rms: Order a writer's stores before its readers see it leave

Readers check rms->writers without a fence, and rms_wunlock() drops
it with a plain store.  rms_wlock() interrupts every CPU before the
first writer blocks readers, but nothing orders them when the last
writer leaves: on a weakly ordered CPU a reader can see the count
reach zero and still read what the writer replaced, such as callbacks
of a module unloaded under the lock.

Fence every CPU before the last writer drops the count: rename
rms_wlock_switch() to rms_block_readers() and add
rms_unblock_readers(), a wrapper for cpus_fence_seq_cst().  Readers
stay fence-free; the cost lands on the rare unlock.

Reviewed by:		mjg
Discussed with:		kib
Fixes:			1f162fef76 ("Add read-mostly sleepable locks")
MFC after:		2 weeks
Sponsored by:		Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D60658
This commit is contained in:
R. Christian McDonald committed 2026-10-12 16:45:29 -04:00
1 parent 39d6e22264
commit 2b911d947b
1 file changed
+20 -3
+20 -3
View File
@@ -875,7 +875,9 @@ db_show_rm(const struct lock_object *lock)
*
* Neither rms_rlock nor rms_runlock use thread fences. Instead interrupt
* fences are inserted to ensure ordering with the code executed in the IPI
* handler.
* handler. Writers interrupt all CPUs both to block readers and to unblock
* them, so a reader that sees no writer also sees what the last writer
* stored.
*
* No attempt is made to track which CPUs read locked at least once,
* consequently write locking sends IPIs to all of them. This will become a
@@ -1171,7 +1173,7 @@ rms_assert_no_pcpu_readers(struct rmslock *rms)
#endif
static void
rms_wlock_switch(struct rmslock *rms)
rms_block_readers(struct rmslock *rms)
{
struct rmslock_ipi rmsipi;
@@ -1188,6 +1190,19 @@ rms_wlock_switch(struct rmslock *rms)
&rmsipi.srcra);
}
/*
* Order the writer's stores before the count readers check without a fence.
*/
static void
rms_unblock_readers(struct rmslock *rms)
{
MPASS(rms->readers == 0);
MPASS(rms->writers == 1);
cpus_fence_seq_cst();
}
void
rms_wlock(struct rmslock *rms)
{
@@ -1210,7 +1225,7 @@ rms_wlock(struct rmslock *rms)
KASSERT(rms->owner == RMS_NOOWNER,
("%s: unexpected owner value %p\n", __func__, rms->owner));
rms_wlock_switch(rms);
rms_block_readers(rms);
rms_assert_no_pcpu_readers(rms);
if (rms->readers > 0) {
@@ -1235,6 +1250,8 @@ rms_wunlock(struct rmslock *rms)
("%s: unexpected owner value %p\n", __func__, rms->owner));
MPASS(rms->writers >= 1);
MPASS(rms->readers == 0);
if (rms->writers == 1)
rms_unblock_readers(rms);
rms->writers--;
if (rms->writers > 0) {
wakeup_one(&rms->owner);