Switch pfil(9) to rmlocks
Robert Watson
rwatson at FreeBSD.org
Mon Nov 26 12:37:10 PST 2007
On Fri, 23 Nov 2007, Max Laier wrote:
> attached is a diff to switch the pfil(9) subsystem to rmlocks, which are
> more suited for the task. I'd like some exposure before doing the switch,
> but I don't expect any fallout. This email is going through the patched
> pfil already - twice.
FYI, since people are experimenting with rmlocks as a substitute for rwlocks,
I played with moving the global rwlock used to protect the name space and
linkage of UNIX domain sockets to be an rmlock. Kris didn't see any
measurable change in performance for his MySQL benchmarks, but I figured I'd
post the patches as they give a sense of what change impact things like reader
state management have on code. Attached below. I have no current plans to
commit these changes as they appear not to offer benefit (either because the
rwlock overhead was negigible compared to other costs in the benchmark, or
because the read/write blend was too scewed towards writes -- I think probably
the former rather than the latter).
Robert N M Watson
Computer Laboratory
University of Cambridge
==== //depot/vendor/freebsd/src/sys/kern/uipc_usrreq.c#141 (text+ko) - //depot/user/rwatson/proto/src/sys/kern/uipc_usrreq.c#59 (text+ko) ==== content
@@ -79,6 +79,7 @@
#include <sys/proc.h>
#include <sys/protosw.h>
#include <sys/resourcevar.h>
+#include <sys/rmlock.h>
#include <sys/rwlock.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
@@ -196,6 +197,35 @@
* binding, both of which involve dropping UNIX domain socket locks in order
* to perform namei() and other file system operations.
*/
+
+#define USE_RMLOCKS
+#ifdef USE_RMLOCKS
+
+static struct rmlock unp_global_rmlock;
+
+#define UNP_GLOBAL_LOCK_INIT() rm_init(&unp_global_rmlock, \
+ "unp_global_rmlock", 0)
+
+#define UNP_GLOBAL_LOCK_ASSERT() /*rm_assert(&unp_global_rmlock, \
+ RA_LOCKED) */
+#define UNP_GLOBAL_UNLOCK_ASSERT() /*rm_assert(&unp_global_rmlock, \
+ RA_UNLOCKED) */
+
+#define UNP_GLOBAL_WLOCK() rm_wlock(&unp_global_rmlock)
+#define UNP_GLOBAL_WUNLOCK() rm_wunlock(&unp_global_rmlock)
+#define UNP_GLOBAL_WLOCK_ASSERT() /*rm_assert(&unp_global_rmlock, \
+ RA_WLOCKED) */
+#define UNP_GLOBAL_WOWNED() rm_wowned(&unp_global_rmlock)
+
+#define UNP_GLOBAL_RLOCK() rm_rlock(&unp_global_rmlock, &tr)
+#define UNP_GLOBAL_RUNLOCK() rm_runlock(&unp_global_rmlock, &tr)
+#define UNP_GLOBAL_RLOCK_ASSERT() /*rm_assert(&unp_global_rmlock, \
+ RA_RLOCKED) */
+
+#define RMLOCK_DECL struct rm_priotracker tr
+
+#else /* !USE_RMLOCKS */
+
static struct rwlock unp_global_rwlock;
#define UNP_GLOBAL_LOCK_INIT() rw_init(&unp_global_rwlock, \
@@ -217,6 +247,10 @@
#define UNP_GLOBAL_RLOCK_ASSERT() rw_assert(&unp_global_rwlock, \
RA_RLOCKED)
+#define RMLOCK_DECL __unused struct rm_priotracker tr
+
+#endif /* !USE_RMLOCKS */
+
#define UNP_PCB_LOCK_INIT(unp) mtx_init(&(unp)->unp_mtx, \
"unp_mtx", "unp_mtx", \
MTX_DUPOK|MTX_DEF|MTX_RECURSE)
@@ -225,9 +259,11 @@
#define UNP_PCB_UNLOCK(unp) mtx_unlock(&(unp)->unp_mtx)
#define UNP_PCB_LOCK_ASSERT(unp) mtx_assert(&(unp)->unp_mtx, MA_OWNED)
+
static int unp_connect(struct socket *, struct sockaddr *,
- struct thread *);
-static int unp_connect2(struct socket *so, struct socket *so2, int);
+ struct thread *, struct rm_priotracker *);
+static int unp_connect2(struct socket *so, struct socket *so2, int,
+ struct rm_priotracker *);
static void unp_disconnect(struct unpcb *unp, struct unpcb *unp2);
static void unp_shutdown(struct unpcb *);
static void unp_drop(struct unpcb *, int);
@@ -295,6 +331,7 @@
{
struct unpcb *unp, *unp2;
const struct sockaddr *sa;
+ RMLOCK_DECL;
/*
* Pass back name of connected socket, if it was bound and we are
@@ -493,10 +530,11 @@
uipc_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
{
int error;
+ RMLOCK_DECL;
KASSERT(td == curthread, ("uipc_connect: td != curthread"));
UNP_GLOBAL_WLOCK();
- error = unp_connect(so, nam, td);
+ error = unp_connect(so, nam, td, &tr);
UNP_GLOBAL_WUNLOCK();
return (error);
}
@@ -526,6 +564,7 @@
{
struct unpcb *unp, *unp2;
int error;
+ RMLOCK_DECL;
UNP_GLOBAL_WLOCK();
unp = so1->so_pcb;
@@ -534,7 +573,7 @@
unp2 = so2->so_pcb;
KASSERT(unp2 != NULL, ("uipc_connect2: unp2 == NULL"));
UNP_PCB_LOCK(unp2);
- error = unp_connect2(so1, so2, PRU_CONNECT2);
+ error = unp_connect2(so1, so2, PRU_CONNECT2, &tr);
UNP_PCB_UNLOCK(unp2);
UNP_PCB_UNLOCK(unp);
UNP_GLOBAL_WUNLOCK();
@@ -753,6 +792,7 @@
u_int mbcnt, sbcc;
u_long newhiwat;
int error = 0;
+ RMLOCK_DECL;
unp = sotounpcb(so);
KASSERT(unp != NULL, ("uipc_send: unp == NULL"));
@@ -782,7 +822,7 @@
error = EISCONN;
break;
}
- error = unp_connect(so, nam, td);
+ error = unp_connect(so, nam, td, &tr);
if (error)
break;
unp2 = unp->unp_conn;
@@ -836,7 +876,7 @@
if ((so->so_state & SS_ISCONNECTED) == 0) {
if (nam != NULL) {
UNP_GLOBAL_WLOCK_ASSERT();
- error = unp_connect(so, nam, td);
+ error = unp_connect(so, nam, td, &tr);
if (error)
break; /* XXX */
} else {
@@ -938,6 +978,7 @@
{
struct unpcb *unp, *unp2;
struct socket *so2;
+ RMLOCK_DECL;
unp = sotounpcb(so);
KASSERT(unp != NULL, ("uipc_sense: unp == NULL"));
@@ -1110,7 +1151,8 @@
}
static int
-unp_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
+unp_connect(struct socket *so, struct sockaddr *nam, struct thread *td,
+ struct rm_priotracker *tr)
{
struct sockaddr_un *soun = (struct sockaddr_un *)nam;
struct vnode *vp;
@@ -1249,7 +1291,7 @@
KASSERT(unp2 != NULL, ("unp_connect: unp2 == NULL"));
UNP_PCB_LOCK(unp);
UNP_PCB_LOCK(unp2);
- error = unp_connect2(so, so2, PRU_CONNECT);
+ error = unp_connect2(so, so2, PRU_CONNECT, tr);
UNP_PCB_UNLOCK(unp2);
UNP_PCB_UNLOCK(unp);
bad2:
@@ -1273,7 +1315,8 @@
}
static int
-unp_connect2(struct socket *so, struct socket *so2, int req)
+unp_connect2(struct socket *so, struct socket *so2, int req,
+ struct rm_priotracker *tr)
{
struct unpcb *unp;
struct unpcb *unp2;
@@ -1359,6 +1402,7 @@
struct xunpgen *xug;
struct unp_head *head;
struct xunpcb *xu;
+ RMLOCK_DECL;
head = ((intptr_t)arg1 == SOCK_DGRAM ? &unp_dhead : &unp_shead);
More information about the freebsd-current
mailing list