Hi Rafael,
This is another unplanned patchset for all the platforms that i broke. :)
Okay, there are two important fixes (1 & 4) and two general cleanups (2 & 3). I hope most of the issues would be resolved by these and we would be able to push clean cpufreq core into 3.9.
I have pushed them in my for-rafael branch at:
http://git.linaro.org/gitweb?p=people/vireshk/linux.git%3Ba=shortlog%3Bh=ref...
@Artem & Valdis: Please test them and reply with your Tested-by's (in case they work :) ).
Viresh Kumar (4): cpufreq: governors: Fix WARN_ON() for multi-policy platforms cpufreq: Remove unused HOTPLUG_CPU code cpufreq: Create a macro for unlock_policy_rwsem{read,write} cpufreq: Fix locking issues
drivers/cpufreq/cpufreq.c | 126 ++++++++++++++++++------------------- drivers/cpufreq/cpufreq_governor.c | 32 ++++++---- 2 files changed, 79 insertions(+), 79 deletions(-)
On multi-policy systems there is a single instance of governor for both the policies (if same governor is chosen for both policies). With the code update from following patches:
8eeed09 cpufreq: governors: Get rid of dbs_data->enable field b394058 cpufreq: governors: Reset tunables only for cpufreq_unregister_governor()
We are creating/removing sysfs directory of governor for for every call to GOV_START and STOP. This would fail for multi-policy system as there is a per-policy call to START/STOP.
This patch reuses the governor->initialized variable to detect total users of governor.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 6 ++++-- drivers/cpufreq/cpufreq_governor.c | 32 +++++++++++++++++++------------- 2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index ccc598a..3b941a1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1567,8 +1567,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, policy->cpu, event); ret = policy->governor->governor(policy, event);
- if (!policy->governor->initialized && (event == CPUFREQ_GOV_START)) - policy->governor->initialized = 1; + if (event == CPUFREQ_GOV_START) + policy->governor->initialized++; + else if (event == CPUFREQ_GOV_STOP) + policy->governor->initialized--;
/* we keep one module reference alive for each CPU governed by this CPU */ diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index e4a306c..5a76086 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -247,11 +247,13 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data, dbs_data->gov_dbs_timer); }
- rc = sysfs_create_group(cpufreq_global_kobject, - dbs_data->attr_group); - if (rc) { - mutex_unlock(&dbs_data->mutex); - return rc; + if (!policy->governor->initialized) { + rc = sysfs_create_group(cpufreq_global_kobject, + dbs_data->attr_group); + if (rc) { + mutex_unlock(&dbs_data->mutex); + return rc; + } }
/* @@ -262,13 +264,15 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data, cs_dbs_info->down_skip = 0; cs_dbs_info->enable = 1; cs_dbs_info->requested_freq = policy->cur; - cpufreq_register_notifier(cs_ops->notifier_block, - CPUFREQ_TRANSITION_NOTIFIER);
- if (!policy->governor->initialized) + if (!policy->governor->initialized) { + cpufreq_register_notifier(cs_ops->notifier_block, + CPUFREQ_TRANSITION_NOTIFIER); + dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10); + } } else { od_dbs_info->rate_mult = 1; od_dbs_info->sample_type = OD_NORMAL_SAMPLE; @@ -311,11 +315,13 @@ unlock: mutex_lock(&dbs_data->mutex); mutex_destroy(&cpu_cdbs->timer_mutex);
- sysfs_remove_group(cpufreq_global_kobject, - dbs_data->attr_group); - if (dbs_data->governor == GOV_CONSERVATIVE) - cpufreq_unregister_notifier(cs_ops->notifier_block, - CPUFREQ_TRANSITION_NOTIFIER); + if (policy->governor->initialized == 1) { + sysfs_remove_group(cpufreq_global_kobject, + dbs_data->attr_group); + if (dbs_data->governor == GOV_CONSERVATIVE) + cpufreq_unregister_notifier(cs_ops->notifier_block, + CPUFREQ_TRANSITION_NOTIFIER); + } mutex_unlock(&dbs_data->mutex);
break;
Because the sibling cpu of any online cpu is identified very early in cpufreq_add_dev(), below code is never executed. And so can be removed.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3b941a1..795b0e8 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -858,7 +858,7 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling, static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) { unsigned int j, cpu = dev->id; - int ret = -ENOMEM, found = 0; + int ret = -ENOMEM; struct cpufreq_policy *policy; struct cpufreq_driver *driver; unsigned long flags; @@ -908,6 +908,7 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) goto err_free_cpumask;
policy->cpu = cpu; + policy->governor = CPUFREQ_DEFAULT_GOVERNOR; cpumask_copy(policy->cpus, cpumask_of(cpu));
/* Initially set CPU itself as the policy_cpu */ @@ -918,20 +919,6 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) init_completion(&policy->kobj_unregister); INIT_WORK(&policy->update, handle_update);
- /* Set governor before ->init, so that driver could check it */ -#ifdef CONFIG_HOTPLUG_CPU - for_each_online_cpu(sibling) { - struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling); - if (cp && cp->governor && - (cpumask_test_cpu(cpu, cp->related_cpus))) { - policy->governor = cp->governor; - found = 1; - break; - } - } -#endif - if (!found) - policy->governor = CPUFREQ_DEFAULT_GOVERNOR; /* call driver. From then on the cpufreq must be able * to accept all calls to ->verify and ->setpolicy for this CPU */
On the lines of macro: lock_policy_rwsem, we can create another macro for unlock_policy_rwsem. Lets do it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 795b0e8..c46aed4 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -70,8 +70,7 @@ static DEFINE_PER_CPU(int, cpufreq_policy_cpu); static DEFINE_PER_CPU(struct rw_semaphore, cpu_policy_rwsem);
#define lock_policy_rwsem(mode, cpu) \ -static int lock_policy_rwsem_##mode \ -(int cpu) \ +static int lock_policy_rwsem_##mode(int cpu) \ { \ int policy_cpu = per_cpu(cpufreq_policy_cpu, cpu); \ BUG_ON(policy_cpu == -1); \ @@ -81,23 +80,18 @@ static int lock_policy_rwsem_##mode \ }
lock_policy_rwsem(read, cpu); - lock_policy_rwsem(write, cpu);
-static void unlock_policy_rwsem_read(int cpu) -{ - int policy_cpu = per_cpu(cpufreq_policy_cpu, cpu); - BUG_ON(policy_cpu == -1); - up_read(&per_cpu(cpu_policy_rwsem, policy_cpu)); -} - -static void unlock_policy_rwsem_write(int cpu) -{ - int policy_cpu = per_cpu(cpufreq_policy_cpu, cpu); - BUG_ON(policy_cpu == -1); - up_write(&per_cpu(cpu_policy_rwsem, policy_cpu)); +#define unlock_policy_rwsem(mode, cpu) \ +static void unlock_policy_rwsem_##mode(int cpu) \ +{ \ + int policy_cpu = per_cpu(cpufreq_policy_cpu, cpu); \ + BUG_ON(policy_cpu == -1); \ + up_##mode(&per_cpu(cpu_policy_rwsem, policy_cpu)); \ }
+unlock_policy_rwsem(read, cpu); +unlock_policy_rwsem(write, cpu);
/* internal prototypes */ static int __cpufreq_governor(struct cpufreq_policy *policy,
cpufreq core uses two locks: - cpufreq_driver_lock: General lock for driver and cpufreq_cpu_data array. - cpu_policy_rwsemfix locking: per CPU reader-writer semaphore designed to cure all cpufreq/hotplug/workqueue/etc related lock issues.
These locks were not used properly and are placed against their principle (present before their definition) at various places. This patch is an attempt to fix their use.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 79 +++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 34 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index c46aed4..79511ab 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -59,8 +59,6 @@ static DEFINE_SPINLOCK(cpufreq_driver_lock); * mode before doing so. * * Additional rules: - * - All holders of the lock should check to make sure that the CPU they - * are concerned with are online after they get the lock. * - Governor routines that can be called in cpufreq hotplug path should not * take this sem as top level hotplug notifier handler takes this. * - Lock should not be held across @@ -257,6 +255,7 @@ void cpufreq_notify_transition(struct cpufreq_freqs *freqs, unsigned int state) { struct cpufreq_policy *policy; struct cpufreq_driver *driver; + unsigned long flags;
BUG_ON(irqs_disabled());
@@ -269,7 +268,10 @@ void cpufreq_notify_transition(struct cpufreq_freqs *freqs, unsigned int state) pr_debug("notification %u of frequency transition to %u kHz\n", state, freqs->new);
+ spin_lock_irqsave(&cpufreq_driver_lock, flags); policy = per_cpu(cpufreq_cpu_data, freqs->cpu); + spin_unlock_irqrestore(&cpufreq_driver_lock, flags); + switch (state) {
case CPUFREQ_PRECHANGE: @@ -792,8 +794,10 @@ static int cpufreq_add_dev_interface(unsigned int cpu,
if (ret) { pr_debug("setting policy failed\n"); + spin_lock_irqsave(&cpufreq_driver_lock, flags); if (driver->exit) driver->exit(policy); + spin_unlock_irqrestore(&cpufreq_driver_lock, flags); } return ret;
@@ -814,22 +818,22 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling, policy = cpufreq_cpu_get(sibling); WARN_ON(!policy);
- per_cpu(cpufreq_policy_cpu, cpu) = policy->cpu; - - lock_policy_rwsem_write(cpu); - __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+ lock_policy_rwsem_write(sibling); + spin_lock_irqsave(&cpufreq_driver_lock, flags); + cpumask_set_cpu(cpu, policy->cpus); + per_cpu(cpufreq_policy_cpu, cpu) = policy->cpu; per_cpu(cpufreq_cpu_data, cpu) = policy; spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ unlock_policy_rwsem_write(sibling); + __cpufreq_governor(policy, CPUFREQ_GOV_START); __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
- unlock_policy_rwsem_write(cpu); - ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq"); if (ret) { cpufreq_cpu_put(policy); @@ -878,11 +882,15 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
#ifdef CONFIG_HOTPLUG_CPU /* Check if this cpu was hot-unplugged earlier and has siblings */ + spin_lock_irqsave(&cpufreq_driver_lock, flags); for_each_online_cpu(sibling) { struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling); - if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) + if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) { + spin_unlock_irqrestore(&cpufreq_driver_lock, flags); return cpufreq_add_policy_cpu(cpu, sibling, dev); + } } + spin_unlock_irqrestore(&cpufreq_driver_lock, flags); #endif #endif
@@ -907,20 +915,21 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
/* Initially set CPU itself as the policy_cpu */ per_cpu(cpufreq_policy_cpu, cpu) = cpu; - ret = (lock_policy_rwsem_write(cpu) < 0); - WARN_ON(ret);
init_completion(&policy->kobj_unregister); INIT_WORK(&policy->update, handle_update);
+ spin_lock_irqsave(&cpufreq_driver_lock, flags); /* call driver. From then on the cpufreq must be able * to accept all calls to ->verify and ->setpolicy for this CPU */ ret = driver->init(policy); if (ret) { pr_debug("initialization failed\n"); - goto err_unlock_policy; + spin_unlock_irqrestore(&cpufreq_driver_lock, flags); + goto err_set_policy_cpu; } + spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
/* related cpus should atleast have policy->cpus */ cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus); @@ -950,8 +959,6 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) if (ret) goto err_out_unregister;
- unlock_policy_rwsem_write(cpu); - kobject_uevent(&policy->kobj, KOBJ_ADD); module_put(rcu_dereference(cpufreq_driver)->owner); pr_debug("initialization complete\n"); @@ -967,8 +974,8 @@ err_out_unregister: kobject_put(&policy->kobj); wait_for_completion(&policy->kobj_unregister);
-err_unlock_policy: - unlock_policy_rwsem_write(cpu); +err_set_policy_cpu: + per_cpu(cpufreq_policy_cpu, cpu) = -1; free_cpumask_var(policy->related_cpus); err_free_cpumask: free_cpumask_var(policy->cpus); @@ -1017,12 +1024,14 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
spin_lock_irqsave(&cpufreq_driver_lock, flags); + data = per_cpu(cpufreq_cpu_data, cpu); + per_cpu(cpufreq_cpu_data, cpu) = NULL; + + spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
if (!data) { pr_debug("%s: No cpu_data found\n", __func__); - spin_unlock_irqrestore(&cpufreq_driver_lock, flags); - unlock_policy_rwsem_write(cpu); return -EINVAL; }
@@ -1034,9 +1043,10 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif CPUFREQ_NAME_LEN); #endif
- per_cpu(cpufreq_cpu_data, cpu) = NULL; + WARN_ON(lock_policy_rwsem_write(cpu)); cpus = cpumask_weight(data->cpus); cpumask_clear_cpu(cpu, data->cpus); + unlock_policy_rwsem_write(cpu);
if (cpu != data->cpu) { sysfs_remove_link(&dev->kobj, "cpufreq"); @@ -1047,31 +1057,37 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif ret = kobject_move(&data->kobj, &cpu_dev->kobj); if (ret) { pr_err("%s: Failed to move kobj: %d", __func__, ret); + + WARN_ON(lock_policy_rwsem_write(cpu)); cpumask_set_cpu(cpu, data->cpus); - ret = sysfs_create_link(&cpu_dev->kobj, &data->kobj, - "cpufreq"); + + spin_lock_irqsave(&cpufreq_driver_lock, flags); + per_cpu(cpufreq_cpu_data, cpu) = data; spin_unlock_irqrestore(&cpufreq_driver_lock, flags); + unlock_policy_rwsem_write(cpu); + + ret = sysfs_create_link(&cpu_dev->kobj, &data->kobj, + "cpufreq"); return -EINVAL; }
+ WARN_ON(lock_policy_rwsem_write(cpu)); update_policy_cpu(data, cpu_dev->id); + unlock_policy_rwsem_write(cpu); pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n", __func__, cpu_dev->id, cpu); }
- spin_unlock_irqrestore(&cpufreq_driver_lock, flags); - pr_debug("%s: removing link, cpu: %d\n", __func__, cpu); cpufreq_cpu_put(data); - unlock_policy_rwsem_write(cpu);
/* If cpu is last user of policy, free policy */ if (cpus == 1) { - lock_policy_rwsem_write(cpu); + lock_policy_rwsem_read(cpu); kobj = &data->kobj; cmp = &data->kobj_unregister; - unlock_policy_rwsem_write(cpu); + unlock_policy_rwsem_read(cpu); kobject_put(kobj);
/* we need to make sure that the underlying kobj is actually @@ -1082,10 +1098,10 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif wait_for_completion(cmp); pr_debug("wait complete\n");
- lock_policy_rwsem_write(cpu); + spin_lock_irqsave(&cpufreq_driver_lock, flags); if (driver->exit) driver->exit(data); - unlock_policy_rwsem_write(cpu); + spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
free_cpumask_var(data->related_cpus); free_cpumask_var(data->cpus); @@ -1095,6 +1111,7 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif __cpufreq_governor(data, CPUFREQ_GOV_LIMITS); }
+ per_cpu(cpufreq_policy_cpu, cpu) = -1; return 0; }
@@ -1107,9 +1124,6 @@ static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) if (cpu_is_offline(cpu)) return 0;
- if (unlikely(lock_policy_rwsem_write(cpu))) - BUG(); - retval = __cpufreq_remove_dev(dev, sif); return retval; } @@ -1808,9 +1822,6 @@ static int __cpuinit cpufreq_cpu_callback(struct notifier_block *nfb, break; case CPU_DOWN_PREPARE: case CPU_DOWN_PREPARE_FROZEN: - if (unlikely(lock_policy_rwsem_write(cpu))) - BUG(); - __cpufreq_remove_dev(dev, NULL); break; case CPU_DOWN_FAILED:
On Thursday, February 07, 2013 03:57:42 PM Viresh Kumar wrote:
Hi Rafael,
This is another unplanned patchset for all the platforms that i broke. :)
Okay, there are two important fixes (1 & 4) and two general cleanups (2 & 3). I hope most of the issues would be resolved by these and we would be able to push clean cpufreq core into 3.9.
I have pushed them in my for-rafael branch at:
http://git.linaro.org/gitweb?p=people/vireshk/linux.git%3Ba=shortlog%3Bh=ref...
@Artem & Valdis: Please test them and reply with your Tested-by's (in case they work :) ).
Viresh Kumar (4): cpufreq: governors: Fix WARN_ON() for multi-policy platforms cpufreq: Remove unused HOTPLUG_CPU code cpufreq: Create a macro for unlock_policy_rwsem{read,write} cpufreq: Fix locking issues
drivers/cpufreq/cpufreq.c | 126 ++++++++++++++++++------------------- drivers/cpufreq/cpufreq_governor.c | 32 ++++++---- 2 files changed, 79 insertions(+), 79 deletions(-)
I think they all make sense, so applied to linux-next.
I would prefer not to make any more changes to cpufreq before v3.9 from now on, except for fixes and maybe the Drik's patchset that I kind of scheduled for merging into bleeding-edge later today.
Thanks, Rafael
On 7 February 2013 18:35, Rafael J. Wysocki rjw@sisk.pl wrote:
I think they all make sense, so applied to linux-next.
I would prefer not to make any more changes to cpufreq before v3.9 from now on, except for fixes and maybe the Drik's patchset that I kind of scheduled for
Dirk :)
merging into bleeding-edge later today.
I probably have few more for you. Some sparse warnings to be fixed for Dirks work and an dangling exynos patch which is waiting for your reply :)
-- viresh
On Thursday, February 07, 2013 06:52:20 PM Viresh Kumar wrote:
On 7 February 2013 18:35, Rafael J. Wysocki rjw@sisk.pl wrote:
I think they all make sense, so applied to linux-next.
I would prefer not to make any more changes to cpufreq before v3.9 from now on, except for fixes and maybe the Drik's patchset that I kind of scheduled for
Dirk :)
Yes, sorry Dirk.
merging into bleeding-edge later today.
I probably have few more for you. Some sparse warnings to be fixed for Dirks work and an dangling exynos patch which is waiting for your reply :)
Which Exynos patch?
BTW, there still are locking problems in linux-next. Why do we need to take cpufreq_driver_lock() around driver->init() in cpufreq_add_dev(), in particular?
Rafael
On 8 February 2013 04:37, Rafael J. Wysocki rjw@sisk.pl wrote:
On Thursday, February 07, 2013 06:52:20 PM Viresh Kumar wrote:
On 7 February 2013 18:35, Rafael J. Wysocki rjw@sisk.pl wrote:
I think they all make sense, so applied to linux-next.
I would prefer not to make any more changes to cpufreq before v3.9 from now on, except for fixes and maybe the Drik's patchset that I kind of scheduled for
Dirk :)
Yes, sorry Dirk.
merging into bleeding-edge later today.
I probably have few more for you. Some sparse warnings to be fixed for Dirks work and an dangling exynos patch which is waiting for your reply :)
Which Exynos patch?
https://lkml.org/lkml/2013/1/30/592
BTW, there still are locking problems in linux-next. Why do we need to take cpufreq_driver_lock() around driver->init() in cpufreq_add_dev(), in particular?
I thought cpufreq provides atomicity to all drivers callbacks and that's why i had those around it :(
On 8 February 2013 04:37, Rafael J. Wysocki rjw@sisk.pl wrote:
BTW, there still are locking problems in linux-next. Why do we need to take cpufreq_driver_lock() around driver->init() in cpufreq_add_dev(), in particular?
I thought a bit more and realized there is no such limitation on cpufreq_driver->ops about calling routines which can sleep. And thus we shoudln't have locks around any of these. I have got a patch for it, that i would fold-back into the original patch that introduced locking fixes (attached too for testing):
From: Viresh Kumar viresh.kumar@linaro.org Date: Fri, 8 Feb 2013 10:35:31 +0530 Subject: [PATCH] cpufreq: Remove unnecessary locking
I have placed some locks intentionally around calls to driver->ops (init/exit), which look to be wrong as these calls can call routines that potentially sleep.
Lets remove these locks.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 5d8a422..04aab05 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -795,10 +795,8 @@ static int cpufreq_add_dev_interface(unsigned int cpu,
if (ret) { pr_debug("setting policy failed\n"); - spin_lock_irqsave(&cpufreq_driver_lock, flags); if (driver->exit) driver->exit(policy); - spin_unlock_irqrestore(&cpufreq_driver_lock, flags); } return ret;
@@ -920,17 +918,14 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) init_completion(&policy->kobj_unregister); INIT_WORK(&policy->update, handle_update);
- spin_lock_irqsave(&cpufreq_driver_lock, flags); /* call driver. From then on the cpufreq must be able * to accept all calls to ->verify and ->setpolicy for this CPU */ ret = driver->init(policy); if (ret) { pr_debug("initialization failed\n"); - spin_unlock_irqrestore(&cpufreq_driver_lock, flags); goto err_set_policy_cpu; } - spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
/* related cpus should atleast have policy->cpus */ cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus); @@ -1100,10 +1095,8 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif wait_for_completion(cmp); pr_debug("wait complete\n");
- spin_lock_irqsave(&cpufreq_driver_lock, flags); if (driver->exit) driver->exit(data); - spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
free_cpumask_var(data->related_cpus); free_cpumask_var(data->cpus);
On Fri, Feb 08, 2013 at 10:39:13AM +0530, Viresh Kumar wrote:
On 8 February 2013 04:37, Rafael J. Wysocki rjw@sisk.pl wrote:
BTW, there still are locking problems in linux-next. Why do we need to take cpufreq_driver_lock() around driver->init() in cpufreq_add_dev(), in particular?
I thought a bit more and realized there is no such limitation on cpufreq_driver->ops about calling routines which can sleep. And thus we shoudln't have locks around any of these. I have got a patch for it, that i would fold-back into the original patch that introduced locking fixes (attached too for testing):
Tested this patch on top of linux-pm.git/bleeding-edge Now everything seems to be alright.
From: Viresh Kumar viresh.kumar@linaro.org Date: Fri, 8 Feb 2013 10:35:31 +0530 Subject: [PATCH] cpufreq: Remove unnecessary locking
I have placed some locks intentionally around calls to driver->ops (init/exit), which look to be wrong as these calls can call routines that potentially sleep.
Lets remove these locks.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/cpufreq/cpufreq.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 5d8a422..04aab05 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -795,10 +795,8 @@ static int cpufreq_add_dev_interface(unsigned int cpu,
if (ret) { pr_debug("setting policy failed\n");
spin_lock_irqsave(&cpufreq_driver_lock, flags);spin_unlock_irqrestore(&cpufreq_driver_lock, flags);@@ -920,17 +918,14 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) init_completion(&policy->kobj_unregister); INIT_WORK(&policy->update, handle_update);
spin_lock_irqsave(&cpufreq_driver_lock, flags); /* call driver. From then on the cpufreq must be able
- to accept all calls to ->verify and ->setpolicy for this CPU
*/ ret = driver->init(policy); if (ret) { pr_debug("initialization failed\n");
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);goto err_set_policy_cpu; }
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
/* related cpus should atleast have policy->cpus */ cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
@@ -1100,10 +1095,8 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif wait_for_completion(cmp); pr_debug("wait complete\n");
spin_lock_irqsave(&cpufreq_driver_lock, flags);if (driver->exit) driver->exit(data);
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);free_cpumask_var(data->related_cpus); free_cpumask_var(data->cpus);
Tested-by: Artem Savkov artem.savkov@gmail.com
On Thu, Feb 07, 2013 at 03:57:42PM +0530, Viresh Kumar wrote:
Hi Rafael,
This is another unplanned patchset for all the platforms that i broke. :)
Okay, there are two important fixes (1 & 4) and two general cleanups (2 & 3). I hope most of the issues would be resolved by these and we would be able to push clean cpufreq core into 3.9.
I have pushed them in my for-rafael branch at:
http://git.linaro.org/gitweb?p=people/vireshk/linux.git%3Ba=shortlog%3Bh=ref...
@Artem & Valdis: Please test them and reply with your Tested-by's (in case they work :) ).
Viresh Kumar (4): cpufreq: governors: Fix WARN_ON() for multi-policy platforms cpufreq: Remove unused HOTPLUG_CPU code cpufreq: Create a macro for unlock_policy_rwsem{read,write} cpufreq: Fix locking issues
drivers/cpufreq/cpufreq.c | 126 ++++++++++++++++++------------------- drivers/cpufreq/cpufreq_governor.c | 32 ++++++---- 2 files changed, 79 insertions(+), 79 deletions(-)
Tested out linux-pm.git/linux-next with this patches pulled. It seems that my systemd-sleep issue is fixed, however there is a new 'sleeping in invalid context' bug during boot:
[ 12.736484] BUG: sleeping function called from invalid context at mm/slub.c:925 [ 12.739727] in_atomic(): 1, irqs_disabled(): 1, pid: 1799, name: systemd-modules [ 12.742961] 2 locks held by systemd-modules/1799: [ 12.746153] #0: (subsys mutex#3){......}, at: [<c13f4056>] subsys_interface_register+0x36/0xb0 [ 12.749499] #1: (cpufreq_driver_lock){......}, at: [<c14ba53b>] cpufreq_add_dev+0x22b/0x3d0 [ 12.752865] Pid: 1799, comm: systemd-modules Not tainted 3.8.0-rc6+ #1 [ 12.756175] Call Trace: [ 12.759538] [<c1068150>] __might_sleep+0xe0/0x100 [ 12.762156] [<c112a481>] kmem_cache_alloc_trace+0xb1/0x150 [ 12.765432] [<f804e653>] ? acpi_cpufreq_cpu_init+0x73/0x5c0 [acpi_cpufreq] [ 12.768780] [<f804e653>] acpi_cpufreq_cpu_init+0x73/0x5c0 [acpi_cpufreq] [ 12.772161] [<c14ba53b>] ? cpufreq_add_dev+0x22b/0x3d0 [ 12.775549] [<c1695af7>] ? _raw_spin_lock_irqsave+0x77/0x90 [ 12.778932] [<c14ba53b>] ? cpufreq_add_dev+0x22b/0x3d0 [ 12.782307] [<c14ba548>] cpufreq_add_dev+0x238/0x3d0 [ 12.785652] [<c13f4095>] subsys_interface_register+0x75/0xb0 [ 12.788989] [<f804ec20>] ? do_drv_write+0x80/0x80 [acpi_cpufreq] [ 12.792325] [<c14b986b>] cpufreq_register_driver+0x7b/0x150 [ 12.795657] [<f8075000>] ? 0xf8074fff [ 12.798971] [<f80750ae>] acpi_cpufreq_init+0xae/0x1b3 [acpi_cpufreq] [ 12.802346] [<c1001222>] do_one_initcall+0x112/0x160 [ 12.805723] [<c106145a>] ? __blocking_notifier_call_chain+0x4a/0x80 [ 12.809123] [<c10a048e>] load_module+0xd7e/0x1460 [ 12.812515] [<c1696d82>] ? error_code+0x5a/0x60 [ 12.815891] [<c10a0be8>] sys_init_module+0x78/0xb0 [ 12.819249] [<c169d67a>] sysenter_do_call+0x12/0x2d [ 12.822924] ------------[ cut here ]------------ [ 12.826275] WARNING: at kernel/smp.c:327 smp_call_function_single+0x104/0x130() [ 12.829668] Hardware name: 0578A21 [ 12.833020] Modules linked in: acpi_cpufreq(+) mperf thinkpad_acpi [ 12.836456] Pid: 1799, comm: systemd-modules Not tainted 3.8.0-rc6+ #1 [ 12.839891] Call Trace: [ 12.843268] [<c1036a82>] warn_slowpath_common+0x72/0xa0 [ 12.846617] [<c109b154>] ? smp_call_function_single+0x104/0x130 [ 12.849921] [<c109b154>] ? smp_call_function_single+0x104/0x130 [ 12.853149] [<f804eea0>] ? acpi_cpufreq_target+0x280/0x280 [acpi_cpufreq] [ 12.856361] [<c1036ad2>] warn_slowpath_null+0x22/0x30 [ 12.859518] [<c109b154>] smp_call_function_single+0x104/0x130 [ 12.862691] [<c109b5b4>] smp_call_function_any+0x44/0xb0 [ 12.865788] [<f804eea0>] ? acpi_cpufreq_target+0x280/0x280 [acpi_cpufreq] [ 12.868893] [<f804e13e>] get_cur_val+0x7e/0x100 [acpi_cpufreq] [ 12.871957] [<f804e5bb>] get_cur_freq_on_cpu+0x4b/0x70 [acpi_cpufreq] [ 12.874987] [<f804e9b8>] acpi_cpufreq_cpu_init+0x3d8/0x5c0 [acpi_cpufreq] [ 12.877998] [<c14ba53b>] ? cpufreq_add_dev+0x22b/0x3d0 [ 12.880982] [<c14ba548>] cpufreq_add_dev+0x238/0x3d0 [ 12.883931] [<c13f4095>] subsys_interface_register+0x75/0xb0 [ 12.886840] [<f804ec20>] ? do_drv_write+0x80/0x80 [acpi_cpufreq] [ 12.889717] [<c14b986b>] cpufreq_register_driver+0x7b/0x150 [ 12.892550] [<f8075000>] ? 0xf8074fff [ 12.895339] [<f80750ae>] acpi_cpufreq_init+0xae/0x1b3 [acpi_cpufreq] [ 12.898169] [<c1001222>] do_one_initcall+0x112/0x160 [ 12.900988] [<c106145a>] ? __blocking_notifier_call_chain+0x4a/0x80 [ 12.903820] [<c10a048e>] load_module+0xd7e/0x1460 [ 12.906631] [<c1696d82>] ? error_code+0x5a/0x60 [ 12.909418] [<c10a0be8>] sys_init_module+0x78/0xb0 [ 12.912193] [<c169d67a>] sysenter_do_call+0x12/0x2d [ 12.914956] ---[ end trace e15032846f0195a0 ]---
On 8 February 2013 01:09, Artem Savkov artem.savkov@gmail.com wrote:
Tested out linux-pm.git/linux-next with this patches pulled. It seems that my systemd-sleep issue is fixed, however there is a new 'sleeping in invalid context' bug during boot:
[ 12.736484] BUG: sleeping function called from invalid context at mm/slub.c:925 [ 12.739727] in_atomic(): 1, irqs_disabled(): 1, pid: 1799, name: systemd-modules [ 12.742961] 2 locks held by systemd-modules/1799: [ 12.746153] #0: (subsys mutex#3){......}, at: [<c13f4056>] subsys_interface_register+0x36/0xb0 [ 12.749499] #1: (cpufreq_driver_lock){......}, at: [<c14ba53b>] cpufreq_add_dev+0x22b/0x3d0 [ 12.752865] Pid: 1799, comm: systemd-modules Not tainted 3.8.0-rc6+ #1 [ 12.756175] Call Trace: [ 12.759538] [<c1068150>] __might_sleep+0xe0/0x100 [ 12.762156] [<c112a481>] kmem_cache_alloc_trace+0xb1/0x150 [ 12.765432] [<f804e653>] ? acpi_cpufreq_cpu_init+0x73/0x5c0 [acpi_cpufreq] [ 12.768780] [<f804e653>] acpi_cpufreq_cpu_init+0x73/0x5c0 [acpi_cpufreq] [ 12.772161] [<c14ba53b>] ? cpufreq_add_dev+0x22b/0x3d0 [ 12.775549] [<c1695af7>] ? _raw_spin_lock_irqsave+0x77/0x90 [ 12.778932] [<c14ba53b>] ? cpufreq_add_dev+0x22b/0x3d0 [ 12.782307] [<c14ba548>] cpufreq_add_dev+0x238/0x3d0
Can you please try out this:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 819aa33..a77d0bc 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -919,17 +919,14 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) init_completion(&policy->kobj_unregister); INIT_WORK(&policy->update, handle_update);
- spin_lock_irqsave(&cpufreq_driver_lock, flags); /* call driver. From then on the cpufreq must be able * to accept all calls to ->verify and ->setpolicy for this CPU */ ret = driver->init(policy); if (ret) { pr_debug("initialization failed\n"); - spin_unlock_irqrestore(&cpufreq_driver_lock, flags); goto err_set_policy_cpu; } - spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
/* related cpus should atleast have policy->cpus */ cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
On Thursday, February 07, 2013 03:57:42 PM Viresh Kumar wrote:
Hi Rafael,
This is another unplanned patchset for all the platforms that i broke. :)
Okay, there are two important fixes (1 & 4) and two general cleanups (2 & 3). I hope most of the issues would be resolved by these and we would be able to push clean cpufreq core into 3.9.
I have pushed them in my for-rafael branch at:
http://git.linaro.org/gitweb?p=people/vireshk/linux.git%3Ba=shortlog%3Bh=ref...
@Artem & Valdis: Please test them and reply with your Tested-by's (in case they work :) ).
Viresh Kumar (4): cpufreq: governors: Fix WARN_ON() for multi-policy platforms cpufreq: Remove unused HOTPLUG_CPU code cpufreq: Create a macro for unlock_policy_rwsem{read,write} cpufreq: Fix locking issues
drivers/cpufreq/cpufreq.c | 126 ++++++++++++++++++------------------- drivers/cpufreq/cpufreq_governor.c | 32 ++++++---- 2 files changed, 79 insertions(+), 79 deletions(-)
I should have done that before, sorry about it.
Can you please rework this series on top of linux-pm.git/pm-cpufreq and try to avoid introducing new issues this time?
If this works, we'll rebase all of the other new material on top of it, if possible.
Thanks, Rafael
On Friday, February 08, 2013 12:33:14 AM Rafael J. Wysocki wrote:
On Thursday, February 07, 2013 03:57:42 PM Viresh Kumar wrote:
Hi Rafael,
This is another unplanned patchset for all the platforms that i broke. :)
Okay, there are two important fixes (1 & 4) and two general cleanups (2 & 3). I hope most of the issues would be resolved by these and we would be able to push clean cpufreq core into 3.9.
I have pushed them in my for-rafael branch at:
http://git.linaro.org/gitweb?p=people/vireshk/linux.git%3Ba=shortlog%3Bh=ref...
@Artem & Valdis: Please test them and reply with your Tested-by's (in case they work :) ).
Viresh Kumar (4): cpufreq: governors: Fix WARN_ON() for multi-policy platforms cpufreq: Remove unused HOTPLUG_CPU code cpufreq: Create a macro for unlock_policy_rwsem{read,write} cpufreq: Fix locking issues
drivers/cpufreq/cpufreq.c | 126 ++++++++++++++++++------------------- drivers/cpufreq/cpufreq_governor.c | 32 ++++++---- 2 files changed, 79 insertions(+), 79 deletions(-)
I should have done that before, sorry about it.
Can you please rework this series on top of linux-pm.git/pm-cpufreq and try to avoid introducing new issues this time?
If this works, we'll rebase all of the other new material on top of it, if possible.
I've dropped the pm-cpufreq-next branch from linux-next now, BTW.
Thanks, Rafael
On 8 February 2013 05:03, Rafael J. Wysocki rjw@sisk.pl wrote:
I should have done that before, sorry about it.
Can you please rework this series on top of linux-pm.git/pm-cpufreq and try to avoid introducing new issues this time?
Even i want to do that, but when i fetch your repo i don't see all applied patches in this branch.
On Friday, February 08, 2013 08:20:55 AM Viresh Kumar wrote:
On 8 February 2013 05:03, Rafael J. Wysocki rjw@sisk.pl wrote:
I should have done that before, sorry about it.
Can you please rework this series on top of linux-pm.git/pm-cpufreq and try to avoid introducing new issues this time?
Even i want to do that, but when i fetch your repo i don't see all applied patches in this branch.
The top-most commit in that branch is
commit 73bf0fc2b03d1f4fdada0ec430dc20bfb089cfd5 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Feb 5 22:21:14 2013 +0100
cpufreq: Don't remove sysfs link for policy->cpu
because that's when the locking problems were first reported and I stopped putting new commits into that branch. And since the locking problems were introduced by b8eed8a "cpufreq: Simplify __cpufreq_remove_dev()" I want them to be fixed on top of pm-cpufreq rather than on top of more new stuff that very well may introduce *more* problems.
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
Moreover, I'd very much prefer it if you fixed the problems introduced by b8eed8a "cpufreq: Simplify __cpufreq_remove_dev()" separately and *then* any other locking problems you're seeing in the code, although people are not reporting them.
You seem to have a clear picture of how the code should work now, so that won't be a big trouble I guess.
Thanks, Rafael
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Moreover, I'd very much prefer it if you fixed the problems introduced by b8eed8a "cpufreq: Simplify __cpufreq_remove_dev()" separately and *then* any other locking problems you're seeing in the code, although people are not reporting them.
You seem to have a clear picture of how the code should work now, so that won't be a big trouble I guess.
For me all problems are fixed in the current code. :)
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
Moreover, I'd very much prefer it if you fixed the problems introduced by b8eed8a "cpufreq: Simplify __cpufreq_remove_dev()" separately and *then* any other locking problems you're seeing in the code, although people are not reporting them.
You seem to have a clear picture of how the code should work now, so that won't be a big trouble I guess.
For me all problems are fixed in the current code. :)
Sounds great. :-)
Thanks, Rafael
On Friday, February 08, 2013 09:02:37 PM Rafael J. Wysocki wrote:
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
OK, applied to bleeding-edge. Hopefully it will be build-tested over the weekend and I can move it to linux-next.
I dropped the rwlock/RCU patches from Nathan, though, because I had some doubts about the correctness of the RCU one and the rwlock one alone would conflict with your further changes.
Thanks, Rafael
On 02/08/2013 03:56 PM, Rafael J. Wysocki wrote:
On Friday, February 08, 2013 09:02:37 PM Rafael J. Wysocki wrote:
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
OK, applied to bleeding-edge. Hopefully it will be build-tested over the weekend and I can move it to linux-next.
I dropped the rwlock/RCU patches from Nathan, though, because I had some doubts about the correctness of the RCU one and the rwlock one alone would conflict with your further changes.
One piece of fallout from dropping Nathan patches I had rebased mine on top of them.
This fixes the breakage do you want me to spin my patches or send this separately?:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 0ebdf8c..a008b8e 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1024,7 +1024,7 @@ static int __cpufreq_remove_dev(struct device *dev, struct __cpufreq_governor(data, CPUFREQ_GOV_STOP);
#ifdef CONFIG_HOTPLUG_CPU - if (!driver->setpolicy) + if (!cpufreq_driver->setpolicy) strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name, CPUFREQ_NAME_LEN); #endif @@ -1771,7 +1771,7 @@ int cpufreq_update_policy(unsigned int cpu) pr_debug("Driver did not initialize current freq"); data->cur = policy.cur; } else { - if (data->cur != policy.cur && driver->target) + if (data->cur != policy.cur && cpufreq_driver->target) cpufreq_out_of_sync(cpu, data->cur, policy.cur); }
On Friday, February 08, 2013 04:08:49 PM Dirk Brandewie wrote:
On 02/08/2013 03:56 PM, Rafael J. Wysocki wrote:
On Friday, February 08, 2013 09:02:37 PM Rafael J. Wysocki wrote:
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
OK, applied to bleeding-edge. Hopefully it will be build-tested over the weekend and I can move it to linux-next.
I dropped the rwlock/RCU patches from Nathan, though, because I had some doubts about the correctness of the RCU one and the rwlock one alone would conflict with your further changes.
One piece of fallout from dropping Nathan patches I had rebased mine on top of them.
This fixes the breakage do you want me to spin my patches or send this separately?:
No need to, I'll try to fix that in my tree.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 0ebdf8c..a008b8e 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1024,7 +1024,7 @@ static int __cpufreq_remove_dev(struct device *dev, struct __cpufreq_governor(data, CPUFREQ_GOV_STOP);
#ifdef CONFIG_HOTPLUG_CPU
if (!driver->setpolicy)
if (!cpufreq_driver->setpolicy) strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name, CPUFREQ_NAME_LEN);@@ -1771,7 +1771,7 @@ int cpufreq_update_policy(unsigned int cpu) pr_debug("Driver did not initialize current freq"); data->cur = policy.cur; } else {
if (data->cur != policy.cur && driver->target)
if (data->cur != policy.cur && cpufreq_driver->target) cpufreq_out_of_sync(cpu, data->cur, policy.cur); }
Thanks, Rafael
On 9 February 2013 05:38, Dirk Brandewie dirk.brandewie@gmail.com wrote:
On 02/08/2013 03:56 PM, Rafael J. Wysocki wrote:
On Friday, February 08, 2013 09:02:37 PM Rafael J. Wysocki wrote:
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
OK, applied to bleeding-edge. Hopefully it will be build-tested over the weekend and I can move it to linux-next.
I dropped the rwlock/RCU patches from Nathan, though, because I had some doubts about the correctness of the RCU one and the rwlock one alone would conflict with your further changes.
As soon as i read Rafael's mail, i realized Dirk's patch might be broken and immediately i saw your mail :)
@Rafael: Sorry for not reviewing Nathan's patch well. I didn't knew much about RCU then. I am going through its lwn articles now ;)
On Saturday, February 09, 2013 07:40:26 AM Viresh Kumar wrote:
On 9 February 2013 05:38, Dirk Brandewie dirk.brandewie@gmail.com wrote:
On 02/08/2013 03:56 PM, Rafael J. Wysocki wrote:
On Friday, February 08, 2013 09:02:37 PM Rafael J. Wysocki wrote:
On Friday, February 08, 2013 08:06:52 PM Viresh Kumar wrote:
On 8 February 2013 18:02, Rafael J. Wysocki rjw@sisk.pl wrote:
So as I said, please rework the fixes on top of linux-pm.git/pm-cpufreq.
I already did. Please check for-rafael branch
Cool. This is the one I'm supposed to apply, then?
OK, applied to bleeding-edge. Hopefully it will be build-tested over the weekend and I can move it to linux-next.
I dropped the rwlock/RCU patches from Nathan, though, because I had some doubts about the correctness of the RCU one and the rwlock one alone would conflict with your further changes.
As soon as i read Rafael's mail, i realized Dirk's patch might be broken and immediately i saw your mail :)
@Rafael: Sorry for not reviewing Nathan's patch well. I didn't knew much about RCU then. I am going through its lwn articles now ;)
No biggie, I overlooked that myself first time.
Thanks, Rafael
On 8 February 2013 05:03, Rafael J. Wysocki rjw@sisk.pl wrote:
I should have done that before, sorry about it.
np
Can you please rework this series on top of linux-pm.git/pm-cpufreq and try to avoid introducing new issues this time?
Sorry for this. I didn't got any such issues on my system and i tried to think as widely as possible. But still just a human with some mistakes :)
If this works, we'll rebase all of the other new material on top of it, if possible.
To make your life a bit easy, i have got all cpufreq patches, that you & me have got for 3.9, rebased over pm-cpufreq and these are:
f3843e0 cpufreq: exynos: simplify .init() for setting policy->cpus 7ea6658 cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs 6002fd0 cpufreq/x86: Add P-state driver for sandy bridge. bcfe254 cpufreq_stats: do not remove sysfs files if frequency table is not present 6c82b96 cpufreq: Do not track governor name for scaling drivers with internal governors. 2a6df07 cpufreq: Only call cpufreq_out_of_sync() for driver that implement cpufreq_driver.target() 0893112 cpufreq: Retrieve current frequency from scaling drivers with internal governors e034e73 cpufreq: Fix locking issues 003da79 cpufreq: Create a macro for unlock_policy_rwsem{read,write} 0092c75 cpufreq: Remove unused HOTPLUG_CPU code 34d5833 cpufreq: governors: Fix WARN_ON() for multi-policy platforms e1ee7c8 cpufreq: Convert the cpufreq_driver_lock to use RCU e076b60 cpufreq: Convert the cpufreq_driver_lock to a rwlock 6d919f9 cpufreq: ondemand: Replace down_differential tuner with adj_up_threshold 80dd878 cpufreq / stats: Get rid of CPUFREQ_STATDEVICE_ATTR a7e183d cpufreq: Don't check cpu_online(policy->cpu) 9db0116 cpufreq: add imx6q-cpufreq driver
I have pushed them in for-rafael branch in my repo. Look carefully at the first two patches, they were not present in your latest repo.
This was the exynos patch i was talking about:
f3843e0 cpufreq: exynos: simplify .init() for setting policy->cpus
I don't know if you dropped this one or what ?
7ea6658 cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
-
Artem Savkov -
Dirk Brandewie -
Rafael J. Wysocki -
Viresh Kumar