Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection
From: Mathieu Desnoyers
Date: Thu Jul 09 2026 - 20:03:58 EST
On 2026-07-09 19:57, Boqun Feng wrote:
On Thu, Jul 09, 2026 at 07:22:18PM -0400, Mathieu Desnoyers wrote:
On 2026-07-09 19:05, Paul E. McKenney wrote:
On Thu, Jul 09, 2026 at 05:47:00PM -0400, Mathieu Desnoyers wrote:[...]
I am concerned about it, because I knowingly just used preempt disable+ */
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().
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! :)
I don't find this use-case is very useful, but I guess a different set
of percpu slot for interrupts can resolve this issue?
Correct, but then you add overhead to hazptr synchronize because
it needs to consider additional percpu slots.
And then there are softirqs and nmis, so complexity piles up fast.
Maybe not an issue, but it's certainly a tradeoff.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com