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

[Paul E. McKenney]

On Thu, Jul 09, 2026 at 05:47:00PM -0400, Mathieu Desnoyers wrote:
> On 2026-07-09 16:44, Paul E. McKenney wrote:
> [...]
> 
> > Do the kernel-doc headers below accurately capture the rules of the
> > hazard-pointer road?
> > 
> > I am sure that I am confused on at least a few points, but we have
> > to start somewhere!  ;-)
> 
> Comments inline,
> 
> > 
> > ------------------------------------------------------------------------
> > 
> > commit 64241bf30d2cea1be8114f5be3ceb16b2db38d5f
> > Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > Date:   Thu Jul 9 13:39:09 2026 -0700
> > 
> >      hazptr: Upgrade kernel-doc headers
> >      Upgrade the kernel-doc headers for hazptr_acquire(), hazptr_release(),
> >      and hazptr_detach_from_task().
> >      Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> >      Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> >      Cc: Boqun Feng <boqun@xxxxxxxxxx>
> > 
> > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h
> > index c5aa4fb03b11cf..8b6c0c8cdfb987 100644
> > --- a/include/linux/hazptr.h
> > +++ b/include/linux/hazptr.h
> > @@ -126,6 +126,24 @@ void hazptr_promote_to_backup_slot(struct hazptr_ctx *ctx, struct hazptr_slot *s
> >   	ctx->slot = backup_slot;
> >   }
> > +/**
> > + * hazptr_detach_from_task - Allow a hazard pointer to be released by some other task
> > + *
> > + * @ctx: The hazard-pointer context to be migrated.
> > + *
> > + * By default, a given hazptr_acquire() and the corresponding
> > + * hazptr_release() must run in the context of a single task.  When you
> > + * need to acquire a hazard pointer in one task and release it in another,
> > + * you must first invoke hazptr_detach_from_task() on that hazard pointer's
> > + * context.  It is permissible to invoke hazptr_detach_from_task() multiple
> > + * times on the same @ctx while it is protecting the same pointer.
> > + *
> > + * For example, if a hazard pointer is acquired by a task and released
> > + * by a timer handler, that task would need to pass the hazard pointer's
> > + * context to hazptr_detach_from_task() after the hazptr_acquire() and
> > + * before arming the timer (or at least before the handler had a chance
> > + * to execute).
> 
> Important note: the "detach" must be called from the context of the
> thread "owning" the hazptr ctx (except when it's already been detached).

Good point, upgrading.

> > + */
> >   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().

The actual code would need to be a bit more ornate to ensure that the
irq handler knew that the task it interrupted was ready, willing, and
able to release the hazard pointer before a context switch could occur.

> We'll need to think it through carefully. AFAIR it was OK to have an
> hazptr acquire in thread context interrupted (it's only disabling
> preemption, not interrupts), and another hazptr acquire in interrupt
> context nested on top. This was OK as long as the IRQ handler restores
> the state of the per-cpu hazptr slots to their original content, which
> is what motivated this requirement pairing acquire with release in the
> handler.
> 
> Now I think the slightly relaxed rule should be: either the release
> happens within the interrupt handler (so we're back to the original
> state on irq return), OR the interrupt handler calls "detach form
> task" before returning, which has the same effect of restoring the
> per-cpu hazptr slot to its original state.

This would prohibit the pattern shown above.  Which is quite possibly OK.
But if we do prohibit this pattern, we need a diagnostic that checks for
that pattern, correct?  Especially in this new world of LLM-generated
code.

I have not yet changed the comment (or the later ones) to prohibit this,
as I await your verdict.  If we do prohibit this, please note that
executing on a given CPU does not constitute a context.

> So naming this "detach from execution context" rather than detach from
> task would probably be a better fit. I'm open to suggestions.

I am OK with that name, expecially if we outlaw passing from a
handler the the interrupted task without the benefit of an intervening
hazptr_detach_from_task() or hazptr_detach_from_context(), as the case
might be.

