mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Boqun Feng <boqun@kernel.org>
Cc: "Paul E . McKenney" <paulmck@kernel.org>,
	linux-kernel@vger.kernel.org,
	Bradley Morgan <brads@mainlining.org>,
	Gary Guo <gary@garyguo.net>,
	rcu@vger.kernel.org, lkmm@lists.linux.dev
Subject: Re: [PATCH hazptr 4/4] hazptr: Introduce "try acquire" fast path, fallback to overflow list
Date: Sun, 27 Sep 2026 13:15:39 -0400	[thread overview]
Message-ID: <5c78c338-1be6-456b-b963-bcfc62748aab@efficios.com> (raw)
In-Reply-To: <arlHFVT5EzcX4d1-@MacBook-0RXW5>

On 2026-09-27 12:40, Boqun Feng wrote:
> On Sun, Sep 27, 2026 at 11:51:31AM -0400, Mathieu Desnoyers wrote:
>> Introduce a "try acquire" hazard pointer fast path, which performs an
>> early load of the address to store it into the hazard pointer slot, and
>> then re-loads that address after a barrier to check whether it has
>> changed meanwhile.
>>
>> On comparison failure, rather than re-try, guarantee forward progress by
>> falling back to the __hazptr_acquire slow path on failure.
>>
>> The acquire slow path attempts a try-acquire for any available per-CPU
>> slot. If that fails, it chains the backup slot into the overflow list,
>> therefore guaranteeing forward progress for both hazard pointer
>> read-side and synchronize:
>>
>> - Readers set the wildcard, and then proceed to set the more
>>    specific address to replace the wildcard.
>>
>> - One synchronize alternates between two overflow list periods,
>>    scanning each one while readers are added to the other period,
>>    thus preventing a steady flow of readers from preventing
>>    synchronize forward progress.
>>
>> With this change, the scan on per-CPU slots don't need to expect a
>> wildcard anymore, because none can be produced by readers. Wildcards are
>> only expected within overflow lists.
>>
> 
> Ok, I was missing something, but I think it's better to call it out.
> Wildcards can only exist in the overflow lists when the context is not
> preemptible. In other words, there won't be a preempted readers blocking
> the synchronize_hazptr() with a wilcard in the overflow list.

Exactly ! Wildcard slots only exist during the short time-frame of the
preempt-off read-side code region (few instructions). And with this
patch, this does not even happen very often, because the fast path don't
rely on the wildcards.

> 
> So no more design trade-off question from me :)
> 

Are you sure ? Scrolling down....

>> @@ -196,16 +197,13 @@ void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
>>   	for_each_possible_cpu(cpu) {
>>   		/*
>>   		 * Scan CPU slots.
>> -		 * Forward progress against recurring wildcards is guaranteed
>> -		 * by scanning for one wildcard while new elements use the
>> -		 * other wildcard value (1UL vs 2UL).
>>   		 * Forward progress against recurring single hazard pointer
>>   		 * values is guaranteed by the fact that a hazard pointer
>>   		 * is not reclaimed nor reused until the scan for that hazard
>>   		 * pointer completes, which prevents a steady flow of readers
>>   		 * to acquire that same hazard pointer value.
> 
> (Not a comment to this patch, but I think it's worth bringing up)
> 
> I want to point out this is not true for the lockdep use case, because
> the we need to protect a hash list deletion there, and we use the
> address of the hash bucket there. It's proven fine in practice because
> the readers are rare (we only call the reader is_dynamic_key() in
> register_lock_class(), that is every time you have a new lock class to
> register).
> 
> Maybe what we want to say here is that "if the users guarantee no steady
> flow of the same hazard pointer value, we guarantee forward progress".
> Thoughts?

AFAIU, your approach to protect lockdep linked lists is to use the
address of the hash bucket to protect the traversal. As this address is 
invariant (global array item address), that address should be fine
to fulfill hazptr requirements, but it has downsides: rather than
protecting the specific nodes being retired, the whole hash chain is
protected. This means that, as you point out, many readers retiring
nodes from a given bucket (except the first node) could end up holding a
continuous stream of hazptr for a given hazptr value, preventing
progress of hazptr synchronize.

It's also coarser: per-bucket rather than per-node.

Am I missing something here ?

One honest question: is this pattern something we expect to
see often ? If so, then we may want to introduce a notion of
hazptr protection "period" flip (similar to some RCU implementations),
where we tag the low bit of the slot pointer (0 vs 1), and alternate
between the two periods in synchronize. This would prevent a steady-flow
of same-value readers from preventing synchronize forward progress.

Thoughts ?

Thanks,

Mathieu

> 
> The rest looks good to me.
> 
> Regards,
> Boqun
> 


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

  reply	other threads:[~2026-09-27 17:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 15:51 [PATCH hazptr 0/4] Hazard pointer updates Mathieu Desnoyers
2026-09-27 15:51 ` [PATCH hazptr 1/4] hazptr: Fix two-phase hazptr_synchronize race with detach Mathieu Desnoyers
2026-09-27 15:51 ` [PATCH hazptr 2/4] compiler.h: Introduce ptr_eq() to preserve address dependency Mathieu Desnoyers
2026-09-27 15:51 ` [PATCH hazptr 3/4] Documentation: RCU: Refer to ptr_eq() Mathieu Desnoyers
2026-09-27 15:51 ` [PATCH hazptr 4/4] hazptr: Introduce "try acquire" fast path, fallback to overflow list Mathieu Desnoyers
2026-09-27 16:40   ` Boqun Feng
2026-09-27 17:15     ` Mathieu Desnoyers [this message]
2026-09-27 17:24       ` Boqun Feng
2026-09-27 17:36         ` Mathieu Desnoyers
2026-09-27 18:16           ` Boqun Feng
2026-09-27 17:26       ` Boqun Feng
2026-09-27 22:39       ` Gary Guo
2026-09-28  9:12         ` Boqun Feng
2026-09-28 11:32           ` Gary Guo
2026-09-28 14:56             ` Bradley Morgan
2026-09-28 15:32             ` Boqun Feng
2026-09-28  9:27     ` Kunwu Chan
2026-09-27 16:07 ` [PATCH hazptr 0/4] Hazard pointer updates Bradley Morgan
2026-09-27 16:27   ` Mathieu Desnoyers
2026-09-27 16:33     ` Bradley Morgan
2026-09-27 16:45       ` Mathieu Desnoyers
2026-09-27 16:15 ` Boqun Feng
2026-09-27 16:20   ` Mathieu Desnoyers
2026-09-27 16:22     ` Bradley Morgan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c78c338-1be6-456b-b963-bcfc62748aab@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=boqun@kernel.org \
    --cc=brads@mainlining.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkmm@lists.linux.dev \
    --cc=paulmck@kernel.org \
    --cc=rcu@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®