Switch pfil(9) to rmlocks

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.

Max,

Have you done performance measurements that show rmlocks to
be a win in this 
scenario?  I did some patchs for UNIX domain sockets to
replace the rwlock 
there but it appeared not to have a measurable impact on SQL
benchmarks, 
presumbaly because the read/write blend wasn't right and/or
that wasnt a 
significant source of overhead in the benchmark.  I'd
anticipate a much more 
measurable improvement for pfil, but would be interested in
learning how much 
is seen?

Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"

On Friday 23 November 2007, Robert Watson wrote:
> 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.
>
> Max,
>
> Have you done performance measurements that show
rmlocks to be a win in
> this scenario?  I did some patchs for UNIX domain
sockets to replace
> the rwlock there but it appeared not to have a
measurable impact on SQL
> benchmarks, presumbaly because the read/write blend
wasn't right and/or
> that wasnt a significant source of overhead in the
benchmark.  I'd
> anticipate a much more measurable improvement for pfil,
but would be
> interested in learning how much is seen?

I don't yet, but will see if I can collect some data later
today.  The 
main reason for the switch is shortcomings in rwlock's (not)

implementation of reader recursion as discussed in -arch
"rwlocks, 
correctness over speed."  rmlocks do that correctly,
afaiu.

Unless an artificial no-op hook is used, I don't expect to
see significant 
performance gain, however.  All current pfil(9) consumer
need some form 
of synchronization of their own, which will probably nullify
the gain 
from rmlocks.

-- 
/"  Best regards,                      | mlaierfreebsd.org
 /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.l
ove2party.net/  | mlaierEFnet
/   ASCII Ribbon Campaign              | Against HTML Mail
and News

