mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Gary Guo <gary@garyguo.net>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	rcu@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: kernel-team@meta.com, Boqun Feng <boqun@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	lkmm@lists.linux.dev, Zqiang <qiang.zhang@linux.dev>,
	Wang Lian <lianux.mm@gmail.com>,
	Kunwu Chan <kunwu.chan@gmail.com>,
	Bradley Morgan <brads@mainlining.org>,
	Bradley Morgan <include@grrlz.net>
Subject: Re: [PATCH 26/28] hazptr: Implement two-phase wildcard scan
Date: Fri, 25 Sep 2026 15:19:40 -0400	[thread overview]
Message-ID: <4da27ea8-6f1a-40a6-9be1-8bbd474d8367@efficios.com> (raw)
In-Reply-To: <DLK9MW1N86KD.2GRWUAD2NX4FU@garyguo.net>

On 2026-09-20 11:57, Gary Guo wrote:
> On Sun Sep 20, 2026 at 4:44 PM BST, Mathieu Desnoyers wrote:
>> On 2026-09-20 10:46, Gary Guo wrote:
>>> On Sat Sep 19, 2026 at 1:00 AM BST, Paul E. McKenney wrote:
>>>> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>>>
>>>> Implement a two-phase wildcard scan to guarantee forward progress of
>>>> synchronize_hazptr() even if there is a steady stream of ill-timed
>>>> readers which populate wildcards into per-CPU slots.
>>>
>>> Hmm, I am not sure that I understand the problem here. The per-CPU slot is
>>> scanned only once per CPU, and patch 1 already introduces flipping of the
>>> overflow list. What prevents the forward progress?
>>
>> A steady stream of readers acquiring and releasing various hazard
>> pointers happening concurrently with the percpu slots checks, being
>> unlucky enough that each of the slot is constantly in a "wildcard"
>> state, thus preventing forward progress of the synchronize, just with
>> a steady stream of individually time-bound readers.
> 
> Oh, so the issue is that we cannot progress over a single slot, because with
> ill-timing a new iteration of the inner loop of "smp_cond_load_acquire" could
> see a new reader while waiting for the slot to be released?

Correct.

> 
> So, in essence, the flipping is used to prevent ABA problem on percpu slots?

Yes, specifically an ABA which could theoretically prevent forward
progress of synchronize given a steady flow of hazptr acquire/release.

> 
>>
>>>
>>> I think having a shared global read by all CPUs sounds really undesirable,
>>> especially that it gets flipped for each hazptr_synchronize -- this means that
>>> in the pathological case where there are a steady stream of hazptr_synchronize
>>> calls, each fast-path hazptr_acquire will have a cache miss reading
>>> hazptr_wildcard.
>>
>> There is a straightforward optimization we can do if this happen to
>> cause performance issues: only do the flip when the synchronize
>> encounters a wildcard retry delay beyond a specified threshold.
>> So we ensure synchronize observe the absence of both wildcard
>> values in each cpu slots, and only flip the current wildcard on retry
>> delay.
> 
> Another option would be avoid using WILDCARD if possible. IIRC the wildcard is
> used to ensure forward progress on the reader side, so it avoids the possibility
> of READ_ONCE(*addr_p) changing before and after protecting.

Using the wildcard has a few benefits:

1) Prevents this retry loop on the read-side.

2) Prevents comparison of a loaded pointer value against a re-load of
    that value, which causes issues with compiler optimizations (I did a
    ptr_eq() patch in a prior version of the hazard pointer patches to
    handle this).

It does have a downside though: given a very long preemption by a host
VM, the guest VM could technically keep a wildcard present for a long
time in a per-cpu slot, which would prevent hazptr synchronize from
progressing for a long time in the guest VM kernel.

> 
> One option would be to first use the typical hazard pointer impl that read the
> pointer twice, and when that fails, use the wildcard protection. This would mean
> that in the common case where the hazptr_acquire does not race with a pointer
> update, the WILDCARD protection is not used at all.

So your idea is to use the hazptr load+reload approach (with ptr_eq()
check preventing the compiler from removing the dependency on the
second load), but rather than retry, fallback to the two-phases
wildcard. This way, we get the best of both worlds: guaranteed
progress for the read-side (with the wildcard fallback), and typically
we are immune to long-host-VM preemption delays, because the
wildcard fallback would almost never fire.

I like it. What do you guys think ?

Thanks,

Mathieu

