On 30-Nov 17:02, Juri Lelli wrote:

On 30/11/17 15:41, Patrick Bellasi wrote:

quoted

On 30-Nov 14:12, Juri Lelli wrote:

quoted

Hi,

On 30/11/17 11:47, Patrick Bellasi wrote:

[...]

quoted

diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 2f52ec0f1539..67339ccb5595 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c

@@ -347,6 +347,12 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time,
 
 	sg_cpu->util = util;
 	sg_cpu->max = max;
+
+	/* CPU is entering IDLE, reset flags without triggering an update */
+	if (unlikely(flags & SCHED_CPUFREQ_IDLE)) {
+		sg_cpu->flags = 0;
+		goto done;
+	}

Looks good for now. I'm just thinking that we will happen for DL, as a
CPU that still "has" a sleeping task is not going to be really idle
until the 0-lag time.

AFAIU, for the time being, DL already cannot really rely on this flag
for its behaviors to be correct. Indeed, flags are reset as soon as
a FAIR task wakes up and it's enqueued.

Right, and your flags ORing patch should help with this.

quoted

Only once your DL integration patches are in, we do not depends on
flags anymore since DL will report a ceratain utilization up to the
0-lag time, isn't it?

Utilization won't decrease until 0-lag time, correct.

Then IMO with your DL patches the DL class don't need the flags
anymore since schedutil will know (and account) for the
utlization required by the DL tasks. Isn't it?

I was just wondering if resetting flags before that time (when a CPU
enters idle) might be an issue.

If the above is correct, then flags will be used only for the RT class (and
IO boosting)... and thus this patch will still be useful as it is now:
meaning that once the idle task is selected we do not care anymore
about RT and IOBoosting (only).

quoted

If that's the case, I would say that the flags will be used only to
jump to the max OPP for RT tasks. Thus, this patch should still be valid.

quoted

I guess we could move this at that point in time?

Not sure what you mean here. Right now the new SCHED_CPUFREQ_IDLE flag
is notified only by idle tasks. That's the only code path where we are
sure the CPU is entering IDLE.

W.r.t. the possible issue above, I was thinking that we might want to
reset flags at 0-lag time for DL (if CPU is still idle). Anyway, two
distinct set of patches. Who gets in last will have to ponder the thing
a little bit more. :)

Perhaps I'm still a bit confused but, to me, it seems that with your
patches we completely fix DL but we still can use this exact same
patch just for RT tasks.

Best,

Juri

-- 
#include <best/regards.h>

Patrick Bellasi