Re: [PATCH] sched/fair: Don't increase sd->balance_interval on newidle balance
From: Vincent Guittot
Date: Wed Sep 26 2018 - 09:58:49 EST
On Wed, 26 Sep 2018 at 15:17, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Wed, Sep 26, 2018 at 12:33:25PM +0200, Vincent Guittot wrote:
> > On Wed, 26 Sep 2018 at 11:35, Valentin Schneider
> > > library I use) has some phase where it spawns at lot of tasks at once to do
> > > some setup (busybox, shutils, bash...). Some of those tasks are pinned to a
> > > particular CPU, and that can lead to failed load_balance() - and to make things
> > > worse, there's a lot of idle_balance() in there.
> > >
> > > Eventually when I start running my actual workload a few ~100ms later, it's
> > > impacted by that balance_interval increase.
> > >
> > > Admittedly that's a specific use-case, but I don't think this quick increase
> > > is something that was intended.
> > Yes, this really sounds like a specific use-case. Unluckily you find a
> > way to reach max interval quite easily/every time with your test
> > set-up but keep in mind that this can also happen in real system life
> > and without using the newly idle path.
> > So if it's a problem to have a interval at max value for your unitary
> > test, it probably means that it's a problem for the system and the max
> > value is too high
> > Taking advantage of all load_balance event to update the interval
> > makes sense to me. It seems that you care about a short and regular
> > balance interval more that minimizing overhead of load balancing.
> > At the opposite, i'm sure that you don't complain if newly idle load
> > balance resets the interval to min value and overwrite what the
> > periodic load balance set up previously :-)
> Well, we've excluded newidle balance from updating such stats before. So
> in that respect the patch proposed by Valentin isn't weird.
> Consider for example:
> 58b26c4c0257 ("sched: Increment cache_nice_tries only on periodic lb")
> In general I think it makes perfect sense to exclude newidle balance
> from such stats; you get much more stable results from the regular
Ok so in this case we should exclude all update of the interval
during newly idle and not only some of them