On Fri, Nov 13, 2020 at 09:56:37AM +0100, Vincent Guittot wrote:
> Hi Dan,
>
> Le vendredi 13 nov. 2020 à 11:46:57 (+0300), Dan Carpenter a écrit :
> > Hello Vincent Guittot,
> >
> > The patch b4c9c9f15649: "sched/fair: Prefer prev cpu in asymmetric
> > wakeup path" from Oct 29, 2020, leads to the following static checker
> > warning:
> >
> > kernel/sched/fair.c:6249 select_idle_sibling()
> > error: uninitialized symbol 'task_util'.
> >
> > kernel/sched/fair.c
> > 6233 static int select_idle_sibling(struct task_struct *p, int prev, int
> > target)
> > 6234 {
> > 6235 struct sched_domain *sd;
> > 6236 unsigned long task_util;
> > 6237 int i, recent_used_cpu;
> > 6238
> > 6239 /*
> > 6240 * On asymmetric system, update task utilization because we
> > will check
> > 6241 * that the task fits with cpu's capacity.
> > 6242 */
> >
> > The original comment was a bit more clear... Perhaps "On asymmetric
> > system[s], [record the] task utilization because we will check that the
> > task [can be done within] the cpu's capacity."
>
> The comment "update task utilization because we will check ..." refers to
> sync_entity_load_avg()
>
> >
> > 6243 if (static_branch_unlikely(&sched_asym_cpucapacity)) {
> > 6244 sync_entity_load_avg(&p->se);
> > 6245 task_util = uclamp_task_util(p);
> > 6246 }
> >
> > "task_util" is not initialized on the else path.
>
> no need because it will not be used
>
> >
> > 6247
> > 6248 if ((available_idle_cpu(target) || sched_idle_cpu(target))
> > &&
> > 6249 asym_fits_capacity(task_util, target))
> > ^^^^^^^^^
> > Uninitialized variable warning.
>
> asym_fits_capacity includes the same condition as above when we set task_util
> so task_util can't be used unintialize
>
> static inline bool asym_fits_capacity(int task_util, int cpu)
> {
> if (static_branch_unlikely(&sched_asym_cpucapacity))
> return fits_capacity(task_util, capacity_of(cpu));
>
> return true;
It's an interesting question, because unless the compiler makes this
inline, then it will lead to a KASan/syzbot warning at runtime. The
function is, of course, marked as inline but the compiler, also of
course, generally ignores those hints (use __always_inline if you want
the compiler to pay attention). On the other hand, the compiler will
still probably inline it... So this is *probably* not going to
lead to a runtime warning.
regards,
dan carpenter
