Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Kernel panic in 5.99.24



On Thu, Feb 04, 2010 at 10:20:09AM +0200, Martti Kuparinen wrote:
> YAMAMOTO Takashi wrote:
> 
> >>Here is a compilable, but totally untested patch that may fix the
> >>problem.
> >
> >it seems like a wrong approach because which ioctl can cause recursive
> >calls is out of control of agr driver.
> 
> And the patch does not prevent panic...

Here is a patch, also compiled but untested, that aims both to
synchronize access to the ports list, and to wait to destroy the softc
until all threads of control have left it.  I doubt that the patch will
"just work," but I hope that it is a step in the right direction.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933
Index: sys/net/agr/if_agr.c
===================================================================
RCS file: /cvsroot/src/sys/net/agr/if_agr.c,v
retrieving revision 1.24
diff -p -u -u -p -r1.24 if_agr.c
--- sys/net/agr/if_agr.c        9 Jun 2009 22:21:54 -0000       1.24
+++ sys/net/agr/if_agr.c        4 Feb 2010 23:11:09 -0000
@@ -42,6 +42,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_agr.c,v 1
 #include <sys/sockio.h>
 #include <sys/proc.h>  /* XXX for curproc */
 #include <sys/kauth.h>
+#include <sys/xcall.h>
 
 #if NBPFILTER > 0
 #include <net/bpf.h>
@@ -67,9 +68,9 @@ void agrattach(int);
 static int agr_clone_create(struct if_clone *, int);
 static int agr_clone_destroy(struct ifnet *);
 static void agr_start(struct ifnet *);
-static int agr_setconfig(struct ifnet *, const struct agrreq *);
-static int agr_getconfig(struct ifnet *, struct agrreq *);
-static int agr_getportlist(struct ifnet *, struct agrreq *);
+static int agr_setconfig(struct agr_softc *, const struct agrreq *);
+static int agr_getconfig(struct agr_softc *, struct agrreq *);
+static int agr_getportlist(struct agr_softc *, struct agrreq *);
 static int agr_addport(struct ifnet *, struct ifnet *);
 static int agr_remport(struct ifnet *, struct ifnet *);
 static int agrreq_copyin(const void *, struct agrreq *);
@@ -83,6 +84,16 @@ static int agrport_config_promisc_callba
 static int agrport_config_promisc(struct agr_port *, bool);
 static int agrport_cleanup(struct agr_softc *, struct agr_port *);
 
+static int agr_enter(struct agr_softc *);
+static void agr_exit(struct agr_softc *);
+static int agr_pause(struct agr_softc *);
+static void agr_evacuate(struct agr_softc *);
+static void agr_sync(void);
+static void agr_ports_lock(struct agr_softc *);
+static void agr_ports_unlock(struct agr_softc *);
+static bool agr_ports_enter(struct agr_softc *);
+static void agr_ports_exit(struct agr_softc *);
+
 static struct if_clone agr_cloner =
     IF_CLONE_INITIALIZER("agr", agr_clone_create, agr_clone_destroy);
 
@@ -174,20 +185,6 @@ agr_unlock(struct agr_softc *sc)
        mutex_exit(&sc->sc_lock);
 }
 
-void
-agr_ioctl_lock(struct agr_softc *sc)
-{
-
-       mutex_enter(&sc->sc_ioctl_lock);
-}
-
-void
-agr_ioctl_unlock(struct agr_softc *sc)
-{
-
-       mutex_exit(&sc->sc_ioctl_lock);
-}
-
 /*
  * agr_xmit_frame: transmit a pre-built frame.
  */
@@ -335,8 +332,10 @@ agr_clone_create(struct if_clone *ifc, i
 
        sc = agr_alloc_softc();
        TAILQ_INIT(&sc->sc_ports);
-       mutex_init(&sc->sc_ioctl_lock, MUTEX_DRIVER, IPL_NONE);
-       mutex_init(&sc->sc_lock, MUTEX_DRIVER, IPL_NET);
+       mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_NET);
+       mutex_init(&sc->sc_entry_mtx, MUTEX_DEFAULT, IPL_NONE);
+       cv_init(&sc->sc_insc_cv, "agr_softc");
+       cv_init(&sc->sc_ports_cv, "agrports");
        agrtimer_init(sc);
        ifp = &sc->sc_if;
        snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d",
