[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