Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection

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

On 2026-07-09 19:05, Paul E. McKenney wrote:
> On Thu, Jul 09, 2026 at 05:47:00PM -0400, Mathieu Desnoyers wrote:
[...]
>>> + */
>>>    static inline
>>>    void hazptr_detach_from_task(struct hazptr_ctx *ctx)
>>>    {
>>> @@ -160,17 +178,29 @@ void hazptr_note_context_switch(void)
>>>    	}
>>>    }
>>> -/*
>>> - * hazptr_acquire: Load pointer at address and protect with hazard pointer.
>>> +/**
>>> + * hazptr_acquire - Load pointer at address and protect with hazard pointer.
>>> + *
>>> + * @ctx: The hazard-pointer context to be passed to hazptr_release().
>>> + * @addr_p: Pointer to the pointer that is to be hazard-pointer protected.
>>>     *
>>>     * Load @addr_p, and protect the loaded pointer with hazard pointer.
>>> - * When using hazptr_acquire from interrupt handlers, the acquired slots
>>> - * need to be released before returning from the interrupt handler.
>>
>> I see that you removed wording of a major constraint here which allowed
>> use of hazptr locally in a interrupt handler: the need to pair the
>> acquire/release within the handler.
> 
> I did indeed remove that wording.  You could do something like this:
> 
> 	Task Context			IRQ Handler Interrupts Task
> 	------------			---------------------------
> 	preempt_disable();
> 	ihp = __this_cpu_read(irq_hc);	ihp = __this_cpu_read(irq_hc);
> 					p = hazptr_acquire(ihp, &gp);
> 	lp = xchg(p, NULL);
> 	if (lp) {
> 		do_something(lp);
> 		hazptr_release(ihp, lp);
> 	}
> 	preempt_enable();
> 
> If I understand the rules correctly (ha!), this is perfectly legal
> and does not require a hazptr_detach_from_task().
> 
I am concerned about it, because I knowingly just used preempt disable
to protect hazptr_acquire from the scheduler, but not from interrupt
handlers, because it's faster than irqoff.

I am concerned that this use of hazptr_acquire could confuse the
thread-level hazptr_acquire, let's dig:

void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
{
         struct hazptr_percpu_slots *percpu_slots;
         struct hazptr_slot_item *slot_item;
         struct hazptr_slot *slot;
         void *addr;

         guard(preempt)();
         percpu_slots = this_cpu_ptr(&hazptr_percpu_slots);
         slot_item = &percpu_slots->items[0];
         slot = &slot_item->slot;
[...]

   from here --------------------
         if (unlikely(slot->addr))
                 return __hazptr_acquire(ctx, addr_p);
         WRITE_ONCE(slot->addr, HAZPTR_WILDCARD);        /* Store B */

         /* Memory ordering: Store B before Load A. */
         smp_mb();

         /*
          * Load @addr_p after storing wildcard to the hazard pointer slot.
          */
         addr = READ_ONCE(*addr_p);      /* Load A */

         /*
          * We don't care about ordering of Store C. It will simply
          * replace the wildcard by a more specific address. If addr is
          * NULL, we simply store NULL into the slot.
          */
         WRITE_ONCE(slot->addr, addr);   /* Store C */
    to here ----------------------------

I designed hazptr_acquire so a _nested_ interrupt which _brings back_
the slot addr to its original state will work. Now let's see if
that's still OK if the interrupt handler leaves the slot->addr
populated with an address.

And no. If the interrupt happens right before "Store B", its
reserved slot address is overwritten by the thread. That's incorrect.

So as it is today, the irq handler needs to vacate the slot before it
returns, either through a hazptr release or a detach.

That being said, this is the code as it is today. If someone finds a
clever way to support this use-case and keep it fast and not too complex,
I'm all ears! :)

One possible way to make it work would be to install the HAZPTR_WILDCARD
with a local-cmpxchg expecting a NULL slot->addr. This would close this
race window. But it comes at a non-null overhead price. Another alternative
on x86 would be to use a lock prefixed cmpxchg, which has an implied smp_mb
on success, which may be in the same ballpark as the sequence of WRITE_ONCE+
explicit smp_mb().

Thanks,

Mathieu

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