* [PATCH 1/2] hazptrtorture: Fix hazptr ownership issue @ 2026-07-09 18:29 Mathieu Desnoyers 2026-07-09 18:29 ` [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-09 18:29 UTC (permalink / raw) To: Paul E . McKenney, Boqun Feng; +Cc: linux-kernel, Mathieu Desnoyers hazptr_torture_acquire is invoked from IPI on remote CPUs. It needs to immediately detach the hazptr from its local task so it can be released from other tasks. Without this fix, the hazptrtorture test fails loudly with a OOPS. It passes with the fix applied. [ This applies on top of linux-rcu dev branch at commit e5ee2ea2d00ae5a0ca53ccf9c9100f2357e1bd08 ] Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> --- kernel/rcu/hazptrtorture.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/kernel/rcu/hazptrtorture.c b/kernel/rcu/hazptrtorture.c index 5e7cb594f9e0..4507d0f09b8c 100644 --- a/kernel/rcu/hazptrtorture.c +++ b/kernel/rcu/hazptrtorture.c @@ -370,6 +370,11 @@ static void hazptr_torture_acquire(void *hppp_in) struct hazptr_pending *hppp = hppp_in; hppp->hpp_htp = cur_ops->readlock(&hppp->hpp_hc); + /* + * Acquiring a hazard pointer from a remote CPU. + * Detach hazptr from its task so it can be released by another task. + */ + hazptr_detach_from_task(&hppp->hpp_hc); atomic_long_inc(per_cpu_ptr(&hazptr_torture_acquires_irq, raw_smp_processor_id())); } @@ -411,6 +416,10 @@ hazptr_torture_reader_tail(struct hazptr_pending *hppp, struct torture_random_st preempt_enable(); if (irq_release && !(torture_random(trsp) % irq_release)) { guard(preempt)(); + /* + * Handling release from a remote CPU. + * Detach hazptr from its task so it can be released by another task. + */ hazptr_detach_from_task(&hppp->hpp_hc); cpu = cpumask_next_wrap(smp_processor_id(), cpu_online_mask); smp_call_function_single(cpu, hazptr_torture_release, hppp, 1); @@ -428,8 +437,11 @@ static void hazptr_torture_defer(struct hazptr_pending *hppp, struct torture_ran int cpu = torture_random(trsp) % nr_cpu_ids; struct llist_head *llhp; - hazptr_detach_from_task(&hppp->hpp_hc); guard(preempt)(); + /* + * Handling release from a remote CPU. + * Detach hazptr from its task so it can be released by another task. + */ hazptr_detach_from_task(&hppp->hpp_hc); cpu = cpumask_next_wrap(cpu, cpu_online_mask); llhp = per_cpu_ptr(&hazptr_pending, cpu); -- 2.43.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 18:29 [PATCH 1/2] hazptrtorture: Fix hazptr ownership issue Mathieu Desnoyers @ 2026-07-09 18:29 ` Mathieu Desnoyers 2026-07-09 18:47 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-09 18:29 UTC (permalink / raw) To: Paul E . McKenney, Boqun Feng; +Cc: linux-kernel, Mathieu Desnoyers Introduce Hazard Pointers debug assert, which detects misuse of hazard pointers, namely failure to detach the hazard pointer from its owner thread before releasing it from a different thread. Prints the following to the console when a failure is detected: Hazard Pointer (addr=000000006885a05f) released on remote task without being detached from task. Acquire: caller=hazptr_torture_read_lock+0x43/0xa0 [hazptrtorture], pid=3727, cpu=1. Release: pid=3725, cpu=139. WARNING: ./include/linux/hazptr.h:225 at hazptr_torture_read_unlock+0x68/0xf0 [hazptrtorture], CPU#139: hazptr_torture_/3725 Modules linked in: hazptrtorture torture nft_masq nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink CPU: 139 UID: 0 PID: 3725 Comm: hazptr_torture_ Not tainted 7.1.0-rc4+ #9 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 RIP: 0010:hazptr_torture_read_unlock+0x74/0xf0 [hazptrtorture] Code: 74 31 8b 4f 40 4c 8b 43 48 49 c7 c2 22 c9 56 c0 48 c7 c7 3b c9 56 c0 4c 8d 1d b8 32 f1 ff 52 48 89 fa 4c 89 df 50 51 4c 89 d1 <67> 48 0f b9 3a 48 83 c4 18 48 8b 03 48 8d 53 18 48 c7 00 00 00 00 RSP: 0018:ff621a2a479b3de0 EFLAGS: 00010293 RAX: 0000000000000e8d RBX: ff12ea30d3913488 RCX: ffffffffc056c922 RDX: ffffffffc056c93b RSI: ffffffffc0562280 RDI: ffffffffc0562030 RBP: ff621a2a479b3e50 R08: ffffffffc064ee53 R09: 0000000000000e8f R10: ffffffffc056c922 R11: ffffffffc0562030 R12: 0000000000000000 R13: ffffffffc0562280 R14: ff12ea30d3913488 R15: ff621a2a479b3e50 FS: 0000000000000000(0000) GS:ff12ea5011a84000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f39b6e4b010 CR3: 0000000114c6e005 CR4: 0000000000771ef0 PKRU: 55555554 Call Trace: <TASK> hazptr_torture_reader_tail+0x8e/0x210 [hazptrtorture] hazptr_torture_reader+0x145/0xb30 [hazptrtorture] ? srso_alias_return_thunk+0x5/0xfbef5 ? set_cpus_allowed_ptr+0x36/0x60 ? srso_alias_return_thunk+0x5/0xfbef5 ? srso_alias_return_thunk+0x5/0xfbef5 ? __pfx_hazptr_torture_reader+0x10/0x10 [hazptrtorture] kthread+0xdf/0x120 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x216/0x2d0 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> ---[ end trace 0000000000000000 ]--- Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> --- include/linux/hazptr.h | 38 ++++++++++++++++++++++++++++++++++++++ kernel/rcu/Kconfig.debug | 9 +++++++++ 2 files changed, 47 insertions(+) diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h index d96414575739..c5aa4fb03b11 100644 --- a/include/linux/hazptr.h +++ b/include/linux/hazptr.h @@ -51,6 +51,11 @@ struct hazptr_ctx { /* Backup slot in case all per-CPU slots are used. */ struct hazptr_backup_slot backup_slot; struct hlist_node preempt_node; +#ifdef CONFIG_HAZPTR_DEBUG + bool detach_task, detach_cpu; /* Whether the ctx has been detached from task/cpu. */ + int acquire_pid, acquire_cpu; /* Note the task and cpu number at acquire. */ + unsigned long acquire_caller; /* Acquire instruction pointer. */ +#endif }; struct hazptr_slot_ctx { @@ -127,6 +132,9 @@ void hazptr_detach_from_task(struct hazptr_ctx *ctx) struct hazptr_slot *slot; guard(preempt)(); +#ifdef CONFIG_HAZPTR_DEBUG + ctx->detach_task = ctx->detach_cpu = true; +#endif slot = ctx->slot; if (unlikely(hazptr_slot_is_backup(ctx, slot))) return; @@ -145,6 +153,9 @@ void hazptr_note_context_switch(void) if (!slot->addr) continue; +#ifdef CONFIG_HAZPTR_DEBUG + item->ctx.ctx->detach_cpu = true; +#endif hazptr_promote_to_backup_slot(item->ctx.ctx, slot); } } @@ -173,6 +184,12 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) percpu_slots = this_cpu_ptr(&hazptr_percpu_slots); slot_item = &percpu_slots->items[0]; slot = &slot_item->slot; +#ifdef CONFIG_HAZPTR_DEBUG + ctx->detach_cpu = ctx->detach_task = false; + ctx->acquire_pid = current->pid; + ctx->acquire_cpu = smp_processor_id(); + ctx->acquire_caller = _THIS_IP_; +#endif if (unlikely(slot->addr)) return __hazptr_acquire(ctx, addr_p); WRITE_ONCE(slot->addr, HAZPTR_WILDCARD); /* Store B */ @@ -196,6 +213,26 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) return addr; } +#ifdef CONFIG_HAZPTR_DEBUG +/* Called with preemption disabled. */ +static inline +void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) +{ + int pid = current->pid, cpu = smp_processor_id(); + bool warn_remote_cpu = !ctx->detach_cpu && ctx->acquire_cpu != cpu, + warn_remote_task = !ctx->detach_task && ctx->acquire_pid != pid; + + WARN_ONCE(warn_remote_cpu || warn_remote_task, + "Hazard Pointer (addr=%p) released on remote %s without %s. Acquire: caller=%pS, pid=%d, cpu=%d. Release: pid=%d, cpu=%d.", + addr, + warn_remote_task ? "task" : "cpu", + warn_remote_task ? "being detached from task" : "context switch", + (void *) ctx->acquire_caller, ctx->acquire_pid, ctx->acquire_cpu, pid, cpu); +} +#else +static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { } +#endif + /* Release the protected hazard pointer from @slot. */ static inline void hazptr_release(struct hazptr_ctx *ctx, void *addr) @@ -205,6 +242,7 @@ void hazptr_release(struct hazptr_ctx *ctx, void *addr) if (!addr) return; guard(preempt)(); + hazptr_release_debug(ctx, addr); slot = ctx->slot; smp_store_release(&slot->addr, NULL); if (unlikely(hazptr_slot_is_backup(ctx, slot))) diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug index 7629c345b0b6..57fec4444a97 100644 --- a/kernel/rcu/Kconfig.debug +++ b/kernel/rcu/Kconfig.debug @@ -264,4 +264,13 @@ config TRIVIAL_PREEMPT_RCU This has no value for production and is only for testing. +config HAZPTR_DEBUG + bool "Provide debugging asserts for Hazard Pointers" + depends on DEBUG_KERNEL + default n + help + This option provides consistency checks for Hazard pointers. + + Say Y here if you want to enable those assert, N otherwise. + endmenu # "RCU Debugging" -- 2.43.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 18:29 ` [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection Mathieu Desnoyers @ 2026-07-09 18:47 ` Paul E. McKenney 2026-07-09 18:56 ` Mathieu Desnoyers 2026-07-09 20:45 ` Paul E. McKenney 0 siblings, 2 replies; 20+ messages in thread From: Paul E. McKenney @ 2026-07-09 18:47 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Thu, Jul 09, 2026 at 02:29:33PM -0400, Mathieu Desnoyers wrote: > Introduce Hazard Pointers debug assert, which detects misuse of hazard > pointers, namely failure to detach the hazard pointer from its owner > thread before releasing it from a different thread. > > Prints the following to the console when a failure is detected: Thank you, Mathieu! I am testing this one first to verify the debugging, and then I will test the patch 1/2 hazptrtorture.c changes to verify the fix. We are going to need to carefully document the hazard-pointer handling rules! ;-) Thanx, Paul > Hazard Pointer (addr=000000006885a05f) released on remote task without being detached from task. Acquire: caller=hazptr_torture_read_lock+0x43/0xa0 [hazptrtorture], pid=3727, cpu=1. Release: pid=3725, cpu=139. > WARNING: ./include/linux/hazptr.h:225 at hazptr_torture_read_unlock+0x68/0xf0 [hazptrtorture], CPU#139: hazptr_torture_/3725 > Modules linked in: hazptrtorture torture nft_masq nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink > CPU: 139 UID: 0 PID: 3725 Comm: hazptr_torture_ Not tainted 7.1.0-rc4+ #9 PREEMPT(full) > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 > RIP: 0010:hazptr_torture_read_unlock+0x74/0xf0 [hazptrtorture] > Code: 74 31 8b 4f 40 4c 8b 43 48 49 c7 c2 22 c9 56 c0 48 c7 c7 3b c9 56 c0 4c 8d 1d b8 32 f1 ff 52 48 89 fa 4c 89 df 50 51 4c 89 d1 <67> 48 0f b9 3a 48 83 c4 18 48 8b 03 48 8d 53 18 48 c7 00 00 00 00 > RSP: 0018:ff621a2a479b3de0 EFLAGS: 00010293 > RAX: 0000000000000e8d RBX: ff12ea30d3913488 RCX: ffffffffc056c922 > RDX: ffffffffc056c93b RSI: ffffffffc0562280 RDI: ffffffffc0562030 > RBP: ff621a2a479b3e50 R08: ffffffffc064ee53 R09: 0000000000000e8f > R10: ffffffffc056c922 R11: ffffffffc0562030 R12: 0000000000000000 > R13: ffffffffc0562280 R14: ff12ea30d3913488 R15: ff621a2a479b3e50 > FS: 0000000000000000(0000) GS:ff12ea5011a84000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f39b6e4b010 CR3: 0000000114c6e005 CR4: 0000000000771ef0 > PKRU: 55555554 > Call Trace: > <TASK> > hazptr_torture_reader_tail+0x8e/0x210 [hazptrtorture] > hazptr_torture_reader+0x145/0xb30 [hazptrtorture] > ? srso_alias_return_thunk+0x5/0xfbef5 > ? set_cpus_allowed_ptr+0x36/0x60 > ? srso_alias_return_thunk+0x5/0xfbef5 > ? srso_alias_return_thunk+0x5/0xfbef5 > ? __pfx_hazptr_torture_reader+0x10/0x10 [hazptrtorture] > kthread+0xdf/0x120 > ? __pfx_kthread+0x10/0x10 > ret_from_fork+0x216/0x2d0 > ? __pfx_kthread+0x10/0x10 > ret_from_fork_asm+0x1a/0x30 > </TASK> > ---[ end trace 0000000000000000 ]--- > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > --- > include/linux/hazptr.h | 38 ++++++++++++++++++++++++++++++++++++++ > kernel/rcu/Kconfig.debug | 9 +++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h > index d96414575739..c5aa4fb03b11 100644 > --- a/include/linux/hazptr.h > +++ b/include/linux/hazptr.h > @@ -51,6 +51,11 @@ struct hazptr_ctx { > /* Backup slot in case all per-CPU slots are used. */ > struct hazptr_backup_slot backup_slot; > struct hlist_node preempt_node; > +#ifdef CONFIG_HAZPTR_DEBUG > + bool detach_task, detach_cpu; /* Whether the ctx has been detached from task/cpu. */ > + int acquire_pid, acquire_cpu; /* Note the task and cpu number at acquire. */ > + unsigned long acquire_caller; /* Acquire instruction pointer. */ > +#endif > }; > > struct hazptr_slot_ctx { > @@ -127,6 +132,9 @@ void hazptr_detach_from_task(struct hazptr_ctx *ctx) > struct hazptr_slot *slot; > > guard(preempt)(); > +#ifdef CONFIG_HAZPTR_DEBUG > + ctx->detach_task = ctx->detach_cpu = true; > +#endif > slot = ctx->slot; > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > return; > @@ -145,6 +153,9 @@ void hazptr_note_context_switch(void) > > if (!slot->addr) > continue; > +#ifdef CONFIG_HAZPTR_DEBUG > + item->ctx.ctx->detach_cpu = true; > +#endif > hazptr_promote_to_backup_slot(item->ctx.ctx, slot); > } > } > @@ -173,6 +184,12 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) > percpu_slots = this_cpu_ptr(&hazptr_percpu_slots); > slot_item = &percpu_slots->items[0]; > slot = &slot_item->slot; > +#ifdef CONFIG_HAZPTR_DEBUG > + ctx->detach_cpu = ctx->detach_task = false; > + ctx->acquire_pid = current->pid; > + ctx->acquire_cpu = smp_processor_id(); > + ctx->acquire_caller = _THIS_IP_; > +#endif > if (unlikely(slot->addr)) > return __hazptr_acquire(ctx, addr_p); > WRITE_ONCE(slot->addr, HAZPTR_WILDCARD); /* Store B */ > @@ -196,6 +213,26 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) > return addr; > } > > +#ifdef CONFIG_HAZPTR_DEBUG > +/* Called with preemption disabled. */ > +static inline > +void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) > +{ > + int pid = current->pid, cpu = smp_processor_id(); > + bool warn_remote_cpu = !ctx->detach_cpu && ctx->acquire_cpu != cpu, > + warn_remote_task = !ctx->detach_task && ctx->acquire_pid != pid; > + > + WARN_ONCE(warn_remote_cpu || warn_remote_task, > + "Hazard Pointer (addr=%p) released on remote %s without %s. Acquire: caller=%pS, pid=%d, cpu=%d. Release: pid=%d, cpu=%d.", > + addr, > + warn_remote_task ? "task" : "cpu", > + warn_remote_task ? "being detached from task" : "context switch", > + (void *) ctx->acquire_caller, ctx->acquire_pid, ctx->acquire_cpu, pid, cpu); > +} > +#else > +static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { } > +#endif > + > /* Release the protected hazard pointer from @slot. */ > static inline > void hazptr_release(struct hazptr_ctx *ctx, void *addr) > @@ -205,6 +242,7 @@ void hazptr_release(struct hazptr_ctx *ctx, void *addr) > if (!addr) > return; > guard(preempt)(); > + hazptr_release_debug(ctx, addr); > slot = ctx->slot; > smp_store_release(&slot->addr, NULL); > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug > index 7629c345b0b6..57fec4444a97 100644 > --- a/kernel/rcu/Kconfig.debug > +++ b/kernel/rcu/Kconfig.debug > @@ -264,4 +264,13 @@ config TRIVIAL_PREEMPT_RCU > > This has no value for production and is only for testing. > > +config HAZPTR_DEBUG > + bool "Provide debugging asserts for Hazard Pointers" > + depends on DEBUG_KERNEL > + default n > + help > + This option provides consistency checks for Hazard pointers. > + > + Say Y here if you want to enable those assert, N otherwise. > + > endmenu # "RCU Debugging" > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 18:47 ` Paul E. McKenney @ 2026-07-09 18:56 ` Mathieu Desnoyers 2026-07-09 20:44 ` Paul E. McKenney 2026-07-09 20:45 ` Paul E. McKenney 1 sibling, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-09 18:56 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel On 2026-07-09 14:47, Paul E. McKenney wrote: > On Thu, Jul 09, 2026 at 02:29:33PM -0400, Mathieu Desnoyers wrote: >> Introduce Hazard Pointers debug assert, which detects misuse of hazard >> pointers, namely failure to detach the hazard pointer from its owner >> thread before releasing it from a different thread. >> >> Prints the following to the console when a failure is detected: > > Thank you, Mathieu! > > I am testing this one first to verify the debugging, and then I will > test the patch 1/2 hazptrtorture.c changes to verify the fix. > > We are going to need to carefully document the hazard-pointer handling > rules! ;-) Good point! So the general rule for the hazptr API is that the hazptr context is attached to the task that does the hazptr acquire. The scheduler integration is responsible for ensuring that the hazptr context is moved to a backup slot in case of context switch, which takes care of handling task migrations without user intervention. The case for which the user needs to explicitly invoke "detach from task" is when the hazptr context is handed off to a _different_ task. In that case the scheduler really cannot help, so it needs to be done explicitly. The debug code I proposed here discovers two scenarios: - Release vs acquire are done on different tasks without explicit detach from task (user error), - Release vs acquire are done on different CPUs, without scheduler context switch in between. This could happen if the user has a confused hazptr context ownership model. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 18:56 ` Mathieu Desnoyers @ 2026-07-09 20:44 ` Paul E. McKenney 2026-07-09 21:47 ` Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-09 20:44 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Thu, Jul 09, 2026 at 02:56:15PM -0400, Mathieu Desnoyers wrote: > On 2026-07-09 14:47, Paul E. McKenney wrote: > > On Thu, Jul 09, 2026 at 02:29:33PM -0400, Mathieu Desnoyers wrote: > > > Introduce Hazard Pointers debug assert, which detects misuse of hazard > > > pointers, namely failure to detach the hazard pointer from its owner > > > thread before releasing it from a different thread. > > > > > > Prints the following to the console when a failure is detected: > > > > Thank you, Mathieu! > > > > I am testing this one first to verify the debugging, and then I will > > test the patch 1/2 hazptrtorture.c changes to verify the fix. > > > > We are going to need to carefully document the hazard-pointer handling > > rules! ;-) > > Good point! > > So the general rule for the hazptr API is that the hazptr context > is attached to the task that does the hazptr acquire. > > The scheduler integration is responsible for ensuring that the > hazptr context is moved to a backup slot in case of context switch, > which takes care of handling task migrations without user intervention. > > The case for which the user needs to explicitly invoke "detach from > task" is when the hazptr context is handed off to a _different_ task. > In that case the scheduler really cannot help, so it needs to be > done explicitly. > > The debug code I proposed here discovers two scenarios: > > - Release vs acquire are done on different tasks without explicit detach > from task (user error), > - Release vs acquire are done on different CPUs, without scheduler > context switch in between. This could happen if the user has > a confused hazptr context ownership model. Or if they do hazptr_acquire() in one interrupt handler on one CPU and hazptr_release() in another on some other CPU. Which, as you say, requires that the first interrupt handler invoke hazptr_detach_from_task() before making the hazptr_ctx structure available to the other CPU's handler. 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! ;-) Thanx, Paul ------------------------------------------------------------------------ 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). + */ 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. + * 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. */ 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 + * 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) { ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 20:44 ` Paul E. McKenney @ 2026-07-09 21:47 ` Mathieu Desnoyers 2026-07-09 23:05 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-09 21:47 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 21:47 ` Mathieu Desnoyers @ 2026-07-09 23:05 ` Paul E. McKenney 2026-07-09 23:22 ` Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-09 23:05 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel 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@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). 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@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..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) { ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 23:05 ` Paul E. McKenney @ 2026-07-09 23:22 ` Mathieu Desnoyers 2026-07-09 23:48 ` Paul E. McKenney 2026-07-09 23:57 ` Boqun Feng 0 siblings, 2 replies; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-09 23:22 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 23:22 ` Mathieu Desnoyers @ 2026-07-09 23:48 ` Paul E. McKenney 2026-07-10 0:10 ` Mathieu Desnoyers 2026-07-09 23:57 ` Boqun Feng 1 sibling, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-09 23:48 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel 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: > [...] > > > > + */ > > > > 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(). 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! 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 + * + * @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(). + * + * 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,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. + * + * 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. + * + * 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) { ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 23:48 ` Paul E. McKenney @ 2026-07-10 0:10 ` Mathieu Desnoyers 2026-07-10 19:07 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-10 0:10 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-10 0:10 ` Mathieu Desnoyers @ 2026-07-10 19:07 ` Paul E. McKenney 2026-07-11 13:48 ` Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-10 19:07 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Thu, Jul 09, 2026 at 08:10:00PM -0400, Mathieu Desnoyers wrote: > 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. hazptr_detach_from_context()? I am leaving this alone for the moment, easy to change later. > > + * > > + * @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 ? You miss nothing. It was late and I wasn't sufficiently alert. So this was my hint to back slowly away from the keyboard. ;-) Update attached at the end. > > + * 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. And same response. ;-) > > + * > > + * 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. I deleted this "Please note..." sentence from all three kernel-doc headers and adjusted to suit. Again, please see below. > Maybe just call it hazptr_detach() ? That would be your choice, not mine. ;-) Thanx, Paul > 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) > > { ------------------------------------------------------------------------ diff rather than commit due to not-yet-rebased fixup in my -rcu tree. ------------------------------------------------------------------------ diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h index c5aa4fb03b11cf..57a7cb41df2015 100644 --- a/include/linux/hazptr.h +++ b/include/linux/hazptr.h @@ -126,6 +126,28 @@ 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 a single execution context, for example, + * the context of a single task or a single interrupt handler. When you + * have acquired a hazard pointer in one context 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 same context + * 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 +182,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 later call to hazptr_release(). + * + * By default, the call to hazptr_release() must be running in the same + * execution context as the corresponding hazptr_acquire(), for example, + * within the same task or interrupt handler. When it is necessary + * to instead call hazptr_release() from some other context, pass @ctx + * to hazptr_detach_from_task() in the original context after invoking + * hazptr_acquire() but before making the hazard pointer available to that + * other context. + * + * 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 +269,23 @@ 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(). + * + * It is permissible (though 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) { ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-10 19:07 ` Paul E. McKenney @ 2026-07-11 13:48 ` Mathieu Desnoyers 2026-07-11 23:55 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-11 13:48 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel On 2026-07-10 15:07, Paul E. McKenney wrote: >> >> 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. > > hazptr_detach_from_context()? > > I am leaving this alone for the moment, easy to change later. I just wonder if that name brings confusion. Is "context" the execution context or the "hazptr ctx" context ? I fear this is really a good opportunity to confuse everyone. [...] > >> Maybe just call it hazptr_detach() ? > > That would be your choice, not mine. ;-) I find that hazptr_detach is probably better here. It does not state it detaches "what", but at least there is no shortcut confusion with the overloaded "context" wording. [...] > > +/** > + * 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 a single execution context, for example, > + * the context of a single task or a single interrupt handler. When you > + * have acquired a hazard pointer in one context 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 same context > + * 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). good. > + */ > static inline > void hazptr_detach_from_task(struct hazptr_ctx *ctx) > { > @@ -160,12 +182,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 later call to hazptr_release(). Perhaps worthwhile to hint at the vast performance/scalability/memory footprint/cache line footprint difference between hazptr and refcount to justify why both exist here ? AFAIU there is partial overlap between refcount, hazptr, and RCU, each with their own strengths and weaknesses. We should prepare a summary table for the end users wondering which is the right tool for their use-case. > + * > + * By default, the call to hazptr_release() must be running in the same > + * execution context as the corresponding hazptr_acquire(), for example, > + * within the same task or interrupt handler. When it is necessary > + * to instead call hazptr_release() from some other context, pass @ctx > + * to hazptr_detach_from_task() in the original context after invoking > + * hazptr_acquire() but before making the hazard pointer available to that > + * other context. > + * > + * It is not permissible to invoke hazptr_acquire() twice on the same @ctx > + * without an intervening hazptr_release(). good > * > * Returns a non-NULL protected address if the loaded pointer is non-NULL. > * Returns NULL if the loaded pointer is NULL. > @@ -233,7 +269,23 @@ 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(). > + * > + * It is permissible (though unwise from a maintainability viewpoint) > + * to invoke hazptr_release() twice on the same @ctx without an intervening > + * hazptr_acquire(). Good! Thanks, Mathieu > + */ > static inline > void hazptr_release(struct hazptr_ctx *ctx, void *addr) > { -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-11 13:48 ` Mathieu Desnoyers @ 2026-07-11 23:55 ` Paul E. McKenney 2026-07-14 19:16 ` Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-11 23:55 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Sat, Jul 11, 2026 at 09:48:18AM -0400, Mathieu Desnoyers wrote: > On 2026-07-10 15:07, Paul E. McKenney wrote: > > > > > > 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. > > > > hazptr_detach_from_context()? > > > > I am leaving this alone for the moment, easy to change later. > > I just wonder if that name brings confusion. Is "context" the > execution context or the "hazptr ctx" context ? I fear this is > really a good opportunity to confuse everyone. > > [...] > > > > > > Maybe just call it hazptr_detach() ? > > > > That would be your choice, not mine. ;-) > > I find that hazptr_detach is probably better here. It does not > state it detaches "what", but at least there is no shortcut > confusion with the overloaded "context" wording. Fair enough. Would you like me to make this change? > [...] > > > +/** > > + * 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 a single execution context, for example, > > + * the context of a single task or a single interrupt handler. When you > > + * have acquired a hazard pointer in one context 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 same context > > + * 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). > > good. > > > + */ > > static inline > > void hazptr_detach_from_task(struct hazptr_ctx *ctx) > > { > > @@ -160,12 +182,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 later call to hazptr_release(). > > Perhaps worthwhile to hint at the vast performance/scalability/memory > footprint/cache line footprint difference between hazptr and refcount > to justify why both exist here ? Like this? * Load @addr_p, and protect the loaded pointer with hazard pointer. * This protection is roughly similar to (but way faster than) that of a * reference counter, and ends with a later call to hazptr_release() > AFAIU there is partial overlap between refcount, hazptr, and RCU, each > with their own strengths and weaknesses. We should prepare a summary > table for the end users wondering which is the right tool for their > use-case. We have lots of such tables of varying sizes and levels of detail in numerous C++ Working Papers, to say nothing of Section 9.6 of perfbook. ;-) One short text distinguishing them is: "RCU is a fast and scalable replacement for many reader-writer locking use cases, hazard pointers is a fast and scalable replacement for many reference-counting use cases, reader-writer locking provides mutual exclusion, and reference counting provides instant notice when the counter reaches zero." > > + * > > + * By default, the call to hazptr_release() must be running in the same > > + * execution context as the corresponding hazptr_acquire(), for example, > > + * within the same task or interrupt handler. When it is necessary > > + * to instead call hazptr_release() from some other context, pass @ctx > > + * to hazptr_detach_from_task() in the original context after invoking > > + * hazptr_acquire() but before making the hazard pointer available to that > > + * other context. > > + * > > + * It is not permissible to invoke hazptr_acquire() twice on the same @ctx > > + * without an intervening hazptr_release(). > > good > > > * > > * Returns a non-NULL protected address if the loaded pointer is non-NULL. > > * Returns NULL if the loaded pointer is NULL. > > @@ -233,7 +269,23 @@ 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(). > > + * > > + * It is permissible (though unwise from a maintainability viewpoint) > > + * to invoke hazptr_release() twice on the same @ctx without an intervening > > + * hazptr_acquire(). > > Good! We might actually be converging! ;-) Thanx, Paul ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-11 23:55 ` Paul E. McKenney @ 2026-07-14 19:16 ` Mathieu Desnoyers 2026-07-14 20:54 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-14 19:16 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel On 2026-07-11 19:55, Paul E. McKenney wrote: > On Sat, Jul 11, 2026 at 09:48:18AM -0400, Mathieu Desnoyers wrote: >> On 2026-07-10 15:07, Paul E. McKenney wrote: >>>> >>>> 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. >>> >>> hazptr_detach_from_context()? >>> >>> I am leaving this alone for the moment, easy to change later. >> >> I just wonder if that name brings confusion. Is "context" the >> execution context or the "hazptr ctx" context ? I fear this is >> really a good opportunity to confuse everyone. >> >> [...] >> >>> >>>> Maybe just call it hazptr_detach() ? >>> >>> That would be your choice, not mine. ;-) >> >> I find that hazptr_detach is probably better here. It does not >> state it detaches "what", but at least there is no shortcut >> confusion with the overloaded "context" wording. > > Fair enough. > > Would you like me to make this change? Since you're actively working in that area, please go ahead :) >>> + */ >>> static inline >>> void hazptr_detach_from_task(struct hazptr_ctx *ctx) >>> { >>> @@ -160,12 +182,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 later call to hazptr_release(). >> >> Perhaps worthwhile to hint at the vast performance/scalability/memory >> footprint/cache line footprint difference between hazptr and refcount >> to justify why both exist here ? > > Like this? > > * Load @addr_p, and protect the loaded pointer with hazard pointer. > * This protection is roughly similar to (but way faster than) that of a > * reference counter, and ends with a later call to hazptr_release() works for me! > >> AFAIU there is partial overlap between refcount, hazptr, and RCU, each >> with their own strengths and weaknesses. We should prepare a summary >> table for the end users wondering which is the right tool for their >> use-case. > > We have lots of such tables of varying sizes and levels of detail in > numerous C++ Working Papers, to say nothing of Section 9.6 of perfbook. ;-) > > One short text distinguishing them is: "RCU is a fast and scalable > replacement for many reader-writer locking use cases, hazard pointers is > a fast and scalable replacement for many reference-counting use cases, > reader-writer locking provides mutual exclusion, and reference counting > provides instant notice when the counter reaches zero." lgtm Thanks! Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-14 19:16 ` Mathieu Desnoyers @ 2026-07-14 20:54 ` Paul E. McKenney 2026-07-14 21:06 ` Mathieu Desnoyers 0 siblings, 1 reply; 20+ messages in thread From: Paul E. McKenney @ 2026-07-14 20:54 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Tue, Jul 14, 2026 at 03:16:58PM -0400, Mathieu Desnoyers wrote: > On 2026-07-11 19:55, Paul E. McKenney wrote: > > On Sat, Jul 11, 2026 at 09:48:18AM -0400, Mathieu Desnoyers wrote: > > > On 2026-07-10 15:07, Paul E. McKenney wrote: > > > > > > > > > > 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. > > > > > > > > hazptr_detach_from_context()? > > > > > > > > I am leaving this alone for the moment, easy to change later. > > > > > > I just wonder if that name brings confusion. Is "context" the > > > execution context or the "hazptr ctx" context ? I fear this is > > > really a good opportunity to confuse everyone. > > > > > > [...] > > > > > > > > > > > > Maybe just call it hazptr_detach() ? > > > > > > > > That would be your choice, not mine. ;-) > > > > > > I find that hazptr_detach is probably better here. It does not > > > state it detaches "what", but at least there is no shortcut > > > confusion with the overloaded "context" wording. > > > > Fair enough. > > > > Would you like me to make this change? > > Since you're actively working in that area, please go ahead :) OK, rebasing. ;-) > > > > + */ > > > > static inline > > > > void hazptr_detach_from_task(struct hazptr_ctx *ctx) > > > > { > > > > @@ -160,12 +182,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 later call to hazptr_release(). > > > > > > Perhaps worthwhile to hint at the vast performance/scalability/memory > > > footprint/cache line footprint difference between hazptr and refcount > > > to justify why both exist here ? > > > > Like this? > > > > * Load @addr_p, and protect the loaded pointer with hazard pointer. > > * This protection is roughly similar to (but way faster than) that of a > > * reference counter, and ends with a later call to hazptr_release() > > works for me! Got it, thank you! > > > AFAIU there is partial overlap between refcount, hazptr, and RCU, each > > > with their own strengths and weaknesses. We should prepare a summary > > > table for the end users wondering which is the right tool for their > > > use-case. > > > > We have lots of such tables of varying sizes and levels of detail in > > numerous C++ Working Papers, to say nothing of Section 9.6 of perfbook. ;-) > > > > One short text distinguishing them is: "RCU is a fast and scalable > > replacement for many reader-writer locking use cases, hazard pointers is > > a fast and scalable replacement for many reference-counting use cases, > > reader-writer locking provides mutual exclusion, and reference counting > > provides instant notice when the counter reaches zero." > > lgtm Fair enough! Put this in Documentation/RCU? As a header comment in include/linux/hazptr.h? Somewhere else? Thanx, Paul > Thanks! > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > https://www.efficios.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-14 20:54 ` Paul E. McKenney @ 2026-07-14 21:06 ` Mathieu Desnoyers 2026-07-14 21:31 ` Paul E. McKenney 0 siblings, 1 reply; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-14 21:06 UTC (permalink / raw) To: paulmck; +Cc: Boqun Feng, linux-kernel On 2026-07-14 16:54, Paul E. McKenney wrote: [...] >>>> AFAIU there is partial overlap between refcount, hazptr, and RCU, each >>>> with their own strengths and weaknesses. We should prepare a summary >>>> table for the end users wondering which is the right tool for their >>>> use-case. >>> >>> We have lots of such tables of varying sizes and levels of detail in >>> numerous C++ Working Papers, to say nothing of Section 9.6 of perfbook. ;-) >>> >>> One short text distinguishing them is: "RCU is a fast and scalable >>> replacement for many reader-writer locking use cases, hazard pointers is >>> a fast and scalable replacement for many reference-counting use cases, >>> reader-writer locking provides mutual exclusion, and reference counting >>> provides instant notice when the counter reaches zero." >> >> lgtm > > Fair enough! Put this in Documentation/RCU? As a header comment in > include/linux/hazptr.h? Somewhere else? > I'm tempted to add the documentation into Documentation/RCU and a reference to it in the RCU, hazptr, and refcount headers so people stumbling on one of those three can be pointed to the higher level document which explains how each relate to one another. But is referencing documentation from code even something we typically do ? Also given this would discuss more than just RCU, I'm not sure Doc/RCU is the right home for it, but we don't have a Documentation/existence nor Documentation/synchronization. Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-14 21:06 ` Mathieu Desnoyers @ 2026-07-14 21:31 ` Paul E. McKenney 0 siblings, 0 replies; 20+ messages in thread From: Paul E. McKenney @ 2026-07-14 21:31 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Tue, Jul 14, 2026 at 05:06:37PM -0400, Mathieu Desnoyers wrote: > On 2026-07-14 16:54, Paul E. McKenney wrote: > [...] > > > > > AFAIU there is partial overlap between refcount, hazptr, and RCU, each > > > > > with their own strengths and weaknesses. We should prepare a summary > > > > > table for the end users wondering which is the right tool for their > > > > > use-case. > > > > > > > > We have lots of such tables of varying sizes and levels of detail in > > > > numerous C++ Working Papers, to say nothing of Section 9.6 of perfbook. ;-) > > > > > > > > One short text distinguishing them is: "RCU is a fast and scalable > > > > replacement for many reader-writer locking use cases, hazard pointers is > > > > a fast and scalable replacement for many reference-counting use cases, > > > > reader-writer locking provides mutual exclusion, and reference counting > > > > provides instant notice when the counter reaches zero." > > > > > > lgtm > > > > Fair enough! Put this in Documentation/RCU? As a header comment in > > include/linux/hazptr.h? Somewhere else? > > > > I'm tempted to add the documentation into Documentation/RCU and a > reference to it in the RCU, hazptr, and refcount headers so people > stumbling on one of those three can be pointed to the higher level > document which explains how each relate to one another. > > But is referencing documentation from code even something we typically > do ? > > Also given this would discuss more than just RCU, I'm not sure Doc/RCU > is the right home for it, but we don't have a Documentation/existence > nor Documentation/synchronization. Thoughts ? Rename kernel/rcu to kernel/defer and Documentation/RCU to Documentation/defer? I am sure that this would not be at all disruptive. ;-) Thanx, Paul ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 23:22 ` Mathieu Desnoyers 2026-07-09 23:48 ` Paul E. McKenney @ 2026-07-09 23:57 ` Boqun Feng 2026-07-10 0:03 ` Mathieu Desnoyers 1 sibling, 1 reply; 20+ messages in thread From: Boqun Feng @ 2026-07-09 23:57 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: paulmck, linux-kernel 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: > [...] > > > > + */ > > > > 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! :) > 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? Regards, Boqun > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 23:57 ` Boqun Feng @ 2026-07-10 0:03 ` Mathieu Desnoyers 0 siblings, 0 replies; 20+ messages in thread From: Mathieu Desnoyers @ 2026-07-10 0:03 UTC (permalink / raw) To: Boqun Feng; +Cc: paulmck, linux-kernel 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: >> [...] >>>>> + */ >>>>> 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! :) >> > > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection 2026-07-09 18:47 ` Paul E. McKenney 2026-07-09 18:56 ` Mathieu Desnoyers @ 2026-07-09 20:45 ` Paul E. McKenney 1 sibling, 0 replies; 20+ messages in thread From: Paul E. McKenney @ 2026-07-09 20:45 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: Boqun Feng, linux-kernel On Thu, Jul 09, 2026 at 11:47:08AM -0700, Paul E. McKenney wrote: > On Thu, Jul 09, 2026 at 02:29:33PM -0400, Mathieu Desnoyers wrote: > > Introduce Hazard Pointers debug assert, which detects misuse of hazard > > pointers, namely failure to detach the hazard pointer from its owner > > thread before releasing it from a different thread. > > > > Prints the following to the console when a failure is detected: > > Thank you, Mathieu! > > I am testing this one first to verify the debugging, and then I will > test the patch 1/2 hazptrtorture.c changes to verify the fix. And both tests passed, so thank you! > We are going to need to carefully document the hazard-pointer handling > rules! ;-) And for that, please see my other email in this thread. Thanx, Paul > > Hazard Pointer (addr=000000006885a05f) released on remote task without being detached from task. Acquire: caller=hazptr_torture_read_lock+0x43/0xa0 [hazptrtorture], pid=3727, cpu=1. Release: pid=3725, cpu=139. > > WARNING: ./include/linux/hazptr.h:225 at hazptr_torture_read_unlock+0x68/0xf0 [hazptrtorture], CPU#139: hazptr_torture_/3725 > > Modules linked in: hazptrtorture torture nft_masq nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink > > CPU: 139 UID: 0 PID: 3725 Comm: hazptr_torture_ Not tainted 7.1.0-rc4+ #9 PREEMPT(full) > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 > > RIP: 0010:hazptr_torture_read_unlock+0x74/0xf0 [hazptrtorture] > > Code: 74 31 8b 4f 40 4c 8b 43 48 49 c7 c2 22 c9 56 c0 48 c7 c7 3b c9 56 c0 4c 8d 1d b8 32 f1 ff 52 48 89 fa 4c 89 df 50 51 4c 89 d1 <67> 48 0f b9 3a 48 83 c4 18 48 8b 03 48 8d 53 18 48 c7 00 00 00 00 > > RSP: 0018:ff621a2a479b3de0 EFLAGS: 00010293 > > RAX: 0000000000000e8d RBX: ff12ea30d3913488 RCX: ffffffffc056c922 > > RDX: ffffffffc056c93b RSI: ffffffffc0562280 RDI: ffffffffc0562030 > > RBP: ff621a2a479b3e50 R08: ffffffffc064ee53 R09: 0000000000000e8f > > R10: ffffffffc056c922 R11: ffffffffc0562030 R12: 0000000000000000 > > R13: ffffffffc0562280 R14: ff12ea30d3913488 R15: ff621a2a479b3e50 > > FS: 0000000000000000(0000) GS:ff12ea5011a84000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00007f39b6e4b010 CR3: 0000000114c6e005 CR4: 0000000000771ef0 > > PKRU: 55555554 > > Call Trace: > > <TASK> > > hazptr_torture_reader_tail+0x8e/0x210 [hazptrtorture] > > hazptr_torture_reader+0x145/0xb30 [hazptrtorture] > > ? srso_alias_return_thunk+0x5/0xfbef5 > > ? set_cpus_allowed_ptr+0x36/0x60 > > ? srso_alias_return_thunk+0x5/0xfbef5 > > ? srso_alias_return_thunk+0x5/0xfbef5 > > ? __pfx_hazptr_torture_reader+0x10/0x10 [hazptrtorture] > > kthread+0xdf/0x120 > > ? __pfx_kthread+0x10/0x10 > > ret_from_fork+0x216/0x2d0 > > ? __pfx_kthread+0x10/0x10 > > ret_from_fork_asm+0x1a/0x30 > > </TASK> > > ---[ end trace 0000000000000000 ]--- > > > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > --- > > include/linux/hazptr.h | 38 ++++++++++++++++++++++++++++++++++++++ > > kernel/rcu/Kconfig.debug | 9 +++++++++ > > 2 files changed, 47 insertions(+) > > > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h > > index d96414575739..c5aa4fb03b11 100644 > > --- a/include/linux/hazptr.h > > +++ b/include/linux/hazptr.h > > @@ -51,6 +51,11 @@ struct hazptr_ctx { > > /* Backup slot in case all per-CPU slots are used. */ > > struct hazptr_backup_slot backup_slot; > > struct hlist_node preempt_node; > > +#ifdef CONFIG_HAZPTR_DEBUG > > + bool detach_task, detach_cpu; /* Whether the ctx has been detached from task/cpu. */ > > + int acquire_pid, acquire_cpu; /* Note the task and cpu number at acquire. */ > > + unsigned long acquire_caller; /* Acquire instruction pointer. */ > > +#endif > > }; > > > > struct hazptr_slot_ctx { > > @@ -127,6 +132,9 @@ void hazptr_detach_from_task(struct hazptr_ctx *ctx) > > struct hazptr_slot *slot; > > > > guard(preempt)(); > > +#ifdef CONFIG_HAZPTR_DEBUG > > + ctx->detach_task = ctx->detach_cpu = true; > > +#endif > > slot = ctx->slot; > > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > > return; > > @@ -145,6 +153,9 @@ void hazptr_note_context_switch(void) > > > > if (!slot->addr) > > continue; > > +#ifdef CONFIG_HAZPTR_DEBUG > > + item->ctx.ctx->detach_cpu = true; > > +#endif > > hazptr_promote_to_backup_slot(item->ctx.ctx, slot); > > } > > } > > @@ -173,6 +184,12 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) > > percpu_slots = this_cpu_ptr(&hazptr_percpu_slots); > > slot_item = &percpu_slots->items[0]; > > slot = &slot_item->slot; > > +#ifdef CONFIG_HAZPTR_DEBUG > > + ctx->detach_cpu = ctx->detach_task = false; > > + ctx->acquire_pid = current->pid; > > + ctx->acquire_cpu = smp_processor_id(); > > + ctx->acquire_caller = _THIS_IP_; > > +#endif > > if (unlikely(slot->addr)) > > return __hazptr_acquire(ctx, addr_p); > > WRITE_ONCE(slot->addr, HAZPTR_WILDCARD); /* Store B */ > > @@ -196,6 +213,26 @@ void *hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p) > > return addr; > > } > > > > +#ifdef CONFIG_HAZPTR_DEBUG > > +/* Called with preemption disabled. */ > > +static inline > > +void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) > > +{ > > + int pid = current->pid, cpu = smp_processor_id(); > > + bool warn_remote_cpu = !ctx->detach_cpu && ctx->acquire_cpu != cpu, > > + warn_remote_task = !ctx->detach_task && ctx->acquire_pid != pid; > > + > > + WARN_ONCE(warn_remote_cpu || warn_remote_task, > > + "Hazard Pointer (addr=%p) released on remote %s without %s. Acquire: caller=%pS, pid=%d, cpu=%d. Release: pid=%d, cpu=%d.", > > + addr, > > + warn_remote_task ? "task" : "cpu", > > + warn_remote_task ? "being detached from task" : "context switch", > > + (void *) ctx->acquire_caller, ctx->acquire_pid, ctx->acquire_cpu, pid, cpu); > > +} > > +#else > > +static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { } > > +#endif > > + > > /* Release the protected hazard pointer from @slot. */ > > static inline > > void hazptr_release(struct hazptr_ctx *ctx, void *addr) > > @@ -205,6 +242,7 @@ void hazptr_release(struct hazptr_ctx *ctx, void *addr) > > if (!addr) > > return; > > guard(preempt)(); > > + hazptr_release_debug(ctx, addr); > > slot = ctx->slot; > > smp_store_release(&slot->addr, NULL); > > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > > diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug > > index 7629c345b0b6..57fec4444a97 100644 > > --- a/kernel/rcu/Kconfig.debug > > +++ b/kernel/rcu/Kconfig.debug > > @@ -264,4 +264,13 @@ config TRIVIAL_PREEMPT_RCU > > > > This has no value for production and is only for testing. > > > > +config HAZPTR_DEBUG > > + bool "Provide debugging asserts for Hazard Pointers" > > + depends on DEBUG_KERNEL > > + default n > > + help > > + This option provides consistency checks for Hazard pointers. > > + > > + Say Y here if you want to enable those assert, N otherwise. > > + > > endmenu # "RCU Debugging" > > -- > > 2.43.0 > > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-07-14 21:31 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-07-09 18:29 [PATCH 1/2] hazptrtorture: Fix hazptr ownership issue Mathieu Desnoyers 2026-07-09 18:29 ` [PATCH 2/2] hazptr: Introduce CONFIG_HAZPTR_DEBUG misuse detection Mathieu Desnoyers 2026-07-09 18:47 ` Paul E. McKenney 2026-07-09 18:56 ` Mathieu Desnoyers 2026-07-09 20:44 ` Paul E. McKenney 2026-07-09 21:47 ` Mathieu Desnoyers 2026-07-09 23:05 ` Paul E. McKenney 2026-07-09 23:22 ` Mathieu Desnoyers 2026-07-09 23:48 ` Paul E. McKenney 2026-07-10 0:10 ` Mathieu Desnoyers 2026-07-10 19:07 ` Paul E. McKenney 2026-07-11 13:48 ` Mathieu Desnoyers 2026-07-11 23:55 ` Paul E. McKenney 2026-07-14 19:16 ` Mathieu Desnoyers 2026-07-14 20:54 ` Paul E. McKenney 2026-07-14 21:06 ` Mathieu Desnoyers 2026-07-14 21:31 ` Paul E. McKenney 2026-07-09 23:57 ` Boqun Feng 2026-07-10 0:03 ` Mathieu Desnoyers 2026-07-09 20:45 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome