On Tue, May 8, 2018 at 2:32 PM, Claudio Scordino <[email protected]> wrote: > > > Il 08/05/2018 08:54, Viresh Kumar ha scritto: >> >> On 07-05-18, 16:43, Claudio Scordino wrote: >>> >>> At OSPM, it was mentioned the issue about urgent CPU frequency requests >>> arriving when a frequency switch is already in progress. >>> >>> Besides the various issues (physical time for switching frequency, >>> on-going kthread activity, etc.) one (minor) issue is the kernel >>> "forgetting" such request, thus waiting the next switch time for >>> recomputing the needed frequency and behaving accordingly. >>> >>> This patch makes the kthread serve any urgent request occurred during >>> the previous frequency switch. It introduces a specific flag, only set >>> when the SCHED_DEADLINE scheduling class increases the CPU utilization, >>> aiming at decreasing the likelihood of a deadline miss. >>> >>> Indeed, some preliminary tests in critical conditions (i.e. >>> SCHED_DEADLINE tasks with short periods) have shown reductions of more >>> than 10% of the average number of deadline misses. On the other hand, >>> the increase in terms of energy consumption when running SCHED_DEADLINE >>> tasks (not yet measured) is likely to be not negligible (especially in >>> case of critical scenarios like "ramp up" utilizations). >>> >>> The patch is meant as follow-up discussion after OSPM. >>> >>> Signed-off-by: Claudio Scordino <[email protected]> >>> CC: Viresh Kumar <[email protected]> >>> CC: Rafael J. Wysocki <[email protected]> >>> CC: Peter Zijlstra <[email protected]> >>> CC: Ingo Molnar <[email protected]> >>> CC: Patrick Bellasi <[email protected]> >>> CC: Juri Lelli <[email protected]> >>> Cc: Luca Abeni <[email protected]> >>> CC: Joel Fernandes <[email protected]> >>> CC: [email protected] >>> --- >>> kernel/sched/cpufreq_schedutil.c | 20 ++++++++++++++++++-- >>> 1 file changed, 18 insertions(+), 2 deletions(-) >>> >>> diff --git a/kernel/sched/cpufreq_schedutil.c >>> b/kernel/sched/cpufreq_schedutil.c >>> index d2c6083..4de06b0 100644 >>> --- a/kernel/sched/cpufreq_schedutil.c >>> +++ b/kernel/sched/cpufreq_schedutil.c >>> @@ -41,6 +41,7 @@ struct sugov_policy { >>> bool work_in_progress; >>> bool need_freq_update; >>> + bool urgent_freq_update; >>> }; >>> struct sugov_cpu { >>> @@ -92,6 +93,14 @@ static bool sugov_should_update_freq(struct >>> sugov_policy *sg_policy, u64 time) >>> !cpufreq_can_do_remote_dvfs(sg_policy->policy)) >>> return false; >>> + /* >>> + * Continue computing the new frequency. In case of >>> work_in_progress, >>> + * the kthread will resched a change once the current transition >>> is >>> + * finished. >>> + */ >>> + if (sg_policy->urgent_freq_update) >>> + return true; >>> + >>> if (sg_policy->work_in_progress) >>> return false; >>> @@ -121,6 +130,9 @@ static void sugov_update_commit(struct sugov_policy >>> *sg_policy, u64 time, >>> sg_policy->next_freq = next_freq; >>> sg_policy->last_freq_update_time = time; >>> + if (sg_policy->work_in_progress) >>> + return; >>> + >>> if (policy->fast_switch_enabled) { >>> next_freq = cpufreq_driver_fast_switch(policy, >>> next_freq); >>> if (!next_freq) >>> @@ -274,7 +286,7 @@ static inline bool sugov_cpu_is_busy(struct sugov_cpu >>> *sg_cpu) { return false; } >>> static inline void ignore_dl_rate_limit(struct sugov_cpu *sg_cpu, >>> struct sugov_policy *sg_policy) >>> { >>> if (cpu_util_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->util_dl) >>> - sg_policy->need_freq_update = true; >>> + sg_policy->urgent_freq_update = true; >>> } >>> static void sugov_update_single(struct update_util_data *hook, u64 >>> time, >>> @@ -383,8 +395,11 @@ static void sugov_work(struct kthread_work *work) >>> struct sugov_policy *sg_policy = container_of(work, struct >>> sugov_policy, work); >>> mutex_lock(&sg_policy->work_lock); >>> - __cpufreq_driver_target(sg_policy->policy, sg_policy->next_freq, >>> + do { >>> + sg_policy->urgent_freq_update = false; >>> + __cpufreq_driver_target(sg_policy->policy, >>> sg_policy->next_freq, >>> CPUFREQ_RELATION_L); >> >> >> If we are going to solve this problem, then maybe instead of the added >> complexity and a new flag we can look for need_freq_update flag at this >> location >> and re-calculate the next frequency if required. > > > I agree. > Indeed, I've been in doubt if adding a new flag or relying on the existing > need_freq_update flag (whose name, however, didn't seem to reflect any sense > of urgency). > Maybe we can use need_freq_update but change its name to a more meaningful > string ?
Implicitly, it means "as soon as reasonably possible".
