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

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

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()?

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.