Messages in this thread | | | Subject | Re: [PATCH] sched/fair: Remove the duplicate check from group_has_capacity() | From | Qi Zheng <> | Date | Tue, 11 Aug 2020 11:39:52 +0800 |
| |
On 2020/8/11 上午2:33, Valentin Schneider wrote: > > On 10/08/20 02:00, Qi Zheng wrote: >> 1. The group_has_capacity() function is only called in >> group_classify(). >> 2. The following inequality has already been checked in >> group_is_overloaded() which was also called in >> group_classify(). >> >> (sgs->group_capacity * imbalance_pct) < >> (sgs->group_runnable * 100) >> > > Consider group_is_overloaded() returns false because of the first > condition: > > if (sgs->sum_nr_running <= sgs->group_weight) > return false; > > then group_has_capacity() would be the first place where the group_runnable > vs group_capacity comparison would be done. > > Now in that specific case we'll actually only check it if > > sgs->sum_nr_running == sgs->group_weight > > and the only case where the runnable vs capacity check can fail here is if > there's significant capacity pressure going on. TBH this capacity pressure > could be happening even when there are fewer tasks than CPUs, so I'm not > sure how intentional that corner case is.
Maybe some cpus in sg->cpumask are no longer active at the == case, which causes the significant capacity pressure?
> > > For the > > sgs->sum_nr_running > sgs->group_weight > > case I agree with your patch, there just is that oddity at the == case. > >> So just remove the duplicate check from group_has_capacity(). >> >> Signed-off-by: Qi Zheng <arch0.zheng@gmail.com> >> --- >> kernel/sched/fair.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 2ba8f230feb9..a41903fb327a 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -8234,10 +8234,6 @@ 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)) >> - return false; >> - >> if ((sgs->group_capacity * 100) > >> (sgs->group_util * imbalance_pct)) >> return true;
| |