Re: [PATCH 2/4] sched: cpufreq: Keep track of cpufreq utilization update flags

On Wednesday, December 20, 2017 3:31:00 PM CET Patrick Bellasi wrote:

On 20-Dec 14:28, Peter Zijlstra wrote:

quoted

On Wed, Dec 20, 2017 at 12:55:46PM +0000, Patrick Bellasi wrote:

quoted

On 20-Dec 09:31, Peter Zijlstra wrote:

quoted

quoted

Didn't juri have patches to make DL do something sane? But yes, I think
those flags are part of the problem.

He recently reposted them here:

  https://lkml.kernel.org/r/20171204102325.5110-1-juri.lelli@redhat.com

Yeah, just found them and actually munged them into my queue; did all
the modifications you suggested too. Lets see if it comes apart.

quoted

quoted

quoted

- From the utilization handler, we check runqueues of all three sched
  classes to see if they have some work pending (this can be done
  smartly by checking only RT first and skipping other checks if RT
  has some work).

No that's wrong. DL should provide a minimum required based on existing
reservations, we can add the expected CFS average on top and request
that.

And for RT all we need to know is if current is of that class, otherwise
we don't care.

So, this:

   https://marc.info/?i=20171130114723.29210-3-patrick.bellasi%40arm.com

Right, I was actually looking for those patches, but I'm searching
backwards and hit upon Juri's patches first.

quoted

was actually going in this direction, although still working on top of
flags to not change the existing interface too much.

IMO, the advantage of flags is that they are a sort-of "pro-active"
approach, where the scheduler notify sensible events to schedutil.
But keep adding flags seems to overkilling to me too.

If we remove flags then we have to query the scheduler classes "on
demand"... but, as Peter suggests, once we have DL bits Juri posted,
the only issue if to know if an RT task is running.
This the patch above can be just good enough, with no flags at all and
with just a check for current being RT (or DL for the time being).

Well, we still need flags for crap like IO-WAIT IIRC. That's sugov
internal state and not something the scheduler actually already knows.

Right, that flag is set from:

    core.c::io_schedule_prepare()

for the current task, which is going to be dequeued soon.

Once it wakes up the next time, at enqueue time we trigger a boosting
by passing schedutil that flag.

Thus, unless we are happy to delay the boosting until the task is
actually picked for execution (don't think so), then we need to keep
the flag and signal schedutil at enqueue time.

However, was wondering one thing: should't we already have a vruntime
bonus for IO sleeping tasks? Because in that case, the task is likely
to be on CPU quite soon... and thus, perhaps by removing the flag and
moving the schedutil notification into core.c at the end of
__schedule() should be working to detect both RT and FAIR::IOWAIT
boosted tasks.

schedutil is not the only user of this flag.

Thanks,
Rafael