2016-11-25 04:55:40

by Kirtika Ruchandani

[permalink] [raw]
Subject: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

'struct cpuset* cs' that is set but not used, was introduced in commit
1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
gives the folllowing harmless warning, which we'd like to fix to
reduce the noise with W=1 in the kernel.

kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
struct cpuset *cs;
^

Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
Cc: Tejun Heo <[email protected]>
Signed-off-by: Kirtika Ruchandani <[email protected]>
---
kernel/cpuset.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 29f815d..af51460 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1499,10 +1499,8 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
static void cpuset_cancel_attach(struct cgroup_taskset *tset)
{
struct cgroup_subsys_state *css;
- struct cpuset *cs;

cgroup_taskset_first(tset, &css);
- cs = css_cs(css);

mutex_lock(&cpuset_mutex);
css_cs(css)->attach_in_progress--;
--
2.8.0.rc3.226.g39d4020


2016-11-25 05:49:53

by Zefan Li

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

On 2016/11/25 12:55, Kirtika Ruchandani wrote:
> 'struct cpuset* cs' that is set but not used, was introduced in commit
> 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
> gives the folllowing harmless warning, which we'd like to fix to
> reduce the noise with W=1 in the kernel.
>
> kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
> kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
> struct cpuset *cs;
> ^
>
> Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").

This isn't a bug, so I don't think this tag is proper.

> Cc: Tejun Heo <[email protected]>
> Signed-off-by: Kirtika Ruchandani <[email protected]>

Acked-by: Zefan Li <[email protected]>

> ---
> kernel/cpuset.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 29f815d..af51460 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1499,10 +1499,8 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> static void cpuset_cancel_attach(struct cgroup_taskset *tset)
> {
> struct cgroup_subsys_state *css;
> - struct cpuset *cs;
>
> cgroup_taskset_first(tset, &css);
> - cs = css_cs(css);
>
> mutex_lock(&cpuset_mutex);
> css_cs(css)->attach_in_progress--;
>

2016-11-25 09:48:40

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

On Friday, November 25, 2016 1:46:04 PM CET Zefan Li wrote:
> On 2016/11/25 12:55, Kirtika Ruchandani wrote:
> > 'struct cpuset* cs' that is set but not used, was introduced in commit
> > 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> > cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
> > gives the folllowing harmless warning, which we'd like to fix to
> > reduce the noise with W=1 in the kernel.
> >
> > kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
> > kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
> > struct cpuset *cs;
> > ^
> >
> > Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
>
> This isn't a bug, so I don't think this tag is proper.

I think it's ok since the changelog makes it clear that the
warning is harmless. It's still useful information to know
what commit introduced the warning, and the warning is fixed
by this patch.

Arnd

2016-11-26 00:42:55

by Zefan Li

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

On 2016/11/25 17:46, Arnd Bergmann wrote:
> On Friday, November 25, 2016 1:46:04 PM CET Zefan Li wrote:
>> On 2016/11/25 12:55, Kirtika Ruchandani wrote:
>>> 'struct cpuset* cs' that is set but not used, was introduced in commit
>>> 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
>>> cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
>>> gives the folllowing harmless warning, which we'd like to fix to
>>> reduce the noise with W=1 in the kernel.
>>>
>>> kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
>>> kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
>>> struct cpuset *cs;
>>> ^
>>>
>>> Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
>>
>> This isn't a bug, so I don't think this tag is proper.
>
> I think it's ok since the changelog makes it clear that the
> warning is harmless. It's still useful information to know
> what commit introduced the warning, and the warning is fixed
> by this patch.
>

People like stable tree maintainers use scripts to find out bug fixes
that needs to be backported to older kernels, and those scripts tracks
the Fixes tag. No doubt this patch doesn't require backporting, so
it's better avoid using this tag.

2016-11-28 07:37:22

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

On Sat 26-11-16 08:42:40, Li Zefan wrote:
> On 2016/11/25 17:46, Arnd Bergmann wrote:
> > On Friday, November 25, 2016 1:46:04 PM CET Zefan Li wrote:
> >> On 2016/11/25 12:55, Kirtika Ruchandani wrote:
> >>> 'struct cpuset* cs' that is set but not used, was introduced in commit
> >>> 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> >>> cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
> >>> gives the folllowing harmless warning, which we'd like to fix to
> >>> reduce the noise with W=1 in the kernel.
> >>>
> >>> kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
> >>> kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
> >>> struct cpuset *cs;
> >>> ^
> >>>
> >>> Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> >>
> >> This isn't a bug, so I don't think this tag is proper.
> >
> > I think it's ok since the changelog makes it clear that the
> > warning is harmless. It's still useful information to know
> > what commit introduced the warning, and the warning is fixed
> > by this patch.
> >
>
> People like stable tree maintainers use scripts to find out bug fixes
> that needs to be backported to older kernels, and those scripts tracks
> the Fixes tag. No doubt this patch doesn't require backporting, so
> it's better avoid using this tag.

I would disagree here. Randomly picking up fixes just because they are
Fixing some commit is just too dangerous for the stable trees. Fixes tag
should tell what was the culprit of the issue fixed by the patch,
nothing more and nothing less.

--
Michal Hocko
SUSE Labs

2016-11-28 20:54:08

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

Hello,

On Mon, Nov 28, 2016 at 08:37:09AM +0100, Michal Hocko wrote:
> I would disagree here. Randomly picking up fixes just because they are
> Fixing some commit is just too dangerous for the stable trees. Fixes tag
> should tell what was the culprit of the issue fixed by the patch,
> nothing more and nothing less.

Logically, I agree but then the only time I used the Fixes tag is when
I was tagging patches for stable and I can imagine people grepping for
the tag to backport. Also, given that the offending commit is already
referenced in the description, the tag doesn't make much difference to
human beings. I don't think it's a big deal either way but am more
inclined to follow Li's suggestion here given that he is maintaining
stable trees.

Thanks.

--
tejun

2016-11-28 21:07:28

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable

On Thu, Nov 24, 2016 at 08:55:12PM -0800, Kirtika Ruchandani wrote:
> 'struct cpuset* cs' that is set but not used, was introduced in commit
> 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> cpuset_cancel_attach() uses css_cs(css) instead. Compiling with W=1
> gives the folllowing harmless warning, which we'd like to fix to
> reduce the noise with W=1 in the kernel.
>
> kernel/cpuset.c: In function ‘cpuset_cancel_attach’:
> kernel/cpuset.c:1502:17: warning: variable ‘cs’ set but not used [-Wunused-but-set-variable]
> struct cpuset *cs;
> ^
>
> Fixes: 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling").
> Cc: Tejun Heo <[email protected]>
> Signed-off-by: Kirtika Ruchandani <[email protected]>

Applied to cgroup/for-4.10.

Thanks.

--
tejun