Re: [PATCH RFC -rcu/dev] hazptr: Record per-CPU slot owner after claiming the slot

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2026-10-19 12:18, Andrea Parri wrote:
> An interrupt that uses a hazard pointer while the interrupted task is
> in the acquire slow path can leave its own context recorded as the
> owner of the per-CPU slot the task then publishes into.  The next
> context switch promotes the task's pointer with the interrupt's
> context.
> 
> hazptr_get_free_percpu_slot() records the owner when it picks an empty
> slot, but only Store B in __hazptr_try_acquire() claims the slot.  An
> interrupt in between picks the same slot, as the lowest free one:
> 
>    Task on CPU 0                        IRQ on CPU 0
>    -------------                        ------------
>    hazptr_acquire(ctx)
>      fast path fails
>      __hazptr_acquire(ctx)
>        picks item k (empty)
>        item k->ctx.ctx = ctx
>                                         hazptr_acquire(irq_ctx)
>                                           claims item k
>                                           item k->ctx.ctx = irq_ctx
>                                         hazptr_release(irq_ctx)
>                                           clears slot k
>        __hazptr_try_acquire()
>          Store B into slot k
>        (item k still names irq_ctx)
>    sleeps holding the pointer
>    hazptr_note_context_switch()
>      promotes item k with irq_ctx

You are correct. Writing directly to item->ctx.ctx before having
reserved the slot address (store B) fails to prevent the nested
irq handler from using that slot as well and causing confusion between
the two contexts.

Thanks for catching this !

Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> 
> The task gets there with k > 0 when it already holds an outer hazard
> pointer in slot 0; no other task can, as its slot is promoted when it
> is switched out.
> 
> The task gets there with k == 0 when the fast path leaves slot 0
> empty: either its ptr_eq() check fails because an updater changed the
> pointer, or the pointer is NULL and an updater publishes a non-NULL one
> before the slow path loads it.
> 
> The promotion moves the task's pointer into irq_ctx's backup slot,
> which nothing unchains, so hazptr_synchronize() on that pointer never
> returns.  The task's ctx->slot still names slot k, so its
> hazptr_release() clears whatever another reader on that CPU has since
> published there.
> 
> If the interrupt detaches instead, as hazptr_torture_acquire() does,
> the promotion chains irq_ctx's backup slot a second time while it is
> still linked.  The overflow list then loops on that node, or follows
> LIST_POISON1 once hazptr_unchain_backup_slot() removes it.
> 
> Record the owner after __hazptr_try_acquire() succeeds, as the fast
> path in hazptr_acquire() does.  This should be safe because an
> interrupt that used the slot has released or detached it by the time
> the task resumes, and only hazptr_note_context_switch() reads the
> owner, which cannot run before the task enables preemption.
> 
> Fixes: 50b568234163 ("hazptr: Implement Hazard Pointers")
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
> 
> Notes:
>      With this patch, hazptrtorture's NOPREEMPT scenario passes a 30-minute
>      run on x86_64 with 8 CPUs.  PREEMPT runs clean for 30 minutes but can
>      hang at shutdown, in hazptrtorture's release of deferred hazard
>      pointers; that hang does not involve this patch, and is addressed by
>      "hazptrtorture: Release all deferred hazard pointers before stopping",
>      posted separately.
>      
>      With that fix also applied, 30-minute runs pass for PREEMPT, for
>      NOPREEMPT with torture_type=hazptr, for PREEMPT with
>      torture_type=hazptr-stack, and for PREEMPT with CONFIG_HAZPTR_DEBUG=n.
>      
>      PREEMPT and NOPREEMPT also pass without this patch: their readers hold
>      one hazard pointer each, so they reach the slow path only when the
>      ptr_eq() check fails or the pointer is NULL, which happens only before
>      the writer publishes its first element and after it stops.
>      
>      A KUnit reproducer, with a test-only hook in __hazptr_try_acquire()
>      between the free-slot check and Store B, fails on 940793b926aa and
>      passes with this patch.  I can post it if useful.
> 
>   kernel/hazptr.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/hazptr.c b/kernel/hazptr.c
> index 3ca73b56a5c25..b15b84ebd4ceb 100644
> --- a/kernel/hazptr.c
> +++ b/kernel/hazptr.c
> @@ -54,19 +54,16 @@ unsigned int flip_list_phase(unsigned int phase)
>   }
>   
>   static
> -struct hazptr_slot *hazptr_get_free_percpu_slot(struct hazptr_ctx *ctx)
> +struct hazptr_slot_item *hazptr_get_free_percpu_slot_item(void)
>   {
>   	struct hazptr_percpu_slots *percpu_slots = this_cpu_ptr(&hazptr_percpu_slots);
>   	unsigned int idx;
>   
>   	for (idx = 0; idx < NR_HAZPTR_PERCPU_SLOTS; idx++) {
>   		struct hazptr_slot_item *item = &percpu_slots->items[idx];
> -		struct hazptr_slot *slot = &item->slot;
>   
> -		if (!slot->addr) {
> -			item->ctx.ctx = ctx;
> -			return slot;
> -		}
> +		if (!item->slot.addr)
> +			return item;
>   	}
>   	/* All slots are in use. */
>   	return NULL;
> @@ -78,6 +75,7 @@ struct hazptr_slot *hazptr_get_free_percpu_slot(struct hazptr_ctx *ctx)
>    */
>   void *__hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
>   {
> +	struct hazptr_slot_item *item;
>   	struct hazptr_slot *slot;
>   	void *addr;
>   
> @@ -85,10 +83,17 @@ void *__hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
>   	 * In case we are called due to nested use of hazard pointers,
>   	 * try a slot protection with per-CPU slots.
>   	 */
> -	slot = hazptr_get_free_percpu_slot(ctx);
> -	if (likely(slot)) {
> +	item = hazptr_get_free_percpu_slot_item();
> +	if (likely(item)) {
> +		slot = &item->slot;
>   		addr = __hazptr_try_acquire(ctx, addr_p, slot);
>   		if (addr) {
> +			/*
> +			 * Record the owner only once Store B has claimed the
> +			 * slot, as hazptr_acquire() does: an interrupt may
> +			 * have used this slot since it was chosen.
> +			 */
> +			item->ctx.ctx = ctx;
>   			ctx->slot = slot;
>   			return addr;
>   		}
> 


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com