> 
> Best,
> Gary
> 
>>
>> This would prevent the common "frequent" synchronize case you
>> mention from causing cache misses on the read-side.
>>
>> But I preferred to keep it simple and wait until we hit this
>> level of synchronize call throughput until adding that extra
>> complexity.
>>
>> Thanks,
>>
>> Mathieu
> 


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

  reply	other threads:[~2026-09-25 19:19 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 23:59 [PATCH RFC v3 0/28] Simple hazard-pointer implementation and torture tests Paul E. McKenney
2026-09-19  0:00 ` [PATCH 01/28] hazptr: Implement Hazard Pointers Paul E. McKenney
2026-09-19 11:12   ` Bradley Morgan
2026-09-19 16:30   ` Linus Torvalds
2026-09-19 16:34     ` Bradley Morgan
2026-09-19 17:00       ` Linus Torvalds
2026-09-19 17:09         ` Mathieu Desnoyers
2026-09-19 18:09           ` Boqun Feng
2026-09-19 17:19         ` Mathieu Desnoyers
2026-09-19 18:18         ` Paul E. McKenney
2026-09-19 23:58         ` Bradley Morgan
2026-09-19 16:41   ` Bradley Morgan
2026-09-19 17:56     ` Paul E. McKenney
2026-09-19  0:00 ` [PATCH 02/28] hazptr: Add refscale test Paul E. McKenney
2026-09-19  0:00 ` [PATCH 03/28] torture: Add a hazptrtorture.c torture test Paul E. McKenney
2026-09-19  0:00 ` [PATCH 04/28] hazptrtorture: Add testing of on-stack hazptr_ctx structures Paul E. McKenney
2026-09-19  0:00 ` [PATCH 05/28] hazptrtorture: Add microsecond-scale sleep in readers Paul E. McKenney
2026-09-19  0:00 ` [PATCH 06/28] hazptrtorture: Enable system-independent CPU overcommit Paul E. McKenney
2026-09-19  0:00 ` [PATCH 07/28] torture: Add a stutter_will_wait() function Paul E. McKenney
2026-09-19  0:00 ` [PATCH 08/28] hazptrtorture: Use mnemonic local variables for context information Paul E. McKenney
2026-09-19  0:00 ` [PATCH 09/28] hazptrtorture: Split hazptr_torture_reader_tail() from hazptr_torture_reader() Paul E. McKenney
2026-09-19  0:00 ` [PATCH 10/28] hazptrtorture: Add kthread to release deferred hazard pointers Paul E. McKenney
2026-09-19  0:00 ` [PATCH 11/28] hazptrtorture: Defer release of " Paul E. McKenney
2026-09-19  0:00 ` [PATCH 12/28] hazptrtorture: Add irq_acquire to acquire hazptr from irq Paul E. McKenney
2026-09-19  0:00 ` [PATCH 13/28] hazptrtorture: Use task_state_to_char() for task-state reporting Paul E. McKenney
2026-09-19  0:00 ` [PATCH 14/28] hazptrtorture: Pass hazptr_pending to hazptr_torture_reader_tail() Paul E. McKenney
2026-09-19  0:00 ` [PATCH 15/28] hazptrtorture: Add the ability to disable the writer kthread Paul E. McKenney
2026-09-19  0:00 ` [PATCH 16/28] hazptrtorture: Add irq_release to release hazptr from irq Paul E. McKenney
2026-09-19  0:00 ` [PATCH 17/28] hazptrtorture: Accumulate operation statistics Paul E. McKenney
2026-09-19  0:00 ` [PATCH 18/28] doc: Add hazptrtorture module parameters Paul E. McKenney
2026-09-19  0:00 ` [PATCH 19/28] hazptr: Permit detaching hazard pointers from contexts Paul E. McKenney
2026-09-19 11:42   ` Boqun Feng
2026-09-19 11:43     ` Boqun Feng
2026-09-19  0:00 ` [PATCH 20/28] hazptrtorture: Detach deferred and IPIed hazard pointers Paul E. McKenney
2026-09-19  0:00 ` [PATCH 21/28] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection Paul E. McKenney
2026-09-19  0:00 ` [PATCH 22/28] hazptrtorture: Fix hazptr ownership issue Paul E. McKenney
2026-09-19  0:00 ` [PATCH 23/28] hazptrtorture: Enable CONFIG_HAZPTR_DEBUG Paul E. McKenney
2026-09-19  0:00 ` [PATCH 24/28] hazptr: Upgrade kernel-doc headers Paul E. McKenney
2026-09-19  0:00 ` [PATCH 25/28] hazptrtorture: Fix inverted sleep condition in do_pending kthread Paul E. McKenney
2026-09-19  0:00 ` [PATCH 26/28] hazptr: Implement two-phase wildcard scan Paul E. McKenney
2026-09-19 13:28   ` Boqun Feng
2026-09-20 12:46     ` Mathieu Desnoyers
2026-09-20 15:55       ` Boqun Feng
2026-09-25 19:08         ` Mathieu Desnoyers
2026-09-20 14:46   ` Gary Guo
2026-09-20 15:44     ` Mathieu Desnoyers
2026-09-20 15:57       ` Gary Guo
2026-09-25 19:19         ` Mathieu Desnoyers [this message]
2026-09-25 20:17           ` Gary Guo
2026-09-25 20:18           ` Mathieu Desnoyers
2026-09-19  0:00 ` [PATCH 27/28] hazptr: handle NULL address in hazptr_detach Paul E. McKenney
2026-09-19  0:00 ` [PATCH 28/28] torture.sh: Add hazptr torturing Paul E. McKenney
2026-09-19 11:18   ` 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=4da27ea8-6f1a-40a6-9be1-8bbd474d8367@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=boqun@kernel.org \
    --cc=brads@mainlining.org \
    --cc=gary@garyguo.net \
    --cc=include@grrlz.net \
    --cc=kernel-team@meta.com \
    --cc=kunwu.chan@gmail.com \
    --cc=lianux.mm@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkmm@lists.linux.dev \
    --cc=paulmck@kernel.org \
    --cc=qiang.zhang@linux.dev \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.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®