Messages in this thread | | | Subject | Re: [PATCH] sched/core: add unlikely in group_has_capacity() | From | Qi Zheng <> | Date | Fri, 7 Aug 2020 10:47:54 +0800 |
| |
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@gmail.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@gmail.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 >
| |