Re: [PATCH] mm/slub: quiet the clang warning with -Wunused-function enabled

[Vlastimil Babka <vbabka@suse.cz>]

On 7/10/24 11:40 PM, Suren Baghdasaryan wrote:
> On Wed, Jul 10, 2024 at 8:02 AM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On Wed, Jul 10, 2024 at 04:03:33AM +0100, Matthew Wilcox wrote:
>> > On Wed, Jul 10, 2024 at 10:54:18AM +0800, sxwjean@me.com wrote:
>> > > From: Xiongwei Song <xiongwei.song@linux.dev>
>> > >
>> > > The only user of prepare_slab_obj_exts_hook() is
>> > > alloc_tagging_slab_alloc_hook(), which can build with
>> > > CONFIG_MEM_ALLOC_PROFILING enabled. So, the warning was triggerred
>> > > when disabling CONFIG_MEM_ALLOC_PROFILING. Let's add "__maybe_unused"
>> > > for prepare_slab_obj_exts_hook().
>> >
>> > Perhaps instead clang can be fixed to match gcc's behaviour?
>>
>> Clang only differs from GCC on warning for unused static inline functions in .c
>> files, not .h files. The kernel already handles this in
>> include/linux/compiler_types.h but it disables this workaround for W=1 to catch
>> unused functions like this as a result of commit 6863f5643dd7 ("kbuild: allow
>> Clang to find unused static inline functions for W=1 build"):
>>
>> /*
>>  * GCC does not warn about unused static inline functions for -Wunused-function.
>>  * Suppress the warning in clang as well by using __maybe_unused, but enable it
>>  * for W=1 build. This will allow clang to find unused functions. Remove the
>>  * __inline_maybe_unused entirely after fixing most of -Wunused-function warnings.
>>  */
>> #ifdef KBUILD_EXTRA_WARN1
>> #define __inline_maybe_unused
>> #else
>> #define __inline_maybe_unused __maybe_unused
>> #endif
>>
>> So I don't really think there is much for clang to do here and I think having
>> the ability to find unused static inline functions in .c files is useful (you
>> might disagree, perhaps a revert could still be discussed). I guess
>> IS_ENABLED() can't be used there, so it seems like either taking this patch,
>> ignoring the warning, or refactoring the code in some other way are the only
>> options I see.
> 
> I think this is the consequence of the recent refactoring I've done in
> https://lore.kernel.org/all/20240704135941.1145038-1-surenb@google.com/.
> There should be a cleaner way to fix this. I'll post it later today or
> tomorrow morning.

Yeah looks like the non-empty prepare_slab_obj_exts_hook() could move to the
#ifdef CONFIG_MEM_ALLOC_PROFILING section above
alloc_tagging_slab_alloc_hook() and the empty one just removed.

> Thanks,
> Suren.
> 
>>
>> > > Reported-by: kernel test robot <lkp@intel.com>
>> > > Closes: https://lore.kernel.org/oe-kbuild-all/202407050845.zNONqauD-lkp@intel.com/
>> > > Signed-off-by: Xiongwei Song <xiongwei.song@linux.dev>
>> > > ---
>> > >  mm/slub.c | 4 ++--
>> > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/mm/slub.c b/mm/slub.c
>> > > index ce39544acf7c..2e26f20759c0 100644
>> > > --- a/mm/slub.c
>> > > +++ b/mm/slub.c
>> > > @@ -2027,7 +2027,7 @@ static inline bool need_slab_obj_ext(void)
>> > >     return false;
>> > >  }
>> > >
>> > > -static inline struct slabobj_ext *
>> > > +static inline struct slabobj_ext * __maybe_unused
>> > >  prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
>> > >  {
>> > >     struct slab *slab;
>> > > @@ -2068,7 +2068,7 @@ static inline bool need_slab_obj_ext(void)
>> > >     return false;
>> > >  }
>> > >
>> > > -static inline struct slabobj_ext *
>> > > +static inline struct slabobj_ext * __maybe_unused
>> > >  prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
>> > >  {
>> > >     return NULL;
>> > > --
>> > > 2.34.1
>> > >
>> > >