Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 26 Aug 2009 23:37:55 +0100
From:      Andrew Brampton <brampton+freebsd@gmail.com>
To:        freebsd-current@freebsd.org
Subject:   [patch] rw_try_wlock does not set recursive bit when recursing
Message-ID:  <d41814900908261537r1726c420p11d3547afde98bbc@mail.gmail.com>

index | next in thread | raw e-mail

[-- Attachment #1 --]
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

[-- Attachment #2 --]
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,

[-- Attachment #3 --]
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,
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d41814900908261537r1726c420p11d3547afde98bbc>