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

[Bradley Morgan]

On 19 October 2026 17:18:48 BST, Andrea Parri <parri.andrea@xxxxxxxxx>
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
>
>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

Awesome! 

Acked-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>


>Signed-off-by: Andrea Parri <parri.andrea@xxxxxxxxx>
>---
>
>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;
> 		}
>
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds