[src/trunk]: src/sys/net ifnet(9): Defer if_watchdog (a.k.a. if_slowtimo) to ...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/ec9e647db392
branches:  trunk
changeset: 369484:ec9e647db392
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Aug 20 11:09:24 2022 +0000

description:
ifnet(9): Defer if_watchdog (a.k.a. if_slowtimo) to workqueue.

This is necessary to make mii_down and the *_init/stop routines that
call it to sleep waiting for MII callouts on other CPUs.

Mark the workqueue and callout MP-safe; only take the kernel lock
around the callback.

No kernel bump despite change to struct ifnet because the change is
ABI-compatible and using the callout outside net/if.c has never been
kosher.

diffstat:

 sys/net/if.c |  107 +++++++++++++++++++++++++++++++++++++++++++---------------
 sys/net/if.h |    4 +-
 2 files changed, 80 insertions(+), 31 deletions(-)

diffs (190 lines):

diff -r 270d67592e08 -r ec9e647db392 sys/net/if.c
--- a/sys/net/if.c      Sat Aug 20 10:55:27 2022 +0000
+++ b/sys/net/if.c      Sat Aug 20 11:09:24 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.512 2022/08/17 19:56:28 rillig Exp $  */
+/*     $NetBSD: if.c,v 1.513 2022/08/20 11:09:24 riastradh 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.512 2022/08/17 19:56:28 rillig Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.513 2022/08/20 11:09:24 riastradh Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -197,6 +197,8 @@
 static pserialize_t            ifnet_psz;
 static struct workqueue                *ifnet_link_state_wq __read_mostly;
 
+static struct workqueue                *if_slowtimo_wq __read_mostly;
+
 static kmutex_t                        if_clone_mtx;
 
 struct ifnet *lo0ifp;
@@ -221,7 +223,8 @@
 static void if_detach_queues(struct ifnet *, struct ifqueue *);
 static void sysctl_sndq_setup(struct sysctllog **, const char *,
     struct ifaltq *);
-static void if_slowtimo(void *);
+static void if_slowtimo_intr(void *);
+static void if_slowtimo_work(struct work *, void *);
 static void if_attachdomain1(struct ifnet *);
 static int ifconf(u_long, void *);
 static int if_transmit(struct ifnet *, struct mbuf *);
@@ -255,6 +258,15 @@
 static void if_deferred_start_common(struct ifnet *);
 static void if_deferred_start_destroy(struct ifnet *);
 
+struct if_slowtimo_data {
+       kmutex_t                isd_lock;
+       struct callout          isd_ch;
+       struct work             isd_work;
+       struct ifnet            *isd_ifp;
+       bool                    isd_queued;
+       bool                    isd_dying;
+};
+
 #if defined(INET) || defined(INET6)
 static void sysctl_net_pktq_setup(struct sysctllog **, int);
 #endif
@@ -333,6 +345,10 @@
        KASSERT(error == 0);
        PSLIST_INIT(&ifnet_pslist);
 
+       error = workqueue_create(&if_slowtimo_wq, "ifwdog",
+           if_slowtimo_work, NULL, PRI_SOFTNET, IPL_SOFTCLOCK, WQ_MPSAFE);
+       KASSERTMSG(error == 0, "error=%d", error);
+
        if_indexlim = 8;
 
        if_pfil = pfil_head_create(PFIL_TYPE_IFNET, NULL);
@@ -779,11 +795,17 @@
        rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
 
        if (ifp->if_slowtimo != NULL) {
-               ifp->if_slowtimo_ch =
-                   kmem_zalloc(sizeof(*ifp->if_slowtimo_ch), KM_SLEEP);
-               callout_init(ifp->if_slowtimo_ch, 0);
-               callout_setfunc(ifp->if_slowtimo_ch, if_slowtimo, ifp);
-               if_slowtimo(ifp);
+               struct if_slowtimo_data *isd;
+
+               isd = kmem_zalloc(sizeof(*isd), KM_SLEEP);
+               mutex_init(&isd->isd_lock, MUTEX_DEFAULT, IPL_SOFTCLOCK);
+               callout_init(&isd->isd_ch, CALLOUT_MPSAFE);
+               callout_setfunc(&isd->isd_ch, if_slowtimo_intr, ifp);
+               isd->isd_ifp = ifp;
+
+               ifp->if_slowtimo_data = isd;
+
+               if_slowtimo_intr(ifp);
        }
 
        if (ifp->if_transmit == NULL || ifp->if_transmit == if_nulltransmit)
@@ -1350,11 +1372,20 @@
        pserialize_perform(ifnet_psz);
        IFNET_GLOBAL_UNLOCK();
 
-       if (ifp->if_slowtimo != NULL && ifp->if_slowtimo_ch != NULL) {
-               ifp->if_slowtimo = NULL;
-               callout_halt(ifp->if_slowtimo_ch, NULL);
-               callout_destroy(ifp->if_slowtimo_ch);
-               kmem_free(ifp->if_slowtimo_ch, sizeof(*ifp->if_slowtimo_ch));
+       if (ifp->if_slowtimo != NULL) {
+               struct if_slowtimo_data *isd = ifp->if_slowtimo_data;
+
+               mutex_enter(&isd->isd_lock);
+               isd->isd_dying = true;
+               mutex_exit(&isd->isd_lock);
+               callout_halt(&isd->isd_ch, NULL);
+               workqueue_wait(if_slowtimo_wq, &isd->isd_work);
+               callout_destroy(&isd->isd_ch);
+               mutex_destroy(&isd->isd_lock);
+               kmem_free(isd, sizeof(*isd));
+
+               ifp->if_slowtimo_data = NULL; /* paraonia */
+               ifp->if_slowtimo = NULL;      /* paranoia */
        }
        if_deferred_start_destroy(ifp);
 
