lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Wed,  5 Jun 2013 14:34:33 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	rjw@...k.pl
Cc:	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	cpufreq@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, robin.randhawa@....com,
	Steve.Bannister@....com, Liviu.Dudau@....com,
	charles.garcia-tobin@....com, arvind.chauhan@....com,
	chenxg.marvell@...il.com, Viresh Kumar <viresh.kumar@...aro.org>
Subject: [PATCH] cpufreq: userspace: Simplify governor

Userspace governor has got more code than what it needs for its functioning.
Lets simplify it.

Over that it will fix issues in cpufreq_governor_userspace(), which isn't doing
right things in START/STOP. It is working per-cpu currently whereas it just
required to manage policy->cpu.

Reported-by: Xiaoguang Chen <chenxg.marvell@...il.com>
Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
---
@Rafael:

I don't know why this code was initially added. Please let me know if I am doing
something stupid.

Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.

 drivers/cpufreq/cpufreq_userspace.c | 108 ++++--------------------------------
 1 file changed, 12 insertions(+), 96 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index bbeb9c0..5dc77b7 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -13,55 +13,13 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/smp.h>
-#include <linux/init.h>
-#include <linux/spinlock.h>
-#include <linux/interrupt.h>
 #include <linux/cpufreq.h>
-#include <linux/cpu.h>
-#include <linux/types.h>
-#include <linux/fs.h>
-#include <linux/sysfs.h>
+#include <linux/init.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 
-/**
- * A few values needed by the userspace governor
- */
-static DEFINE_PER_CPU(unsigned int, cpu_max_freq);
-static DEFINE_PER_CPU(unsigned int, cpu_min_freq);
-static DEFINE_PER_CPU(unsigned int, cpu_cur_freq); /* current CPU freq */
-static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by
-							userspace */
 static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
-
 static DEFINE_MUTEX(userspace_mutex);
-static int cpus_using_userspace_governor;
-
-/* keep track of frequency transitions */
-static int
-userspace_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
-	void *data)
-{
-	struct cpufreq_freqs *freq = data;
-
-	if (!per_cpu(cpu_is_managed, freq->cpu))
-		return 0;
-
-	if (val == CPUFREQ_POSTCHANGE) {
-		pr_debug("saving cpu_cur_freq of cpu %u to be %u kHz\n",
-				freq->cpu, freq->new);
-		per_cpu(cpu_cur_freq, freq->cpu) = freq->new;
-	}
-
-	return 0;
-}
-
-static struct notifier_block userspace_cpufreq_notifier_block = {
-	.notifier_call  = userspace_cpufreq_notifier
-};
-
 
 /**
  * cpufreq_set - set the CPU frequency
@@ -80,13 +38,6 @@ static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)
 	if (!per_cpu(cpu_is_managed, policy->cpu))
 		goto err;
 
-	per_cpu(cpu_set_freq, policy->cpu) = freq;
-
-	if (freq < per_cpu(cpu_min_freq, policy->cpu))
-		freq = per_cpu(cpu_min_freq, policy->cpu);
-	if (freq > per_cpu(cpu_max_freq, policy->cpu))
-		freq = per_cpu(cpu_max_freq, policy->cpu);
-
 	/*
 	 * We're safe from concurrent calls to ->target() here
 	 * as we hold the userspace_mutex lock. If we were calling
@@ -107,7 +58,7 @@ static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)
 
 static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
 {
-	return sprintf(buf, "%u\n", per_cpu(cpu_cur_freq, policy->cpu));
+	return sprintf(buf, "%u\n", policy->cur);
 }
 
 static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
@@ -119,66 +70,31 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
 	switch (event) {
 	case CPUFREQ_GOV_START:
 		BUG_ON(!policy->cur);
-		mutex_lock(&userspace_mutex);
-
-		if (cpus_using_userspace_governor == 0) {
-			cpufreq_register_notifier(
-					&userspace_cpufreq_notifier_block,
-					CPUFREQ_TRANSITION_NOTIFIER);
-		}
-		cpus_using_userspace_governor++;
+		pr_debug("started managing cpu %u\n", cpu);
 
+		mutex_lock(&userspace_mutex);
 		per_cpu(cpu_is_managed, cpu) = 1;
-		per_cpu(cpu_min_freq, cpu) = policy->min;
-		per_cpu(cpu_max_freq, cpu) = policy->max;
-		per_cpu(cpu_cur_freq, cpu) = policy->cur;
-		per_cpu(cpu_set_freq, cpu) = policy->cur;
-		pr_debug("managing cpu %u started "
-			"(%u - %u kHz, currently %u kHz)\n",
-				cpu,
-				per_cpu(cpu_min_freq, cpu),
-				per_cpu(cpu_max_freq, cpu),
-				per_cpu(cpu_cur_freq, cpu));
-
 		mutex_unlock(&userspace_mutex);
 		break;
 	case CPUFREQ_GOV_STOP:
-		mutex_lock(&userspace_mutex);
-		cpus_using_userspace_governor--;
-		if (cpus_using_userspace_governor == 0) {
-			cpufreq_unregister_notifier(
-					&userspace_cpufreq_notifier_block,
-					CPUFREQ_TRANSITION_NOTIFIER);
-		}
+		pr_debug("managing cpu %u stopped\n", cpu);
 
+		mutex_lock(&userspace_mutex);
 		per_cpu(cpu_is_managed, cpu) = 0;
-		per_cpu(cpu_min_freq, cpu) = 0;
-		per_cpu(cpu_max_freq, cpu) = 0;
-		per_cpu(cpu_set_freq, cpu) = 0;
-		pr_debug("managing cpu %u stopped\n", cpu);
 		mutex_unlock(&userspace_mutex);
 		break;
 	case CPUFREQ_GOV_LIMITS:
 		mutex_lock(&userspace_mutex);
-		pr_debug("limit event for cpu %u: %u - %u kHz, "
-			"currently %u kHz, last set to %u kHz\n",
+		pr_debug("limit event for cpu %u: %u - %u kHz, currently %u kHz\n",
 			cpu, policy->min, policy->max,
-			per_cpu(cpu_cur_freq, cpu),
-			per_cpu(cpu_set_freq, cpu));
-		if (policy->max < per_cpu(cpu_set_freq, cpu)) {
+			policy->cur);
+
+		if (policy->max < policy->cur)
 			__cpufreq_driver_target(policy, policy->max,
 						CPUFREQ_RELATION_H);
-		} else if (policy->min > per_cpu(cpu_set_freq, cpu)) {
+		else if (policy->min > policy->cur)
 			__cpufreq_driver_target(policy, policy->min,
 						CPUFREQ_RELATION_L);
-		} else {
-			__cpufreq_driver_target(policy,
-						per_cpu(cpu_set_freq, cpu),
-						CPUFREQ_RELATION_L);
-		}
-		per_cpu(cpu_min_freq, cpu) = policy->min;
-		per_cpu(cpu_max_freq, cpu) = policy->max;
-		per_cpu(cpu_cur_freq, cpu) = policy->cur;
 		mutex_unlock(&userspace_mutex);
 		break;
 	}
-- 
1.7.12.rc2.18.g61b472e

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