I think this will do the job for you.  There are also some ethernet
    drivers which use the extmbuf interface for jumbo packets which I
    have to fix but this should be enough to fix the crash you are getting.

    It compiles but it is not well tested.  Please test with intr_mpsafe set
    to 1!

    I serialize the cluster ref code, and use a neat trick to avoid having
    to call the serializer for the common case (which hopefully is not bogus).
    The SFBUF and the VM_*() routines are not MPSAFE, however, so at the
    moment the sendfile() code is non-optimal because it has to get the
    Big Giant Lock to unwire a VM page.  This needs some work post-release.

                                                -Matt

Index: kern/uipc_mbuf.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.53
diff -u -r1.53 uipc_mbuf.c
--- kern/uipc_mbuf.c    25 Nov 2005 17:52:53 -0000      1.53
+++ kern/uipc_mbuf.c    29 Nov 2005 18:08:51 -0000
@@ -100,6 +100,7 @@
 #include <sys/uio.h>
 #include <sys/thread.h>
 #include <sys/globaldata.h>
+#include <sys/serialize.h>
 #include <sys/thread2.h>
 
 #include <vm/vm.h>
@@ -116,6 +117,7 @@
 struct mbcluster {
        int32_t mcl_refs;
        void    *mcl_data;
+       struct lwkt_serialize mcl_serializer;
 };
 
 static void mbinit(void *);
@@ -273,6 +275,7 @@
                return (FALSE);
        cl->mcl_refs = 0;
        cl->mcl_data = buf;
+       lwkt_serialize_init(&cl->mcl_serializer);
        return (TRUE);
 }
 
@@ -297,7 +300,7 @@
        m->m_ext.ext_ref = m_mclref;
        m->m_ext.ext_free = m_mclfree;
        m->m_ext.ext_size = MCLBYTES;
-       ++cl->mcl_refs;
+       atomic_add_int(&cl->mcl_refs, 1);
 
        m->m_data = m->m_ext.ext_buf;
        m->m_flags |= M_EXT | M_EXT_CLUSTER;
@@ -642,6 +645,18 @@
        }
 }
 
+/*
+ * Updates to mbcluster must be MPSAFE.  Only an entity which already has
+ * a reference to the cluster can ref it, so we are in no danger of 
+ * racing an add with a subtract.  But the operation must still be atomic
+ * 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.
+ */
 static void
 m_mclref(void *arg)
 {
@@ -655,13 +670,20 @@
 {
        struct mbcluster *mcl = arg;
 
-       /* XXX interrupt race.  Currently called from a critical section */
-       if (mcl->mcl_refs > 1) {
-               atomic_subtract_int(&mcl->mcl_refs, 1);
-       } else {
-               KKASSERT(mcl->mcl_refs == 1);
+       if (mcl->mcl_refs == 1) {
                mcl->mcl_refs = 0;
                objcache_put(mclmeta_cache, mcl);
+       } else {
+               lwkt_serialize_enter(&mcl->mcl_serializer);
+               if (mcl->mcl_refs > 1) {
+                       atomic_subtract_int(&mcl->mcl_refs, 1);
+                       lwkt_serialize_exit(&mcl->mcl_serializer);
+               } else {
+                       lwkt_serialize_exit(&mcl->mcl_serializer);
+                       KKASSERT(mcl->mcl_refs == 1);
+                       mcl->mcl_refs = 0;
+                       objcache_put(mclmeta_cache, mcl);
+               }
        }
 }
 
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.58
diff -u -r1.58 uipc_syscalls.c
--- kern/uipc_syscalls.c        2 Sep 2005 07:16:58 -0000       1.58
+++ kern/uipc_syscalls.c        29 Nov 2005 18:14:54 -0000
@@ -74,6 +74,7 @@
 #include <vm/vm_extern.h>
 #include <sys/file2.h>
 #include <sys/signalvar.h>
+#include <sys/serialize.h>
 
 #include <sys/thread2.h>
 #include <sys/msgport2.h>
@@ -85,6 +86,7 @@
 struct sfbuf_mref {
        struct sf_buf   *sf;
        int             mref_count;
+       struct lwkt_serialize serializer;
 };
 
 static MALLOC_DEFINE(M_SENDFILE, "sendfile", "sendfile sfbuf ref structures");
@@ -1247,16 +1249,22 @@
  * Detach a mapped page and release resources back to the system.
  * We must release our wiring and if the object is ripped out
  * from under the vm_page we become responsible for freeing the
- * page.
+ * page.  These routines must be MPSAFE.
  *
  * XXX HACK XXX TEMPORARY UNTIL WE IMPLEMENT EXT MBUF REFERENCE COUNTING
+ *
+ * XXX vm_page_*() routines are not MPSAFE yet, the MP lock is required.
  */
 static void
 sf_buf_mref(void *arg)
 {
        struct sfbuf_mref *sfm = arg;
 
-       ++sfm->mref_count;
+       /*
+        * We must already hold a ref so there is no race to 0, just 
+        * atomically increment the count.
+        */
+       atomic_add_int(&sfm->mref_count, 1);
 }
 
 static void
@@ -1266,15 +1274,40 @@
        vm_page_t m;
 
        KKASSERT(sfm->mref_count > 0);
-       if (--sfm->mref_count == 0) {
-               m = sf_buf_page(sfm->sf);
-               sf_buf_free(sfm->sf);
-               crit_enter();
-               vm_page_unwire(m, 0);
-               if (m->wire_count == 0 && m->object == NULL)
-                       vm_page_try_to_free(m);
-               crit_exit();
-               free(sfm, M_SENDFILE);
+       if (sfm->mref_count == 1) {
+               /*
+                * We are the only holder so no further locking is required,
+                * the sfbuf can simply be freed.
+                */
+               sfm->mref_count = 0;
+               goto freeit;
+       } else {
+               /*
+                * There may be other holders, we must obtain the serializer
+                * to protect against a sf_buf_mfree() race to 0.  An atomic
+                * operation is still required for races against 
+                * sf_buf_mref().
+                *
+                * XXX vm_page_*() and SFBUF routines not MPSAFE yet.
+                */
+               lwkt_serialize_enter(&sfm->serializer);
+               atomic_subtract_int(&sfm->mref_count, 1);
+               if (sfm->mref_count == 0) {
+                       lwkt_serialize_exit(&sfm->serializer);
+freeit:
+                       get_mplock();
+                       crit_enter();
+                       m = sf_buf_page(sfm->sf);
+                       sf_buf_free(sfm->sf);
+                       vm_page_unwire(m, 0);
+                       if (m->wire_count == 0 && m->object == NULL)
+                               vm_page_try_to_free(m);
+                       crit_exit();
+                       rel_mplock();
+                       free(sfm, M_SENDFILE);
+               } else {
+                       lwkt_serialize_exit(&sfm->serializer);
+               }
        }
 }
 
@@ -1582,6 +1615,7 @@
                sfm = malloc(sizeof(struct sfbuf_mref), M_SENDFILE, M_WAITOK);
                sfm->sf = sf;
                sfm->mref_count = 1;
+               lwkt_serialize_init(&sfm->serializer);
 
                m->m_ext.ext_free = sf_buf_mfree;
                m->m_ext.ext_ref = sf_buf_mref;

Reply via email to