Messages in this thread | | | From | Vincent Guittot <> | Date | Wed, 11 Apr 2018 17:41:24 +0200 | Subject | Re: [PATCH] sched/fair: schedutil: update only with all info available |
| |
On 11 April 2018 at 17:37, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Apr 11, 2018 at 05:29:01PM +0200, Vincent Guittot wrote: >> On 11 April 2018 at 17:14, Peter Zijlstra <peterz@infradead.org> wrote: >> > On Tue, Apr 10, 2018 at 12:04:12PM +0100, Patrick Bellasi wrote: >> >> On 09-Apr 10:51, Vincent Guittot wrote: >> > >> >> > Peter, >> >> > what was your goal with adding the condition "if >> >> > (rq->cfs.h_nr_running)" for the aggragation of CFS utilization >> >> >> >> The original intent was to get rid of sched class flags, used to track >> >> which class has tasks runnable from within schedutil. The reason was >> >> to solve some misalignment between scheduler class status and >> >> schedutil status. >> >> >> >> The solution, initially suggested by Viresh, and finally proposed by >> >> Peter was to exploit RQ knowledges directly from within schedutil. >> >> >> >> The problem is that now schedutil updated depends on two information: >> >> utilization changes and number of RT and CFS runnable tasks. >> >> >> >> Thus, using cfs_rq::h_nr_running is not the problem... it's actually >> >> part of a much more clean solution of the code we used to have. >> >> >> >> The problem, IMO is that we now depend on other information which >> >> needs to be in sync before calling schedutil... and the patch I >> >> proposed is meant to make it less likely that all the information >> >> required are not aligned (also in the future). >> > >> > Specifically, the h_nr_running test was get rid of >> > >> > if (delta_ns > TICK_NSEC) { >> > j_sg_cpu->iowait_boost = 0; >> > j_sg_cpu->iowait_boost_pending = false; >> > - j_sg_cpu->util_cfs = 0; >> > >> > ^^^^^^^^^^^^^^^^^^^^^^^ that.. >> > >> > - if (j_sg_cpu->util_dl == 0) >> > - continue; >> > } >> > >> > >> > because that felt rather arbitrary. >> >> yes I agree. >> >> With the patch that updates blocked idle load, we should not have the >> problem of blocked utilization anymore and get rid of the code above >> and h_nr_running test > > Yes, these patches predate those, but indeed, now that we age the > blocked load consistently it should no longer be required. > > Of course, you still have that weird regression report against those > patches... :-)
Yes. and to be honest I don't have any clues of the root cause :-( Heiner mentioned that it's much better in latest linux-next but I haven't seen any changes related to the code of those patches
| |