Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753058AbdDJKjB (ORCPT ); Mon, 10 Apr 2017 06:39:01 -0400 Received: from foss.arm.com ([217.140.101.70]:46074 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752144AbdDJKi7 (ORCPT ); Mon, 10 Apr 2017 06:38:59 -0400 References: <3498238.liCqOyIkGA@aspire.rjw.lan> <2407280.n9qVSLCrF5@aspire.rjw.lan> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Brendan Jackman To: "Rafael J. Wysocki" Cc: Linux PM , Juri Lelli , LKML , Peter Zijlstra , Srinivas Pandruvada , Viresh Kumar , Vincent Guittot , Patrick Bellasi , Joel Fernandes , Morten Rasmussen Subject: Re: [RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers In-reply-to: <2407280.n9qVSLCrF5@aspire.rjw.lan> Date: Mon, 10 Apr 2017 11:38:55 +0100 Message-ID: <87k26sv96o.fsf@arm.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4106 Lines: 103 Hi Rafael, On Mon, Apr 10 2017 at 00:10, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > Make the schedutil governor compute the initial (default) value of > the rate_limit_us sysfs attribute by multiplying the transition > latency by a multiplier depending on the policy and set by the > scaling driver (instead of using a constant for this purpose). > > That will allow scaling drivers to make schedutil use smaller default > values of rate_limit_us and reduce the default average time interval > between consecutive frequency changes. > I've been thinking about this over the last couple of days, and I'm thinking (in opposition to what I said at OSPM Pisa) that allowing drivers to specify a _multiplier_ isn't ideal, since you lose granularity when you want your rate limit to be close to your transition latency (i.e. if your multiplier would be 2.5 or something). Can we instead just have an independent field policy->default_rate_limit_us or similar? Drivers know the transition latency so intel_pstate can still use a multiplier if it wants. Cheers, Brendan > Make intel_pstate use the opportunity to reduce the rate limit > somewhat. > > Signed-off-by: Rafael J. Wysocki > --- > drivers/cpufreq/cpufreq.c | 1 + > drivers/cpufreq/intel_pstate.c | 2 ++ > include/linux/cpufreq.h | 8 ++++++++ > kernel/sched/cpufreq_schedutil.c | 2 +- > 4 files changed, 12 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/cpufreq/cpufreq.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/cpufreq.c > +++ linux-pm/drivers/cpufreq/cpufreq.c > @@ -1072,6 +1072,7 @@ static struct cpufreq_policy *cpufreq_po > init_waitqueue_head(&policy->transition_wait); > init_completion(&policy->kobj_unregister); > INIT_WORK(&policy->update, handle_update); > + policy->latency_multiplier = LATENCY_MULTIPLIER; > > policy->cpu = cpu; > return policy; > Index: linux-pm/include/linux/cpufreq.h > =================================================================== > --- linux-pm.orig/include/linux/cpufreq.h > +++ linux-pm/include/linux/cpufreq.h > @@ -120,6 +120,14 @@ struct cpufreq_policy { > bool fast_switch_possible; > bool fast_switch_enabled; > > + /* > + * Multiplier to apply to the transition latency to obtain the preferred > + * average time interval between consecutive invocations of the driver > + * to set the frequency for this policy. Initialized by the core to the > + * LATENCY_MULTIPLIER value. > + */ > + unsigned int latency_multiplier; > + > /* Cached frequency lookup from cpufreq_driver_resolve_freq. */ > unsigned int cached_target_freq; > int cached_resolved_idx; > Index: linux-pm/kernel/sched/cpufreq_schedutil.c > =================================================================== > --- linux-pm.orig/kernel/sched/cpufreq_schedutil.c > +++ linux-pm/kernel/sched/cpufreq_schedutil.c > @@ -530,7 +530,7 @@ static int sugov_init(struct cpufreq_pol > goto stop_kthread; > } > > - tunables->rate_limit_us = LATENCY_MULTIPLIER; > + tunables->rate_limit_us = policy->latency_multiplier; > lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC; > if (lat) > tunables->rate_limit_us *= lat; > Index: linux-pm/drivers/cpufreq/intel_pstate.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > +++ linux-pm/drivers/cpufreq/intel_pstate.c > @@ -41,6 +41,7 @@ > #define INTEL_PSTATE_HWP_SAMPLING_INTERVAL (50 * NSEC_PER_MSEC) > > #define INTEL_CPUFREQ_TRANSITION_LATENCY 20000 > +#define INTEL_CPUFREQ_LATENCY_MULTIPLIER 250 > > #ifdef CONFIG_ACPI > #include > @@ -2237,6 +2238,7 @@ static int intel_cpufreq_cpu_init(struct > return ret; > > policy->cpuinfo.transition_latency = INTEL_CPUFREQ_TRANSITION_LATENCY; > + policy->latency_multiplier = INTEL_CPUFREQ_LATENCY_MULTIPLIER; > /* This reflects the intel_pstate_get_cpu_pstates() setting. */ > policy->cur = policy->cpuinfo.min_freq; >