> > + * This protection is roughly similar to that of a reference counter.
> > + * It is not permissible to invoke hazptr_acquire() twice on the same @ctx
> > + * without an intervening hazptr_release().
> >    *
> >    * Returns a non-NULL protected address if the loaded pointer is non-NULL.
> >    * Returns NULL if the loaded pointer is NULL.
> >    *
> >    * On success the protected hazptr slot is stored in @ctx->slot.
> > + *
> > + * By default, the call to hazptr_release() must be running in the context
> > + * of the same task that executed hazptr_acquire().  When it is necessary
> > + * to instead call hazptr_release() from the context of some other task,
> > + * pass @ctx to hazptr_detach_from_task() after invoking hazptr_acquire()
> > + * but before making the hazard pointer available to that other task.
> > + * Please note that "in the context of some other task" includes things
> > + * like interrupt handlers.
> 
> My earlier comments apply to this paragraph as well.
> 
> >    */
> >   static inline
> >   void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
> > @@ -233,7 +263,22 @@ void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr)
> >   static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { }
> >   #endif
> > -/* Release the protected hazard pointer from @slot. */
> > +/**
> > + * hazptr_release - Release the specified hazard pointer
> > + *
> > + * @ctx: The hazard-pointer context that was passed to hazptr_acquire().
> > + * @addr_p: The pointer that is to be hazard-pointer unprotected.
> > + *
> > + * Release the protected hazard pointer recorded in @ctx.
> > + *
> > + * By default, hazptr_release() must execute in the context of the
> > + * same task that executed the corresponding hazptr_acquire(), but
> 
> same comment about interrupt context requirements.

Please see below for an update.  Again, this assumes that the task is
the context, regardless of things like interrupts, and would need further
changes to treat things like interrupt handlers as separate contexts.

							Thanx, Paul

> Thanks!
> 
> Mathieu
> 
> > + * please see hazptr_detach_from_task().
> > + *
> > + * It is permissible (though likely unwise from a maintainability
> > + * viewpoint) to invoke hazptr_release() twice on the same @ctx without
> > + * an intervening hazptr_acquire().
> > + */
> >   static inline
> >   void hazptr_release(struct hazptr_ctx *ctx, void *addr)
> >   {

------------------------------------------------------------------------

commit 6c17eb2a1bc4affc0963a40ddc52cac639800558
Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
Date:   Thu Jul 9 13:39:09 2026 -0700

    hazptr: Upgrade kernel-doc headers
    
    Upgrade the kernel-doc headers for hazptr_acquire(), hazptr_release(),
    and hazptr_detach_from_task().
    
    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
    Cc: Boqun Feng <boqun@xxxxxxxxxx>

diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h
index c5aa4fb03b11cf..de5afe0cce1d80 100644
--- a/include/linux/hazptr.h
+++ b/include/linux/hazptr.h
@@ -126,6 +126,29 @@ void hazptr_promote_to_backup_slot(struct hazptr_ctx *ctx, struct hazptr_slot *s
 	ctx->slot = backup_slot;
 }
 
+/**
+ * hazptr_detach_from_task - Allow a hazard pointer to be released by some other task
+ *
+ * @ctx: The hazard-pointer context to be migrated.
+ *
+ * By default, a given hazptr_acquire() and the corresponding
+ * hazptr_release() must run in the context of a single task.
+ * Please note that things like interrupt handlers execute in the
+ * context of the interrupted task.  When you have acquired a hazard
+ * pointer in one task and need to release it in another, you must
+ * invoke hazptr_detach_from_task() on that hazard pointer's context.
+ * It is permissible to invoke hazptr_detach_from_task() multiple times
+ * on the same @ctx while it is protecting the same pointer, however,
+ * the first invocation absolutely must be in the context of the task
+ * that did the hazptr_acquire(), and must take place after the return
+ * from that hazptr_acquire().
+ *
+ * For example, if a hazard pointer is acquired by a task and
+ * released by a timer handler, that task would need to pass the hazard
+ * pointer's context to hazptr_detach_from_task() after return from the
+ * hazptr_acquire() and before arming the timer (or at least before the
+ * handler had a chance to access that hazard-pointer context).
+ */
 static inline
 void hazptr_detach_from_task(struct hazptr_ctx *ctx)
 {
@@ -160,12 +183,26 @@ 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.
+ * This protection is roughly similar to that of a reference counter, and
+ * ends with a hazptr_release().
+ *
+ * By default, the call to hazptr_release() must be running in the context
+ * of the same task that executed hazptr_acquire().  When it is necessary
+ * to instead call hazptr_release() from the context of some other task,
+ * pass @ctx to hazptr_detach_from_task() after invoking hazptr_acquire()
+ * but before making the hazard pointer available to that other task.
+ * Please note that things like interrupt handlers execute in the context
+ * of the interrupted task.
+ *
+ * It is not permissible to invoke hazptr_acquire() twice on the same @ctx
+ * without an intervening hazptr_release().
  *
  * Returns a non-NULL protected address if the loaded pointer is non-NULL.
  * Returns NULL if the loaded pointer is NULL.
@@ -233,7 +270,24 @@ void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr)
 static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { }
 #endif
 
-/* Release the protected hazard pointer from @slot. */
+/**
+ * hazptr_release - Release the specified hazard pointer
+ *
+ * @ctx: The hazard-pointer context that was passed to hazptr_acquire().
+ * @addr_p: The pointer that is to be hazard-pointer unprotected.
+ *
+ * Release the protected hazard pointer recorded in @ctx.
+ *
+ * By default, hazptr_release() must execute in the context of the
+ * same task that executed the corresponding hazptr_acquire(), but
+ * if this restriction is problematic for your use case, please see
+ * hazptr_detach_from_task().  Please note that things like interrupt
+ * handlers execute in the context of the interrupted task.
+ *
+ * It is permissible (though likely unwise from a maintainability
+ * viewpoint) to invoke hazptr_release() twice on the same @ctx without
+ * an intervening hazptr_acquire().
+ */
 static inline
 void hazptr_release(struct hazptr_ctx *ctx, void *addr)
 {