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

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

On 2026-07-09 19:48, Paul E. McKenney wrote:
[...]
> Let's start by me correctly documenting the current rules.  Calling
> hazptr_detach_from_task() should not be all that much of a burden,
> especially if the proper diagnostics are in place.
> 
> Please see below, and thank you for bearing with me!
> 

Comments below,

> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit b8de842c6708de99335e2f700d3668e18aeeb3eb
> 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..dbaea42a68745b 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

We should really rename this given that it detaches the hazptr ctx from
the execution _context_ (irq handler or thread). Not sure how to name it
though.

> + *
> + * @ctx: The hazard-pointer context to be migrated.
> + *
> + * By default, a given hazptr_acquire() and the corresponding
> + * hazptr_release() must run in a single execution context, for example, the
> + * context of a single task or a single interrupt handler.  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().
> + *

The text above does not discuss the need to either release or detach
before returning from interrupt handler. Or am I missing something ?

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

ok

> + */
>   static inline
>   void hazptr_detach_from_task(struct hazptr_ctx *ctx)
>   {
> @@ -160,12 +183,27 @@ 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 same
> + * execution context that executed the corresponding hazptr_acquire(), for
> + * example, within the same task or interrupt handler.  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.

Same comment about use of detach before returning from irq.

> + *
> + * 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 +271,25 @@ 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 same execution context
> + * that invoked the corresponding hazptr_acquire(), for example, within the
> + * same task or the same interrupt handler.  However, 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.

Again, this statement is misleading: one could think that they don't
need to detach the ctx before returning to the _same task_. The naming
of the detach helper is actively confusing here.

Maybe just call it hazptr_detach() ?

Thanks,

Mathieu

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