[src/trunk]: src/sys Revert "Make if_timer MP-safe if IFEF_MPSAFE"

[ozaki-r <ozaki-r%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/069bdfad38ad
branches:  trunk
changeset: 358015:069bdfad38ad
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Fri Dec 08 05:22:23 2017 +0000

description:
Revert "Make if_timer MP-safe if IFEF_MPSAFE"

Because it has decreased the performance of wm. And also I found that
wm_watchdog doesn't work well with if_watchdog framework at all. Sharing one
counter (if_timer) with multiple instances (hardware multi-queues) can't detect
a single (or some) stall of them because other instances reset the counter even
if the stalled one want the watchdog to fire.

Interfaces without IFEF_MPSAFE works safely with the original if_watchdog thanks
to KENREL_LOCK. OTOH, interfaces with IFEF_MPSAFE shouldn't use if_watchdog and
should implement their own watchdog timer that works with multiple instances.

diffstat:

 sys/dev/pci/if_wm.c |  12 ++++++------
 sys/net/if.c        |  43 +++++--------------------------------------
 sys/net/if.h        |   5 +----
 3 files changed, 12 insertions(+), 48 deletions(-)

diffs (166 lines):

diff -r 2d67c5fbe97a -r 069bdfad38ad sys/dev/pci/if_wm.c
--- a/sys/dev/pci/if_wm.c       Fri Dec 08 04:37:15 2017 +0000
+++ b/sys/dev/pci/if_wm.c       Fri Dec 08 05:22:23 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_wm.c,v 1.548 2017/12/07 00:38:38 msaitoh Exp $      */
+/*     $NetBSD: if_wm.c,v 1.549 2017/12/08 05:22:23 ozaki-r Exp $      */
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -83,7 +83,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.548 2017/12/07 00:38:38 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.549 2017/12/08 05:22:23 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -5937,7 +5937,7 @@
 
        /* Mark the interface as down and cancel the watchdog timer. */
        ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
-       if_watchdog_stop(ifp);
+       ifp->if_timer = 0;
 
        if (disable) {
                for (i = 0; i < sc->sc_nqueues; i++) {
@@ -7373,7 +7373,7 @@
 
        if (txq->txq_free != ofree) {
                /* Set a watchdog timer in case the chip flakes out. */
-               if_watchdog_reset(ifp, 5);
+               ifp->if_timer = 5;
        }
 }
 
@@ -7945,7 +7945,7 @@
 
        if (sent) {
                /* Set a watchdog timer in case the chip flakes out. */
-               if_watchdog_reset(ifp, 5);
+               ifp->if_timer = 5;
        }
 }
 
@@ -8082,7 +8082,7 @@
         * timer.
         */
        if (txq->txq_sfree == WM_TXQUEUELEN(txq))
-               if_watchdog_stop(ifp);
+               ifp->if_timer = 0;
 
        return processed;
 }
diff -r 2d67c5fbe97a -r 069bdfad38ad sys/net/if.c
--- a/sys/net/if.c      Fri Dec 08 04:37:15 2017 +0000
+++ b/sys/net/if.c      Fri Dec 08 05:22:23 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.410 2017/12/08 04:03:51 ozaki-r Exp $ */
+/*     $NetBSD: if.c,v 1.411 2017/12/08 05:22:23 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.410 2017/12/08 04:03:51 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.411 2017/12/08 05:22:23 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -771,11 +771,9 @@
        rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
 
        if (ifp->if_slowtimo != NULL) {
-               int flags = ISSET(ifp->if_extflags, IFEF_MPSAFE) ?
-                   CALLOUT_MPSAFE : 0;
                ifp->if_slowtimo_ch =
                    kmem_zalloc(sizeof(*ifp->if_slowtimo_ch), KM_SLEEP);
-               callout_init(ifp->if_slowtimo_ch, flags);
+               callout_init(ifp->if_slowtimo_ch, 0);
                callout_setfunc(ifp->if_slowtimo_ch, if_slowtimo, ifp);
                if_slowtimo(ifp);
        }
@@ -2539,18 +2537,6 @@
 }
 
 /*
- * XXX reusing (ifp)->if_snd->ifq_lock rather than having another spin mutex
- * for each ifnet.  It doesn't matter because:
- * - if IFEF_MPSAFE is enabled, if_snd isn't used and lock contention on
- *   ifq_lock don't happen
- * - if IFEF_MPSAFE is disabled, there is no lock contention on ifq_lock
- *   because if_snd and if_watchdog_reset is used with KERNEL_LOCK on packet
- *   transmissions and if_slowtimo is also called with KERNEL_LOCK
- */
-#define IF_WATCHDOG_LOCK(ifp)  mutex_enter((ifp)->if_snd.ifq_lock)
-#define IF_WATCHDOG_UNLOCK(ifp)        mutex_exit((ifp)->if_snd.ifq_lock)
-
-/*
  * Handle interface slowtimo timer routine.  Called
  * from softclock, we decrement timer (if set) and
  * call the appropriate interface routine on expiration.
@@ -2561,40 +2547,21 @@
        void (*slowtimo)(struct ifnet *);
        struct ifnet *ifp = arg;
        int s;
-       bool fire;
 
        slowtimo = ifp->if_slowtimo;
        if (__predict_false(slowtimo == NULL))
                return;
 
        s = splnet();
-       IF_WATCHDOG_LOCK(ifp);
-       fire = (ifp->if_timer != 0 && --ifp->if_timer == 0);
-       IF_WATCHDOG_UNLOCK(ifp);
-       if (fire)
+       if (ifp->if_timer != 0 && --ifp->if_timer == 0)
                (*slowtimo)(ifp);
+
        splx(s);
 
        if (__predict_true(ifp->if_slowtimo != NULL))
                callout_schedule(ifp->if_slowtimo_ch, hz / IFNET_SLOWHZ);
 }
 
-void
-if_watchdog_reset(struct ifnet *ifp, short v)
-{
-
-       IF_WATCHDOG_LOCK(ifp);
-       ifp->if_timer = v;
-       IF_WATCHDOG_UNLOCK(ifp);
-}
-
-void
-if_watchdog_stop(struct ifnet *ifp)
-{
-
-       if_watchdog_reset(ifp, 0);
-}
-
 /*
  * Mark an interface up and notify protocols of
  * the transition.
diff -r 2d67c5fbe97a -r 069bdfad38ad sys/net/if.h
--- a/sys/net/if.h      Fri Dec 08 04:37:15 2017 +0000
+++ b/sys/net/if.h      Fri Dec 08 05:22:23 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.250 2017/12/08 04:03:51 ozaki-r Exp $ */
+/*     $NetBSD: if.h,v 1.251 2017/12/08 05:22:23 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -1045,9 +1045,6 @@
 
 void   if_input(struct ifnet *, struct mbuf *);
 
-void   if_watchdog_reset(struct ifnet *, short);
-void   if_watchdog_stop(struct ifnet *);
-
 struct if_percpuq *
        if_percpuq_create(struct ifnet *);
 void   if_percpuq_destroy(struct if_percpuq *);