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

[Vlastimil Babka <vbabka@suse.cz>]

On 9/12/25 18:39, Alexei Starovoitov wrote:
> On Fri, Sep 12, 2025 at 6:23 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> --- 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.

TBH when I see some code with a side-effect wrapped in a WARN or assert that
can be configured off, it already raises a question if that gets executed
when configured off. There are examples where it does, and where it doesn't.

> what clang warning suggested is imo cleaner:

That silences the warning about flags, however it means that when lockdep
gets disabled on runtime, the locks stop locking (but not unlocking). While
debug_locks_off() only seems to get called in a serious situations
currently, we should not make things even worse then.

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

And so I've folded in mine.