Re: [vbabka-slab:slab/for-6.18/kmalloc_nolock 14/14] mm/slub.c:3866:2: warning: variable 'flags' is used uninitialized whenever '&&' condition is false

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

On Fri, Sep 12, 2025 at 6:23 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/12/25 12:25, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git slab/for-6.18/kmalloc_nolock
> > head:   8014922e0e72dde7684abebeab3404720401679f
> > commit: 8014922e0e72dde7684abebeab3404720401679f [14/14] slab: Introduce kmalloc_nolock() and kfree_nolock().
> > config: x86_64-buildonly-randconfig-004-20250912 (https://download.01.org/0day-ci/archive/20250912/202509121822.aOV6H1ts-lkp@intel.com/config)
> > compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250912/202509121822.aOV6H1ts-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202509121822.aOV6H1ts-lkp@intel.com/
> >
> > All warnings (new ones prefixed by >>):
> >
> >>> mm/slub.c:3866:2: warning: variable 'flags' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized]
> >     3866 |         local_lock_cpu_slab(s, flags);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    mm/slub.c:3776:2: note: expanded from macro 'local_lock_cpu_slab'
> >     3776 |         lockdep_assert(local_trylock_irqsave(&(s)->cpu_slab->lock, flags))
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/lockdep.h:279:15: note: expanded from macro 'lockdep_assert'
> >      279 |         do { WARN_ON(debug_locks && !(cond)); } while (0)
> >          |                      ^~~~~~~~~~~
> >    include/asm-generic/bug.h:171:25: note: expanded from macro 'WARN_ON'
> >      171 |         int __ret_warn_on = !!(condition);                              \
> >          |                                ^~~~~~~~~
> >    mm/slub.c:3891:27: note: uninitialized use occurs here
> >     3891 |         local_unlock_cpu_slab(s, flags);
> >          |                                  ^~~~~
> >    mm/slub.c:3780:48: note: expanded from macro 'local_unlock_cpu_slab'
> >     3780 |         local_unlock_irqrestore(&(s)->cpu_slab->lock, flags)
> >          |                                                       ^~~~~
> >    include/linux/local_lock.h:52:48: note: expanded from macro 'local_unlock_irqrestore'
> >       52 |         __local_unlock_irqrestore(this_cpu_ptr(lock), flags)
> >          |                                                       ^~~~~
> >    include/linux/local_lock_internal.h:202:21: note: expanded from macro '__local_unlock_irqrestore'
> >      202 |                 local_irq_restore(flags);                       \
> >          |                                   ^~~~~
> >    include/linux/irqflags.h:240:61: note: expanded from macro 'local_irq_restore'
> >      240 | #define local_irq_restore(flags) do { raw_local_irq_restore(flags); } while (0)
> >          |                                                             ^~~~~
> >    include/linux/irqflags.h:179:26: note: expanded from macro 'raw_local_irq_restore'
> >      179 |                 arch_local_irq_restore(flags);          \
> >          |                                        ^~~~~
> >    mm/slub.c:3866:2: note: remove the '&&' if its condition is always true
> >     3866 |         local_lock_cpu_slab(s, flags);
> >          |         ^
> >    mm/slub.c:3776:2: note: expanded from macro 'local_lock_cpu_slab'
> >     3776 |         lockdep_assert(local_trylock_irqsave(&(s)->cpu_slab->lock, flags))
> >          |         ^
> >    include/linux/lockdep.h:279:15: note: expanded from macro 'lockdep_assert'
> >      279 |         do { WARN_ON(debug_locks && !(cond)); } while (0)
> >          |                      ^
> >    mm/slub.c:3863:21: note: initialize the variable 'flags' to silence this warning
> >     3863 |         unsigned long flags;
> >          |                            ^
> >          |                             = 0
> >    1 warning generated.
> >
> >
> > vim +3866 mm/slub.c
>
> >   3852
> >   3853        /*
> >   3854         * Put a slab into a partial slab slot if available.
> >   3855         *
> >   3856         * If we did not find a slot then simply move all the partials to the
> >   3857         * per node partial list.
> >   3858         */
> >   3859        static void put_cpu_partial(struct kmem_cache *s, struct slab *slab, int drain)
> >   3860        {
> >   3861                struct slab *oldslab;
> >   3862                struct slab *slab_to_put = NULL;
> >   3863                unsigned long flags;
> >   3864                int slabs = 0;
> >   3865
> >> 3866         local_lock_cpu_slab(s, flags);
>
> I guess this?
>
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3772,8 +3772,11 @@ static void deactivate_slab(struct kmem_cache *s, struct slab *slab,
>  #define local_lock_cpu_slab(s, flags)  \
>         local_lock_irqsave(&(s)->cpu_slab->lock, flags)
>  #else
> -#define local_lock_cpu_slab(s, flags)  \
> -       lockdep_assert(local_trylock_irqsave(&(s)->cpu_slab->lock, flags))
> +#define gg(s, flags)                                         \
> +       do {                                                                   \
> +               bool __l = local_trylock_irqsave(&(s)->cpu_slab->lock, flags); \
> +               lockdep_assert(__l);

That works, but would raise questions for anyone looking at
these two lines as-is, since it's an implementation
detail of lockdep_assert() and combination of .config flags.

what clang warning suggested is imo cleaner:

diff --git a/mm/slub.c b/mm/slub.c
index 4079abb5d0e1..a6b50cd1d95f 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3865,7 +3865,7 @@ static void put_cpu_partial(struct kmem_cache
*s, struct slab *slab, int drain)
 {
        struct slab *oldslab;
        struct slab *slab_to_put = NULL;
-       unsigned long flags;
+       unsigned long flags = 0;
        int slabs = 0;

Could you fold one of them in?