PERFORCE change 125224 for review
Marko Zec
zec at FreeBSD.org
Thu Aug 16 10:41:18 PDT 2007
http://perforce.freebsd.org/chv.cgi?CH=125224
Change 125224 by zec at zec_tpx32 on 2007/08/16 17:40:55
Initial attempt at locking of the global vnet list.
The global vnet list is quite static, i.e. it is being
read-only traversed relatively often, mostly from timers,
currently at around 110 times per second regardless of HZ
setting; will be more in HZ range once dummynet gets loaded.
It only needs to be updated when new vnets are created or
existing ones are deleted. Hence, the vnet list might look
like an ideal candidate for read-often write-seldom type of
locks, such as sx(9) or rwlock(9). However, so far my
experiments with locking the vnet list while processing timers
using sx, rwlock, or default mutexes, have been futile; each
variant leading to a different LOR storm and / or to temporary
system lockups. Therefore I'm attempting to protect the vnet
list using a handcrafted shared / exclusive locking scheme...
The basic idea is to allow shared read-only access to the vnet
list using a refcounted scheme, with the refcount itself being
protected by a mutex, while for granting exclusive access
the shared refcount must be zero and the mutex must be held
during the entire critical section. I.e. the mutex is not
held during the shared access sections, but only while bumping
the refcount. A condvar(9) is used by read-only threads to
wake up the thread(s) waiting to enter an exclusively-locked
section. The extra overhead this scheme introduces is that
for each read-only section we need two mtx_lock() and two
mtx_unlock() calls plus one cv_signal() invocation.
This mechanism has yet to be stress-tested on a SMP machine.
Given that those are basically my first steps in the strange
world of kernel-level multithreading, any comments from more
knowledgeable people would be much appreciated...
Affected files ...
.. //depot/projects/vimage/src/sys/kern/kern_vimage.c#34 edit
.. //depot/projects/vimage/src/sys/sys/vimage.h#34 edit
Differences ...
==== //depot/projects/vimage/src/sys/kern/kern_vimage.c#34 (text+ko) ====
@@ -33,31 +33,19 @@
#include "opt_ddb.h"
#include "opt_vimage.h"
-#include <sys/types.h>
#include <sys/param.h>
-#include <sys/systm.h>
+#include <sys/kernel.h>
+#include <sys/linker.h>
#include <sys/malloc.h>
-#include <sys/domain.h>
-#include <sys/protosw.h>
-#include <sys/kernel.h>
-#include <sys/sysproto.h>
-#include <sys/sysent.h>
#include <sys/sockio.h>
-#include <sys/proc.h>
-#include <sys/linker.h>
-#include <sys/queue.h>
-#include <sys/socketvar.h>
-#include <sys/sysctl.h>
#include <sys/priv.h>
#include <sys/vimage.h>
-#include <sys/vmmeter.h>
#ifdef DDB
#include <ddb/ddb.h>
#endif
#include <net/vnet.h>
-#include <net/bpf.h>
#include <net/if_types.h>
#include <net/if_dl.h>
#include <net/if_clone.h>
@@ -121,6 +109,21 @@
struct vprocg_list_head vprocg_head;
struct vcpu_list_head vcpu_head;
+struct cv vnet_list_condvar;
+struct mtx vnet_list_refc_mtx;
+int vnet_list_refc = 0;
+
+#define VNET_LIST_LOCK() \
+ mtx_lock(&vnet_list_refc_mtx); \
+ while (vnet_list_refc != 0) { \
+ cv_wait(&vnet_list_condvar, &vnet_list_refc_mtx); \
+ printf("XXX vnet_list_refc = %d in %s\n", \
+ vnet_list_refc, __FUNCTION__); \
+ }
+
+#define VNET_LIST_UNLOCK() \
+ mtx_unlock(&vnet_list_refc_mtx);
+
static int last_vi_id = 0;
static TAILQ_HEAD(vnet_modlink_head, vnet_modlink) vnet_modlink_head;
@@ -430,6 +433,7 @@
if (error)
return (error);
+ VNET_LIST_LOCK(); /* XXX should lock vimage list... */
if (strlen(vi_req->vi_name)) {
LIST_FOREACH(tvip, &vimage_head, vi_le)
if (strcmp(vi_req->vi_name, tvip->vi_name)==0) {
@@ -437,9 +441,11 @@
break;
}
if (vip_r == NULL && !(vi_req->req_action & VI_CREATE)) {
+ VNET_LIST_UNLOCK(); /* XXX */
return (ESRCH);
}
if (vip_r != NULL && vi_req->req_action & VI_CREATE) {
+ VNET_LIST_UNLOCK(); /* XXX */
return (EADDRINUSE);
}
if (vi_req->req_action == VI_GETNEXT) {
@@ -447,6 +453,7 @@
if ((vip_r = LIST_NEXT(vip_r, vi_le)) == 0)
vip_r = LIST_FIRST(&vimage_head);
if (vip_r == vip) {
+ VNET_LIST_UNLOCK(); /* XXX */
return (ESRCH);
}
if (!vi_child_of(vip, vip_r))
@@ -455,6 +462,7 @@
} else
vip_r = vip;
+ VNET_LIST_UNLOCK(); /* XXX */
if (vip_r && !vi_child_of(vip, vip_r) &&
vi_req->req_action != VI_GET && vi_req->req_action != VI_GETNEXT)
@@ -515,9 +523,8 @@
vip_r->vi_parent = vip;
}
- if (vip == vip_r && vip != &vimage_0) {
+ if (vip == vip_r && vip != &vimage_0)
return (EPERM);
- }
}
return (error);
@@ -606,10 +613,12 @@
CURVNET_RESTORE();
- LIST_INSERT_HEAD(&vimage_head, vip, vi_le);
+ VNET_LIST_LOCK(); /* XXX should lock other lists separately */
LIST_INSERT_HEAD(&vnet_head, vnet, vnet_le);
LIST_INSERT_HEAD(&vprocg_head, vprocg, vprocg_le);
LIST_INSERT_HEAD(&vcpu_head, vcpu, vcpu_le);
+ LIST_INSERT_HEAD(&vimage_head, vip, vi_le);
+ VNET_LIST_UNLOCK();
vi_alloc_done:
return (vip);
@@ -630,8 +639,9 @@
struct ifnet *ifp, *nifp;
struct vnet_modlink *vml;
- /* XXX should have the vnet list locked here!!! */
+ VNET_LIST_LOCK();
LIST_REMOVE(vnet, vnet_le);
+ VNET_LIST_UNLOCK();
CURVNET_SET_QUIET(vnet);
INIT_VNET_NET(vnet);
@@ -663,6 +673,7 @@
vnet->vnet_magic_n = -1;
vi_free(vnet, M_VNET);
+ /* XXX lock those bellow... */
LIST_REMOVE(vprocg, vprocg_le);
vi_free(vprocg, M_VPROCG);
@@ -772,6 +783,9 @@
vnet_0.vnet_magic_n = VNET_MAGIC_N;
+ mtx_init(&vnet_list_refc_mtx, "vnet_list_refc_mtx", NULL, MTX_DEF);
+ cv_init(&vnet_list_condvar, "vnet_list_condvar");
+
/* We MUST clear curvnet in vi_init_done before going SMP. */
curvnet = &vnet_0;
}
==== //depot/projects/vimage/src/sys/sys/vimage.h#34 (text+ko) ====
@@ -30,17 +30,13 @@
* XXX RCS tag goes here
*/
-
#ifndef _NET_VIMAGE_H_
#define _NET_VIMAGE_H_
-
-#include <sys/resource.h>
-#include <sys/dkstat.h>
-#include <sys/msgbuf.h>
-#include <sys/select.h>
-#include <sys/callout.h>
+#include <sys/lock.h>
#include <sys/proc.h>
+#include <sys/condvar.h>
+#include <sys/mutex.h>
#ifdef INVARIANTS
#define VNET_DEBUG
@@ -218,20 +214,20 @@
#define VNET_ITERLOOP_BEGIN() \
struct vnet *vnet_iter; \
- /* XXX LOCK vnet list */ \
+ VNET_LIST_REF(); \
LIST_FOREACH(vnet_iter, &vnet_head, vnet_le) { \
CURVNET_SET(vnet_iter);
#define VNET_ITERLOOP_BEGIN_QUIET() \
struct vnet *vnet_iter; \
- /* XXX LOCK vnet list */ \
+ VNET_LIST_REF(); \
LIST_FOREACH(vnet_iter, &vnet_head, vnet_le) { \
CURVNET_SET_QUIET(vnet_iter);
#define VNET_ITERLOOP_END() \
CURVNET_RESTORE(); \
} \
- /* XXX UNLOCK vnet list */
+ VNET_LIST_UNREF();
#else /* !VNET_DEBUG */
@@ -252,7 +248,7 @@
#define VNET_ITERLOOP_BEGIN() \
struct vnet *vnet_iter; \
- /* XXX LOCK vnet list */ \
+ VNET_LIST_REF(); \
LIST_FOREACH(vnet_iter, &vnet_head, vnet_le) { \
CURVNET_SET(vnet_iter);
@@ -261,7 +257,7 @@
#define VNET_ITERLOOP_END() \
CURVNET_RESTORE(); \
} \
- /* XXX UNLOCK vnet list */
+ VNET_LIST_UNREF();
#endif /* !VNET_DEBUG */
@@ -326,7 +322,24 @@
extern struct vnet vnet_0;
LIST_HEAD(vnet_list_head, vnet);
extern struct vnet_list_head vnet_head;
+extern int vnet_list_refc;
+extern struct mtx vnet_list_refc_mtx;
+extern struct cv vnet_list_condvar;
+#define VNET_LIST_REF() \
+ mtx_lock(&vnet_list_refc_mtx); \
+ vnet_list_refc++; \
+ if (vnet_list_refc > 1) \
+ printf ("XXX vnet_list_refc = %d in %s\n", \
+ vnet_list_refc, __FUNCTION__); \
+ mtx_unlock(&vnet_list_refc_mtx);
+
+#define VNET_LIST_UNREF() \
+ mtx_lock(&vnet_list_refc_mtx); \
+ vnet_list_refc--; \
+ mtx_unlock(&vnet_list_refc_mtx); \
+ cv_signal(&vnet_list_condvar);
+
#define IS_VNET_0(arg) ((arg) == &vnet_0 ? 1 : 0)
/*
@@ -345,7 +358,6 @@
struct vnet *v_vnet;
};
-
struct vprocg {
LIST_ENTRY(vprocg) vprocg_le;
@@ -383,7 +395,6 @@
#endif
};
-
struct vcpu {
LIST_ENTRY(vcpu) vcpu_le;
More information about the p4-projects
mailing list