On 25 September 2026 20:49:42 BST, 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]> >--- > 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 <[email protected]> --- Thanks! https://lore.kernel.org/all/[email protected]/
