In my testing lately I've often run into a high context switch rate over RW
locks.
The RW locks we have now are "half adaptive" so to speak. Lock waiters can
spin while the lock is write locked and the owner is running, because the
owner can be identified in that case, and then it can be determined whether
or not the owner is running on a CPU: if so, the waiter spins, if not, it
sleeps.
The same does not happen if a RW lock is read locked and someone wants to
take a write hold, because in that case there is no identifiable owner, and
so it can't be determined whether the owner(s) are running or not. In that
case the lock waiter always sleeps. It then has a kind of snowball effect
because it delays the lock getting back to its unheld state, and every
thread arriving during the aftermath sleeps too.
If you only take mostly read holds, or only take mostly write holds on an
individual RW lock this isn't a big deal because it occurs rarely. But if
you introduce more of a mix of read/write you start to get the poor
behaviour. The correct answer here may be "don't do that", but it's not a
very practical one.
I'd like to reduce this blocking activity, so here is a patch against
-current that implements the adaptive behaviour in the reader case too, by
having each LWP holding RW locks track those holds, and if going to sleep,
cause any held RW locks to be temporarily put into "sleep mode", with the
default being "spin mode". In my testing with mixed vnode locks this works
well.
http://www.netbsd.org/~ad/2020/rwlock.diff
i guess in rw_downgrade
newown = (owner & RW_NODEBUG) | RW_SPIN;
should be
newown = (owner & (RW_NODEBUG | RW_SPIN));
as the owner might have failed to record the lock on acquisition.
This removes the assembly stubs but the compiler seems to do a pretty good
job on the C ones I added, and in microbenchmarking it the difference is
tiny. I've only included the amd64 MD changes in the diff.
In trying this out on builds it seems that tracking at most 2 holds catches
~~99.8% of activity, but I have made the limit 4 (it's a soft limit, if
exceeded all further taken locks are put into "sleep mode").
I also added a rw_owner_running() for the pagedaemon, to match mutexes (not
used yet).
Thoughts?
cheers,
Andrew