Re: [RFC PATCH v4 3/4] hazptr: Implement Hazard Pointers

[Frederic Weisbecker <frederic@kernel.org>]

Le Fri, Dec 19, 2025 at 09:22:19AM -0500, Mathieu Desnoyers a écrit :
> On 2025-12-18 19:43, Boqun Feng wrote:
> > On Thu, Dec 18, 2025 at 12:35:18PM -0500, Mathieu Desnoyers wrote:
> > [...]
> > > > Could you utilize this[1] to see a
> > > > comparison of the reader-side performance against RCU/SRCU?
> > > 
> > > Good point ! Let's see.
> > > 
> > > On a AMD 2x EPYC 9654 96-Core Processor with 192 cores,
> > > hyperthreading disabled,
> > > CONFIG_PREEMPT=y,
> > > CONFIG_PREEMPT_RCU=y,
> > > CONFIG_PREEMPT_HAZPTR=y.
> > > 
> > > scale_type                 ns
> > > -----------------------
> > > hazptr-smp-mb             13.1   <- this implementation
> > > hazptr-barrier            11.5   <- replace smp_mb() on acquire with barrier(), requires IPIs on synchronize.
> > > hazptr-smp-mb-hlist       12.7   <- replace per-task hp context and per-cpu overflow lists by hlist.
> > > rcu                       17.0
> > 
> > Hmm.. now looking back, how is it possible that hazptr is faster than
> > RCU on the reader-side? Because a grace period was happening and
> > triggered rcu_read_unlock_special()? This is actualy more interesting.
> So I could be entirely misreading the code, but, we have:
> 
> rcu_flavor_sched_clock_irq():
> [...]
>         /* If GP is oldish, ask for help from rcu_read_unlock_special(). */
>         if (rcu_preempt_depth() > 0 &&
>             __this_cpu_read(rcu_data.core_needs_qs) &&
>             __this_cpu_read(rcu_data.cpu_no_qs.b.norm) &&
>             !t->rcu_read_unlock_special.b.need_qs &&
>             time_after(jiffies, rcu_state.gp_start + HZ))
>                 t->rcu_read_unlock_special.b.need_qs = true;
> 
> which means we set need_qs = true as a result from observing
> cpu_no_qs.b.norm == true.
> 
> This is sufficient to trigger calls (plural) to rcu_read_unlock_special()
> from __rcu_read_unlock.
> 
> But then if we look at rcu_preempt_deferred_qs_irqrestore()
> which we would expect to clear the rcu_read_unlock_special.b.need_qs
> state, we have this:
> 
>         special = t->rcu_read_unlock_special;
>         if (!special.s && !rdp->cpu_no_qs.b.exp) {
>                 local_irq_restore(flags);
>                 return;
>         }
>         t->rcu_read_unlock_special.s = 0;
> 
> which skips over clearing the state unless there is an expedited
> grace period required.
> 
> So unless I'm missing something, we should _also_ clear that state
> when it's invoked after rcu_flavor_sched_clock_irq, so the next
> __rcu_read_unlock won't all call into rcu_read_unlock_special().
> 
> I'm adding a big warning about sleep deprivation and possibly
> misunderstanding the whole thing. What am I missing ?

As far as I can tell, this skips clearing the state if the state is
already cleared. Or am I even more sleep deprived than you? :o)

Thanks.

-- 
Frederic Weisbecker
SUSE Labs