From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Bellasi Subject: Re: [PATCH] sched/fair: schedutil: update only with all info available Date: Wed, 11 Apr 2018 11:15:17 +0100 Message-ID: <20180411101517.GL14248@e110439-lin> References: <20180406172835.20078-1-patrick.bellasi@arm.com> <20180410110412.GG14248@e110439-lin> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Vincent Guittot Cc: Peter Zijlstra , linux-kernel , "open list:THERMAL" , Ingo Molnar , "Rafael J . Wysocki" , Viresh Kumar , Juri Lelli , Joel Fernandes , Steve Muckle , Dietmar Eggemann , Morten Rasmussen List-Id: linux-pm@vger.kernel.org On 11-Apr 08:57, Vincent Guittot wrote: > On 10 April 2018 at 13:04, Patrick Bellasi wrote: > > On 09-Apr 10:51, Vincent Guittot wrote: > >> On 6 April 2018 at 19:28, Patrick Bellasi 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. > > This was mainly for RT tasks but it was not the case for cfs task > before commit 8f111bc357aa True, but with his solution Peter has actually come up with a unified interface which is now (and can be IMO) based just on RUNNABLE counters for each class. > > 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. > > So there are 2 problems there: > - using cfs_rq::h_nr_running when aggregating cfs utilization which > generates a lot of frequency drop You mean because we now completely disregard the blocked utilization where a CPU is idle, right? Given how PELT works and the recent support for IDLE CPUs updated, we should probably always add contributions for the CFS class. > - making sure that the nr-running are up-to-date when used in sched_util Right... but, if we always add the cfs_rq (to always account for blocked utilization), we don't have anymore this last dependency, isn't it? We still have to account for the util_est dependency. Should I add a patch to this series to disregard cfs_rq::h_nr_running from schedutil as you suggested? > > 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). > > > > -- > > #include > > > > Patrick Bellasi -- #include Patrick Bellasi