[PATCH -next] mm/hugetlb_cgroup: fix a -Wunused-but-set-variable

[PATCH -next] mm/hugetlb_cgroup: fix a -Wunused-but-set-variable

[Qian Cai]

The commit c32300516047 ("hugetlb_cgroup: add interface for
charge/uncharge hugetlb reservations") forgot to remove an unused
variable,

mm/hugetlb_cgroup.c: In function 'hugetlb_cgroup_migrate':
mm/hugetlb_cgroup.c:777:25: warning: variable 'h_cg' set but not used
[-Wunused-but-set-variable]
  struct hugetlb_cgroup *h_cg;
                         ^~~~

Signed-off-by: Qian Cai <cai@lca.pw>
---
 mm/hugetlb_cgroup.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index ad777fecad28..8a86a2b62bef 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -774,7 +774,6 @@ void __init hugetlb_cgroup_file_init(void)
  */
 void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 {
-	struct hugetlb_cgroup *h_cg;
 	struct hugetlb_cgroup *h_cg_rsvd;
 	struct hstate *h = page_hstate(oldhpage);
 
@@ -783,7 +782,6 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 
 	VM_BUG_ON_PAGE(!PageHuge(oldhpage), oldhpage);
 	spin_lock(&hugetlb_lock);
-	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	h_cg_rsvd = hugetlb_cgroup_from_page_rsvd(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
 
-- 
1.8.3.1

Re: [PATCH -next] mm/hugetlb_cgroup: fix a -Wunused-but-set-variable

[Mina Almasry]

On Mon, Feb 17, 2020 at 7:31 AM Qian Cai <cai@lca.pw> wrote:
>
> The commit c32300516047 ("hugetlb_cgroup: add interface for
> charge/uncharge hugetlb reservations") forgot to remove an unused
> variable,
>
> mm/hugetlb_cgroup.c: In function 'hugetlb_cgroup_migrate':
> mm/hugetlb_cgroup.c:777:25: warning: variable 'h_cg' set but not used
> [-Wunused-but-set-variable]
>   struct hugetlb_cgroup *h_cg;
>                          ^~~~
>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>  mm/hugetlb_cgroup.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index ad777fecad28..8a86a2b62bef 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -774,7 +774,6 @@ void __init hugetlb_cgroup_file_init(void)
>   */
>  void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
>  {
> -       struct hugetlb_cgroup *h_cg;
>         struct hugetlb_cgroup *h_cg_rsvd;
>         struct hstate *h = page_hstate(oldhpage);
>
> @@ -783,7 +782,6 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
>
>         VM_BUG_ON_PAGE(!PageHuge(oldhpage), oldhpage);
>         spin_lock(&hugetlb_lock);
> -       h_cg = hugetlb_cgroup_from_page(oldhpage);
>         h_cg_rsvd = hugetlb_cgroup_from_page_rsvd(oldhpage);
>         set_hugetlb_cgroup(oldhpage, NULL);
>
> --
> 1.8.3.1
>

Hi Qian,

Thank you very much for the fix to remove the warning, but actually
the real fix is I'm missing a 'set_hugetlb_cgroup(newhpage, h_cg);'
which will use the variable and set the cgroup on newhpage which is
needed. I'll submit the proper fix.

What bothers me though is that locally when I checkout the broken
patch and try to build I don't see the warning:

make -j80 mm/hugetlb_cgroup.o
no warning.
make -j80 mm/hugetlb_cgroup.o CFLAGS_KERNEL="-Wall"
no warning
make -j80 mm/hugetlb_cgroup.o CFLAGS_KERNEL="-Wunused-but-set-variable"
I see the warning.

So it seems there is a bunch of warnings I need to explicitly turn on
otherwise I will continually submit patches that introduce warnings in
your build. Any idea why I'm running into this? Do you also have to
turn on these warnings manually on your make line? Is it related to
gcc version? My gcc version is:
gcc version 9.2.1 20190909 (Debian 9.2.1-8)

Re: [PATCH -next] mm/hugetlb_cgroup: fix a -Wunused-but-set-variable

[Qian Cai]

On Tue, 2020-02-18 at 10:58 -0800, Mina Almasry wrote:
> On Mon, Feb 17, 2020 at 7:31 AM Qian Cai <cai@lca.pw> wrote:
> > 
> > The commit c32300516047 ("hugetlb_cgroup: add interface for
> > charge/uncharge hugetlb reservations") forgot to remove an unused
> > variable,
> > 
> > mm/hugetlb_cgroup.c: In function 'hugetlb_cgroup_migrate':
> > mm/hugetlb_cgroup.c:777:25: warning: variable 'h_cg' set but not used
> > [-Wunused-but-set-variable]
> >   struct hugetlb_cgroup *h_cg;
> >                          ^~~~
> > 
> > Signed-off-by: Qian Cai <cai@lca.pw>
> > ---
> >  mm/hugetlb_cgroup.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> > index ad777fecad28..8a86a2b62bef 100644
> > --- a/mm/hugetlb_cgroup.c
> > +++ b/mm/hugetlb_cgroup.c
> > @@ -774,7 +774,6 @@ void __init hugetlb_cgroup_file_init(void)
> >   */
> >  void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
> >  {
> > -       struct hugetlb_cgroup *h_cg;
> >         struct hugetlb_cgroup *h_cg_rsvd;
> >         struct hstate *h = page_hstate(oldhpage);
> > 
> > @@ -783,7 +782,6 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
> > 
> >         VM_BUG_ON_PAGE(!PageHuge(oldhpage), oldhpage);
> >         spin_lock(&hugetlb_lock);
> > -       h_cg = hugetlb_cgroup_from_page(oldhpage);
> >         h_cg_rsvd = hugetlb_cgroup_from_page_rsvd(oldhpage);
> >         set_hugetlb_cgroup(oldhpage, NULL);
> > 
> > --
> > 1.8.3.1
> > 
> 
> Hi Qian,
> 
> Thank you very much for the fix to remove the warning, but actually
> the real fix is I'm missing a 'set_hugetlb_cgroup(newhpage, h_cg);'
> which will use the variable and set the cgroup on newhpage which is
> needed. I'll submit the proper fix.
> 
> What bothers me though is that locally when I checkout the broken
> patch and try to build I don't see the warning:
> 
> make -j80 mm/hugetlb_cgroup.o
> no warning.
> make -j80 mm/hugetlb_cgroup.o CFLAGS_KERNEL="-Wall"
> no warning
> make -j80 mm/hugetlb_cgroup.o CFLAGS_KERNEL="-Wunused-but-set-variable"
> I see the warning.
> 
> So it seems there is a bunch of warnings I need to explicitly turn on
> otherwise I will continually submit patches that introduce warnings in
> your build. Any idea why I'm running into this? Do you also have to
> turn on these warnings manually on your make line? Is it related to
> gcc version? My gcc version is:
> gcc version 9.2.1 20190909 (Debian 9.2.1-8)

I am doing "make W=1" which will turn on those warnings. Quite noisy but you can
"grep" what you are interested in.