On Tue, 08 Sep 2026 23:26:06 +0100 David Woodhouse <[email protected]> wrote:
> On Tue, 2026-09-08 at 13:29 -0700, Paul E. McKenney wrote: > > On Mon, Sep 07, 2026 at 03:58:19PM +0800, Kunwu Chan wrote: > > > From: Kunwu Chan <[email protected]> > > > > > > synchronize_srcu_atomic() is restricted to srcu_read_lock_atomic() and > > > srcu_read_unlock_atomic(), whose read-side critical sections disable > > > preemption. In the common case where there are no readers at all, the > > > grace period therefore need not do the index flip. Add a fastpath > > > that sums both ranks of the per-CPU ->srcu_ctrs[] counters and, if the > > > lock counts match the unlock counts on both ranks, ends the grace > > > period immediately, skipping the srcu_advance_state() scans, mirroring > > > the similar Tiny SRCU fastpath. > > > > > > Correctness requires the counter-sum proof to follow the grace-period > > > anchor written by srcu_gp_start(); placing it before the anchor could > > > let this grace period miss a pre-existing reader and return without > > > waiting for it. The smp_mb() between the unlock and lock sums pairs > > > with the smp_mb() in __srcu_read_lock(). The grace period is ended > > > manually under ->lock and ->srcu_atomic_gp_flag. > > > > > > Signed-off-by: Kunwu Chan <[email protected]> > > > > Adding David Woodhouse on CC. > > > > David, does this provide appropriate performance in your use case? > > Compared with the early bail that I had before, there's a *slight* > shift into the higher-latency buckets as expected, but definitely not > enough that I care. We're doing this whole thing to eliminate the > multi-millisecond p100 latencies that happen when we invoke the > workqueue, and the differences we're looking at in the table below are > *well* below what we care about: > > > ┌─────────────┬───────────────────┬───────────────────┬───────┐ > │ bucket │ try-first (#10) │ Kunwu (#11) │ Δ │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [0,1µs) │ 5,338,474 (60.8%) │ 4,953,657 (56.8%) │ −7% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [1,2) │ 1,077,998 │ 1,087,133 │ ≈ │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [2,4) │ 803,244 │ 790,489 │ ≈ │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [4,8) │ 811,708 │ 880,673 │ +8% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [8,16) │ 366,183 │ 597,443 │ +63% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [16,32) │ 338,731 │ 361,243 │ +7% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [32,64) │ 14,290 │ 28,823 │ ×2.0 │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [64,128) │ 2,549 │ 3,655 │ +43% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [128,256) │ 555 │ 783 │ +41% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [256,512) │ 456 │ 625 │ +37% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ [512µs+) │ 13 │ 8 │ ≈ │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ max │ 1.07ms │ 1.56ms │ +46% │ > ├─────────────┼───────────────────┼───────────────────┼───────┤ > │ total walks │ 8,777,514 │ 8,723,453 │ −0.6% │ > └─────────────┴───────────────────┴───────────────────┴───────┘ > > (192-CPU PREEMPT_RT host, 12 concurrent gfn_to_pfn_cache invalidation > reproducers, 300-second windows; each "walk" is one invalidation drain > which includes the synchronize_srcu_atomic() call. #10/#11 are just > local build numbers.) > Thanks, David, for testing this with your KVM workload and confirming the current approach works well. I’ll keep looking into the fastpath for synchronize_srcu_expedited() as well, while preserving the existing SRCU semantics. Thanks, KunWu
