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

[Nathan Chancellor]

On Wed, Jul 10, 2024 at 04:03:33AM +0100, Matthew Wilcox wrote:
> On Wed, Jul 10, 2024 at 10:54:18AM +0800, sxwjean@xxxxxx wrote:
> > From: Xiongwei Song <xiongwei.song@xxxxxxxxx>
> > 
> > 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.

> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202407050845.zNONqauD-lkp@xxxxxxxxx/
> > Signed-off-by: Xiongwei Song <xiongwei.song@xxxxxxxxx>
> > ---
> >  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
> > 
> >