tree: https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/core
head: 2dfdb3d20ad50e2ae2cb84cbceb0f0fc75e79e5d
commit: 445d9e8ba05d5e9e4b26956b7fe529223e29d8d1 [13/19] sched: make struct sched_statistics independent of fair sched class
config: m68k-buildonly-randconfig-r002-20210910 (attached as .config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?id=445d9e8ba05d5e9e4b26956b7fe529223e29d8d1
git remote add peterz-queue https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git
git fetch --no-tags peterz-queue sched/core
git checkout 445d9e8ba05d5e9e4b26956b7fe529223e29d8d1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
kernel/sched/fair.c: In function 'update_curr':
kernel/sched/fair.c:860:42: warning: unused variable 'stats' [-Wunused-variable]
860 | struct sched_statistics *stats = __schedstats_from_se(curr);
| ^~~~~
kernel/sched/fair.c: In function 'update_stats_wait_start':
>> kernel/sched/fair.c:892:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
892 | struct sched_statistics *stats;
| ^~~~~
kernel/sched/fair.c: In function 'update_stats_wait_end':
kernel/sched/fair.c:912:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
912 | struct sched_statistics *stats;
| ^~~~~
kernel/sched/fair.c: In function 'update_stats_enqueue_sleeper':
kernel/sched/fair.c:956:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
956 | struct sched_statistics *stats;
| ^~~~~
kernel/sched/fair.c: In function 'set_next_entity':
kernel/sched/fair.c:4517:42: warning: unused variable 'stats' [-Wunused-variable]
4517 | struct sched_statistics *stats = __schedstats_from_se(se);
| ^~~~~
kernel/sched/fair.c: At top level:
kernel/sched/fair.c:5505:6: warning: no previous prototype for 'init_cfs_bandwidth' [-Wmissing-prototypes]
5505 | void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
| ^~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11661:6: warning: no previous prototype for 'free_fair_sched_group' [-Wmissing-prototypes]
11661 | void free_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11663:5: warning: no previous prototype for 'alloc_fair_sched_group' [-Wmissing-prototypes]
11663 | int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11668:6: warning: no previous prototype for 'online_fair_sched_group' [-Wmissing-prototypes]
11668 | void online_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/sched/fair.c:11670:6: warning: no previous prototype for 'unregister_fair_sched_group' [-Wmissing-prototypes]
11670 | void unregister_fair_sched_group(struct task_group *tg) { }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/stats +892 kernel/sched/fair.c
840
841 /*
842 * Update the current task's runtime statistics.
843 */
844 static void update_curr(struct cfs_rq *cfs_rq)
845 {
846 struct sched_entity *curr = cfs_rq->curr;
847 u64 now = rq_clock_task(rq_of(cfs_rq));
848 u64 delta_exec;
849
850 if (unlikely(!curr))
851 return;
852
853 delta_exec = now - curr->exec_start;
854 if (unlikely((s64)delta_exec <= 0))
855 return;
856
857 curr->exec_start = now;
858
859 if (schedstat_enabled()) {
> 860 struct sched_statistics *stats = __schedstats_from_se(curr);
861
862 __schedstat_set(stats->exec_max,
863 max(delta_exec, stats->exec_max));
864 }
865
866 curr->sum_exec_runtime += delta_exec;
867 schedstat_add(cfs_rq->exec_clock, delta_exec);
868
869 curr->vruntime += calc_delta_fair(delta_exec, curr);
870 update_min_vruntime(cfs_rq);
871
872 if (entity_is_task(curr)) {
873 struct task_struct *curtask = task_of(curr);
874
875 trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
876 cgroup_account_cputime(curtask, delta_exec);
877 account_group_exec_runtime(curtask, delta_exec);
878 }
879
880 account_cfs_rq_runtime(cfs_rq, delta_exec);
881 }
882
883 static void update_curr_fair(struct rq *rq)
884 {
885 update_curr(cfs_rq_of(&rq->curr->se));
886 }
887
888 static inline void
889 update_stats_wait_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
890 {
891 u64 wait_start, prev_wait_start;
> 892 struct sched_statistics *stats;
893
894 if (!schedstat_enabled())
895 return;
896
897 stats = __schedstats_from_se(se);
898
899 wait_start = rq_clock(rq_of(cfs_rq));
900 prev_wait_start = schedstat_val(stats->wait_start);
901
902 if (entity_is_task(se) && task_on_rq_migrating(task_of(se)) &&
903 likely(wait_start > prev_wait_start))
904 wait_start -= prev_wait_start;
905
906 __schedstat_set(stats->wait_start, wait_start);
907 }
908
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Sat, Sep 11, 2021 at 2:21 AM kernel test robot <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/core
> head: 2dfdb3d20ad50e2ae2cb84cbceb0f0fc75e79e5d
> commit: 445d9e8ba05d5e9e4b26956b7fe529223e29d8d1 [13/19] sched: make struct sched_statistics independent of fair sched class
> config: m68k-buildonly-randconfig-r002-20210910 (attached as .config)
> compiler: m68k-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?id=445d9e8ba05d5e9e4b26956b7fe529223e29d8d1
> git remote add peterz-queue https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git
> git fetch --no-tags peterz-queue sched/core
> git checkout 445d9e8ba05d5e9e4b26956b7fe529223e29d8d1
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=m68k
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> kernel/sched/fair.c: In function 'update_curr':
> kernel/sched/fair.c:860:42: warning: unused variable 'stats' [-Wunused-variable]
> 860 | struct sched_statistics *stats = __schedstats_from_se(curr);
> | ^~~~~
> kernel/sched/fair.c: In function 'update_stats_wait_start':
> >> kernel/sched/fair.c:892:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
> 892 | struct sched_statistics *stats;
> | ^~~~~
> kernel/sched/fair.c: In function 'update_stats_wait_end':
> kernel/sched/fair.c:912:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
> 912 | struct sched_statistics *stats;
> | ^~~~~
> kernel/sched/fair.c: In function 'update_stats_enqueue_sleeper':
> kernel/sched/fair.c:956:34: warning: variable 'stats' set but not used [-Wunused-but-set-variable]
> 956 | struct sched_statistics *stats;
> | ^~~~~
> kernel/sched/fair.c: In function 'set_next_entity':
> kernel/sched/fair.c:4517:42: warning: unused variable 'stats' [-Wunused-variable]
> 4517 | struct sched_statistics *stats = __schedstats_from_se(se);
> | ^~~~~
> kernel/sched/fair.c: At top level:
> kernel/sched/fair.c:5505:6: warning: no previous prototype for 'init_cfs_bandwidth' [-Wmissing-prototypes]
> 5505 | void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
> | ^~~~~~~~~~~~~~~~~~
> kernel/sched/fair.c:11661:6: warning: no previous prototype for 'free_fair_sched_group' [-Wmissing-prototypes]
> 11661 | void free_fair_sched_group(struct task_group *tg) { }
> | ^~~~~~~~~~~~~~~~~~~~~
> kernel/sched/fair.c:11663:5: warning: no previous prototype for 'alloc_fair_sched_group' [-Wmissing-prototypes]
> 11663 | int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
> | ^~~~~~~~~~~~~~~~~~~~~~
> kernel/sched/fair.c:11668:6: warning: no previous prototype for 'online_fair_sched_group' [-Wmissing-prototypes]
> 11668 | void online_fair_sched_group(struct task_group *tg) { }
> | ^~~~~~~~~~~~~~~~~~~~~~~
> kernel/sched/fair.c:11670:6: warning: no previous prototype for 'unregister_fair_sched_group' [-Wmissing-prototypes]
> 11670 | void unregister_fair_sched_group(struct task_group *tg) { }
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +/stats +892 kernel/sched/fair.c
>
> 840
> 841 /*
> 842 * Update the current task's runtime statistics.
> 843 */
> 844 static void update_curr(struct cfs_rq *cfs_rq)
> 845 {
> 846 struct sched_entity *curr = cfs_rq->curr;
> 847 u64 now = rq_clock_task(rq_of(cfs_rq));
> 848 u64 delta_exec;
> 849
> 850 if (unlikely(!curr))
> 851 return;
> 852
> 853 delta_exec = now - curr->exec_start;
> 854 if (unlikely((s64)delta_exec <= 0))
> 855 return;
> 856
> 857 curr->exec_start = now;
> 858
> 859 if (schedstat_enabled()) {
> > 860 struct sched_statistics *stats = __schedstats_from_se(curr);
> 861
> 862 __schedstat_set(stats->exec_max,
> 863 max(delta_exec, stats->exec_max));
> 864 }
> 865
> 866 curr->sum_exec_runtime += delta_exec;
> 867 schedstat_add(cfs_rq->exec_clock, delta_exec);
> 868
> 869 curr->vruntime += calc_delta_fair(delta_exec, curr);
> 870 update_min_vruntime(cfs_rq);
> 871
> 872 if (entity_is_task(curr)) {
> 873 struct task_struct *curtask = task_of(curr);
> 874
> 875 trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
> 876 cgroup_account_cputime(curtask, delta_exec);
> 877 account_group_exec_runtime(curtask, delta_exec);
> 878 }
> 879
> 880 account_cfs_rq_runtime(cfs_rq, delta_exec);
> 881 }
> 882
> 883 static void update_curr_fair(struct rq *rq)
> 884 {
> 885 update_curr(cfs_rq_of(&rq->curr->se));
> 886 }
> 887
> 888 static inline void
> 889 update_stats_wait_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
> 890 {
> 891 u64 wait_start, prev_wait_start;
> > 892 struct sched_statistics *stats;
> 893
> 894 if (!schedstat_enabled())
> 895 return;
> 896
> 897 stats = __schedstats_from_se(se);
> 898
> 899 wait_start = rq_clock(rq_of(cfs_rq));
> 900 prev_wait_start = schedstat_val(stats->wait_start);
> 901
> 902 if (entity_is_task(se) && task_on_rq_migrating(task_of(se)) &&
> 903 likely(wait_start > prev_wait_start))
> 904 wait_start -= prev_wait_start;
> 905
> 906 __schedstat_set(stats->wait_start, wait_start);
> 907 }
> 908
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
Thanks for the report.
This build warning happens when CONFIG_SCHEDSTATS is not set, in which
case the schedstat_* functions will be none.
Should add '__maybe_unused' to fix it.
I will send a fix.
--
Thanks
Yafang
On Sat, Sep 11, 2021 at 02:21:26AM +0800, kernel test robot wrote:
> compiler: m68k-linux-gcc (GCC) 11.2.0
> kernel/sched/fair.c: In function 'update_curr':
> kernel/sched/fair.c:860:42: warning: unused variable 'stats' [-Wunused-variable]
> 860 | struct sched_statistics *stats = __schedstats_from_se(curr);
> | ^~~~~
OK, compiler guys, this code reads like:
#define schedstats_enabled() (0)
#define __schedstat_set(x, y) do { } while (0)
if (schedstats_enabled()) {
struct sched_statistics *stats = __schedstats_from_se(curr);
__schedstat_set(stats->exec_max,
max(delta_exec, stats->exec_max));
}
So yes, we initialize a variable that then isn't used, but the whole
bloody thing is inside if (0) which will not ever get ran *anyway*.
This is a crap warning if ever I saw one...
Hi!
On Sat, Sep 11, 2021 at 02:20:49PM +0200, Peter Zijlstra wrote:
> On Sat, Sep 11, 2021 at 02:21:26AM +0800, kernel test robot wrote:
> > compiler: m68k-linux-gcc (GCC) 11.2.0
>
> > kernel/sched/fair.c: In function 'update_curr':
> > kernel/sched/fair.c:860:42: warning: unused variable 'stats' [-Wunused-variable]
> > 860 | struct sched_statistics *stats = __schedstats_from_se(curr);
> > | ^~~~~
>
> OK, compiler guys, this code reads like:
>
> #define schedstats_enabled() (0)
> #define __schedstat_set(x, y) do { } while (0)
>
>
> if (schedstats_enabled()) {
> struct sched_statistics *stats = __schedstats_from_se(curr);
>
> __schedstat_set(stats->exec_max,
> max(delta_exec, stats->exec_max));
> }
>
> So yes, we initialize a variable that then isn't used, but the whole
> bloody thing is inside if (0) which will not ever get ran *anyway*.
>
> This is a crap warning if ever I saw one...
Yes, we really should warn "do not use a preprocessor macro if what you
want is a function"? The variable really *is* unused, with this macro.
If we would remove dead code before warning about unused variables
there would be many *more* false positives, fwiw.
Segher
On Sat, Sep 11, 2021 at 04:30:41PM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Sat, Sep 11, 2021 at 02:20:49PM +0200, Peter Zijlstra wrote:
> > On Sat, Sep 11, 2021 at 02:21:26AM +0800, kernel test robot wrote:
> > > compiler: m68k-linux-gcc (GCC) 11.2.0
> >
> > > kernel/sched/fair.c: In function 'update_curr':
> > > kernel/sched/fair.c:860:42: warning: unused variable 'stats' [-Wunused-variable]
> > > 860 | struct sched_statistics *stats = __schedstats_from_se(curr);
> > > | ^~~~~
> >
> > OK, compiler guys, this code reads like:
> >
> > #define schedstats_enabled() (0)
> > #define __schedstat_set(x, y) do { } while (0)
> >
> >
> > if (schedstats_enabled()) {
> > struct sched_statistics *stats = __schedstats_from_se(curr);
> >
> > __schedstat_set(stats->exec_max,
> > max(delta_exec, stats->exec_max));
> > }
> >
> > So yes, we initialize a variable that then isn't used, but the whole
> > bloody thing is inside if (0) which will not ever get ran *anyway*.
> >
> > This is a crap warning if ever I saw one...
>
> Yes, we really should warn "do not use a preprocessor macro if what you
> want is a function"? The variable really *is* unused, with this macro.
Why would I want to write a bunch of one-off functions and preprocessor
guard them? That's going to be a mess.
> If we would remove dead code before warning about unused variables
> there would be many *more* false positives, fwiw.
Not if you also remove any variables declared in dead code and all
variables only used in the dead code.