[PATCH] cpuset: Remove unused 'struct cpuset*' variable

[PATCH] cpuset: Remove unused 'struct cpuset*' variable

[Kirtika Ruchandani]

'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 <tj@kernel.org>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
 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

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

[Zefan Li]

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 <tj@kernel.org>
> Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>

Acked-by: Zefan Li <lizefan@huawei.com>

> ---
>  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--;
> 

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

[Zefan Li]

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 <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Kirtika Ruchandani <kirtika-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Acked-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

> ---
>  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--;
> 

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

[Arnd Bergmann]

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

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

[Arnd Bergmann]

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

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

[Zefan Li]

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.

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

[Zefan Li]

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.

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

[Michal Hocko]

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

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

[Michal Hocko]

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

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

[Tejun Heo]

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

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

[Tejun Heo]

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

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

[Tejun Heo]

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 <tj@kernel.org>
> Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>

Applied to cgroup/for-4.10.

Thanks.

-- 
tejun