kernel - Finish MPSAFEing uipc_mbuf.c
authorMatthew Dillon <dillon@apollo.backplane.com>
Thu, 9 Sep 2010 05:51:03 +0000 (22:51 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Thu, 9 Sep 2010 05:52:48 +0000 (22:52 -0700)
* Make the mbuf tracking debug code MPSAFE.

* Get rid of worthless critical sections.

* Code cleanup.

sys/kern/uipc_mbuf.c
sys/kern/uipc_socket.c

index c5bb02b..18f9b56 100644 (file)
@@ -1,4 +1,6 @@
 /*
+ * (MPSAFE)
+ *
  * Copyright (c) 2004 Jeffrey M. Hsu.  All rights reserved.
  * Copyright (c) 2004 The DragonFly Project.  All rights reserved.
  * 
@@ -83,7 +85,9 @@
 #include <sys/uio.h>
 #include <sys/thread.h>
 #include <sys/globaldata.h>
+
 #include <sys/thread2.h>
+#include <sys/spinlock2.h>
 
 #include <machine/atomic.h>
 #include <machine/limits.h>
@@ -134,18 +138,21 @@ mbtrack_cmp(struct mbtrack *mb1, struct mbtrack *mb2)
 RB_GENERATE2(mbuf_rb_tree, mbtrack, rb_node, mbtrack_cmp, struct mbuf *, m);
 
 struct mbuf_rb_tree    mbuf_track_root;
+static struct spinlock mbuf_track_spin = SPINLOCK_INITIALIZER(mbuf_track_spin);
 
 static void
 mbuftrack(struct mbuf *m)
 {
        struct mbtrack *mbt;
 
-       crit_enter();
        mbt = kmalloc(sizeof(*mbt), M_MTRACK, M_INTWAIT|M_ZERO);
+       spin_lock(&mbuf_track_spin);
        mbt->m = m;
-       if (mbuf_rb_tree_RB_INSERT(&mbuf_track_root, mbt))
+       if (mbuf_rb_tree_RB_INSERT(&mbuf_track_root, mbt)) {
+               spin_unlock(&mbuf_track_spin);
                panic("mbuftrack: mbuf %p already being tracked\n", m);
-       crit_exit();
+       }
+       spin_unlock(&mbuf_track_spin);
 }
 
 static void
@@ -153,15 +160,16 @@ mbufuntrack(struct mbuf *m)
 {
        struct mbtrack *mbt;
 
-       crit_enter();
+       spin_lock(&mbuf_track_spin);
        mbt = mbuf_rb_tree_RB_LOOKUP(&mbuf_track_root, m);
        if (mbt == NULL) {
-               kprintf("mbufuntrack: mbuf %p was not tracked\n", m);
+               spin_unlock(&mbuf_track_spin);
+               panic("mbufuntrack: mbuf %p was not tracked\n", m);
        } else {
                mbuf_rb_tree_RB_REMOVE(&mbuf_track_root, mbt);
                kfree(mbt, M_MTRACK);
        }
-       crit_exit();
+       spin_unlock(&mbuf_track_spin);
 }
 
 void
@@ -170,18 +178,21 @@ mbuftrackid(struct mbuf *m, int trackid)
        struct mbtrack *mbt;
        struct mbuf *n;
 
-       crit_enter();
+       spin_lock(&mbuf_track_spin);
        while (m) { 
                n = m->m_nextpkt;
                while (m) {
                        mbt = mbuf_rb_tree_RB_LOOKUP(&mbuf_track_root, m);
-                       if (mbt)
-                               mbt->trackid = trackid;
+                       if (mbt == NULL) {
+                               spin_unlock(&mbuf_track_spin);
+                               panic("mbuftrackid: mbuf %p not tracked", m);
+                       }
+                       mbt->trackid = trackid;
                        m = m->m_next;
                }
                m = n;
        }
-       crit_exit();
+       spin_unlock(&mbuf_track_spin);
 }
 
 static int
@@ -193,7 +204,9 @@ mbuftrack_callback(struct mbtrack *mbt, void *arg)
 
        ksnprintf(buf, sizeof(buf), "mbuf %p track %d\n", mbt->m, mbt->trackid);
 
+       spin_unlock(&mbuf_track_spin);
        error = SYSCTL_OUT(req, buf, strlen(buf));
+       spin_lock(&mbuf_track_spin);
        if (error)      
                return(-error);
        return(0);
@@ -204,10 +217,10 @@ mbuftrack_show(SYSCTL_HANDLER_ARGS)
 {
        int error;
 
-       crit_enter();
+       spin_lock(&mbuf_track_spin);
        error = mbuf_rb_tree_RB_SCAN(&mbuf_track_root, NULL,
                                     mbuftrack_callback, req);
-       crit_exit();
+       spin_unlock(&mbuf_track_spin);
        return (-error);
 }
 SYSCTL_PROC(_kern_ipc, OID_AUTO, showmbufs, CTLFLAG_RD|CTLTYPE_STRING,
@@ -638,14 +651,14 @@ m_reclaim(void)
        struct domain *dp;
        struct protosw *pr;
 
-       crit_enter();
+       kprintf("Debug: m_reclaim() called\n");
+
        SLIST_FOREACH(dp, &domains, dom_next) {
                for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) {
                        if (pr->pr_drain)
                                (*pr->pr_drain)();
                }
        }
-       crit_exit();
        atomic_add_long_nonlocked(&mbstat[mycpu->gd_cpuid].m_drain, 1);
 }
 
@@ -680,7 +693,8 @@ retryonce:
                if ((how & MB_TRYWAIT) && ntries++ == 0) {
                        struct objcache *reclaimlist[] = {
                                mbufphdr_cache,
-                               mbufcluster_cache, mbufphdrcluster_cache
+                               mbufcluster_cache,
+                               mbufphdrcluster_cache
                        };
                        const int nreclaims = __arysize(reclaimlist);
 
@@ -853,7 +867,8 @@ m_mclget(struct mbuf *m, int how)
        mcl = objcache_get(mclmeta_cache, MBTOM(how));
        if (mcl != NULL) {
                linkcluster(m, mcl);
-               atomic_add_long_nonlocked(&mbstat[mycpu->gd_cpuid].m_clusters, 1);
+               atomic_add_long_nonlocked(&mbstat[mycpu->gd_cpuid].m_clusters,
+                                         1);
        } else {
                ++mbstat[mycpu->gd_cpuid].m_drops;
        }
@@ -866,10 +881,7 @@ m_mclget(struct mbuf *m, int how)
  * since multiple entities may have a reference on the cluster.
  *
  * m_mclfree() is almost the same but it must contend with two entities
- * freeing the cluster at the same time.  If there is only one reference
- * count we are the only entity referencing the cluster and no further
- * locking is required.  Otherwise we must protect against a race to 0
- * with the serializer.
+ * freeing the cluster at the same time.
  */
 static void
 m_mclref(void *arg)
@@ -957,7 +969,6 @@ m_free(struct mbuf *m)
         * and is totally separate from whether the mbuf is currently
         * associated with a cluster.
         */
-       crit_enter();
        switch(m->m_flags & (M_CLCACHE | M_EXT | M_EXT_CLUSTER)) {
        case M_CLCACHE | M_EXT | M_EXT_CLUSTER:
                /*
@@ -1036,17 +1047,14 @@ m_free(struct mbuf *m)
                        panic("bad mbuf flags %p %08x\n", m, m->m_flags);
                break;
        }
-       crit_exit();
        return (n);
 }
 
 void
 m_freem(struct mbuf *m)
 {
-       crit_enter();
        while (m)
                m = m_free(m);
-       crit_exit();
 }
 
 /*
@@ -1097,7 +1105,7 @@ m_copym(const struct mbuf *m, int off0, int len, int wait)
 
        KASSERT(off >= 0, ("m_copym, negative off %d", off));
        KASSERT(len >= 0, ("m_copym, negative len %d", len));
-       if (off == 0 && m->m_flags & M_PKTHDR)
+       if (off == 0 && (m->m_flags & M_PKTHDR))
                copyhdr = 1;
        while (off > 0) {
                KASSERT(m != NULL, ("m_copym, offset > size of mbuf chain"));
@@ -1107,7 +1115,7 @@ m_copym(const struct mbuf *m, int off0, int len, int wait)
                m = m->m_next;
        }
        np = &top;
-       top = 0;
+       top = NULL;
        while (len > 0) {
                if (m == NULL) {
                        KASSERT(len == M_COPYALL, 
index 12bf054..96e4261 100644 (file)
@@ -537,10 +537,17 @@ sosend(struct socket *so, struct sockaddr *addr, struct uio *uio,
        int atomic = sosendallatonce(so) || top;
        int pru_flags;
 
-       if (uio)
+       if (uio) {
                resid = uio->uio_resid;
-       else
+       } else {
                resid = (size_t)top->m_pkthdr.len;
+#ifdef INVARIANTS
+               len = 0;
+               for (m = top; m; m = m->m_next)
+                       len += m->m_len;
+               KKASSERT(top->m_pkthdr.len == len);
+#endif
+       }
 
        /*
         * WARNING!  resid is unsigned, space and len are signed.  space