Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751733AbdCCGYj (ORCPT ); Fri, 3 Mar 2017 01:24:39 -0500 Received: from mail-pf0-f174.google.com ([209.85.192.174]:33008 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751521AbdCCGYM (ORCPT ); Fri, 3 Mar 2017 01:24:12 -0500 Date: Fri, 3 Mar 2017 10:49:43 +0530 From: Viresh Kumar To: Patrick Bellasi Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Steven Rostedt , Vincent Guittot , John Stultz , Juri Lelli , Todd Kjos , Tim Murray , Andres Oportus , Joel Fernandes , Morten Rasmussen , Dietmar Eggemann , Chris Redpath , Ingo Molnar , Peter Zijlstra , "Rafael J . Wysocki" Subject: Re: [PATCH 2/6] cpufreq: schedutil: ignore the sugov kthread for frequencies selections Message-ID: <20170303051943.GI13760@vireshk-i7> References: <1488469507-32463-1-git-send-email-patrick.bellasi@arm.com> <1488469507-32463-3-git-send-email-patrick.bellasi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1488469507-32463-3-git-send-email-patrick.bellasi@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2701 Lines: 68 On 02-03-17, 15:45, Patrick Bellasi wrote: > In system where multiple CPUs shares the same frequency domain a small > workload on a CPU can still be subject frequency spikes, generated by > the activation of the sugov's kthread. > > Since the sugov kthread is a special RT task, which goal is just that to > activate a frequency transition, it does not make sense for it to bias > the schedutil's frequency selection. > > This patch exploits the information related to the current task to silently > ignore cpufreq_update_this_cpu() calls, coming from the RT scheduler, while > the sugov kthread is running. > > Signed-off-by: Patrick Bellasi > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Rafael J. Wysocki > Cc: Viresh Kumar > Cc: linux-kernel@vger.kernel.org > Cc: linux-pm@vger.kernel.org > --- > kernel/sched/cpufreq_schedutil.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c > index 084a98b..a3fe5e4 100644 > --- a/kernel/sched/cpufreq_schedutil.c > +++ b/kernel/sched/cpufreq_schedutil.c > @@ -274,6 +274,8 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time, > { > struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_util); > struct sugov_policy *sg_policy = sg_cpu->sg_policy; > + unsigned int cpu = smp_processor_id(); > + struct task_struct *curr = cpu_curr(cpu); Maybe merge these two as you have done in the next patch. > unsigned long util, max; > unsigned int next_f; > > @@ -287,6 +289,10 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time, > goto done; > } > > + /* Skip updates generated by sugov kthreads */ > + if (curr == sg_policy->thread) > + goto done; > + I always wanted to avoid such hacks when I moved to the RT thread :( I was discussing the exact same problem with Vincent few days back and one of the ideas we had was to clear the flags when any RT task is dequeued from a rq. AFAIU, the problem we are discussing here shouldn't normally occur while the sugov RT thread is running as the work_in_progress flag makes sure we don't reevaluate the load while the RT thread is updating the frequency. The problem perhaps occurs as the flag for CPU X is never cleared, and so on the next callback from the scheduler (after the frequency is updated and work_in_progress is cleared) we move to the highest frequency. So what about clearing the flags, just like the previous patch, when the RT or DL task has finished? Sorry for the noise if it was all nonsense :) -- viresh