| Message ID | 20191001142227.1227176-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memcontrol.c: fix another unused function warning | expand |
On Tue, 2019-10-01 at 16:22 +0200, Arnd Bergmann wrote: > Removing the mem_cgroup_id_get() stub function introduced a new warning > of the same kind when CONFIG_MMU is disabled: Shouldn't CONFIG_MEMCG depends on CONFIG_MMU instead? > > mm/memcontrol.c:4929:13: error: unused function 'mem_cgroup_id_get_many' [-Werror,-Wunused-function] > > Address this using a __maybe_unused annotation. > > Note: alternatively, this could be moved into an #ifdef block. Marking it > 'static inline' would not work here as that would still produce the > warning on clang, which only ignores unused inline functions declared > in header files instead of .c files. > > Fixes: 4d0e3230a56a ("mm/memcontrol.c: fix a -Wunused-function warning") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > mm/memcontrol.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c313c49074ca..5f9f90e3cef8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -4921,7 +4921,8 @@ static void mem_cgroup_id_remove(struct mem_cgroup *memcg) > } > } > > -static void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) > +static void __maybe_unused > +mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) > { > refcount_add(n, &memcg->id.ref); > }
On Tue, Oct 1, 2019 at 4:40 PM Qian Cai <cai@lca.pw> wrote: > > On Tue, 2019-10-01 at 16:22 +0200, Arnd Bergmann wrote: > > Removing the mem_cgroup_id_get() stub function introduced a new warning > > of the same kind when CONFIG_MMU is disabled: > > Shouldn't CONFIG_MEMCG depends on CONFIG_MMU instead? Maybe. Generally we allow building a lot of stuff without CONFIG_MMU that may not make sense, so I just followed the same idea here. Arnd
On Tue, Oct 1, 2019 at 7:22 AM Arnd Bergmann <arnd@arndb.de> wrote: > > Removing the mem_cgroup_id_get() stub function introduced a new warning > of the same kind when CONFIG_MMU is disabled: > > mm/memcontrol.c:4929:13: error: unused function 'mem_cgroup_id_get_many' [-Werror,-Wunused-function] > > Address this using a __maybe_unused annotation. > > Note: alternatively, this could be moved into an #ifdef block. Marking it Hi Arnd, Thank you for the patch! I would prefer to move the definition to the correct set of #ifdef guards rather than __maybe_unused. Maybe move the definition of mem_cgroup_id_get_many() to just before __mem_cgroup_clear_mc()? I find __maybe_unused to be a code smell. > 'static inline' would not work here as that would still produce the > warning on clang, which only ignores unused inline functions declared > in header files instead of .c files. > > Fixes: 4d0e3230a56a ("mm/memcontrol.c: fix a -Wunused-function warning") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > mm/memcontrol.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c313c49074ca..5f9f90e3cef8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -4921,7 +4921,8 @@ static void mem_cgroup_id_remove(struct mem_cgroup *memcg) > } > } > > -static void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) > +static void __maybe_unused > +mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) > { > refcount_add(n, &memcg->id.ref); > } > -- -- Thanks, ~Nick Desaulniers
On Tue, 2019-10-01 at 18:00 +0200, Arnd Bergmann wrote: > On Tue, Oct 1, 2019 at 4:40 PM Qian Cai <cai@lca.pw> wrote: > > > > On Tue, 2019-10-01 at 16:22 +0200, Arnd Bergmann wrote: > > > Removing the mem_cgroup_id_get() stub function introduced a new warning > > > of the same kind when CONFIG_MMU is disabled: > > > > Shouldn't CONFIG_MEMCG depends on CONFIG_MMU instead? > > Maybe. Generally we allow building a lot of stuff without CONFIG_MMU that > may not make sense, so I just followed the same idea here. Those blindly mark __maybe_unused might just mask important warnings off in the future, and they are ugly. Let's fix it properly.
On Tue 01-10-19 10:40:05, Qian Cai wrote: > On Tue, 2019-10-01 at 16:22 +0200, Arnd Bergmann wrote: > > Removing the mem_cgroup_id_get() stub function introduced a new warning > > of the same kind when CONFIG_MMU is disabled: > > Shouldn't CONFIG_MEMCG depends on CONFIG_MMU instead? I wanted to do that in the past but there was a clear disagreement from Johannes. -- Michal Hocko SUSE Labs
On Tue 01-10-19 09:36:24, Nick Desaulniers wrote: > On Tue, Oct 1, 2019 at 7:22 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > Removing the mem_cgroup_id_get() stub function introduced a new warning > > of the same kind when CONFIG_MMU is disabled: > > > > mm/memcontrol.c:4929:13: error: unused function 'mem_cgroup_id_get_many' [-Werror,-Wunused-function] > > > > Address this using a __maybe_unused annotation. > > > > Note: alternatively, this could be moved into an #ifdef block. Marking it > > Hi Arnd, > Thank you for the patch! I would prefer to move the definition to the > correct set of #ifdef guards rather than __maybe_unused. Maybe move > the definition of mem_cgroup_id_get_many() to just before > __mem_cgroup_clear_mc()? I find __maybe_unused to be a code smell. Agreed! -- Michal Hocko SUSE Labs
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index c313c49074ca..5f9f90e3cef8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4921,7 +4921,8 @@ static void mem_cgroup_id_remove(struct mem_cgroup *memcg) } } -static void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) +static void __maybe_unused +mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n) { refcount_add(n, &memcg->id.ref); }
Removing the mem_cgroup_id_get() stub function introduced a new warning of the same kind when CONFIG_MMU is disabled: mm/memcontrol.c:4929:13: error: unused function 'mem_cgroup_id_get_many' [-Werror,-Wunused-function] Address this using a __maybe_unused annotation. Note: alternatively, this could be moved into an #ifdef block. Marking it 'static inline' would not work here as that would still produce the warning on clang, which only ignores unused inline functions declared in header files instead of .c files. Fixes: 4d0e3230a56a ("mm/memcontrol.c: fix a -Wunused-function warning") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- mm/memcontrol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.20.0