Re: [PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[Bradley Morgan]

On 25 September 2026 20:49:42 BST, Mathieu Desnoyers
<mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>Boqun Feng pointed out that a detach happening concurrently with
>hazptr_synchronize can miss a slot. Indeed, scanning the per-CPU
>slots needs to be done *before* scanning the overflow lists. Fix the
>implementation accordingly.
>
>Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
>Reported-by: Boqun Feng <boqun@xxxxxxxxxx>
>Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
>Cc: Boqun Feng <boqun@xxxxxxxxxx>
>Cc: Bradley Morgan <brads@xxxxxxxxxxxxxx>
>Cc: Gary Guo <gary@xxxxxxxxxxx>
>Cc: <rcu@xxxxxxxxxxxxxxx>
>Cc: <lkmm@xxxxxxxxxxxxxxx>
>---
> kernel/hazptr.c | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
>diff --git a/kernel/hazptr.c b/kernel/hazptr.c
>index d3d1050d92cf..ccc19843edbd 100644
>--- a/kernel/hazptr.c
>+++ b/kernel/hazptr.c
>@@ -24,6 +24,9 @@ static DEFINE_MUTEX(hazptr_wildcard_lock);	/* Protect the wildcard flip. */
> void *hazptr_wildcard = (void *) 1UL;
> EXPORT_SYMBOL_GPL(hazptr_wildcard);
> 
>+/* The current overflow list phase. */
>+static unsigned int hazptr_overflow_list_phase;
>+
> struct hazptr_overflow_list {
> 	raw_spinlock_t lock;		/* Lock protecting overflow list and list generation. */
> 	struct hlist_head head;		/* Overflow list head. */
>@@ -53,6 +56,12 @@ void *flip_wildcard(void *wildcard)
> 	return ((unsigned long) wildcard == 1UL) ? (void *) 2UL : (void *) 1UL;
> }
> 
>+static
>+unsigned int flip_list_phase(unsigned int phase)
>+{
>+	return 1 - phase;
>+}
>+
> static
> bool is_wildcard(void *addr)
> {
>@@ -178,15 +187,12 @@ void hazptr_synchronize_cpu_slots(int cpu, void *addr, void *scan_wildcard)
> }
> 
> static
>-void hazptr_scan_period(void *addr, void *scan_wildcard)
>+void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
> {
>-	unsigned int scan_idx = (unsigned long) scan_wildcard - 1;
> 	int cpu;
> 
> 	/* Scan all CPUs slots. */
> 	for_each_possible_cpu(cpu) {
>-		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
>-
> 		/*
> 		 * Scan CPU slots.
> 		 * Forward progress against recurring wildcards is guaranteed
>@@ -199,6 +205,17 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
> 		 * to acquire that same hazard pointer value.
> 		 */
> 		hazptr_synchronize_cpu_slots(cpu, addr, scan_wildcard);
>+	}
>+}
>+
>+static
>+void hazptr_scan_overflow_list_period(void *addr, unsigned int scan_idx)
>+{
>+	int cpu;
>+
>+	/* Scan all CPUs overflow lists. */
>+	for_each_possible_cpu(cpu) {
>+		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
> 
> 		/*
> 		 * Scan backup slots in percpu overflow lists.
>@@ -218,6 +235,7 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
>  */
> void hazptr_synchronize(void *addr)
> {
>+	unsigned int scan_list_phase;
> 	void *scan_wildcard;
> 
> 	/*
>@@ -236,10 +254,21 @@ void hazptr_synchronize(void *addr)
> 	smp_mb();
> 
> 	guard(mutex)(&hazptr_wildcard_lock);
>+
>+	/* Scan per-CPU slots. */
> 	scan_wildcard = flip_wildcard(hazptr_wildcard);
>-	hazptr_scan_period(addr, scan_wildcard);
>-	WRITE_ONCE(hazptr_wildcard, scan_wildcard);	/* Flip the current wildcard. */
>-	hazptr_scan_period(addr, flip_wildcard(scan_wildcard));
>+	hazptr_scan_cpu_slots_period(addr, scan_wildcard);
>+	WRITE_ONCE(hazptr_wildcard, scan_wildcard);			/* Flip the current wildcard. */
>+	hazptr_scan_cpu_slots_period(addr, flip_wildcard(scan_wildcard));
>+
>+	/*
>+	 * Scan overflow lists *after* scanning per-CPU slots. See
>+	 * hazptr_promote_to_backup_slot() for scan ordering requirement.
>+	 */
>+	scan_list_phase = flip_list_phase(hazptr_overflow_list_phase);
>+	hazptr_scan_overflow_list_period(addr, scan_list_phase);
>+	WRITE_ONCE(hazptr_overflow_list_phase, scan_list_phase);	/* Flip the current list phase. */
>+	hazptr_scan_overflow_list_period(addr, flip_list_phase(scan_list_phase));
> }
> EXPORT_SYMBOL_GPL(hazptr_synchronize);
> 
>
Nice!

Reviewed-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@xxxxxxxxx/