Re: [PATCH] srcu: Add try_synchronize_srcu() for callers which can prove readers absent

Hi David,

Thanks for the detailed explanations and for addressing the review
comments in v2.

On Fri, Aug 21, 2026 at 7:53 PM David Woodhouse <[email protected]> wrote:
>
> On Fri, 2026-08-21 at 11:22 +0800, Kunwu Chan wrote:
> > If the common case is that there are no readers and the new fast path
> > is intended to avoid the SRCU grace-period machinery, could you explain
> > what property of SRCU is still required by the proposed users that
> > prevents using ordinary RCU instead?
>
> My original version did use RCU, but Sean asked me to switch to a
> dedicated per-vm SRCU¹ because a system-wide grace period doesn't work
> well on large KVM hosting systems.

The rationale for the per-VM SRCU and the no-reader fast path is much
clearer now. In particular, I understand that the per-VM SRCU is mainly
about having a dedicated synchronization domain, while the try path
avoids the unnecessary workqueue and scheduling latency in the
no-reader case.

>
> ¹ https://lore.kernel.org/all/[email protected]/
>
> > Could this use case instead be addressed by making exp_holdoff
> > configurable per srcu_struct, or otherwise allowing a dedicated SRCU
> > domain to use a more aggressive expedite policy?
>
> That doesn't help much; the other concern is latency. And the mere act
> of bouncing to the workqueue only to realise we can wake up again was
> adding 100µs or so every time for nothing. *Plus* once you schedule at
> all, you start to see really high tail latencies of >20ms too — which
> is why we've also started looking at synchronize_srcu_atomic() to
> eliminate that completely. If we get *that*, then maybe I don't
> personally care as much about try_synchronize_srcu(), although it might
> be a useful extension to synchronize_srcu_expedited() — this *would* be
> the 'more aggressive expedite policy' of which you speak, for the case
> where there are no readers.
>

In particular, tuning exp_holdoff would not address the workqueue round-trip
and the resulting tail latency.

> > Could you also clarify why observing no readers during this
> > check is sufficient for the reclamation/happens-before guarantee
> > required by the proposed callers?
>
> Both halves of the grace-period guarantee are provided when it returns
> true. For any reader, either:
>
>    • its lock increment was observed by the sums and its unlock was
>      too — the reader's final smp_mb() then orders its whole critical
>      section before the caller's subsequent accesses (that reader has
>      completed), or
>
>    • its lock increment was not observed — in which case the
>      store-buffering pairing with the smp_mb() in __srcu_read_lock()
>      guarantees that reader observes every store the caller made
>      before calling: it is a "new" reader for the purposes of the
>      grace period, exactly as if it had started after a real
>      synchronize_srcu() returned, or
>
>    • its lock increment was observed but its unlock was not — the one
>      case where the reader may still be running with no guarantee it
>      saw anything. But that reader inflates the lock sum above the
>      unlock sum, so this is precisely the case in which we return
>      false and the caller takes the real grace period.
>
> It's the same argument srcu_readers_active_idx_check() makes; the only
> novelty is summing both epochs so no flip is needed.
>
> > Since a successful try_synchronize_srcu() does not advance srcu_gp_seq,
> > flip the SRCU index, or process callbacks, could we document more
> > explicitly that a successful try provides the required
> > reclamation/happens-before guarantee to the caller, but does not
> > constitute an SRCU grace-period completion for the state/callback APIs?
>
> Yes; done in v2.
>
> > What ordering edge does the first smp_mb() establish that is not
> > already provided by the middle and final barriers?
>
> Er.... none? :)
>
> We need the caller's prior stores ordered before the *lock*-counter
> reads, but the middle barrier already does that for us. Dropped in v2,
> and the comment on the middle barrier updated.
>
> > Would it make sense to perform the same lockdep check in the try path?
>
> Ack.
>
> > When applying this to current rcu/dev, the patch also needs to account
> > for the additional bool is_atomic argument to
> > check_init_srcu_struct(). Please rebase accordingly.
>
> Ooh, shiny, I hadn't spotted that the atomic thing had landed. Maybe I
> should retract my 'if we get that' above... but let's see how it
> survives.
>
> I also have more than just this patch to rebase onto rcu/dev — unlike
> some, I'm not just hacking RCU for the fun of it; I get my jollies
> elsewhere :)
>
> Btw, the current commit c68271c3f83a message says "no index flip, no
> grace-period sequence update" but that isn't true of the implementation
> any more, is it? In fact, it *always* does those things, and doesn't
> take the try-path optimisation by counting both epochs, that I'm doing
> here?
>

The v2 changes address the points I raised, including removing the
redundant leading smp_mb(), adding the lockdep check, and clarifying
that a successful try does not constitute an SRCU grace-period
completion for the state/callback APIs.

I'll take another look at v2, in particular the two-epoch counter proof
and its correspondence with srcu_readers_active_idx_check().

Thanks again for the detailed explanations.

Thanks,
Kunwu

> Thanks for the careful review.