Messages in this thread | | | Date | Mon, 10 Jan 2022 16:40:27 +0000 | From | Vincent Donnefort <> | Subject | Re: [PATCH 3/3] sched/fair: Do not raise overutilized for idle CPUs |
| |
On Wed, Dec 22, 2021 at 09:20:17AM +0100, Vincent Guittot wrote: > On Mon, 20 Dec 2021 at 12:43, Vincent Donnefort > <vincent.donnefort@arm.com> wrote: > > > > During a migration, the lock for the previous runqueue is not taken and > > hence, the task contribution isn't directly removed from that runqueue > > utilization but instead temporarily saved, until the next PELT signals > > update where it would be accounted. There is then a window in which a > > CPU can ben idle be nonetheless overutilized. > > > > The load balancer wouldn't be able to do anything to help a sleeping CPU, > > it brings then no gain to raise overutilized there, only the risk of > > spuriously doing it. > > But how do you make the difference between a very short idle time of > an overutilized CPU and a idle cpu with outdated utilization
No distinction here, but if the CPU is idle there's nothing to pull, so the load balance wouldn't do anything with this information.
> > Being idle is not a good reason for not being overutilized (ie ~80% of > average utilisation) > > > > > Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com> > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 51f6f55abb37..37f737c5f0b8 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -8641,26 +8641,28 @@ static inline void update_sg_lb_stats(struct lb_env *env, > > > > nr_running = rq->nr_running; > > sgs->sum_nr_running += nr_running; > > - > > - if (nr_running > 1) > > - *sg_status |= SG_OVERLOAD; > > - > > - if (cpu_overutilized(i)) > > - *sg_status |= SG_OVERUTILIZED; > > - > > #ifdef CONFIG_NUMA_BALANCING > > sgs->nr_numa_running += rq->nr_numa_running; > > sgs->nr_preferred_running += rq->nr_preferred_running; > > #endif > > + if (nr_running > 1) > > + *sg_status |= SG_OVERLOAD; > > Why do you move this code related to overload ?
This was a cosmetic change to put the NUMA related stats next to the other ones.
[...]
| |