On 8 September 2026 17:22:09 BST, "Paul E. McKenney" <[email protected]>
wrote:
>On Tue, Sep 08, 2026 at 11:29:31AM -0400, Mathieu Desnoyers wrote:
>> On 2026-09-08 11:24, Bradley Morgan wrote:
>> > On 8 September 2026 16:22:14 BST, Mathieu Desnoyers
>> > <[email protected]> wrote:
>> > > When hazptr_acquire loads a NULL pointer, it sets:
>> > > 
>> > > - slot_item->slot.addr = NULL,
>> > > - slot_item->ctx.ctx = ctx
>> > > - ctx->slot = slot
>> > > 
>> > > And it returns NULL.
>> > > 
>> > > Then hazptr_detach is called on this ctx, it will act on the ctx as
>if
>> > > needed to be promoted to backup slot, even though it has a NULL
>addr.
>> > > 
>> > > Looking at what hazptr_note_context_switch() does before promoting
>> > > to backup slot, it checks for a NULL slot->addr, which is exactly
>> > > what is missing from hazptr_detach.
>> > > 
>> > > With this in place there would be no need to explicitly check the
>> > > hazptr_acquire() return value before calling hazptr_detach().
>> > > 
>> > > hazptr_release() has a early return check for NULL addr as well, so
>it
>> > > makes sense that detach does an early return (no-op) similarly.
>> > > 
>> > 
>> > You shall kill me for this!!
>> > 
>> > Could you perhaps do a splat in ze commit description pls?
>> 
>> The splat is available at the "Closes" URL below. I'm not sure whether
>> we should duplicate this verbose information ?
>> 
>> Paul ?
>
>I am fine either way, as long as the information is reasonably easily
>accessible.  Which is the case either way.  ;-)
>
>                                                       Thanx, Paul
>

Hi, what do you reckon, do you wanna merge it?

I can co develop hazptr if u want, it's school season for me but oh well.
Commit to Linux anyway!

>> Thanks,
>> 
>> Mathieu
>> 
>> > 
>> > 
>> > > Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue")
>> > > Reported-by: kernel test robot <[email protected]>
>> > > Closes:
>https://lore.kernel.org/oe-lkp/[email protected]
>> > > Signed-off-by: Mathieu Desnoyers <[email protected]>
>> > > Reviewed-by: Bradley Morgan <[email protected]>
>> > > Cc: Paul E. McKenney <[email protected]>
>> > > Cc: Boqun Feng <[email protected]>
>> > > Cc: Bradley Morgan <[email protected]>
>> > > Cc: <[email protected]>
>> > > Cc: <[email protected]>
>> > > ---
>> > > include/linux/hazptr.h | 4 +++-
>> > > 1 file changed, 3 insertions(+), 1 deletion(-)
>> > > 
>> > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h
>> > > index 43122c5673bd..d1670121947a 100644
>> > > --- a/include/linux/hazptr.h
>> > > +++ b/include/linux/hazptr.h
>> > > @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx)
>> > >  struct hazptr_slot *slot;
>> > > 
>> > >  guard(preempt)();
>> > > +        slot = ctx->slot;
>> > > +        if (!slot->addr)
>> > > +                return;
>> > > #ifdef CONFIG_HAZPTR_DEBUG
>> > >  ctx->detach_task = ctx->detach_cpu = true;
>> > > #endif
>> > > -        slot = ctx->slot;
>> > >  if (unlikely(hazptr_slot_is_backup(ctx, slot)))
>> > >          return;
>> > >  hazptr_promote_to_backup_slot(ctx, slot);
>> > > 
>> > 
>> > --- Thanks!
>> >
>https://lore.kernel.org/all/[email protected]/
>> 
>> 
>> -- 
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> https://www.efficios.com

--- Thanks!
https://lore.kernel.org/all/[email protected]/

Reply via email to