Re: [PATCH 03/13] srcutree: Add reader-free fastpath to synchronize_srcu_atomic()
From: Paul E. McKenney
Date: Tue Sep 08 2026 - 18:56:02 EST
On Tue, Sep 08, 2026 at 11:26:06PM +0100, David Woodhouse 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 <kunwu.chan@xxxxxxxxx>
> > >
> > > 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 <kunwu.chan@xxxxxxxxx>
> >
> > 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.)
Thank you very much!!!
I am interpreting this to mean that Kunwu's current approach does what
you need, so that we can keep current get_state_synchronize_srcu()
and poll_state_synchronize_srcu() semantics.
Thanx, Paul