Hi all,
After merging the tip tree, today's linux-next build (powerpc_ppc64
defconfig) produced this warning:
kernel/sched/debug.c: In function 'print_cfs_group_stats':
kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
460 | struct sched_statistics *stats = __schedstats_from_se(se);
| ^~~~~
Caused by commit
cb3e971c435d ("sched: Make struct sched_statistics independent of fair sched class")
# CONFIG_SCHEDSTATS is not set
--
Cheers,
Stephen Rothwell
On Mon, Sep 20, 2021 at 9:33 AM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> After merging the tip tree, today's linux-next build (powerpc_ppc64
> defconfig) produced this warning:
>
> kernel/sched/debug.c: In function 'print_cfs_group_stats':
> kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> | ^~~~~
>
> Caused by commit
>
> cb3e971c435d ("sched: Make struct sched_statistics independent of fair sched class")
>
> # CONFIG_SCHEDSTATS is not set
>
Hi Stephen,
Thanks for the report.
We have discussed this issue before[1].
This warning happens when CONFIG_SCHEDSTATS is not set and
schedstat_enabled() is 0, so the whole scope should be not compiled.
It seems that we don't need to fix this warning.
[1]. https://lore.kernel.org/lkml/[email protected]/
--
Thanks
Yafang
Hi Yafang,
On Mon, 20 Sep 2021 14:55:29 +0800 Yafang Shao <[email protected]> wrote:
>
> On Mon, Sep 20, 2021 at 9:33 AM Stephen Rothwell <[email protected]> wrote:
> >
> > Hi all,
> >
> > After merging the tip tree, today's linux-next build (powerpc_ppc64
> > defconfig) produced this warning:
> >
> > kernel/sched/debug.c: In function 'print_cfs_group_stats':
> > kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> > 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> > | ^~~~~
> >
> > Caused by commit
> >
> > cb3e971c435d ("sched: Make struct sched_statistics independent of fair sched class")
> >
> > # CONFIG_SCHEDSTATS is not set
> >
>
> Thanks for the report.
>
> We have discussed this issue before[1].
> This warning happens when CONFIG_SCHEDSTATS is not set and
> schedstat_enabled() is 0, so the whole scope should be not compiled.
> It seems that we don't need to fix this warning.
>
> [1]. https://lore.kernel.org/lkml/[email protected]/
Clearly it will be compiled if CONFIG_SCHEDSTATS is not set as that is
exactly what this build has ... even sections of code guarded by "if
(0)" are compiled, they may just not produce any output in the binary.
Also, I do not have W=1 for this build.
If you turned schedstat_val() into a static inline function, then this
warning would go away. That also means that argument types and return
values will be better checked.
So, please fix this.
--
Cheers,
Stephen Rothwell
Hi all,
On Mon, 20 Sep 2021 18:30:16 +1000 Stephen Rothwell <[email protected]> wrote:
>
> On Mon, 20 Sep 2021 14:55:29 +0800 Yafang Shao <[email protected]> wrote:
> >
> > On Mon, Sep 20, 2021 at 9:33 AM Stephen Rothwell <[email protected]> wrote:
> > >
> > > After merging the tip tree, today's linux-next build (powerpc_ppc64
> > > defconfig) produced this warning:
> > >
> > > kernel/sched/debug.c: In function 'print_cfs_group_stats':
> > > kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> > > 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> > > | ^~~~~
> > >
> > > Caused by commit
> > >
> > > cb3e971c435d ("sched: Make struct sched_statistics independent of fair sched class")
> > >
> > > # CONFIG_SCHEDSTATS is not set
> > >
> >
> > Thanks for the report.
> >
> > We have discussed this issue before[1].
> > This warning happens when CONFIG_SCHEDSTATS is not set and
> > schedstat_enabled() is 0, so the whole scope should be not compiled.
> > It seems that we don't need to fix this warning.
> >
> > [1]. https://lore.kernel.org/lkml/[email protected]/
>
> Clearly it will be compiled if CONFIG_SCHEDSTATS is not set as that is
> exactly what this build has ... even sections of code guarded by "if
> (0)" are compiled, they may just not produce any output in the binary.
>
> Also, I do not have W=1 for this build.
>
> If you turned schedstat_val() into a static inline function, then this
> warning would go away. That also means that argument types and return
> values will be better checked.
I take it back, that won't work :-(
> So, please fix this.
Still it needs to be fixed to keep unnecessary warnings out of our builds.
--
Cheers,
Stephen Rothwell
On Mon, Sep 20, 2021 at 11:33:30AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the tip tree, today's linux-next build (powerpc_ppc64
> defconfig) produced this warning:
>
> kernel/sched/debug.c: In function 'print_cfs_group_stats':
> kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> | ^~~~~
>
So I've not seen that one *yet*, I've dont a bunch of SCHEDSTAT=n
builds. I do think GCC is being quite stupid for giving it, seeing how
the whole thing is within if (0). The GCC people seem to disagree when I
brought it up.
Anyway, what I did in other parts was the below; that seems to 'upgrade'
the warnings from -Wunused-variable to -Wunused-but-set-variable, and
that latter *is* in W=1, and I'm arguing it should be promoted to W=2 or
thereabout.
Given that whole if(0) {} thing, I don't feel motivated to change things
overly much and quite strongly feel this is the compiler being daft.
---
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 935dad7dffb7..ef71de01e4d7 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -457,7 +457,8 @@ static void print_cfs_group_stats(struct seq_file *m, int cpu, struct task_group
PN(se->sum_exec_runtime);
if (schedstat_enabled()) {
- struct sched_statistics *stats = __schedstats_from_se(se);
+ struct sched_statistics *stats;
+ stats = __schedstats_from_se(se);
PN_SCHEDSTAT(wait_start);
PN_SCHEDSTAT(sleep_start);
Hi Peter,
On Mon, 20 Sep 2021 13:18:53 +0200 Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Sep 20, 2021 at 11:33:30AM +1000, Stephen Rothwell wrote:
> >
> > After merging the tip tree, today's linux-next build (powerpc_ppc64
> > defconfig) produced this warning:
> >
> > kernel/sched/debug.c: In function 'print_cfs_group_stats':
> > kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> > 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> > | ^~~~~
>
> So I've not seen that one *yet*, I've dont a bunch of SCHEDSTAT=n
> builds. I do think GCC is being quite stupid for giving it, seeing how
> the whole thing is within if (0). The GCC people seem to disagree when I
> brought it up.
>
> Anyway, what I did in other parts was the below; that seems to 'upgrade'
> the warnings from -Wunused-variable to -Wunused-but-set-variable, and
> that latter *is* in W=1, and I'm arguing it should be promoted to W=2 or
> thereabout.
>
> Given that whole if(0) {} thing, I don't feel motivated to change things
> overly much and quite strongly feel this is the compiler being daft.
>
> ---
>
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index 935dad7dffb7..ef71de01e4d7 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -457,7 +457,8 @@ static void print_cfs_group_stats(struct seq_file *m, int cpu, struct task_group
> PN(se->sum_exec_runtime);
>
> if (schedstat_enabled()) {
> - struct sched_statistics *stats = __schedstats_from_se(se);
> + struct sched_statistics *stats;
> + stats = __schedstats_from_se(se);
>
> PN_SCHEDSTAT(wait_start);
> PN_SCHEDSTAT(sleep_start);
Any progress on this? I am still getting the warning.
--
Cheers,
Stephen Rothwell
On Wed, Oct 06, 2021 at 02:06:55PM +1100, Stephen Rothwell wrote:
> Hi Peter,
>
> On Mon, 20 Sep 2021 13:18:53 +0200 Peter Zijlstra <[email protected]> wrote:
> >
> > On Mon, Sep 20, 2021 at 11:33:30AM +1000, Stephen Rothwell wrote:
> > >
> > > After merging the tip tree, today's linux-next build (powerpc_ppc64
> > > defconfig) produced this warning:
> > >
> > > kernel/sched/debug.c: In function 'print_cfs_group_stats':
> > > kernel/sched/debug.c:460:41: warning: unused variable 'stats' [-Wunused-variable]
> > > 460 | struct sched_statistics *stats = __schedstats_from_se(se);
> > > | ^~~~~
> >
> > So I've not seen that one *yet*, I've dont a bunch of SCHEDSTAT=n
> > builds. I do think GCC is being quite stupid for giving it, seeing how
> > the whole thing is within if (0). The GCC people seem to disagree when I
> > brought it up.
> >
> > Anyway, what I did in other parts was the below; that seems to 'upgrade'
> > the warnings from -Wunused-variable to -Wunused-but-set-variable, and
> > that latter *is* in W=1, and I'm arguing it should be promoted to W=2 or
> > thereabout.
> >
> > Given that whole if(0) {} thing, I don't feel motivated to change things
> > overly much and quite strongly feel this is the compiler being daft.
> >
> > ---
> >
> > diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> > index 935dad7dffb7..ef71de01e4d7 100644
> > --- a/kernel/sched/debug.c
> > +++ b/kernel/sched/debug.c
> > @@ -457,7 +457,8 @@ static void print_cfs_group_stats(struct seq_file *m, int cpu, struct task_group
> > PN(se->sum_exec_runtime);
> >
> > if (schedstat_enabled()) {
> > - struct sched_statistics *stats = __schedstats_from_se(se);
> > + struct sched_statistics *stats;
> > + stats = __schedstats_from_se(se);
> >
> > PN_SCHEDSTAT(wait_start);
> > PN_SCHEDSTAT(sleep_start);
>
> Any progress on this? I am still getting the warning.
Urgh, sorry, fell through cracks. Will commit fixup in a few.