Messages in this thread | | | From | Vincent Guittot <> | Date | Fri, 8 Apr 2022 09:39:25 +0200 | Subject | Re: [PATCH] sched/fair: Simplify the scene where both local and busiest are group_fully_busy |
| |
On Thu, 7 Apr 2022 at 04:38, 彭志刚 <zgpeng.linux@gmail.com> wrote: > > When the type of the local group is group_has_spare, it does not affect it, and it also has the opportunity > > to pull the process to the local cpu. >
My point is :
local group is group_fully_busy busiest group is group_fully_busy
but
local cpu is idle or newly idle otherwise we would have already returned
Currently, calculate_imbalance returns imbalance=0 because it is based on avg_load which is not set for busiest group. Instead of skipping calculate_imbalance, we might compute an imbalance similarly to what is done for spare_capacity because local cpu being idle is an opportunity to run something else
> > > This patch deals with scenarios where both the local group and the busiest group type are group_fully_busy. > > In this scenario, because the avg_load of the group of type group_fully_busy is not calculated, this value is 0. > > Therefore, the condition of local->avg_load >= busiest->avg_load in calculate_imbalance is satisfied, so the > > imbalance will be set to 0; Therefore, in this scenario, the original logic has no chance to pull the process to > > the local cpu for execution. I think it can be judged at the upper level, and there is no need to go into > > calculate_imbalance to do some useless work. > > > Vincent Guittot <vincent.guittot@linaro.org> 于2022年4月6日周三 23:41写道: >> >> On Wed, 6 Apr 2022 at 13:46, zgpeng <zgpeng.linux@gmail.com> wrote: >> > >> > When both local and busiest group are group_fully_busy type, because >> > the avg_load of the group of type group_fully_busy is not calculated, the >> > avg_load value is equal to 0. In this case, load balancing will not actually >> > done, because after a series of calculations in the calculate_imbalance, it >> > will be considered that load balance is not required. Therefore,it is not >> > necessary to enter calculate_imbalance to do some useless work. >> > >> > Signed-off-by: zgpeng <zgpeng@tencent.com> >> > Reviewed-by: Samuel Liao <samuelliao@tencent.com> >> > --- >> > kernel/sched/fair.c | 12 ++++++++++++ >> > 1 file changed, 12 insertions(+) >> > >> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> > index 9f75303..cc1d6c4 100644 >> > --- a/kernel/sched/fair.c >> > +++ b/kernel/sched/fair.c >> > @@ -9634,6 +9634,18 @@ static struct sched_group *find_busiest_group(struct lb_env *env) >> > * busiest doesn't have any tasks waiting to run >> > */ >> > goto out_balanced; >> > + >> >> We are there because both local and busiest are not overloaded, local >> is idle or newly_idle and there might be an opportunity to pull a >> waiting task on local to use this local cpu. I wonder if we should not >> cover this opportunity in calculate_imbalance instead of skipping it >> >> > + if (local->group_type == group_fully_busy) >> > + /* >> > + * If local group is group_fully_busy, the code goes here, >> > + * the type of busiest group must also be group_fully_busy. >> > + * Because the avg_load of the group_fully_busy type is not >> > + * calculated at present, it is actually equal to 0. In this >> > + * scenario, load balance is not performed. therefore, it can >> > + * be returned directly here, and there is no need to do some >> > + * useless work in calculate_imbalance. >> > + */ >> > + goto out_balanced; >> > } >> > >> > force_balance: >> > -- >> > 2.9.5 >> >
| |