From: Vincent Guittot <vincent.guittot@linaro.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Valentin Schneider <valentin.schneider@arm.com>,
linux-kernel@vger.kernel.org
Subject: Re: [bug report] sched/fair: Prefer prev cpu in asymmetric wakeup path
Date: Fri, 13 Nov 2020 09:56:37 +0100 [thread overview]
Message-ID: <20201113085637.GA31601@vingu-book> (raw)
In-Reply-To: <20201113084657.GA86197@mwanda>
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;
}
>
> 6250 return target;
> 6251
> 6252 /*
> 6253 * If the previous CPU is cache affine and idle, don't be stupid:
> 6254 */
> 6255 if (prev != target && cpus_share_cache(prev, target) &&
> 6256 (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
> 6257 asym_fits_capacity(task_util, prev))
> 6258 return prev;
> 6259
> 6260 /*
> 6261 * Allow a per-cpu kthread to stack with the wakee if the
>
> regards,
> dan carpenter
next prev parent reply other threads:[~2020-11-13 8:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-13 8:46 [bug report] sched/fair: Prefer prev cpu in asymmetric wakeup path Dan Carpenter
2020-11-13 8:56 ` Vincent Guittot [this message]
2020-11-13 11:49 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201113085637.GA31601@vingu-book \
--to=vincent.guittot@linaro.org \
--cc=dan.carpenter@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=valentin.schneider@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.