Hi Mathieu, Scanning all per-CPU slots before the overflow lists addresses the race where a backup slot can be moved to a list that has already been scanned.
Using a separate overflow-list phase also decouples list selection from the wildcard phase. Reviewed-by: Kunwu Chan <[email protected]> Thanks, Kunwu On Mon, Oct 12, 2026 at 3:49 AM Mathieu Desnoyers <[email protected]> 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 <[email protected]> > Reported-by: Boqun Feng <[email protected]> > Cc: Paul E. McKenney <[email protected]> > Cc: Boqun Feng <[email protected]> > Cc: Bradley Morgan <[email protected]> > Cc: Gary Guo <[email protected]> > Cc: <[email protected]> > Cc: <[email protected]> > Cc: Lian Wang <[email protected]> > Cc: Kunwu Chan <[email protected]> > --- > Changes since v0: > - Load the new overflow list phase word in "hazptr_chain_backup_slot". > - Rename the lock to hazptr_phase_lock to make it clear that it > protects both the wildcard and the overflow list phases. > --- > kernel/hazptr.c | 50 +++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 40 insertions(+), 10 deletions(-) > > diff --git a/kernel/hazptr.c b/kernel/hazptr.c > index d3d1050d92cf..13faa5ba7677 100644 > --- a/kernel/hazptr.c > +++ b/kernel/hazptr.c > @@ -13,6 +13,8 @@ > #include <linux/list.h> > #include <linux/export.h> > > +static DEFINE_MUTEX(hazptr_phase_lock); /* Protect the wildcard and > list phase flip. */ > + > /* > * The current hazard pointer wildcard. Flips between 1UL and 2UL to > guarantee > * hazptr_synchronize forward progress even with a steady stream of readers. > @@ -20,10 +22,12 @@ > * This also affects the overflow list selection: the current list used by > * readers is array[(unsigned long) hazptr_wildcard - 1]. > */ > -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 +57,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 +188,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 +206,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 +236,7 @@ void hazptr_scan_period(void *addr, void *scan_wildcard) > */ > void hazptr_synchronize(void *addr) > { > + unsigned int scan_list_phase; > void *scan_wildcard; > > /* > @@ -235,18 +254,29 @@ void hazptr_synchronize(void *addr) > /* Memory ordering: Store A before Load B. */ > smp_mb(); > > - guard(mutex)(&hazptr_wildcard_lock); > + guard(mutex)(&hazptr_phase_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); > > struct hazptr_slot *hazptr_chain_backup_slot(struct hazptr_ctx *ctx) > { > struct hazptr_overflow_list_flip *overflow_list_flip = > this_cpu_ptr(&percpu_overflow_list_flip); > - unsigned int list_idx = (unsigned long) READ_ONCE(hazptr_wildcard) - > 1; > + unsigned int list_idx = READ_ONCE(hazptr_overflow_list_phase); > struct hazptr_overflow_list *overflow_list = > &overflow_list_flip->array[list_idx]; > struct hazptr_slot *slot = &ctx->backup_slot.slot; > > -- > 2.43.0 >
