On Wed, Sep 2, 2026 at 11:40 PM Paul E. McKenney <[email protected]> wrote:
>
> On Wed, Sep 02, 2026 at 05:47:14PM +0800, KunWu Chan wrote:
> > On Wed, Sep 2, 2026 at 1:20 PM Paul E. McKenney <[email protected]> wrote:
> > > On Wed, Sep 02, 2026 at 11:05:22AM +0800, KunWu Chan wrote:
> > > > On Wed, Sep 2, 2026 at 12:52 AM Paul E. McKenney <[email protected]> 
> > > > wrote:
> > > > > On Tue, Sep 01, 2026 at 04:29:05PM +0800, KunWu Chan wrote:
> > > > > > On Tue, Sep 1, 2026 at 11:10 AM Paul E. McKenney 
> > > > > > <[email protected]> wrote:
> > > > > > > On Tue, Sep 01, 2026 at 10:00:24AM +0800, KunWu Chan wrote:
> > > > > > > > On Tue, Sep 1, 2026 at 9:04 AM Paul E. McKenney 
> > > > > > > > <[email protected]> wrote:
> > > > > > > > > On Mon, Aug 31, 2026 at 03:49:37PM +0800, Kunwu Chan wrote:
> > > > > > > > > > From: Kunwu Chan <[email protected]>
> > > > > > > > > > Add atomic SRCU operation checks and the associated state 
> > > > > > > > > > to Tiny
> > > > > > > > > > SRCU.
> > > > > > > > > >
> > > > > > > > > > An atomic SRCU domain does not use the normal SRCU callback 
> > > > > > > > > > and
> > > > > > > > > > grace-period machinery.  In particular, a callback queued 
> > > > > > > > > > with
> > > > > > > > > > call_srcu() would never be processed.  Use WARN_ON_ONCE() 
> > > > > > > > > > to reject
> > > > > > > > > > call_srcu() and srcu_barrier() on atomic SRCU domains.
> > > > > > > > > >
> > > > > > > > > > For synchronize_srcu(), redirect atomic SRCU domains to
> > > > > > > > > > synchronize_srcu_atomic().
> > > > > > > > > >
> > > > > > > > > > Add srcu_reader_flavor to the Tiny SRCU state for these 
> > > > > > > > > > checks.
> > > > > > > > > > Tiny SRCU does not currently set the flavor for atomic SRCU 
> > > > > > > > > > domains,
> > > > > > > > > > but keeping the flavor in the common state allows the 
> > > > > > > > > > operation
> > > > > > > > > > checks to enforce the restriction once atomic flavor 
> > > > > > > > > > tracking is
> > > > > > > > > > enabled.
> > > > > > > > > >
> > > > > > > > > > Also initialize srcu_atomic_gp_flag, which was previously 
> > > > > > > > > > left
> > > > > > > > > > uninitialized.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Kunwu Chan <[email protected]>
> > > > > > > > > > ---
> > > > > > > > > >  include/linux/srcutiny.h |  1 +
> > > > > > > > > >  kernel/rcu/srcutiny.c    | 12 ++++++++++++
> > > > > > > > > >  2 files changed, 13 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/include/linux/srcutiny.h 
> > > > > > > > > > b/include/linux/srcutiny.h
> > > > > > > > > > index 47a368f945e3..2b293336525a 100644
> > > > > > > > > > --- a/include/linux/srcutiny.h
> > > > > > > > > > +++ b/include/linux/srcutiny.h
> > > > > > > > > > @@ -20,6 +20,7 @@ struct srcu_struct {
> > > > > > > > > >       u8 srcu_gp_running;             /* GP workqueue 
> > > > > > > > > > running? */
> > > > > > > > > >       u8 srcu_gp_waiting;             /* GP waiting for 
> > > > > > > > > > readers? */
> > > > > > > > > >       u8 srcu_atomic_gp_flag;         /* Serialize atomic 
> > > > > > > > > > GP work.*/
> > > > > > > > > > +     u8 srcu_reader_flavor;          /* Values: 
> > > > > > > > > > SRCU_READ_FLAVOR_.*  */
> > > > > > > > > >       unsigned long srcu_idx;         /* Current reader 
> > > > > > > > > > array element in bit 0x2. */
> > > > > > > > > >       unsigned long srcu_idx_max;     /* Furthest future 
> > > > > > > > > > srcu_idx request. */
> > > > > > > > > >       struct swait_queue_head srcu_wq;
> > > > > > > > > > diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> > > > > > > > > > index 26ea4bfbeaf2..22f7716cbb0e 100644
> > > > > > > > > > --- a/kernel/rcu/srcutiny.c
> > > > > > > > > > +++ b/kernel/rcu/srcutiny.c
> > > > > > > > > > @@ -41,6 +41,7 @@ static int init_srcu_struct_fields(struct 
> > > > > > > > > > srcu_struct *ssp)
> > > > > > > > > >       ssp->srcu_cb_tail = &ssp->srcu_cb_head;
> > > > > > > > > >       ssp->srcu_gp_running = false;
> > > > > > > > > >       ssp->srcu_gp_waiting = false;
> > > > > > > > > > +     ssp->srcu_atomic_gp_flag = 0;
> > > > > > > > >
> > > > > > > > > Good catch!  I will be folding this into the base commit with 
> > > > > > > > > attribution
> > > > > > > > > on my next rebase:
> > > > > > > > >
> > > > > > > > > 9a2e9996ccec ("srcutiny: Add an atomic Tiny SRCU")
> > > > > > > > >
> > > > > > > > > >       ssp->srcu_idx = 0;
> > > > > > > > > >       ssp->srcu_idx_max = 0;
> > > > > > > > > >       INIT_WORK(&ssp->srcu_work, srcu_drive_gp);
> > > > > > > > > > @@ -289,6 +290,9 @@ EXPORT_SYMBOL_GPL(srcu_defer_drain);
> > > > > > > > > >  void call_srcu(struct srcu_struct *ssp, struct rcu_head 
> > > > > > > > > > *rhp,
> > > > > > > > > >              rcu_callback_t func)
> > > > > > > > > >  {
> > > > > > > > > > +     if (WARN_ON_ONCE(ssp->srcu_reader_flavor == 
> > > > > > > > > > SRCU_READ_FLAVOR_ATOMIC))
> > > > > > > > > > +             return;
> > > > > > > > > > +
> > > > > > > > > >       if (should_rcu_defer()) {
> > > > > > > > > >               /* A re-entrant call_srcu() during the drain 
> > > > > > > > > > would livelock it. */
> > > > > > > > > >               if (READ_ONCE(srcu_defer_draining) && 
> > > > > > > > > > !in_nmi()) {
> > > > > > > > > > @@ -319,6 +323,11 @@ void synchronize_srcu(struct 
> > > > > > > > > > srcu_struct *ssp)
> > > > > > > > > >  {
> > > > > > > > > >       struct rcu_synchronize rs;
> > > > > > > > > >
> > > > > > > > > > +     if (WARN_ON_ONCE(ssp->srcu_reader_flavor == 
> > > > > > > > > > SRCU_READ_FLAVOR_ATOMIC)) {
> > > > > > > > > > +             synchronize_srcu_atomic(ssp);
> > > > > > > > > > +             return;
> > > > > > > > > > +     }
> > > > > > > > > > +
> > > > > > > > > >       srcu_lock_sync(&ssp->dep_map);
> > > > > > > > > >
> > > > > > > > > >       RCU_LOCKDEP_WARN(lockdep_is_held(ssp) ||
> > > > > > > > > > @@ -415,6 +424,9 @@ 
> > > > > > > > > > EXPORT_SYMBOL_GPL(synchronize_srcu_atomic);
> > > > > > > > > >  /* Register any deferred callbacks, then wait for all 
> > > > > > > > > > in-flight ones. */
> > > > > > > > > >  void srcu_barrier(struct srcu_struct *ssp)
> > > > > > > > > >  {
> > > > > > > > > > +     if (WARN_ON_ONCE(ssp->srcu_reader_flavor == 
> > > > > > > > > > SRCU_READ_FLAVOR_ATOMIC))
> > > > > > > > > > +             return;
> > > > > > > > > > +
> > > > > > > > > >       __srcu_defer_drain(ssp);
> > > > > > > > > >       synchronize_srcu(ssp);
> > > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > The rest is good as far as it goes, but don't we need to set 
> > > > > > > > > the value
> > > > > > > > > of ssp->srcu_reader_flavor somewhere for atomic srcu_struct 
> > > > > > > > > structures?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Hi Paul,
> > > > > > > >
> > > > > > > > Yes, agreed. I have the flavor tracking changes implemented 
> > > > > > > > locally
> > > > > > > > and am testing them now.
> > > > > > > > I’ll send the updated patch shortly.
> > > > > > > >
> > > > > > > > I also have draft patches for the documentation and the fast 
> > > > > > > > path, as
> > > > > > > > well as rcutorture testing for
> > > > > > > > tiny atomic srcu , which I’ll send separately.
> > > > > > >
> > > > > > > Sounds good, and I am looking forward to seeing them.
> > > > > >
> > > > > > Hi Paul,
> > > > > > I’ve sent the flavor tracking changes as [1].
> > > > > > [1] 
> > > > > > https://lore.kernel.org/all/[email protected]/
> > > > > >
> > > > > > >
> > > > > > > I will admit that I am curious as to why Tiny Atomic SRCU needs 
> > > > > > > different
> > > > > > > rcutorture testing than does Tree Atomic SRCU, but I will ask 
> > > > > > > myself
> > > > > > > that question again when I see your patches.  ;-)
> > > > > >
> > > > > > You’re right that the existing SRCU torture configuration can 
> > > > > > exercise
> > > > > > the atomic path with rcutorture.reader_flavor=0x10. I had prepared
> > > > > > the Tiny atomic torture test as a separate draft, mainly adding 
> > > > > > local
> > > > > > configurations for convenience while validating the Tiny and Tree
> > > > > > atomic paths. There is no Tiny-specific torture model here.
> > > > > >
> > > > > > I can adjust the final rcutorture configuration and naming based on 
> > > > > > your
> > > > > > preference.
> > > > >
> > > > > The best approach is to add a section to this script:
> > > > >
> > > > > tools/testing/selftests/rcutorture/bin/torture.sh
> > > > >
> > > > > This would require adding a command-line parameter such as
> > > > > --do-atomic-srcu and friends.  The effect would be that people like
> > > > > me would run short tests of atomic SRCU frequently.
> > > >
> > > > Thanks Paul. I’ll add an atomic SRCU option to
> > > > torture.sh so that it can be run as a short, frequent test.
> > >
> > > Very good, looking forward to seeing it!
> > >
> > > > During my atomic SRCU testing, I noticed that kvm-recheck-rcu.sh
> > > > can report “Reader Batch close calls” even though the kernel reports
> > > > End of test: SUCCESS with no WARNING/BUG. The atomic GP rate is also
> > > > much higher than normal SRCU.
> > > >
> > > > I’m still checking whether this is inherent to atomic SRCU, related to
> > > > my earlier fastpath change, or an interaction with the torture reader
> > > > delay. I’ll continue investigating this as part of the testing work.
> > >
> > > The close calls can happen due to memory reordering.  But this is new
> > > code, so please do check them carefully.
> >
> > Thanks, Paul. That gives me a useful reference point.
> >
> > > The high atomic grace-period rate is expected behavior.  On my x86
> > > laptop, I see about 75/second for Tree Atomic SRCU and about 300
> > > for Tiny.  But at your end, how high is high?
> >
> > On my x86 PC, with the earlier fastpath change, I was seeing:
> > Tree Atomic SRCU: about 250–530 GPs/sec
> > Tiny Atomic SRCU: about 290 GPs/sec
> >
> > After reverting the fastpath change, Tree Atomic SRCU dropped to about
> > 129 GPs/sec, while Tiny remained around 291 GPs/sec. So the Tiny result
> > is close to your ~300/sec reference, while the Tree result is somewhat
> > higher than your ~75/sec observation.
> >
> > The close-call behavior also changed with the fastpath. With the fastpath,
> > batch[2] was non-zero (6–334 in my runs). After reverting it, batch[2] was
> > consistently 0. batch[1] can still occur on Tree SRCU, but I have not
> > seen readers spanning 3 or more GPs.
>
> These results suggest that you should very carefully check memory ordering
> in the fastpath code.
>

Thanks, Paul. I appreciate the reminder.
I'll carefully verify the memory-ordering assumptions in the fastpath and
run more torture tests with the relevant configurations/architectures.

> > I'll continue checking the close-call behavior with repeated torture runs, 
> > with
> > particular attention to the memory-ordering aspect you mentioned.
>
> Sounds good, thank you!

Thanks again!
Kunwu

>
>                                                         Thanx, Paul

Reply via email to