Re: [RFC PATCH 6/7] sched/cpufreq: Improve sugov_cpu_is_busy accuracy

From: Patrick Bellasi
Date: Thu Jun 20 2019 - 07:11:04 EST


On 19-Jun 17:19, Douglas Raillard wrote:
> Hi Patrick,

Hi!

> On 5/16/19 1:55 PM, Patrick Bellasi wrote:
> > On 08-May 18:43, douglas.raillard@xxxxxxx wrote:
> > > From: Douglas RAILLARD <douglas.raillard@xxxxxxx>
> > >
> > > Avoid assuming a CPU is busy when it has begun being idle before
> > > get_next_freq() is called. This is achieved by making sure the CPU will
> > > not be detected as busy by other CPUs whenever its utilization is
> > > decreasing.
> >
> > If I understand it correctly, what you are after here is a "metric"
> > which tells you (in a shared performance domain) if a CPU has been
> > busy for a certain amount of time.
> > You do that by reworking the way idle_calls are accounted for the
> > sugov_update_single() case.
> >
> > That approach could work but it looks a bit convoluted in the way it's
> > coded and it's also difficult to exclude there could be corner cases
> > with wired behaviors.
> > Isn't that why you "fix" the saved_idle_calls counter after all?
> >
> > What about a different approach where we:
> >
> > 1. we annotate the timestamp a CPU wakes up from IDLE (last_wakeup_time)
> >
> > 2. we use that annotated last_wake_time and the rq->nr_running to
> > define the "cpu is busy" heuristic.
> >
> > Looking at a sibling CPU, I think we can end up with two main cases:
> >
> > 1. CPU's nr_running is == 0
> > then we don't consider busy that CPU
> >
> > 2. CPU's nr_running is > 0
> > then the CPU is busy iff
> > (current_time - last_wakeup_tim) >= busy_threshold
> >
> > Notice that, when a CPU is active, its rq clock is periodically
> > updated, at least once per tick. Thus, provided a tick time is not too
> > long to declare busy a CPU, then the above logic should work.
> >
> > Perhaps the busy_threshold can also be defined considering the PELT
> > dynamics and starting from an expected utilization increase converted
> > in time.
>
> After experimenting with quite a few combinations, I managed to get a heuristic
> based on util_avg and util_est_enqueued that seems to work better in my case.
> Key differences are:
> * this new heuristic only really takes into account CFS signals
> (current one based on idle calls takes into account everything that executes
> on a given CPU.)

Right, that should not be an issue. You are after boosting for CFS
tasks at the end, while for RT and DL we don't need boosting since
they have their own mechanisms.

> * it will mark a CPU as busy less often, since it should only trigger when
> there is a change in the utilization of a currently enqueued tasks.

That sounds right too.

> Util changes due to enqueue/dequeue will not trigger it, which IMHO
> is desirable, since we only want to bias frequency selection
> when we know that we don't have precise utilization values for the
> enqueued tasks (because the task has changed its behavior).

Agree.

Best,
Patrick

--
#include <best/regards.h>

Patrick Bellasi