| Message ID | 20240710025418.394321-1-sxwjean@me.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/slub: quiet the clang warning with -Wunused-function enabled | expand |
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? > 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 > >
July 10, 2024 at 11:03 AM, "Matthew Wilcox" <willy@infradead.org> 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? Maybe, I'm not sure. Thanks, Xiongwei > > > > > 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 > > >
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. > > 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 > > > >
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. 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 > > > > > >
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 >> > > >> > >
On Thu, Jul 11, 2024 at 12:43 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > 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. Exactly my plan. I'll post a patch once I reach the office. > > > 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 > >> > > > >> > > >
On Thu, Jul 11, 2024 at 6:56 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Thu, Jul 11, 2024 at 12:43 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > 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. > > 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@google.com/ [2] https://lore.kernel.org/all/20240711170216.1149695-1-surenb@google.com/ > > > > > > 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 > > >> > > > > >> > > > >
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;