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);

_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to