Re: [PATCH hazptr v3 1/4] hazptr: Fix two-phase hazptr_synchronize race with detach

[KunWu Chan]

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 <chentao@xxxxxxxxxx>

Thanks,
Kunwu

On Mon, Oct 12, 2026 at 3:49 AM 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>
> Cc: Lian Wang <lianux.mm@xxxxxxxxx>
> Cc: Kunwu Chan <kunwu.chan@xxxxxxxxx>
> ---
> 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
>