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

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

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@kernel.org>
> 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@kernel.org>
>      Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>      Cc: Boqun Feng <boqun@kernel.org>
> 
> 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).

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

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.

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

> + * 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.

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)
>   {


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