Re: [PATCH 2/2] mm/slab: use prandom if !allow_spin

[Harry Yoo <harry.yoo@oracle.com>]

On Fri, Feb 06, 2026 at 11:22:27AM -0800, Alexei Starovoitov wrote:
> On Fri, Feb 6, 2026 at 10:27 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 2/6/26 18:13, Harry Yoo wrote:
> > > When CONFIG_SLAB_FREELIST_RANDOM is enabled and get_random_u32()
> > > is called in an NMI context, lockdep complains because it acquires
> > > a local_lock:
> > >
> > >   ================================
> > >   WARNING: inconsistent lock state
> > >   6.19.0-rc5-slab-for-next+ #325 Tainted: G                 N
> > >   --------------------------------
> > >   inconsistent {INITIAL USE} -> {IN-NMI} usage.
> > >   kunit_try_catch/8312 [HC2[2]:SC0[0]:HE0:SE1] takes:
> > >   ffff88a02ec49cc0 (batched_entropy_u32.lock){-.-.}-{3:3}, at: get_random_u32+0x7f/0x2e0
> > >   {INITIAL USE} state was registered at:
> > >     lock_acquire+0xd9/0x2f0
> > >     get_random_u32+0x93/0x2e0
> > >     __get_random_u32_below+0x17/0x70
> > >     cache_random_seq_create+0x121/0x1c0
> > >     init_cache_random_seq+0x5d/0x110
> > >     do_kmem_cache_create+0x1e0/0xa30
> > >     __kmem_cache_create_args+0x4ec/0x830
> > >     create_kmalloc_caches+0xe6/0x130
> > >     kmem_cache_init+0x1b1/0x660
> > >     mm_core_init+0x1d8/0x4b0
> > >     start_kernel+0x620/0xcd0
> > >     x86_64_start_reservations+0x18/0x30
> > >     x86_64_start_kernel+0xf3/0x140
> > >     common_startup_64+0x13e/0x148
> > >   irq event stamp: 76
> > >   hardirqs last  enabled at (75): [<ffffffff8298b77a>] exc_nmi+0x11a/0x240
> > >   hardirqs last disabled at (76): [<ffffffff8298b991>] sysvec_irq_work+0x11/0x110
> > >   softirqs last  enabled at (0): [<ffffffff813b2dda>] copy_process+0xc7a/0x2350
> > >   softirqs last disabled at (0): [<0000000000000000>] 0x0
> > >
> > >   other info that might help us debug this:
> > >    Possible unsafe locking scenario:
> > >
> > >          CPU0
> > >          ----
> > >     lock(batched_entropy_u32.lock);
> > >     <Interrupt>
> > >       lock(batched_entropy_u32.lock);
> > >
> > >    *** DEADLOCK ***
> > >
> > > Fix this by using pseudo-random number generator if !allow_spin.
> > > This means kmalloc_nolock() users won't get truly random numbers,
> > > but there is not much we can do about it.
> > >
> > > Note that an NMI handler might interrupt prandom_u32_state() and
> > > change the random state, but that's safe.
> > >
> > > Link: https://lore.kernel.org/all/0c33bdee-6de8-4d9f-92ca-4f72c1b6fb9f@suse.cz
> > > Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
> > > ---
> > >  mm/slub.c | 28 ++++++++++++++++++++++++----
> > >  1 file changed, 24 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/mm/slub.c b/mm/slub.c
> > > index d46464654c15..4d76af84f018 100644
> > > --- a/mm/slub.c
> > > +++ b/mm/slub.c
> > > @@ -43,6 +43,7 @@
> > >  #include <linux/prefetch.h>
> > >  #include <linux/memcontrol.h>
> > >  #include <linux/random.h>
> > > +#include <linux/prandom.h>
> > >  #include <kunit/test.h>
> > >  #include <kunit/test-bug.h>
> > >  #include <linux/sort.h>
> > > @@ -3308,8 +3309,11 @@ static void *next_freelist_entry(struct kmem_cache *s,
> > >       return (char *)start + idx;
> > >  }
> > >
> > > +static DEFINE_PER_CPU(struct rnd_state, slab_rnd_state);
> > > +
> > >  /* Shuffle the single linked freelist based on a random pre-computed sequence */
> > > -static bool shuffle_freelist(struct kmem_cache *s, struct slab *slab)
> > > +static bool shuffle_freelist(struct kmem_cache *s, struct slab *slab,
> > > +                          bool allow_spin)
> > >  {
> > >       void *start;
> > >       void *cur;
> > > @@ -3320,7 +3324,19 @@ static bool shuffle_freelist(struct kmem_cache *s, struct slab *slab)
> > >               return false;
> > >
> > >       freelist_count = oo_objects(s->oo);
> > > -     pos = get_random_u32_below(freelist_count);
> > > +     if (allow_spin) {
> > > +             pos = get_random_u32_below(freelist_count);
> > > +     } else {
> > > +             struct rnd_state *state;
> > > +
> > > +             /*
> > > +              * kmalloc_nolock() called in an NMI context might interrupt
> > > +              * and change the state in the middle.
> > > +              */
> > > +             state = &get_cpu_var(slab_rnd_state);
> > > +             pos = prandom_u32_state(state) % freelist_count;
> > > +             put_cpu_var(slab_rnd_state);
> >
> > I don't think this prevents the changing in the middle? We just stored the
> > pointer in a local variable state, but the prandom call will still access
> > the percpu variable through that?
> >
> > So we might need to disable irq here, and have another percpu state that's
> > used when in_nmi()?

Oh, my intention was not preventing state changes in the middle.
I was thinking "Hmm if we can't disable NMI, do we even need to disable
IRQ? just add some comment saying it might be interrupted
in the middle".

I was even thinking of using raw_cpu_ptr() instead without disabling
preemption through get/put_cpu_var()...

> imo this is all overkill.
> Just prandom_u32_state() without any protection is fine.
> Even if it reenters there is no harm. Just more randomness.

Yeah.

-- 
Cheers,
Harry / Hyeonggon