Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530068 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable Arnd Bergmann <arnd@arndb.de> - 2016-11-25 10:50 +0100
    Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable Zefan Li <lizefan@huawei.com> - 2016-11-26 01:50 +0100
      Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable Michal Hocko <mhocko@kernel.org> - 2016-11-28 08:40 +0100
        Re: [PATCH] cpuset: Remove unused 'struct cpuset*' variable Tejun Heo <tj@kernel.org> - 2016-11-28 22:00 +0100

#1530068 — 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

[toc] | [next] | [standalone]

#1530574

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.

[toc] | [prev] | [next] | [standalone]

#1531082

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

[toc] | [prev] | [next] | [standalone]

#1531654

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web