Re: [PATCH 1/2] mm/slab: skip get_from_any_partial() if !allow_spin

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Fri, Feb 6, 2026 at 10:10 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 2/6/26 18:13, Harry Yoo wrote:
> > Lockdep complains when get_from_any_partial() is called in an NMI
> > context, because current->mems_allowed_seq is seqcount_spinlock_t and
> > not NMI-safe:
> >
> >   ================================
> >   WARNING: inconsistent lock state
> >   6.19.0-rc5-kfree-rcu+ #315 Tainted: G                 N
> >   --------------------------------
> >   inconsistent {INITIAL USE} -> {IN-NMI} usage.
> >   kunit_try_catch/9989 [HC1[1]:SC0[0]:HE0:SE1] takes:
> >   ffff889085799820 (&____s->seqcount#3){.-.-}-{0:0}, at: ___slab_alloc+0x58f/0xc00
> >   {INITIAL USE} state was registered at:
> >     lock_acquire+0x185/0x320
> >     kernel_init_freeable+0x391/0x1150
> >     kernel_init+0x1f/0x220
> >     ret_from_fork+0x736/0x8f0
> >     ret_from_fork_asm+0x1a/0x30
> >   irq event stamp: 56
> >   hardirqs last  enabled at (55): [<ffffffff850a68d7>] _raw_spin_unlock_irq+0x27/0x70
> >   hardirqs last disabled at (56): [<ffffffff850858ca>] __schedule+0x2a8a/0x6630
> >   softirqs last  enabled at (0): [<ffffffff81536711>] copy_process+0x1dc1/0x6a10
> >   softirqs last disabled at (0): [<0000000000000000>] 0x0
> >
> >   other info that might help us debug this:
> >    Possible unsafe locking scenario:
> >
> >          CPU0
> >          ----
> >     lock(&____s->seqcount#3);
> >     <Interrupt>
> >       lock(&____s->seqcount#3);
> >
> >    *** DEADLOCK ***
> >
> > According to Documentation/locking/seqlock.rst, seqcount_t is not
> > NMI-safe and seqcount_latch_t should be used when read path can interrupt
> > the write-side critical section. In this case, return NULL and fall back
> > to slab allocation if !allow_spin.
> >
> > Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
> > ---
> >  mm/slub.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 102fb47ae013..d46464654c15 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3789,6 +3789,14 @@ static void *get_from_any_partial(struct kmem_cache *s, struct partial_context *
> >       enum zone_type highest_zoneidx = gfp_zone(pc->flags);
> >       unsigned int cpuset_mems_cookie;
> >
> > +     /*
> > +      * read_mems_allow_begin() accesses current->mems_allowed_seq,
> > +      * a seqcount_spinlock_t that is not NMI-safe. Skip allocation
> > +      * when GFP flags indicate spinning is not allowed.
> > +      */
> > +     if (!gfpflags_allow_spinning(pc->flags))
> > +             return NULL;
>
> I think it would be less restrictive to just continue, but skip the
> read_mems_allowed_retry() part in the do-while loop, so just make it one
> iteration for !allow_spin. If lockdep doesn't like even the
> read_mems_allowed_begin() (not clear to me), skip it too?

+1
Just unconditional return NULL seems too restrictive.