On Saturday 24 November 2007, Kris Kennaway wrote:
> Max Laier wrote:
> > On Friday 23 November 2007, Robert Watson wrote:
> >> 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.
> >>
> >> Max,
> >>
> >> Have you done performance measurements that
show rmlocks to be a win
> >> in this scenario?  I did some patchs for UNIX
domain sockets to
> >> replace the rwlock there but it appeared not
to have a measurable
> >> impact on SQL benchmarks, presumbaly because
the read/write blend
> >> wasn't right and/or that wasnt a significant
source of overhead in
> >> the benchmark.  I'd anticipate a much more
measurable improvement
> >> for pfil, but would be interested in learning
how much is seen?
> >
> > I had to roll an artificial benchmark in order to
see a significant
> > change (attached - it's a hack!).
> >
> > Using 3 threads on a 4 CPU machine I get the
following results:
> > null hook: ~13% +/- 2
> > mtx hook: up to 40% [*]
> > rw hook: ~5% +/- 1
> > rm hook: ~35% +/- 5
> >
> > [*] The mtx hook is inconclusive as my
measurements vary a lot.  If
> > one thread gets lucky and keeps running the
overall time obviously
> > goes down by a magnitude.  It seems however, that
rmlocks greatly
> > increase the chance of that happening - not sure
if that's a good
> > thing, though.  If all threads receive
approximately equal runtime
> > (which is almost always the case for rwlocks) the
difference is
> > somewhere around 10%.
>
> Is that something we can try to arrange to happen for
improved
> performance in more general situations?

I don't think so.  It's a scheduling problem, but one you
can't (easily) 
predict.  The gain comes from reduced congestion after one
thread is 
done, which doesn't happen in real world situations.  You
are never done.  
The only way to reduce congestion is to shrink critical
sections and to 
use read locks whenever possible.

-- 
/"  Best regards,                      | mlaierfreebsd.org
 /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.l
ove2party.net/  | mlaierEFnet
/   ASCII Ribbon Campaign              | Against HTML Mail
and News

On Sunday 25 November 2007, Darren Reed wrote:
> Max Laier wrote:
> > On Friday 23 November 2007, Robert Watson wrote:
> > > 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.
> > >
> > > Max,
> > >
> > > Have you done performance measurements that
show rmlocks to be a
> > > win in this scenario?  I did some patchs for
UNIX domain sockets to
> > > replace the rwlock there but it appeared not
to have a measurable
> > > impact on SQL benchmarks, presumbaly because
the read/write blend
> > > wasn't right and/or that wasnt a significant
source of overhead in
> > > the benchmark.  I'd anticipate a much more
measurable improvement
> > > for pfil, but would be interested in learning
how much is seen?
> >
> > I had to roll an artificial benchmark in order to
see a significant
> > change (attached - it's a hack!).
> >
> > Using 3 threads on a 4 CPU machine I get the
following results:
> > null hook: ~13% +/- 2
> > mtx hook: up to 40% [*]
> > rw hook: ~5% +/- 1
> > rm hook: ~35% +/- 5
>
> Is that 13%/5%/35% faster or slower or improvement or
degradation?
> If "rw hook" (using rwlock like we have
today?) is 5%, whas is the
> baseline?
>
> I'm expecting that at least one of these should be a
0%...

Sorry for the sparse explanation.  All numbers above are
gain with rmlocks 
i.e. rmlocks are faster in all scenarios.  The test cases
are different 
hook functions.  Every hook has a DELAY(1) and a lock/unlock
call around 
it of the respective lock type.  read lock acquisitions for
rw and rm.  
Please look at the code I posted a bit later for more
details.

-- 
/"  Best regards,                      | mlaierfreebsd.org
 /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.l
ove2party.net/  | mlaierEFnet
/   ASCII Ribbon Campaign              | Against HTML Mail
and News

On Sunday 25 November 2007, Darren Reed wrote:
> Max Laier wrote:
> > On Sunday 25 November 2007, Darren Reed wrote:
> > > Max Laier wrote:
> > > > On Friday 23 November 2007, Robert
Watson wrote:
> > > > > 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.
> > > > >
> > > > > Max,
> > > > >
> > > > > Have you done performance
measurements that show rmlocks to be
> > > > > a win in this scenario?  I did some
patchs for UNIX domain
> > > > > sockets to replace the rwlock there
but it appeared not to have
> > > > > a measurable impact on SQL
benchmarks, presumbaly because the
> > > > > read/write blend wasn't right
and/or that wasnt a significant
> > > > > source of overhead in the
benchmark.  I'd anticipate a much
> > > > > more measurable improvement for
pfil, but would be interested
> > > > > in learning how much is seen?
> > > >
> > > > I had to roll an artificial benchmark in
order to see a
> > > > significant change (attached - it's a
hack!).
> > > >
> > > > Using 3 threads on a 4 CPU machine I get
the following results:
> > > > null hook: ~13% +/- 2
> > > > mtx hook: up to 40% [*]
> > > > rw hook: ~5% +/- 1
> > > > rm hook: ~35% +/- 5
> > >
> > > Is that 13%/5%/35% faster or slower or
improvement or degradation?
> > > If "rw hook" (using rwlock like we
have today?) is 5%, whas is the
> > > baseline?
> > >
> > > I'm expecting that at least one of these
should be a 0%...
> >
> > Sorry for the sparse explanation.  All numbers
above are gain with
> > rmlocks i.e. rmlocks are faster in all scenarios. 
The test cases are
> > different hook functions.  Every hook has a
DELAY(1) and a
> > lock/unlock call around it of the respective lock
type.  read lock
> > acquisitions for rw and rm. Please look at the
code I posted a bit
> > later for more details.
>
> Thanks for the clarification.
> That makes rmlocks very interesting.
> And the kind of lock that both ipf and ipfw could
benefit from,
> especially since you're planning on changing the pfil
locks to be
> this way.  Are you (or is someone else?) intending on
following
> up moving ipfw to them, where appropriate?

It's unclear yet, where they are appropriate.  As the name
suggests (read 
mostly locks) they are for cases where you acquire a write
lock only once 
in a blue moon.  As such the write lock acquisition is a
very expensive 
operation.  This might be appropriate for some cases of IPFW
rulesets, 
but certainly not the general case.  Things like address
tables and 
dynamic rules (states) take a lot of writes.  It's not yet
clear where 
the balance for rmlocks really is and my benchmark doesn't
answer that.

For pfil the case is obvious as you hardly ever change the
hooks.

> I'm tempted to suggest them to other
platforms...although I'd expect
> some amount nay-saying because of NIH, it would be good
if others
> also picked up on them, if the benefits are this clear
cut...

Again ... only for a select set of cases.  But they are
really great for 
cases where you don't want to use proper synchronization
because of the 
slow down and the relative small chance of ever hitting the
race.  Now we 
have a tool to properly protect against these races without
sacrificing 
performance.

-- 
/"  Best regards,                      | mlaierfreebsd.org
 /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.l
ove2party.net/  | mlaierEFnet
/   ASCII Ribbon Campaign              | Against HTML Mail
and News

Max Laier wrote:
> On Sunday 25 November 2007, Darren Reed wrote:
> > Max Laier wrote:
> > > On Sunday 25 November 2007, Darren Reed
wrote:
> > > > Max Laier wrote:
> > > > > On Friday 23 November 2007, Robert
Watson wrote:
> > > > > > 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.
> > > > > >
> > > > > > Max,
> > > > > >
> > > > > > Have you done performance
measurements that show rmlocks to be
> > > > > > a win in this scenario?  I did
some patchs for UNIX domain
> > > > > > sockets to replace the rwlock
there but it appeared not to have
> > > > > > a measurable impact on SQL
benchmarks, presumbaly because the
> > > > > > read/write blend wasn't right
and/or that wasnt a significant
> > > > > > source of overhead in the
benchmark.  I'd anticipate a much
> > > > > > more measurable improvement
for pfil, but would be interested
> > > > > > in learning how much is seen?
> > > > >
> > > > > I had to roll an artificial
benchmark in order to see a
> > > > > significant change (attached - it's
a hack!).
> > > > >
> > > > > Using 3 threads on a 4 CPU machine
I get the following results:
> > > > > null hook: ~13% +/- 2
> > > > > mtx hook: up to 40% [*]
> > > > > rw hook: ~5% +/- 1
> > > > > rm hook: ~35% +/- 5
> > > >
> > > > Is that 13%/5%/35% faster or slower or
improvement or degradation?
> > > > If "rw hook" (using rwlock
like we have today?) is 5%, whas is the
> > > > baseline?
> > > >
> > > > I'm expecting that at least one of these
should be a 0%...
> > >
> > > Sorry for the sparse explanation.  All
numbers above are gain with
> > > rmlocks i.e. rmlocks are faster in all
scenarios.  The test cases are
> > > different hook functions.  Every hook has a
DELAY(1) and a
> > > lock/unlock call around it of the respective
lock type.  read lock
> > > acquisitions for rw and rm. Please look at
the code I posted a bit
> > > later for more details.
> >
> > Thanks for the clarification.
> > That makes rmlocks very interesting.
> > And the kind of lock that both ipf and ipfw could
benefit from,
> > especially since you're planning on changing the
pfil locks to be
> > this way.  Are you (or is someone else?) intending
on following
> > up moving ipfw to them, where appropriate?
>
> It's unclear yet, where they are appropriate.  As the
name suggests (read 
> mostly locks) they are for cases where you acquire a
write lock only once 
> in a blue moon.  As such the write lock acquisition is
a very expensive 
> operation.  This might be appropriate for some cases of
IPFW rulesets, 
> but certainly not the general case.
...

Which is along the lines of what I was thinking.

Is there any documentation around on just how expensive the
write is 
with these locks?

And if the write is expensive, at what point of the write
acquisition do 
reads get blocked?

Darren

_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"

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_rml
ock,	
+					    "unp_global_rmlock", 0)
+
+#define	UNP_GLOBAL_LOCK_ASSERT()	/*rm_assert(&unp_globa
l_rmlock,	
+					    RA_LOCKED) */
+#define	UNP_GLOBAL_UNLOCK_ASSERT()	/*rm_assert(&unp_glo
bal_rmlock,	
+					    RA_UNLOCKED) */
+
+#define	UNP_GLOBAL_WLOCK()		rm_wlock(&unp_global_rmlock
)
+#define	UNP_GLOBAL_WUNLOCK()		rm_wunlock(&unp_global_rm
lock)
+#define	UNP_GLOBAL_WLOCK_ASSERT()	/*rm_assert(&unp_glob
al_rmlock,	
+					    RA_WLOCKED) */
+#define	UNP_GLOBAL_WOWNED()		rm_wowned(&unp_global_rmlo
ck)
+
+#define	UNP_GLOBAL_RLOCK()		rm_rlock(&unp_global_rmlock
, &tr)
+#define	UNP_GLOBAL_RUNLOCK()		rm_runlock(&unp_global_rm
lock, &tr)
+#define	UNP_GLOBAL_RLOCK_ASSERT()	/*rm_assert(&unp_glob
al_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_rwlo
ck,	
 -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_m
tx)
 
#define	UNP_PCB_LOCK_ASSERT(unp)	mtx_assert(&(unp)->u
np_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-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"