crash with network01.patch (with patch)
Matthew Dillon
dillon at apollo.backplane.com
Tue Nov 29 10:24:35 PST 2005
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;
More information about the Bugs
mailing list