[patch] rw_try_wlock does not set recursive bit when recursing

Andrew Brampton brampton+freebsd at gmail.com
Wed Aug 26 22:37:57 UTC 2009


Hi,
The following sequence of commands fails on line 4 with an assertion
that the lock is not currently held:

1: rw_wlock(&rw);
2: if ( rw_try_wlock(&rw) )
3:    rw_wunlock(&rw);
4: rw_wunlock(&rw);

This is because after line 3 is executed the rw lock is no longer
held. I tracked this bug down to _rw_try_wlock which correctly
increments rw_recurse, but does not set the RW_LOCK_RECURSED bit.
Without this bit the third line unlocks the lock and leaves it in a
unlocked state (when it should still be locked). Adding a line to set
this bit makes _rw_try_wlock match the code in _rw_wlock_hard.

I have attached a one line patch which fixes this problem on my 7.2
system, and looking over the CURRENT source it should also apply
cleanly (but I have not tested it on CURRENT). I have also attached
another (slightly longer) patch which fixes the problem as well as
adding an a extra assert. It is up to the FreeBSD developer to decide
which patch they prefer.

thanks
Andrew
-------------- next part --------------
Index: kern_rwlock.c
===================================================================
--- kern_rwlock.c	(revision 191765)
+++ kern_rwlock.c	(working copy)
@@ -202,8 +202,12 @@ _rw_try_wlock(struct rwlock *rw, const char *file,
 	KASSERT(rw->rw_lock != RW_DESTROYED,
 	    ("rw_try_wlock() of destroyed rwlock @ %s:%d", file, line));
 
-	if (rw_wlocked(rw) && (rw->lock_object.lo_flags & RW_RECURSE) != 0) {
+	if (rw_wlocked(rw)) {
+		KASSERT(rw->lock_object.lo_flags & RW_RECURSE,
+		    ("%s: recursing but non-recursive rw %s @ %s:%d\n",
+		    __func__, rw->lock_object.lo_name, file, line));
 		rw->rw_recurse++;
+		atomic_set_ptr(&rw->rw_lock, RW_LOCK_RECURSED);
 		rval = 1;
 	} else
 		rval = atomic_cmpset_acq_ptr(&rw->rw_lock, RW_UNLOCKED,
-------------- next part --------------
Index: kern_rwlock.c
===================================================================
--- kern_rwlock.c	(revision 191765)
+++ kern_rwlock.c	(working copy)
@@ -204,6 +204,7 @@ _rw_try_wlock(struct rwlock *rw, const char *file,
 
 	if (rw_wlocked(rw) && (rw->lock_object.lo_flags & RW_RECURSE) != 0) {
 		rw->rw_recurse++;
+		atomic_set_ptr(&rw->rw_lock, RW_LOCK_RECURSED);
 		rval = 1;
 	} else
 		rval = atomic_cmpset_acq_ptr(&rw->rw_lock, RW_UNLOCKED,


More information about the freebsd-current mailing list