@@ -371,26 +370,24 @@ agr_clone_destroy(struct ifnet *ifp)
        struct agr_softc *sc = ifp->if_softc;
        int error;
 
-       agr_ioctl_lock(sc);
-
-       AGR_LOCK(sc);
-       if (sc->sc_nports > 0) {
-               error = EBUSY;
-       } else {
-               error = 0;
-       }
-       AGR_UNLOCK(sc);
-
-       agr_ioctl_unlock(sc);
+       if ((error = agr_pause(sc)) != 0)
+               return error;
 
-       if (error == 0) {
-               if_detach(ifp);
-               mutex_destroy(&sc->sc_ioctl_lock);
-               mutex_destroy(&sc->sc_lock);
-               agr_free_softc(sc);
-       }
+       if_detach(ifp);
+       agrtimer_destroy(sc);
+       /* Now that the ifnet has been detached, and our
+        * component ifnets are disconnected, there can be
+        * no new threads in the softc.  Wait for every
+        * thread to get out of the softc.
+        */
+       agr_evacuate(sc);
+       mutex_destroy(&sc->sc_lock);
+       mutex_destroy(&sc->sc_entry_mtx);
+       cv_destroy(&sc->sc_insc_cv);
+       cv_destroy(&sc->sc_ports_cv);
+       agr_free_softc(sc);
 
-       return error;
+       return 0;
 }
 
 static struct agr_port *
@@ -464,8 +461,9 @@ agr_start(struct ifnet *ifp)
 }
 
 static int
