lkml.org 
[lkml]   [2009]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/7] CPUFREQ: ondemand/conservative: sanitize sampling_rate restrictions
On Tue,  3 Feb 2009 17:46:42 +0100 Thomas Renninger <trenn@suse.de> wrote:

> Limit sampling rate to transition_latency * 100 or kernel limits.
> If sampling_rate is tried to be set too low, set the lowest allowed value.
>
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> ---
> Documentation/cpu-freq/governors.txt | 14 +++++++++++++-
> drivers/cpufreq/cpufreq_conservative.c | 19 ++++++++++++++++---
> drivers/cpufreq/cpufreq_ondemand.c | 19 +++++++++++++++----
> 3 files changed, 44 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/cpu-freq/governors.txt b/Documentation/cpu-freq/governors.txt
> index 9b18512..ce73f3e 100644
> --- a/Documentation/cpu-freq/governors.txt
> +++ b/Documentation/cpu-freq/governors.txt
> @@ -117,7 +117,19 @@ accessible parameters:
> sampling_rate: measured in uS (10^-6 seconds), this is how often you
> want the kernel to look at the CPU usage and to make decisions on
> what to do about the frequency. Typically this is set to values of
> -around '10000' or more.
> +around '10000' or more. It's default value is (cmp. with users-guide.txt):
> +transition_latency * 1000
> +The lowest value you can set is:
> +transition_latency * 100 or it may get restricted to a value where it
> +makes not sense for the kernel anymore to poll that often which depends
> +on your HZ config variable (HZ=1000: max=20000us, HZ=250: max=5000).
> +Be aware that transition latency is in ns and sampling_rate is in us, so you
> +get the same sysfs value by default.
> +Sampling rate should always get adjusted considering the transition latency
> +To set the sampling rate 750 times as high as the transition latency
> +in the bash (as said, 1000 is default), do:
> +echo `$(($(cat cpuinfo_transition_latency) * 750 / 1000)) \
> + >ondemand/sampling_rate
>
> show_sampling_rate_(min|max): THIS INTERFACE IS DEPRECATED, DON'T USE IT.
> You can use wider ranges now and the general
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 5ba0a3f..b8bbd4a 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -54,7 +54,18 @@ static unsigned int def_sampling_rate;
> (MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
> #define MIN_SAMPLING_RATE \
> (def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
> +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> + * Define the minimal settable sampling rate to the greater of:
> + * - "HW transition latency" * 100 (same as default sampling / 10)
> + * - MIN_STAT_SAMPLING_RATE
> + * To avoid that userspace shoots itself.
> +*/
> +#define MINIMUM_SAMPLING_RATE \
> + ((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> + ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))

Use
max(def_sampling_rate / 10, MIN_STAT_SAMPLING_RATE)
?

But really, this should be a plain old C function. Hiding a bunch of C
code inside a macro which purports to be a compile-time constant is rude.

> +/* This will also vanish soon with removing sampling_rate_max */
> #define MAX_SAMPLING_RATE (500 * def_sampling_rate)

Ditto, really.

> +
> #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER (1000)
> #define DEF_SAMPLING_DOWN_FACTOR (1)
> #define MAX_SAMPLING_DOWN_FACTOR (10)
> @@ -197,12 +208,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
> ret = sscanf (buf, "%u", &input);
>
> mutex_lock(&dbs_mutex);
> - if (ret != 1 || input > MAX_SAMPLING_RATE || input < MIN_SAMPLING_RATE) {
> + if (ret != 1) {
> mutex_unlock(&dbs_mutex);
> return -EINVAL;
> }
> -
> - dbs_tuners_ins.sampling_rate = input;
> + if (input < MINIMUM_SAMPLING_RATE)
> + dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> + else
> + dbs_tuners_ins.sampling_rate = input;

dbs_tuners_ins.sampling_rate = max(MINIMUM_SAMPLING_RATE, input);

> mutex_unlock(&dbs_mutex);
>
> return count;
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 4b42e36..70e0841 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -52,6 +52,16 @@ static unsigned int def_sampling_rate;
> (MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
> #define MIN_SAMPLING_RATE \
> (def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
> +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> + * Define the minimal settable sampling rate to the greater of:
> + * - "HW transition latency" * 100 (same as default sampling / 10)
> + * - MIN_STAT_SAMPLING_RATE
> + * To avoid that userspace shoots itself.
> +*/
> +#define MINIMUM_SAMPLING_RATE \
> + ((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> + ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
> +/* This will also vanish soon with removing sampling_rate_max */
> #define MAX_SAMPLING_RATE (500 * def_sampling_rate)
> #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER (1000)
> #define TRANSITION_LATENCY_LIMIT (10 * 1000 * 1000)
> @@ -243,13 +253,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
> ret = sscanf(buf, "%u", &input);
>
> mutex_lock(&dbs_mutex);
> - if (ret != 1 || input > MAX_SAMPLING_RATE
> - || input < MIN_SAMPLING_RATE) {
> + if (ret != 1) {
> mutex_unlock(&dbs_mutex);
> return -EINVAL;
> }
> -
> - dbs_tuners_ins.sampling_rate = input;
> + if (input < MINIMUM_SAMPLING_RATE)
> + dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> + else
> + dbs_tuners_ins.sampling_rate = input;
> mutex_unlock(&dbs_mutex);

etceterata.


\
 
 \ /
  Last update: 2009-02-04 06:11    [W:0.096 / U:0.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site