@@ -2651,24 +2682,42 @@
  * call the appropriate interface routine on expiration.
  */
 static void
-if_slowtimo(void *arg)
+if_slowtimo_intr(void *arg)
 {
-       void (*slowtimo)(struct ifnet *);
        struct ifnet *ifp = arg;
-       int s;
-
-       slowtimo = ifp->if_slowtimo;
-       if (__predict_false(slowtimo == NULL))
-               return;
-
-       s = splnet();
-       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);
+       struct if_slowtimo_data *isd = ifp->if_slowtimo_data;
+
+       mutex_enter(&isd->isd_lock);
+       if (!isd->isd_dying) {
+               if (ifp->if_timer != 0 &&
+                   --ifp->if_timer == 0 &&
+                   !isd->isd_queued) {
+                       isd->isd_queued = true;
+                       workqueue_enqueue(if_slowtimo_wq, &isd->isd_work,
+                           NULL);
+               } else {
+                       callout_schedule(&isd->isd_ch, hz / IFNET_SLOWHZ);
+               }
+       }
+       mutex_exit(&isd->isd_lock);
+}
+
+static void
+if_slowtimo_work(struct work *work, void *arg)
+{
+       struct if_slowtimo_data *isd =
+           container_of(work, struct if_slowtimo_data, isd_work);
+       struct ifnet *ifp = isd->isd_ifp;
+
+       KERNEL_LOCK(1, NULL);
+       (*ifp->if_slowtimo)(ifp);
+       KERNEL_UNLOCK_ONE(NULL);
+
+       mutex_enter(&isd->isd_lock);
+       isd->isd_queued = false;
+       if (!isd->isd_dying)
+               callout_schedule(&isd->isd_ch, hz / IFNET_SLOWHZ);
+       mutex_exit(&isd->isd_lock);
 }
 
 /*
diff -r 270d67592e08 -r ec9e647db392 sys/net/if.h
--- a/sys/net/if.h      Sat Aug 20 10:55:27 2022 +0000
+++ b/sys/net/if.h      Sat Aug 20 11:09:24 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.299 2022/07/28 15:15:29 skrll Exp $   */
+/*     $NetBSD: if.h,v 1.300 2022/08/20 11:09:24 riastradh Exp $       */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -414,7 +414,7 @@
        kmutex_t        *if_ioctl_lock; /* :: */
        char            *if_description;        /* i: interface description */
 #ifdef _KERNEL /* XXX kvm(3) */
-       struct callout  *if_slowtimo_ch;/* :: */
+       struct if_slowtimo_data *if_slowtimo_data; /* :: */
        struct krwlock  *if_afdata_lock;/* :: */
        struct if_percpuq
                        *if_percpuq;    /* :: we should remove it in the future */