[CPUFREQ] return error when failing to set minfreq

I just stumbled on this bug/feature, this is how to reproduce it:

# echo 450000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
# echo 450000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
# echo powersave > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
# cpufreq-info -p
450000 450000 powersave
# echo 1800000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq ; echo $?
0
# cpufreq-info -p
450000 450000 powersave

Here it is. The kernel refuses to set a min_freq higher than the
max_freq but it allows a max_freq lower than min_freq (lowering min_freq
also).

This behaviour is pretty straightforward (but undocumented) and it
doesn't return an error altough failing to accomplish the requested
action (set min_freq).
The problem (IMO) is basically that userspace is not allowed to set a
full policy atomically while the kernel always does that thus it must
enforce an ordering on operations.

The attached patch returns -EINVAL if trying to increase frequencies
starting from scaling_min_freq and documents the correct ordering of writes.

Signed-off-by: Mattia Dongili <malattia@linux.it>
Signed-off-by: Dominik Brodowski <linux at dominikbrodowski.net>
Signed-off-by: Dave Jones <davej@redhat.com>
--

diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
index 7fedc00c3d30b78688cf0693f1512234c7e00f9f..555c8cf3650a5e6bb6523b9bdc69c75b2bc94159 100644 (file)
--- a/Documentation/cpu-freq/user-guide.txt
+++ b/Documentation/cpu-freq/user-guide.txt
@@ -153,10 +153,13 @@ scaling_governor,         and by "echoing" the name of another
                                that some governors won't load - they only
                                work on some specific architectures or
                                processors.
-scaling_min_freq and 
+scaling_min_freq and
 scaling_max_freq               show the current "policy limits" (in
                                kHz). By echoing new values into these
                                files, you can change these limits.
+                               NOTE: when setting a policy you need to
+                               first set scaling_max_freq, then
+                               scaling_min_freq.
 
 
 If you have selected the "userspace" governor which allows you to

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index bc1088d9b379a97843c58b050f046b8b0df023f5..ad996c772c8b7219a4d587f91fb47df4dff4e7f7 100644 (file)
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1343,6 +1343,11 @@ static int __cpufreq_set_policy(struct cpufreq_policy *data, struct cpufreq_poli
 
        memcpy(&policy->cpuinfo, &data->cpuinfo, sizeof(struct cpufreq_cpuinfo));
 
+       if (policy->min > data->min && policy->min > policy->max) {
+               ret = -EINVAL;
+               goto error_out;
+       }
+
        /* verify the cpu speed can be set within this limit */
        ret = cpufreq_driver->verify(policy);
        if (ret)