Thread (89 messages) 89 messages, 6 authors, 2019-01-25

Re: [PATCH v6 11/16] sched/fair: Add uclamp support to energy_compute()

From: Patrick Bellasi <hidden>
Date: 2019-01-22 15:01:45
Also in: linux-pm, lkml

On 22-Jan 14:39, Quentin Perret wrote:
On Tuesday 22 Jan 2019 at 14:26:06 (+0000), Patrick Bellasi wrote:
quoted
On 22-Jan 13:29, Quentin Perret wrote:
quoted
On Tuesday 22 Jan 2019 at 12:45:46 (+0000), Patrick Bellasi wrote:
quoted
On 22-Jan 12:13, Quentin Perret wrote:
quoted
On Tuesday 15 Jan 2019 at 10:15:08 (+0000), Patrick Bellasi wrote:
quoted
The Energy Aware Scheduler (AES) estimates the energy impact of waking
[...]
quoted
quoted
Ah, sorry, I guess my message was misleading. I'm saying this is
changing the way _EAS_ deals with RT tasks. Right now we don't actually
consider the RT-go-to-max thing at all in the EAS prediction. Your
patch is changing that AFAICT. It actually changes the way EAS sees RT
tasks even without uclamp ...
Lemme see if I get it right.

Currently, whenever we look at CPU utilization for ENERGY_UTIL, we
always use cpu_util_rt() for RT tasks:

---8<---
        util = util_cfs;
        util += cpu_util_rt(rq);
        util += dl_util;
---8<---

Thus, even when RT tasks are RUNNABLE, we don't always assume the CPU
running at the max capacity but just whatever is the aggregated
utilization across all the classes.

With uclamp, we have:

---8<---
        util = cpu_util_rt(rq) + util_cfs;
        if (type == FREQUENCY_UTIL)
            util = uclamp_util_with(rq, util, p);
        dl_util = cpu_util_dl(rq);
        if (type == ENERGY_UTIL)
            util += dl_util;
---8<---

So, I would say that, in terms of ENERGY_UTIL we do the same both
w/ and w/o uclamp. Isn't it?
Yes but now you use FREQUENCY_UTIL for computing 'max_util' in the EAS
prediction.
Right, I overlook that "little" detail... :/
Let's take an example. You have a perf domain with two CPUs. One CPU is
busy running a RT task, the other CPU runs a CFS task. Right now in
compute_energy() we only use ENERGY_UTIL, so 'max_util' ends up being
the max between the utilization of the two tasks. So we don't predict
we're going to max freq.
+1
With your patch, we use FREQUENCY_UTIL to compute 'max_util', so we
_will_ predict that we're going to max freq.
Right, with the default conf yes.
And we will do that even if CONFIG_UCLAMP_TASK=n.
While this should not happen, as I wrote in the RT integration patch,
that's happening because I'm missing some compilation guard or
similar. In this configurations we should always go to max... will
look into that.
The default EAS calculation will be different with your patch when there
are runnable RT tasks in the system. This needs to be documented, I
think.
Sure...
quoted
quoted
But I'm not hostile to the idea since it's possible to enable uclamp and
set the min cap to 0 for RT if you want. So there is a story there.
However, I think this needs be documented somewhere, at the very least.
The only difference I see is that the actual frequency could be
different (lower then max) when a clamped RT task is RUNNABLE.

Are you worried that running RT on a lower freq could have side
effects on the estimated busy time the CPU ?

I also still don't completely get why you say it could be useful to
   "set the min cap to 0 for RT if you want"
I'm not saying it's useful, I'm saying userspace can decide to do that
if it thinks it is a good idea. The default should be min_cap = 1024 for
RT, no questions. But you _can_ change it at runtime if you want to.
That's my point. And doing that basically provides the same behaviour as
what we have right now in terms of EAS calculation (but it changes the
freq selection obviously) which is why I'm not fundamentally opposed to
your patch.
Well, I think it's tricky to say whether the current or new approach
is better... it probably depends on the use-case.
So in short, I'm fine with the behavioural change, but please at least
mention it somewhere :-)
Anyway... agree, it's just that to add some documentation I need to
get what you are pointing out ;)

Will come up with some additional text to be added to the changelog

Maybe we can add a more detailed explanation of the different
behaviors you can get in the EAS documentation which is coming to
mainline ?
Thanks,
Quentin
-- 
#include <best/regards.h>

Patrick Bellasi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help