On Sun, May 08, 2022 at 07:55:44PM +0200, Alexandr Nedvedicky wrote:
> my question is why do we reset RWLOCK_WAIT and RWLOCK_WRWANT flags?
This is a very good question.
> I think those flags should be reset the last reader gone. Perhaps
> the else branch for reader requires this:
>
> else {
> set = (owner - RWLOCK_READ_INCR) &
> ~(RWLOCK_WAIT|RWLOCK_WRWANT)
> if (set != 0)
> set |= (owner & RWLOCK_MASK);
> }
Why should a reader change RWLOCK_WRWANT at all? The writer sets
and clears it. This code was moved to this place in rev 1.8.
Before rw_exit_read() had this comment:
/*
* Potential MP race here. If the owner had WRWANT set we cleared
* it and a reader can sneak in before a writer. Do we care?
*/
I will run my tests with the diff below.
bluhm
Index: kern/kern_rwlock.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.47
diff -u -p -r1.47 kern_rwlock.c
--- kern/kern_rwlock.c 8 Feb 2021 08:18:45 -0000 1.47
+++ kern/kern_rwlock.c 8 May 2022 18:55:52 -0000
@@ -81,7 +81,7 @@ static const struct rwlock_op {
},
{ /* RW_READ */
RWLOCK_READ_INCR,
- RWLOCK_WRLOCK,
+ RWLOCK_WRLOCK | RWLOCK_WRWANT,
RWLOCK_WAIT,
0,
PLOCK
@@ -103,7 +103,7 @@ rw_enter_read(struct rwlock *rwl)
{
unsigned long owner = rwl->rwl_owner;
- if (__predict_false((owner & RWLOCK_WRLOCK) ||
+ if (__predict_false((owner & (RWLOCK_WRLOCK | RWLOCK_WRWANT)) ||
rw_cas(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
rw_enter(rwl, RW_READ);
else {
@@ -343,8 +343,7 @@ rw_do_exit(struct rwlock *rwl, unsigned
if (wrlock)
set = 0;
else
- set = (owner - RWLOCK_READ_INCR) &
- ~(RWLOCK_WAIT|RWLOCK_WRWANT);
+ set = (owner - RWLOCK_READ_INCR) & ~RWLOCK_WAIT;
} while (__predict_false(rw_cas(&rwl->rwl_owner, owner, set)));
if (owner & RWLOCK_WAIT)
