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

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.

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. I was just
wondering if resetting flags before that time (when a CPU enters idle)
might be an issue.

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. :)

Best,

Juri