On Thu, Jul 11, 2024 at 6:56 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote: > > On Thu, Jul 11, 2024 at 12:43 AM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > > > On 7/10/24 11:40 PM, Suren Baghdasaryan wrote: > > > On Wed, Jul 10, 2024 at 8:02 AM Nathan Chancellor <nathan@xxxxxxxxxx> 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@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. > > > > > > I think this is the consequence of the recent refactoring I've done in > > > https://lore.kernel.org/all/20240704135941.1145038-1-surenb@xxxxxxxxxx/. > > > 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. > > Exactly my plan. I'll post a patch once I reach the office. Actually I was wrong and the problem exists even without my refactoring in [1]. I posted the fix at [2] and I based it on slab/for-next because that's the only branch that contains [1]. Not because [2] requires [1] but because they are changing adjacent code, so would create merge problems if merged separately. [1] https://lore.kernel.org/all/20240704135941.1145038-1-surenb@xxxxxxxxxx/ [2] https://lore.kernel.org/all/20240711170216.1149695-1-surenb@xxxxxxxxxx/ > > > > > > Thanks, > > > Suren. > > > > > >> > > >> > > 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 > > >> > > > > >> > > > >