Re: [PATCH v2 1/3] xen/rcu: introduce the concept of RCU epoch

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Wed, Sep 30, 2026 at 07:58:55AM +0200, Jan Beulich wrote:
> On 30.09.2026 00:14, Alejandro Vallejo wrote:
> > On Tue Sep 29, 2026 at 4:52 PM CEST, Jan Beulich wrote:
> >> On 29.09.2026 16:20, Roger Pau Monné wrote:
> >>> On Mon, Sep 28, 2026 at 10:24:04AM +0200, Alejandro Vallejo wrote:
> >>>> On Fri Sep 25, 2026 at 6:30 PM CEST, Roger Pau Monne wrote:
> >>>>>  static inline void rcu_quiesce_disable(void)
> >>>>>  {
> >>>>> +    unsigned int cpu = smp_processor_id();
> >>>>> +
> >>>>>      preempt_disable();
> >>>>> -    this_cpu(rcu_lock_cnt)++;
> >>>>> -    barrier();
> >>>>> +    if ( !ACCESS_ONCE(per_cpu(rcu_lock_cnt, cpu))++ )
> >>>>> +        ACCESS_ONCE(per_cpu(rcu_lock_epoch, cpu)) = 
> >>>>> ACCESS_ONCE(rcu_epoch);
> >>>>
> >>>> I'm not terribly convinced about using ACCESS_ONCE() for atomic
> >>>> accesses. In particular...
> >>>>
> >>>>> +    smp_mb();
> >>>>>  }
> >>>>>  
> >>>>>  static inline void rcu_quiesce_enable(void)
> >>>>>  {
> >>>>> -    barrier();
> >>>>> -    this_cpu(rcu_lock_cnt)--;
> >>>>> +
> >>>>> +    smp_mb();
> >>>>> +    ACCESS_ONCE(this_cpu(rcu_lock_cnt))--;
> >>>>
> >>>> ... this variable is to be read by remote CPUs and autodecrement seems
> >>>> like the wrong tool for an atomic decrement.
> >>>
> >>> I can use {read,write}_atomic() instead, but there's been a tendency
> >>> to phase out the usage of those functions.  On x86 this should be a
> >>> plain `addl` or `subl` instruction (the non-locked version of
> >>> atomic_{inc,dec}()).
> > 
> > I don't think the compiler can merge volatile reads and writes. It can
> > shatter them (thus my fear), but they are meant to be distinct because
> > each might have different side effects. Jan already mentioned this in
> > other words.
> > 
> > You don't need an atomic decrement though, just an atomic write. Note
> > IRQs restore it on exit and other CPUs don't write to it.
> > 
> >   unsigned int *lock_cnt = &this_cpu(rcu_lock_cnt);
> >   write_atomic(lock_cnt, *lock_cnt - 1);
> > 
> > and this yields a plain mov, through a static inline stub (plus the
> > mov from the read access + dec)
> > 
> > The problem with ACCESS_ONCE() is that the write isn't guaranteed to be
> > atomic.
> 
> It may not be properly written down anywhere, but at least the compilers
> we use are known to not tear aligned accesses up to a machine word in
> width.

Then I possibly don't need the ACCESS_ONCE() at all, as I was using it
to guarantee unshattered accesses.  However this should be written
somewhere, maybe in docs/process/coding-best-practices.pandoc?

Thanks, Roger.