Messages in this thread | | | From | Vincent Guittot <> | Date | Wed, 11 Apr 2018 17:29:01 +0200 | Subject | Re: [PATCH] sched/fair: schedutil: update only with all info available |
| |
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
| |