-agr_setconfig(struct ifnet *ifp, const struct agrreq *ar)
+agr_setconfig(struct agr_softc *sc, const struct agrreq *ar)
 {
+       struct ifnet *ifp = &sc->sc_if;
        int cmd = ar->ar_cmd;
        struct ifnet *ifp_port;
        int error = 0;
@@ -482,6 +480,7 @@ agr_setconfig(struct ifnet *ifp, const s
                return ENOENT;
        }
 
+       agr_ports_lock(sc);
        switch (cmd) {
        case AGRCMD_ADDPORT:
                error = agr_addport(ifp, ifp_port);
@@ -495,14 +494,14 @@ agr_setconfig(struct ifnet *ifp, const s
                error = EINVAL;
                break;
        }
+       agr_ports_unlock(sc);
 
        return error;
 }
 
 static int
-agr_getportlist(struct ifnet *ifp, struct agrreq *ar)
+agr_getportlist(struct agr_softc *sc, struct agrreq *ar)
 {
-       struct agr_softc *sc = ifp->if_softc;
        struct agr_port *port;
        struct agrportlist apl;
        struct agrportinfo api;
@@ -557,20 +556,22 @@ agr_getportlist(struct ifnet *ifp, struc
 }
 
 static int
-agr_getconfig(struct ifnet *ifp, struct agrreq *ar)
+agr_getconfig(struct agr_softc *sc, struct agrreq *ar)
 {
        int cmd = ar->ar_cmd;
        int error;
 
+       (void)agr_ports_enter(sc);
        switch (cmd) {
        case AGRCMD_PORTLIST:
-               error = agr_getportlist(ifp, ar);
+               error = agr_getportlist(sc, ar);
                break;
 
        default:
                error = EINVAL;
                break;
        }
+       agr_ports_exit(sc);
 
        return error;
 }
@@ -933,22 +934,142 @@ agrreq_copyout(void *ubuf, struct agrreq
        return 0;
 }
 
+/* Make sure that if any interrupt handlers are out of the softc. */
+static void
+agr_sync(void)
+{
+       uint64_t h;
+
+       if (!mp_online)
+               return;
+
+       h = xc_broadcast(0, (xcfunc_t)nullop, NULL, NULL);
+       xc_wait(h);
+}
+
+static int
+agr_pause(struct agr_softc *sc)
+{
+       int error;
+
+       mutex_enter(&sc->sc_entry_mtx);
+       if ((error = sc->sc_noentry) != 0)
+               goto out;
+
+       sc->sc_noentry = EBUSY;
+
+       while (sc->sc_insc != 0)
+               cv_wait(&sc->sc_insc_cv, &sc->sc_entry_mtx);
+
+       if (sc->sc_nports == 0) {
+               sc->sc_noentry = ENXIO;
+       } else {
+               sc->sc_noentry = 0;
+               error = EBUSY;
+       }
+       cv_broadcast(&sc->sc_insc_cv);
+out:
+       mutex_exit(&sc->sc_entry_mtx);
+       return error;
+}
+
+static void
+agr_evacuate(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       cv_broadcast(&sc->sc_insc_cv);
+       while (sc->sc_insc != 0 || sc->sc_paused != 0)
+               cv_wait(&sc->sc_insc_cv, &sc->sc_entry_mtx);
+       mutex_exit(&sc->sc_entry_mtx);
+
+       agr_sync();
+}
+
+static int
+agr_enter(struct agr_softc *sc)
+{
+       int error;
+
+       mutex_enter(&sc->sc_entry_mtx);
+       sc->sc_paused++;
+       while ((error = sc->sc_noentry) == EBUSY)
+               cv_wait(&sc->sc_insc_cv, &sc->sc_entry_mtx);
+       sc->sc_paused--;
+       if (error == 0)
+               sc->sc_insc++;
+       mutex_exit(&sc->sc_entry_mtx);
+
+       return error;
+}
+
+static void
+agr_exit(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       if (--sc->sc_insc == 0)
+               cv_signal(&sc->sc_insc_cv);
+       mutex_exit(&sc->sc_entry_mtx);
+}
+
+static bool
+agr_ports_enter(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       while (sc->sc_wrports != 0)
+               cv_wait(&sc->sc_ports_cv, &sc->sc_entry_mtx);
+       sc->sc_rdports++;
+       mutex_exit(&sc->sc_entry_mtx);
+
+       return true;
+}
+
+static void
+agr_ports_exit(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       if (--sc->sc_rdports == 0)
+               cv_signal(&sc->sc_ports_cv);
+       mutex_exit(&sc->sc_entry_mtx);
+}
+
+static void
+agr_ports_lock(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       while (sc->sc_rdports != 0)
+               cv_wait(&sc->sc_ports_cv, &sc->sc_entry_mtx);
+       sc->sc_wrports = true;
+       mutex_exit(&sc->sc_entry_mtx);
+}
+
+static void
+agr_ports_unlock(struct agr_softc *sc)
+{
+       mutex_enter(&sc->sc_entry_mtx);
+       sc->sc_wrports = false;
+       cv_signal(&sc->sc_ports_cv);
+       mutex_exit(&sc->sc_entry_mtx);
+}
+
 static int
-agr_ioctl(struct ifnet *ifp, u_long cmd, void *data)
+agr_ioctl(struct ifnet *ifp, const u_long cmd, void *data)
 {
        struct agr_softc *sc = ifp->if_softc;
        struct ifreq *ifr = (struct ifreq *)data;
        struct ifaddr *ifa = (struct ifaddr *)data;
        struct agrreq ar;
-       int error = 0;
+       int error;
+       bool in_ports = false;
        int s;
 
-       agr_ioctl_lock(sc);
+       if ((error = agr_enter(sc)) != 0)
+               return error;
 
        s = splnet();
 
        switch (cmd) {
        case SIOCINITIFADDR:
+               in_ports = agr_ports_enter(sc);
                if (sc->sc_nports == 0) {
                        error = EINVAL;
                        break;
@@ -992,7 +1113,7 @@ agr_ioctl(struct ifnet *ifp, u_long cmd,
                        error = agrreq_copyin(ifr->ifr_data, &ar);
                }
                if (!error) {
-                       error = agr_setconfig(ifp, &ar);
+                       error = agr_setconfig(sc, &ar);
                }
                s = splnet();
                break;
@@ -1001,7 +1122,7 @@ agr_ioctl(struct ifnet *ifp, u_long cmd,
                splx(s);
                error = agrreq_copyin(ifr->ifr_data, &ar);
                if (!error) {
-                       error = agr_getconfig(ifp, &ar);
+                       error = agr_getconfig(sc, &ar);
                }
                if (!error) {
                        error = agrreq_copyout(ifr->ifr_data, &ar);
@@ -1011,11 +1132,11 @@ agr_ioctl(struct ifnet *ifp, u_long cmd,
 
        case SIOCADDMULTI:
        case SIOCDELMULTI:
-               if (sc->sc_nports == 0) {
+               in_ports = agr_ports_enter(sc);
+               if (sc->sc_nports == 0)
                        error = EINVAL;
-                       break;
-               }
-               error = agr_ioctl_multi(ifp, cmd, ifr);
+               else
+                       error = agr_ioctl_multi(ifp, cmd, ifr);
                break;
 
        default:
@@ -1023,9 +1144,12 @@ agr_ioctl(struct ifnet *ifp, u_long cmd,
                break;
        }
 
+       if (in_ports)
+               agr_ports_exit(sc);
+
        splx(s);
 
-       agr_ioctl_unlock(sc);
+       agr_exit(sc);
 
        return error;
 }
Index: sys/net/agr/if_agrtimer.c
===================================================================
RCS file: /cvsroot/src/sys/net/agr/if_agrtimer.c,v
retrieving revision 1.5
diff -p -u -u -p -r1.5 if_agrtimer.c
--- sys/net/agr/if_agrtimer.c   9 Jul 2007 21:11:01 -0000       1.5
+++ sys/net/agr/if_agrtimer.c   4 Feb 2010 23:11:09 -0000
@@ -51,6 +51,13 @@ agrtimer_init(struct agr_softc *sc)
 }
 
 void
+agrtimer_destroy(struct agr_softc *sc)
+{
+
+       callout_destroy(&sc->sc_callout);
+}
+
+void
 agrtimer_start(struct agr_softc *sc)
 {
 
Index: sys/net/agr/if_agrvar_impl.h
===================================================================
RCS file: /cvsroot/src/sys/net/agr/if_agrvar_impl.h,v
retrieving revision 1.8
diff -p -u -u -p -r1.8 if_agrvar_impl.h
--- sys/net/agr/if_agrvar_impl.h        29 May 2009 04:57:05 -0000      1.8
+++ sys/net/agr/if_agrvar_impl.h        4 Feb 2010 23:11:09 -0000
@@ -103,8 +103,15 @@ struct agr_ifreq {
 };
 
 struct agr_softc {
-       kmutex_t sc_ioctl_lock;
+       kmutex_t sc_entry_mtx;
        kmutex_t sc_lock;
+       kcondvar_t sc_ports_cv;
+       kcondvar_t sc_insc_cv;
+       int sc_noentry;
+       int sc_insc;
+       int sc_wrports;
+       int sc_rdports;
+       int sc_paused;
        struct callout sc_callout;
        int sc_nports;
        TAILQ_HEAD(, agr_port) sc_ports;
@@ -125,9 +132,6 @@ struct agr_softc {
 void agr_lock(struct agr_softc *);
 void agr_unlock(struct agr_softc *);
 
-void agr_ioctl_lock(struct agr_softc *);
-void agr_ioctl_unlock(struct agr_softc *);
-
 int agrport_ioctl(struct agr_port *, u_long, void *);
 
 struct agr_softc *agr_alloc_softc(void);
@@ -139,6 +143,7 @@ int agr_xmit_frame(struct ifnet *, struc
 #define        AGR_STATIC(sc)          (((sc)->sc_if.if_flags & IFF_LINK1) != 
0)
 
 void agrtimer_init(struct agr_softc *);
+void agrtimer_destroy(struct agr_softc *);
 void agrtimer_start(struct agr_softc *);
 void agrtimer_stop(struct agr_softc *);
 


Home | Main Index | Thread Index | Old Index