lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 7 Aug 2020 10:47:54 +0800 From: Qi Zheng <arch0.zheng@...il.com> To: Ingo Molnar <mingo@...nel.org> Cc: mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com, vincent.guittot@...aro.org, dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de, linux-kernel@...r.kernel.org Subject: Re: [PATCH] sched/core: add unlikely in group_has_capacity() Yeah, because of the following two points, I also think the probability is 0%: a) the sd is protected by rcu lock, and load_balance() func is between rcu_read_lock() and rcu_read_unlock(). b) the sgs is a local variable. So in the group_classify(), the env->sd->imbalance_pct and the sgs will not be changed. May I remove the duplicate check from group_has_capacity() and resubmit a patch? Yours, Qi Zheng On 2020/8/6 下午10:45, Ingo Molnar wrote: > > * Qi Zheng <arch0.zheng@...il.com> wrote: > >> 1. The group_has_capacity() function is only called in >> group_classify(). >> 2. Before calling the group_has_capacity() function, >> group_is_overloaded() will first judge the following >> formula, if it holds, the group_classify() will directly >> return the group_overloaded. >> >> (sgs->group_capacity * imbalance_pct) < >> (sgs->group_runnable * 100) >> >> Therefore, when the group_has_capacity() is called, the >> probability that the above formalu holds is very small. Hint >> compilers about that. >> >> Signed-off-by: Qi Zheng <arch0.zheng@...il.com> >> --- >> kernel/sched/fair.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 2ba8f230feb9..9074fd5e23b2 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -8234,8 +8234,8 @@ group_has_capacity(unsigned int imbalance_pct, struct sg_lb_stats *sgs) >> if (sgs->sum_nr_running < sgs->group_weight) >> return true; >> >> - if ((sgs->group_capacity * imbalance_pct) < >> - (sgs->group_runnable * 100)) >> + if (unlikely((sgs->group_capacity * imbalance_pct) < >> + (sgs->group_runnable * 100))) >> return false; > > Isn't the probability that this second check will match around 0%? > > I.e. wouldn't the right fix be to remove the duplicate check from > group_has_capacity(), because it's already been checked in > group_classify()? Maybe while leaving a comment in place? > > Thanks, > > Ingo >
Powered by blists - more mailing lists