Re: [RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-10 Thread Rafael J. Wysocki
On Mon, Apr 10, 2017 at 12:38 PM, Brendan Jackman
 wrote:
> 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?

Yes, we can.

Let me cut a v2 of this, shouldn't be too hard. :-)

Thanks,
Rafael


Re: [RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-10 Thread Rafael J. Wysocki
On Mon, Apr 10, 2017 at 12:38 PM, Brendan Jackman
 wrote:
> 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?

Yes, we can.

Let me cut a v2 of this, shouldn't be too hard. :-)

Thanks,
Rafael


Re: [RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-10 Thread Brendan Jackman
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(>transition_wait);
>   init_completion(>kobj_unregister);
>   INIT_WORK(>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 {
>   boolfast_switch_possible;
>   boolfast_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 intlatency_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 2
> +#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;
>



Re: [RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-10 Thread Brendan Jackman
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(>transition_wait);
>   init_completion(>kobj_unregister);
>   INIT_WORK(>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 {
>   boolfast_switch_possible;
>   boolfast_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 intlatency_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 2
> +#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;
>



[RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-09 Thread Rafael J. Wysocki
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.

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(>transition_wait);
init_completion(>kobj_unregister);
INIT_WORK(>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 {
boolfast_switch_possible;
boolfast_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 intlatency_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   2
+#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;
 



[RFC/RFT][PATCH 1/2] cpufreq: schedutil: Use policy-dependent latency multupliers

2017-04-09 Thread Rafael J. Wysocki
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.

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(>transition_wait);
init_completion(>kobj_unregister);
INIT_WORK(>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 {
boolfast_switch_possible;
boolfast_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 intlatency_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   2
+#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;