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); -- 2.43.0
