On 04/01/2013 11:28 AM, Viresh Kumar wrote:
Hi Nathan,
Welcome back :)
On 1 April 2013 21:03, Nathan Zimmer <nzimmer@xxxxxxx> wrote:
You need to resent this patch as we don't want current mail subject as commit
subject.. You could have used the area after three dashes "-" inside the
commit for logs which you don't want to commit.
Ok.
The cpufreq_driver_lock is hot with some configs.
This lock covers both cpufreq_driver and cpufreq_cpu_data so part one of the
s/ so/, so/
proposed fix is to split up the lock abit.
s/abit/a bit/
What's the other part?
cpufreq_cpu_data is now covered by the cpufreq_data_lock.
cpufreq_driver is now covered by the cpufreq_driver lock and the rcu.
This means that the cpufreq_driver_lock is no longer hot.
There remains some measurable heat on the cpufreq_data_lock it is significantly
s/it/but it/
less then previous measured though.
Cc: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxx>
Signed-off-by: Nathan Zimmer <nzimmer@xxxxxxx>
---
drivers/cpufreq/cpufreq.c | 305 +++++++++++++++++++++++++++++++++-------------
1 file changed, 222 insertions(+), 83 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
@@ -329,11 +339,23 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy,
struct cpufreq_governor **governor)
{
int err = -EINVAL;
-
- if (!cpufreq_driver)
+ struct cpufreq_driver *driver;
+ int (*setpolicy)(struct cpufreq_policy *policy);
+ int (*target)(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int relation);
You can keep bools here instead of complex function pointers.
setpolicy_supported and target_supported
Good point. In a few places I needed the function pointer but not here.
I'll convert the unneeded ones to bools and resend.
+ rcu_read_lock();
+ driver = rcu_dereference(cpufreq_driver);
+ if (!driver) {
+ rcu_read_unlock();
goto out;
+ }
+ setpolicy = driver->setpolicy;
+ target = driver->target;
+ rcu_read_unlock();
- if (cpufreq_driver->setpolicy) {
+ if (setpolicy) {
if (!strnicmp(str_governor, "performance", CPUFREQ_NAME_LEN)) {
*policy = CPUFREQ_POLICY_PERFORMANCE;
err = 0;
@@ -342,7 +364,7 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy,
*policy = CPUFREQ_POLICY_POWERSAVE;
err = 0;
}
- } else if (cpufreq_driver->target) {
+ } else if (target) {
struct cpufreq_governor *t;
mutex_lock(&cpufreq_governor_mutex);
@@ -731,6 +766,8 @@ static int cpufreq_add_dev_interface(unsigned int cpu,
{
struct cpufreq_policy new_policy;
struct freq_attr **drv_attr;
+ struct cpufreq_driver *driver;
+ int (*exit)(struct cpufreq_policy *policy);
Declare it in the block which used it.
if (ret) {
pr_debug("setting policy failed\n");
- if (cpufreq_driver->exit)
- cpufreq_driver->exit(policy);
+ rcu_read_lock();
+ exit = rcu_dereference(cpufreq_driver)->exit;
+ if (exit)
+ exit(policy);
+ rcu_read_unlock();
+
}
@@ -1002,32 +1059,42 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif
unsigned int cpu = dev->id, ret, cpus;
unsigned long flags;
struct cpufreq_policy *data;
+ struct cpufreq_driver *driver;
struct kobject *kobj;
struct completion *cmp;That
struct device *cpu_dev;
+ int (*target)(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int relation);
can be bool?
+ int (*exit)(struct cpufreq_policy *policy);
One more generic comment: What about a reader-writer lock for
cpufreq_data_lock??
I had been looking for ways to use the rcu but wasn't having much success.
Let me try a rwlock and grab some numbers after lunch.
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
