* [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:09 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem Viresh Kumar
` (9 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
lock_policy_rwsem_{read|write}() currently has return type of int but it always
return zero and hence its return type must be void instead. This patch makes its
return type void and fixes all users of it as well.
Reported-by: Jon Medhurst<tixy@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 04548f7..eb993d9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -67,13 +67,11 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
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 void lock_policy_rwsem_##mode(int cpu) \
{ \
struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
BUG_ON(!policy); \
down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
- \
- return 0; \
}
lock_policy_rwsem(read, cpu);
@@ -653,13 +651,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
{
struct cpufreq_policy *policy = to_policy(kobj);
struct freq_attr *fattr = to_attr(attr);
- ssize_t ret = -EINVAL;
+ ssize_t ret;
if (!down_read_trylock(&cpufreq_rwsem))
- goto exit;
+ return -EINVAL;
- if (lock_policy_rwsem_read(policy->cpu) < 0)
- goto up_read;
+ lock_policy_rwsem_read(policy->cpu);
if (fattr->show)
ret = fattr->show(policy, buf);
@@ -667,10 +664,8 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
ret = -EIO;
unlock_policy_rwsem_read(policy->cpu);
-
-up_read:
up_read(&cpufreq_rwsem);
-exit:
+
return ret;
}
@@ -689,8 +684,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
if (!down_read_trylock(&cpufreq_rwsem))
goto unlock;
- if (lock_policy_rwsem_write(policy->cpu) < 0)
- goto up_read;
+ lock_policy_rwsem_write(policy->cpu);
if (fattr->store)
ret = fattr->store(policy, buf, count);
@@ -699,7 +693,6 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
unlock_policy_rwsem_write(policy->cpu);
-up_read:
up_read(&cpufreq_rwsem);
unlock:
put_online_cpus();
@@ -1147,7 +1140,7 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
if (ret) {
pr_err("%s: Failed to move kobj: %d", __func__, ret);
- WARN_ON(lock_policy_rwsem_write(old_cpu));
+ lock_policy_rwsem_write(old_cpu);
cpumask_set_cpu(old_cpu, policy->cpus);
unlock_policy_rwsem_write(old_cpu);
@@ -1243,7 +1236,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
return -EINVAL;
}
- WARN_ON(lock_policy_rwsem_write(cpu));
+ lock_policy_rwsem_write(cpu);
cpus = cpumask_weight(policy->cpus);
if (cpus > 1)
@@ -1466,14 +1459,11 @@ unsigned int cpufreq_get(unsigned int cpu)
if (!down_read_trylock(&cpufreq_rwsem))
return 0;
- if (unlikely(lock_policy_rwsem_read(cpu)))
- goto out_policy;
+ lock_policy_rwsem_read(cpu);
ret_freq = __cpufreq_get(cpu);
unlock_policy_rwsem_read(cpu);
-
-out_policy:
up_read(&cpufreq_rwsem);
return ret_freq;
@@ -1697,14 +1687,12 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
{
int ret = -EINVAL;
- if (unlikely(lock_policy_rwsem_write(policy->cpu)))
- goto fail;
+ lock_policy_rwsem_write(policy->cpu);
ret = __cpufreq_driver_target(policy, target_freq, relation);
unlock_policy_rwsem_write(policy->cpu);
-fail:
return ret;
}
EXPORT_SYMBOL_GPL(cpufreq_driver_target);
@@ -1995,10 +1983,7 @@ int cpufreq_update_policy(unsigned int cpu)
goto no_policy;
}
- if (unlikely(lock_policy_rwsem_write(cpu))) {
- ret = -EINVAL;
- goto fail;
- }
+ lock_policy_rwsem_write(cpu);
pr_debug("updating policy for CPU %u\n", cpu);
memcpy(&new_policy, policy, sizeof(*policy));
@@ -2027,7 +2012,6 @@ int cpufreq_update_policy(unsigned int cpu)
unlock_policy_rwsem_write(cpu);
-fail:
cpufreq_cpu_put(policy);
no_policy:
return ret;
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void
2013-10-02 8:43 ` [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void Viresh Kumar
@ 2013-10-03 7:09 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:09 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> lock_policy_rwsem_{read|write}() currently has return type of int but it always
> return zero and hence its return type must be void instead. This patch makes its
> return type void and fixes all users of it as well.
>
> Reported-by: Jon Medhurst<tixy@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/cpufreq.c | 38 +++++++++++---------------------------
> 1 file changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 04548f7..eb993d9 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -67,13 +67,11 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
> 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 void lock_policy_rwsem_##mode(int cpu) \
> { \
> struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> BUG_ON(!policy); \
> down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> - \
> - return 0; \
> }
>
> lock_policy_rwsem(read, cpu);
> @@ -653,13 +651,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> {
> struct cpufreq_policy *policy = to_policy(kobj);
> struct freq_attr *fattr = to_attr(attr);
> - ssize_t ret = -EINVAL;
> + ssize_t ret;
>
> if (!down_read_trylock(&cpufreq_rwsem))
> - goto exit;
> + return -EINVAL;
>
> - if (lock_policy_rwsem_read(policy->cpu) < 0)
> - goto up_read;
> + lock_policy_rwsem_read(policy->cpu);
>
> if (fattr->show)
> ret = fattr->show(policy, buf);
> @@ -667,10 +664,8 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> ret = -EIO;
>
> unlock_policy_rwsem_read(policy->cpu);
> -
> -up_read:
> up_read(&cpufreq_rwsem);
> -exit:
> +
> return ret;
> }
>
> @@ -689,8 +684,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
> if (!down_read_trylock(&cpufreq_rwsem))
> goto unlock;
>
> - if (lock_policy_rwsem_write(policy->cpu) < 0)
> - goto up_read;
> + lock_policy_rwsem_write(policy->cpu);
>
> if (fattr->store)
> ret = fattr->store(policy, buf, count);
> @@ -699,7 +693,6 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
>
> unlock_policy_rwsem_write(policy->cpu);
>
> -up_read:
> up_read(&cpufreq_rwsem);
> unlock:
> put_online_cpus();
> @@ -1147,7 +1140,7 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
> if (ret) {
> pr_err("%s: Failed to move kobj: %d", __func__, ret);
>
> - WARN_ON(lock_policy_rwsem_write(old_cpu));
> + lock_policy_rwsem_write(old_cpu);
> cpumask_set_cpu(old_cpu, policy->cpus);
> unlock_policy_rwsem_write(old_cpu);
>
> @@ -1243,7 +1236,7 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> return -EINVAL;
> }
>
> - WARN_ON(lock_policy_rwsem_write(cpu));
> + lock_policy_rwsem_write(cpu);
> cpus = cpumask_weight(policy->cpus);
>
> if (cpus > 1)
> @@ -1466,14 +1459,11 @@ unsigned int cpufreq_get(unsigned int cpu)
> if (!down_read_trylock(&cpufreq_rwsem))
> return 0;
>
> - if (unlikely(lock_policy_rwsem_read(cpu)))
> - goto out_policy;
> + lock_policy_rwsem_read(cpu);
>
> ret_freq = __cpufreq_get(cpu);
>
> unlock_policy_rwsem_read(cpu);
> -
> -out_policy:
> up_read(&cpufreq_rwsem);
>
> return ret_freq;
> @@ -1697,14 +1687,12 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
> {
> int ret = -EINVAL;
>
> - if (unlikely(lock_policy_rwsem_write(policy->cpu)))
> - goto fail;
> + lock_policy_rwsem_write(policy->cpu);
>
> ret = __cpufreq_driver_target(policy, target_freq, relation);
>
> unlock_policy_rwsem_write(policy->cpu);
>
> -fail:
> return ret;
> }
> EXPORT_SYMBOL_GPL(cpufreq_driver_target);
> @@ -1995,10 +1983,7 @@ int cpufreq_update_policy(unsigned int cpu)
> goto no_policy;
> }
>
> - if (unlikely(lock_policy_rwsem_write(cpu))) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + lock_policy_rwsem_write(cpu);
>
> pr_debug("updating policy for CPU %u\n", cpu);
> memcpy(&new_policy, policy, sizeof(*policy));
> @@ -2027,7 +2012,6 @@ int cpufreq_update_policy(unsigned int cpu)
>
> unlock_policy_rwsem_write(cpu);
>
> -fail:
> cpufreq_cpu_put(policy);
> no_policy:
> return ret;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:23 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev() Viresh Kumar
` (8 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
We have per-cpu cpu_policy_rwsem for cpufreq core, but we never use all of them.
We always use rwsem of policy->cpu and so we can actually make this rwsem per
policy instead.
This patch does this change. With this change other tricky situations are also
avoided now, like which lock to take while we are changing policy->cpu, etc.
Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 110 +++++++++++++---------------------------------
include/linux/cpufreq.h | 14 ++++++
2 files changed, 45 insertions(+), 79 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index eb993d9..ae866b1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -48,47 +48,6 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
#endif
/*
- * cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
- * all cpufreq/hotplug/workqueue/etc related lock issues.
- *
- * The rules for this semaphore:
- * - Any routine that wants to read from the policy structure will
- * do a down_read on this semaphore.
- * - Any routine that will write to the policy structure and/or may take away
- * the policy altogether (eg. CPU hotplug), will hold this lock in write
- * mode before doing so.
- *
- * Additional rules:
- * - 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
- * __cpufreq_governor(data, CPUFREQ_GOV_STOP);
- */
-static DEFINE_PER_CPU(struct rw_semaphore, cpu_policy_rwsem);
-
-#define lock_policy_rwsem(mode, cpu) \
-static void lock_policy_rwsem_##mode(int cpu) \
-{ \
- struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
- BUG_ON(!policy); \
- down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
-}
-
-lock_policy_rwsem(read, cpu);
-lock_policy_rwsem(write, cpu);
-
-#define unlock_policy_rwsem(mode, cpu) \
-static void unlock_policy_rwsem_##mode(int cpu) \
-{ \
- struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
- BUG_ON(!policy); \
- up_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
-}
-
-unlock_policy_rwsem(read, cpu);
-unlock_policy_rwsem(write, cpu);
-
-/*
* rwsem to guarantee that cpufreq driver module doesn't unload during critical
* sections
*/
@@ -656,14 +615,14 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
if (!down_read_trylock(&cpufreq_rwsem))
return -EINVAL;
- lock_policy_rwsem_read(policy->cpu);
+ down_read(&policy->rwsem);
if (fattr->show)
ret = fattr->show(policy, buf);
else
ret = -EIO;
- unlock_policy_rwsem_read(policy->cpu);
+ up_read(&policy->rwsem);
up_read(&cpufreq_rwsem);
return ret;
@@ -684,14 +643,14 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
if (!down_read_trylock(&cpufreq_rwsem))
goto unlock;
- lock_policy_rwsem_write(policy->cpu);
+ down_write(&policy->rwsem);
if (fattr->store)
ret = fattr->store(policy, buf, count);
else
ret = -EIO;
- unlock_policy_rwsem_write(policy->cpu);
+ up_write(&policy->rwsem);
up_read(&cpufreq_rwsem);
unlock:
@@ -868,7 +827,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
}
}
- lock_policy_rwsem_write(policy->cpu);
+ down_write(&policy->rwsem);
write_lock_irqsave(&cpufreq_driver_lock, flags);
@@ -876,7 +835,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
per_cpu(cpufreq_cpu_data, cpu) = policy;
write_unlock_irqrestore(&cpufreq_driver_lock, flags);
- unlock_policy_rwsem_write(policy->cpu);
+ up_write(&policy->rwsem);
if (has_target) {
if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
@@ -923,6 +882,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void)
goto err_free_cpumask;
INIT_LIST_HEAD(&policy->policy_list);
+ init_rwsem(&policy->rwsem);
+
return policy;
err_free_cpumask:
@@ -945,19 +906,12 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
if (cpu == policy->cpu)
return;
- /*
- * Take direct locks as lock_policy_rwsem_write wouldn't work here.
- * Also lock for last cpu is enough here as contention will happen only
- * after policy->cpu is changed and after it is changed, other threads
- * will try to acquire lock for new cpu. And policy is already updated
- * by then.
- */
- down_write(&per_cpu(cpu_policy_rwsem, policy->cpu));
+ down_write(&policy->rwsem);
policy->last_cpu = policy->cpu;
policy->cpu = cpu;
- up_write(&per_cpu(cpu_policy_rwsem, policy->last_cpu));
+ up_write(&policy->rwsem);
#ifdef CONFIG_CPU_FREQ_TABLE
cpufreq_frequency_table_update_policy_cpu(policy);
@@ -1140,9 +1094,9 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
if (ret) {
pr_err("%s: Failed to move kobj: %d", __func__, ret);
- lock_policy_rwsem_write(old_cpu);
+ down_write(&policy->rwsem);
cpumask_set_cpu(old_cpu, policy->cpus);
- unlock_policy_rwsem_write(old_cpu);
+ up_write(&policy->rwsem);
ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
"cpufreq");
@@ -1193,9 +1147,9 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
policy->governor->name, CPUFREQ_NAME_LEN);
#endif
- lock_policy_rwsem_read(cpu);
+ down_read(&policy->rwsem);
cpus = cpumask_weight(policy->cpus);
- unlock_policy_rwsem_read(cpu);
+ up_read(&policy->rwsem);
if (cpu != policy->cpu) {
if (!frozen)
@@ -1236,12 +1190,12 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
return -EINVAL;
}
- lock_policy_rwsem_write(cpu);
+ down_write(&policy->rwsem);
cpus = cpumask_weight(policy->cpus);
if (cpus > 1)
cpumask_clear_cpu(cpu, policy->cpus);
- unlock_policy_rwsem_write(cpu);
+ up_write(&policy->rwsem);
/* If cpu is last user of policy, free policy */
if (cpus == 1) {
@@ -1256,10 +1210,10 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
}
if (!frozen) {
- lock_policy_rwsem_read(cpu);
+ down_read(&policy->rwsem);
kobj = &policy->kobj;
cmp = &policy->kobj_unregister;
- unlock_policy_rwsem_read(cpu);
+ up_read(&policy->rwsem);
kobject_put(kobj);
/*
@@ -1451,19 +1405,22 @@ static unsigned int __cpufreq_get(unsigned int cpu)
*/
unsigned int cpufreq_get(unsigned int cpu)
{
+ struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
unsigned int ret_freq = 0;
if (cpufreq_disabled() || !cpufreq_driver)
return -ENOENT;
+ BUG_ON(!policy);
+
if (!down_read_trylock(&cpufreq_rwsem))
return 0;
- lock_policy_rwsem_read(cpu);
+ down_read(&policy->rwsem);
ret_freq = __cpufreq_get(cpu);
- unlock_policy_rwsem_read(cpu);
+ up_read(&policy->rwsem);
up_read(&cpufreq_rwsem);
return ret_freq;
@@ -1687,11 +1644,11 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
{
int ret = -EINVAL;
- lock_policy_rwsem_write(policy->cpu);
+ down_write(&policy->rwsem);
ret = __cpufreq_driver_target(policy, target_freq, relation);
- unlock_policy_rwsem_write(policy->cpu);
+ up_write(&policy->rwsem);
return ret;
}
@@ -1922,10 +1879,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
/* end old governor */
if (policy->governor) {
__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
- unlock_policy_rwsem_write(new_policy->cpu);
+ up_write(&new_policy->rwsem);
__cpufreq_governor(policy,
CPUFREQ_GOV_POLICY_EXIT);
- lock_policy_rwsem_write(new_policy->cpu);
+ down_write(&new_policy->rwsem);
}
/* start new governor */
@@ -1934,10 +1891,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
if (!__cpufreq_governor(policy, CPUFREQ_GOV_START)) {
failed = 0;
} else {
- unlock_policy_rwsem_write(new_policy->cpu);
+ up_write(&new_policy->rwsem);
__cpufreq_governor(policy,
CPUFREQ_GOV_POLICY_EXIT);
- lock_policy_rwsem_write(new_policy->cpu);
+ down_write(&new_policy->rwsem);
}
}
@@ -1983,7 +1940,7 @@ int cpufreq_update_policy(unsigned int cpu)
goto no_policy;
}
- lock_policy_rwsem_write(cpu);
+ down_write(&policy->rwsem);
pr_debug("updating policy for CPU %u\n", cpu);
memcpy(&new_policy, policy, sizeof(*policy));
@@ -2010,7 +1967,7 @@ int cpufreq_update_policy(unsigned int cpu)
ret = __cpufreq_set_policy(policy, &new_policy);
- unlock_policy_rwsem_write(cpu);
+ up_write(&policy->rwsem);
cpufreq_cpu_put(policy);
no_policy:
@@ -2167,14 +2124,9 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
static int __init cpufreq_core_init(void)
{
- int cpu;
-
if (cpufreq_disabled())
return -ENODEV;
- for_each_possible_cpu(cpu)
- init_rwsem(&per_cpu(cpu_policy_rwsem, cpu));
-
cpufreq_global_kobject = kobject_create();
BUG_ON(!cpufreq_global_kobject);
register_syscore_ops(&cpufreq_syscore_ops);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6b199ed..a72bac2 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -85,6 +85,20 @@ struct cpufreq_policy {
struct list_head policy_list;
struct kobject kobj;
struct completion kobj_unregister;
+
+ /*
+ * The rules for this semaphore:
+ * - Any routine that wants to read from the policy structure will
+ * do a down_read on this semaphore.
+ * - Any routine that will write to the policy structure and/or may take away
+ * the policy altogether (eg. CPU hotplug), will hold this lock in write
+ * mode before doing so.
+ *
+ * Additional rules:
+ * - Lock should not be held across
+ * __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
+ */
+ struct rw_semaphore rwsem;
};
/* Only for ACPI */
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem
2013-10-02 8:43 ` [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem Viresh Kumar
@ 2013-10-03 7:23 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:23 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> We have per-cpu cpu_policy_rwsem for cpufreq core, but we never use all of them.
> We always use rwsem of policy->cpu and so we can actually make this rwsem per
> policy instead.
>
> This patch does this change. With this change other tricky situations are also
> avoided now, like which lock to take while we are changing policy->cpu, etc.
>
> Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
This is a very nice improvement!
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/cpufreq.c | 110 +++++++++++++---------------------------------
> include/linux/cpufreq.h | 14 ++++++
> 2 files changed, 45 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index eb993d9..ae866b1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -48,47 +48,6 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
> #endif
>
> /*
> - * cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
> - * all cpufreq/hotplug/workqueue/etc related lock issues.
> - *
> - * The rules for this semaphore:
> - * - Any routine that wants to read from the policy structure will
> - * do a down_read on this semaphore.
> - * - Any routine that will write to the policy structure and/or may take away
> - * the policy altogether (eg. CPU hotplug), will hold this lock in write
> - * mode before doing so.
> - *
> - * Additional rules:
> - * - 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
> - * __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> - */
> -static DEFINE_PER_CPU(struct rw_semaphore, cpu_policy_rwsem);
> -
> -#define lock_policy_rwsem(mode, cpu) \
> -static void lock_policy_rwsem_##mode(int cpu) \
> -{ \
> - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> - BUG_ON(!policy); \
> - down_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> -}
> -
> -lock_policy_rwsem(read, cpu);
> -lock_policy_rwsem(write, cpu);
> -
> -#define unlock_policy_rwsem(mode, cpu) \
> -static void unlock_policy_rwsem_##mode(int cpu) \
> -{ \
> - struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu); \
> - BUG_ON(!policy); \
> - up_##mode(&per_cpu(cpu_policy_rwsem, policy->cpu)); \
> -}
> -
> -unlock_policy_rwsem(read, cpu);
> -unlock_policy_rwsem(write, cpu);
> -
> -/*
> * rwsem to guarantee that cpufreq driver module doesn't unload during critical
> * sections
> */
> @@ -656,14 +615,14 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> if (!down_read_trylock(&cpufreq_rwsem))
> return -EINVAL;
>
> - lock_policy_rwsem_read(policy->cpu);
> + down_read(&policy->rwsem);
>
> if (fattr->show)
> ret = fattr->show(policy, buf);
> else
> ret = -EIO;
>
> - unlock_policy_rwsem_read(policy->cpu);
> + up_read(&policy->rwsem);
> up_read(&cpufreq_rwsem);
>
> return ret;
> @@ -684,14 +643,14 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
> if (!down_read_trylock(&cpufreq_rwsem))
> goto unlock;
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> if (fattr->store)
> ret = fattr->store(policy, buf, count);
> else
> ret = -EIO;
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> up_read(&cpufreq_rwsem);
> unlock:
> @@ -868,7 +827,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> }
> }
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> write_lock_irqsave(&cpufreq_driver_lock, flags);
>
> @@ -876,7 +835,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
> per_cpu(cpufreq_cpu_data, cpu) = policy;
> write_unlock_irqrestore(&cpufreq_driver_lock, flags);
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> if (has_target) {
> if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
> @@ -923,6 +882,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void)
> goto err_free_cpumask;
>
> INIT_LIST_HEAD(&policy->policy_list);
> + init_rwsem(&policy->rwsem);
> +
> return policy;
>
> err_free_cpumask:
> @@ -945,19 +906,12 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
> if (cpu == policy->cpu)
> return;
>
> - /*
> - * Take direct locks as lock_policy_rwsem_write wouldn't work here.
> - * Also lock for last cpu is enough here as contention will happen only
> - * after policy->cpu is changed and after it is changed, other threads
> - * will try to acquire lock for new cpu. And policy is already updated
> - * by then.
> - */
> - down_write(&per_cpu(cpu_policy_rwsem, policy->cpu));
> + down_write(&policy->rwsem);
>
> policy->last_cpu = policy->cpu;
> policy->cpu = cpu;
>
> - up_write(&per_cpu(cpu_policy_rwsem, policy->last_cpu));
> + up_write(&policy->rwsem);
>
> #ifdef CONFIG_CPU_FREQ_TABLE
> cpufreq_frequency_table_update_policy_cpu(policy);
> @@ -1140,9 +1094,9 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
> if (ret) {
> pr_err("%s: Failed to move kobj: %d", __func__, ret);
>
> - lock_policy_rwsem_write(old_cpu);
> + down_write(&policy->rwsem);
> cpumask_set_cpu(old_cpu, policy->cpus);
> - unlock_policy_rwsem_write(old_cpu);
> + up_write(&policy->rwsem);
>
> ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
> "cpufreq");
> @@ -1193,9 +1147,9 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
> policy->governor->name, CPUFREQ_NAME_LEN);
> #endif
>
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
> cpus = cpumask_weight(policy->cpus);
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
>
> if (cpu != policy->cpu) {
> if (!frozen)
> @@ -1236,12 +1190,12 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> return -EINVAL;
> }
>
> - lock_policy_rwsem_write(cpu);
> + down_write(&policy->rwsem);
> cpus = cpumask_weight(policy->cpus);
>
> if (cpus > 1)
> cpumask_clear_cpu(cpu, policy->cpus);
> - unlock_policy_rwsem_write(cpu);
> + up_write(&policy->rwsem);
>
> /* If cpu is last user of policy, free policy */
> if (cpus == 1) {
> @@ -1256,10 +1210,10 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> }
>
> if (!frozen) {
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
> kobj = &policy->kobj;
> cmp = &policy->kobj_unregister;
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
> kobject_put(kobj);
>
> /*
> @@ -1451,19 +1405,22 @@ static unsigned int __cpufreq_get(unsigned int cpu)
> */
> unsigned int cpufreq_get(unsigned int cpu)
> {
> + struct cpufreq_policy *policy = per_cpu(cpufreq_cpu_data, cpu);
> unsigned int ret_freq = 0;
>
> if (cpufreq_disabled() || !cpufreq_driver)
> return -ENOENT;
>
> + BUG_ON(!policy);
> +
> if (!down_read_trylock(&cpufreq_rwsem))
> return 0;
>
> - lock_policy_rwsem_read(cpu);
> + down_read(&policy->rwsem);
>
> ret_freq = __cpufreq_get(cpu);
>
> - unlock_policy_rwsem_read(cpu);
> + up_read(&policy->rwsem);
> up_read(&cpufreq_rwsem);
>
> return ret_freq;
> @@ -1687,11 +1644,11 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
> {
> int ret = -EINVAL;
>
> - lock_policy_rwsem_write(policy->cpu);
> + down_write(&policy->rwsem);
>
> ret = __cpufreq_driver_target(policy, target_freq, relation);
>
> - unlock_policy_rwsem_write(policy->cpu);
> + up_write(&policy->rwsem);
>
> return ret;
> }
> @@ -1922,10 +1879,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
> /* end old governor */
> if (policy->governor) {
> __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> - unlock_policy_rwsem_write(new_policy->cpu);
> + up_write(&new_policy->rwsem);
> __cpufreq_governor(policy,
> CPUFREQ_GOV_POLICY_EXIT);
> - lock_policy_rwsem_write(new_policy->cpu);
> + down_write(&new_policy->rwsem);
> }
>
> /* start new governor */
> @@ -1934,10 +1891,10 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
> if (!__cpufreq_governor(policy, CPUFREQ_GOV_START)) {
> failed = 0;
> } else {
> - unlock_policy_rwsem_write(new_policy->cpu);
> + up_write(&new_policy->rwsem);
> __cpufreq_governor(policy,
> CPUFREQ_GOV_POLICY_EXIT);
> - lock_policy_rwsem_write(new_policy->cpu);
> + down_write(&new_policy->rwsem);
> }
> }
>
> @@ -1983,7 +1940,7 @@ int cpufreq_update_policy(unsigned int cpu)
> goto no_policy;
> }
>
> - lock_policy_rwsem_write(cpu);
> + down_write(&policy->rwsem);
>
> pr_debug("updating policy for CPU %u\n", cpu);
> memcpy(&new_policy, policy, sizeof(*policy));
> @@ -2010,7 +1967,7 @@ int cpufreq_update_policy(unsigned int cpu)
>
> ret = __cpufreq_set_policy(policy, &new_policy);
>
> - unlock_policy_rwsem_write(cpu);
> + up_write(&policy->rwsem);
>
> cpufreq_cpu_put(policy);
> no_policy:
> @@ -2167,14 +2124,9 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
>
> static int __init cpufreq_core_init(void)
> {
> - int cpu;
> -
> if (cpufreq_disabled())
> return -ENODEV;
>
> - for_each_possible_cpu(cpu)
> - init_rwsem(&per_cpu(cpu_policy_rwsem, cpu));
> -
> cpufreq_global_kobject = kobject_create();
> BUG_ON(!cpufreq_global_kobject);
> register_syscore_ops(&cpufreq_syscore_ops);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 6b199ed..a72bac2 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -85,6 +85,20 @@ struct cpufreq_policy {
> struct list_head policy_list;
> struct kobject kobj;
> struct completion kobj_unregister;
> +
> + /*
> + * The rules for this semaphore:
> + * - Any routine that wants to read from the policy structure will
> + * do a down_read on this semaphore.
> + * - Any routine that will write to the policy structure and/or may take away
> + * the policy altogether (eg. CPU hotplug), will hold this lock in write
> + * mode before doing so.
> + *
> + * Additional rules:
> + * - Lock should not be held across
> + * __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
> + */
> + struct rw_semaphore rwsem;
> };
>
> /* Only for ACPI */
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev()
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 01/11] cpufreq: make return type of lock_policy_rwsem_{read|write}() as void Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 02/11] cpufreq: create per policy rwsem instead of per cpu cpu_policy_rwsem Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:25 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 04/11] cpufreq: Remove extra blank line Viresh Kumar
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Some section of kerneldoc comment for __cpufreq_remove_dev() is invalid now.
Remove it.
Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index ae866b1..d80cbe6 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1260,8 +1260,6 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
* __cpufreq_remove_dev - remove a CPU device
*
* Removes the cpufreq interface for a CPU device.
- * Caller should already have policy_rwsem in write mode for this CPU.
- * This routine frees the rwsem before returning.
*/
static inline int __cpufreq_remove_dev(struct device *dev,
struct subsys_interface *sif,
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev()
2013-10-02 8:43 ` [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev() Viresh Kumar
@ 2013-10-03 7:25 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:25 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Some section of kerneldoc comment for __cpufreq_remove_dev() is invalid now.
> Remove it.
>
> Suggested-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> ---
> drivers/cpufreq/cpufreq.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index ae866b1..d80cbe6 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1260,8 +1260,6 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
> * __cpufreq_remove_dev - remove a CPU device
> *
> * Removes the cpufreq interface for a CPU device.
> - * Caller should already have policy_rwsem in write mode for this CPU.
> - * This routine frees the rwsem before returning.
> */
> static inline int __cpufreq_remove_dev(struct device *dev,
> struct subsys_interface *sif,
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 04/11] cpufreq: Remove extra blank line
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (2 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 03/11] cpufreq: remove invalid comment from __cpufreq_remove_dev() Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:26 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 05/11] cpufreq: don't break string in print statements Viresh Kumar
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
We don't need a blank line just at start of a block, lets remove it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d80cbe6..d149caa 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1155,7 +1155,6 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
if (!frozen)
sysfs_remove_link(&dev->kobj, "cpufreq");
} else if (cpus > 1) {
-
new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen);
if (new_cpu >= 0) {
update_policy_cpu(policy, new_cpu);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 04/11] cpufreq: Remove extra blank line
2013-10-02 8:43 ` [PATCH RESEND 04/11] cpufreq: Remove extra blank line Viresh Kumar
@ 2013-10-03 7:26 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:26 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> We don't need a blank line just at start of a block, lets remove it.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> ---
> drivers/cpufreq/cpufreq.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d80cbe6..d149caa 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1155,7 +1155,6 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
> if (!frozen)
> sysfs_remove_link(&dev->kobj, "cpufreq");
> } else if (cpus > 1) {
> -
> new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen);
> if (new_cpu >= 0) {
> update_policy_cpu(policy, new_cpu);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 05/11] cpufreq: don't break string in print statements
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (3 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 04/11] cpufreq: Remove extra blank line Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 06/11] cpufreq: remove __cpufreq_remove_dev() Viresh Kumar
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
As a rule its better not to break string (quoted inside "") in a print statement
even if it crosses 80 column boundary as that may introduce unwanted bugs and so
this patch rewrites one of the print statements..
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d149caa..065b4dd 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1160,8 +1160,8 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
update_policy_cpu(policy, new_cpu);
if (!frozen) {
- pr_debug("%s: policy Kobject moved to cpu: %d "
- "from: %d\n",__func__, new_cpu, cpu);
+ pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n",
+ __func__, new_cpu, cpu);
}
}
}
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RESEND 06/11] cpufreq: remove __cpufreq_remove_dev()
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (4 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 05/11] cpufreq: don't break string in print statements Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 07/11] cpufreq: Optimize cpufreq_frequency_table_verify() Viresh Kumar
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Nobody except cpufreq_remove_dev() is calling __cpufreq_remove_dev() and so we
don't need separate routines here. Lets merge code from __cpufreq_remove_dev()
to cpufreq_remove_dev() and get rid of __cpufreq_remove_dev().
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 065b4dd..1394dac 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1256,34 +1256,24 @@ static int __cpufreq_remove_dev_finish(struct device *dev,
}
/**
- * __cpufreq_remove_dev - remove a CPU device
+ * cpufreq_remove_dev - remove a CPU device
*
* Removes the cpufreq interface for a CPU device.
*/
-static inline int __cpufreq_remove_dev(struct device *dev,
- struct subsys_interface *sif,
- bool frozen)
-{
- int ret;
-
- ret = __cpufreq_remove_dev_prepare(dev, sif, frozen);
-
- if (!ret)
- ret = __cpufreq_remove_dev_finish(dev, sif, frozen);
-
- return ret;
-}
-
static int cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
{
unsigned int cpu = dev->id;
- int retval;
+ int ret;
if (cpu_is_offline(cpu))
return 0;
- retval = __cpufreq_remove_dev(dev, sif, false);
- return retval;
+ ret = __cpufreq_remove_dev_prepare(dev, sif, false);
+
+ if (!ret)
+ ret = __cpufreq_remove_dev_finish(dev, sif, false);
+
+ return ret;
}
static void handle_update(struct work_struct *work)
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RESEND 07/11] cpufreq: Optimize cpufreq_frequency_table_verify()
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (5 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 06/11] cpufreq: remove __cpufreq_remove_dev() Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 08/11] cpufreq: rename __cpufreq_set_policy() as cpufreq_set_policy() Viresh Kumar
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
cpufreq_frequency_table_verify() is a bit rewritten here to make it more logical
and optimal.
- merge multiple lines for variable declarations together.
- quit early if any frequency between min/max is found.
- don't call cpufreq_verify_within_limits() in case any valid freq is found as
it is of no use.
- count renamed as found and made a boolean.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/freq_table.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 11f6fa9..10f3cfb 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -54,9 +54,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_cpuinfo);
int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table)
{
- unsigned int next_larger = ~0;
- unsigned int i;
- unsigned int count = 0;
+ unsigned int next_larger = ~0, freq, i = 0;
+ bool found = false;
pr_debug("request for verification of policy (%u - %u kHz) for cpu %u\n",
policy->min, policy->max, policy->cpu);
@@ -64,21 +63,23 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
policy->cpuinfo.max_freq);
- for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
- unsigned int freq = table[i].frequency;
+ for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
if (freq == CPUFREQ_ENTRY_INVALID)
continue;
- if ((freq >= policy->min) && (freq <= policy->max))
- count++;
- else if ((next_larger > freq) && (freq > policy->max))
+ if ((freq >= policy->min) && (freq <= policy->max)) {
+ found = true;
+ break;
+ }
+
+ if ((next_larger > freq) && (freq > policy->max))
next_larger = freq;
}
- if (!count)
+ if (!found) {
policy->max = next_larger;
-
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
+ policy->cpuinfo.max_freq);
+ }
pr_debug("verification lead to (%u - %u kHz) for cpu %u\n",
policy->min, policy->max, policy->cpu);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RESEND 08/11] cpufreq: rename __cpufreq_set_policy() as cpufreq_set_policy()
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (6 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 07/11] cpufreq: Optimize cpufreq_frequency_table_verify() Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator Viresh Kumar
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Earlier there used to be two functions named __cpufreq_set_policy() and
cpufreq_set_policy(), but now we only have a single routine lets name it
cpufreq_set_policy() instead of __cpufreq_set_policy().
This also removes some invalid comments or fixes some incorrect comments.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1394dac..267ae99 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -371,7 +371,7 @@ show_one(scaling_min_freq, min);
show_one(scaling_max_freq, max);
show_one(scaling_cur_freq, cur);
-static int __cpufreq_set_policy(struct cpufreq_policy *policy,
+static int cpufreq_set_policy(struct cpufreq_policy *policy,
struct cpufreq_policy *new_policy);
/**
@@ -392,7 +392,7 @@ static ssize_t store_##file_name \
if (ret != 1) \
return -EINVAL; \
\
- ret = __cpufreq_set_policy(policy, &new_policy); \
+ ret = cpufreq_set_policy(policy, &new_policy); \
policy->user_policy.object = policy->object; \
\
return ret ? ret : count; \
@@ -450,11 +450,7 @@ static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
&new_policy.governor))
return -EINVAL;
- /*
- * Do not use cpufreq_set_policy here or the user_policy.max
- * will be wrongly overridden
- */
- ret = __cpufreq_set_policy(policy, &new_policy);
+ ret = cpufreq_set_policy(policy, &new_policy);
policy->user_policy.policy = policy->policy;
policy->user_policy.governor = policy->governor;
@@ -796,11 +792,11 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
int ret = 0;
memcpy(&new_policy, policy, sizeof(*policy));
- /* assure that the starting sequence is run in __cpufreq_set_policy */
+ /* assure that the starting sequence is run in cpufreq_set_policy */
policy->governor = NULL;
/* set default policy */
- ret = __cpufreq_set_policy(policy, &new_policy);
+ ret = cpufreq_set_policy(policy, &new_policy);
policy->user_policy.policy = policy->policy;
policy->user_policy.governor = policy->governor;
@@ -1803,10 +1799,10 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu)
EXPORT_SYMBOL(cpufreq_get_policy);
/*
- * data : current policy.
- * policy : policy to be set.
+ * policy : current policy.
+ * new_policy: policy to be set.
*/
-static int __cpufreq_set_policy(struct cpufreq_policy *policy,
+static int cpufreq_set_policy(struct cpufreq_policy *policy,
struct cpufreq_policy *new_policy)
{
int ret = 0, failed = 1;
@@ -1952,7 +1948,7 @@ int cpufreq_update_policy(unsigned int cpu)
}
}
- ret = __cpufreq_set_policy(policy, &new_policy);
+ ret = cpufreq_set_policy(policy, &new_policy);
up_write(&policy->rwsem);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (7 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 08/11] cpufreq: rename __cpufreq_set_policy() as cpufreq_set_policy() Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:32 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY Viresh Kumar
2013-10-02 8:43 ` [PATCH RESEND 11/11] cpufreq: add new routine cpufreq_verify_within_cpu_limits() Viresh Kumar
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Currently cpufreq_driver's flags are defined directly using 0x1, 0x2, 0x4, 0x8,
etc.. As the list grows it doesn't stays much readable..
Lets use bitwise shift operator << to generate these numbers for respective
positions.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
include/linux/cpufreq.h | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a72bac2..9321059 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -225,13 +225,14 @@ struct cpufreq_driver {
};
/* flags */
-#define CPUFREQ_STICKY 0x01 /* the driver isn't removed even if
- * all ->init() calls failed */
-#define CPUFREQ_CONST_LOOPS 0x02 /* loops_per_jiffy or other kernel
- * "constants" aren't affected by
- * frequency transitions */
-#define CPUFREQ_PM_NO_WARN 0x04 /* don't warn on suspend/resume speed
- * mismatches */
+#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
+ all ->init() calls failed */
+#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
+ kernel "constants" aren't
+ affected by frequency
+ transitions */
+#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
+ speed mismatches */
int cpufreq_register_driver(struct cpufreq_driver *driver_data);
int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator
2013-10-02 8:43 ` [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator Viresh Kumar
@ 2013-10-03 7:32 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:32 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Currently cpufreq_driver's flags are defined directly using 0x1, 0x2, 0x4, 0x8,
> etc.. As the list grows it doesn't stays much readable..
>
> Lets use bitwise shift operator << to generate these numbers for respective
> positions.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> include/linux/cpufreq.h | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a72bac2..9321059 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -225,13 +225,14 @@ struct cpufreq_driver {
> };
>
> /* flags */
> -#define CPUFREQ_STICKY 0x01 /* the driver isn't removed even if
> - * all ->init() calls failed */
> -#define CPUFREQ_CONST_LOOPS 0x02 /* loops_per_jiffy or other kernel
> - * "constants" aren't affected by
> - * frequency transitions */
> -#define CPUFREQ_PM_NO_WARN 0x04 /* don't warn on suspend/resume speed
> - * mismatches */
> +#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
> + all ->init() calls failed */
> +#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
> + kernel "constants" aren't
> + affected by frequency
> + transitions */
> +#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> + speed mismatches */
>
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (8 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 09/11] cpufreq: rewrite cpufreq_driver->flags using shift operator Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-03 7:39 ` Srivatsa S. Bhat
2013-10-02 8:43 ` [PATCH RESEND 11/11] cpufreq: add new routine cpufreq_verify_within_cpu_limits() Viresh Kumar
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Lets use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY instead
of a separate field within cpufreq_driver. This will save some bytes for us.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/arm_big_little.c | 4 ++--
drivers/cpufreq/cpufreq.c | 2 +-
drivers/cpufreq/cpufreq_governor.h | 5 ++++-
include/linux/cpufreq.h | 15 ++++++++-------
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 9acfb82..2c7c134 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -213,13 +213,13 @@ static struct freq_attr *bL_cpufreq_attr[] = {
static struct cpufreq_driver bL_cpufreq_driver = {
.name = "arm-big-little",
- .flags = CPUFREQ_STICKY,
+ .flags = CPUFREQ_STICKY |
+ CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
.verify = bL_cpufreq_verify_policy,
.target = bL_cpufreq_set_target,
.get = bL_cpufreq_get,
.init = bL_cpufreq_init,
.exit = bL_cpufreq_exit,
- .have_governor_per_policy = true,
.attr = bL_cpufreq_attr,
};
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 267ae99..23e0a40 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -92,7 +92,7 @@ static DEFINE_MUTEX(cpufreq_governor_mutex);
bool have_governor_per_policy(void)
{
- return cpufreq_driver->have_governor_per_policy;
+ return !!(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY);
}
EXPORT_SYMBOL_GPL(have_governor_per_policy);
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index 88cd39f..b5f2b86 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -191,7 +191,10 @@ struct common_dbs_data {
struct attribute_group *attr_group_gov_sys; /* one governor - system */
struct attribute_group *attr_group_gov_pol; /* one governor - policy */
- /* Common data for platforms that don't set have_governor_per_policy */
+ /*
+ * Common data for platforms that don't set
+ * CPUFREQ_HAVE_GOVERNOR_PER_POLICY
+ */
struct dbs_data *gdbs_data;
struct cpu_dbs_common_info *(*get_cpu_cdbs)(int cpu);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 9321059..6c9c365 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -194,13 +194,6 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
struct cpufreq_driver {
char name[CPUFREQ_NAME_LEN];
u8 flags;
- /*
- * This should be set by platforms having multiple clock-domains, i.e.
- * supporting multiple policies. With this sysfs directories of governor
- * would be created in cpu/cpu<num>/cpufreq/ directory and so they can
- * use the same governor with different tunables for different clusters.
- */
- bool have_governor_per_policy;
/* needed by all drivers */
int (*init) (struct cpufreq_policy *policy);
@@ -234,6 +227,14 @@ struct cpufreq_driver {
#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
speed mismatches */
+/*
+ * This should be set by platforms having multiple clock-domains, i.e.
+ * supporting multiple policies. With this sysfs directories of governor would
+ * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
+ * governor with different tunables for different clusters.
+ */
+#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
+
int cpufreq_register_driver(struct cpufreq_driver *driver_data);
int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY
2013-10-02 8:43 ` [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY Viresh Kumar
@ 2013-10-03 7:39 ` Srivatsa S. Bhat
0 siblings, 0 replies; 19+ messages in thread
From: Srivatsa S. Bhat @ 2013-10-03 7:39 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, linux-kernel@vger.kernel.org
On 10/02/2013 02:13 PM, Viresh Kumar wrote:
> Lets use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY instead
> of a separate field within cpufreq_driver. This will save some bytes for us.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> drivers/cpufreq/arm_big_little.c | 4 ++--
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/cpufreq/cpufreq_governor.h | 5 ++++-
> include/linux/cpufreq.h | 15 ++++++++-------
> 4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> index 9acfb82..2c7c134 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -213,13 +213,13 @@ static struct freq_attr *bL_cpufreq_attr[] = {
>
> static struct cpufreq_driver bL_cpufreq_driver = {
> .name = "arm-big-little",
> - .flags = CPUFREQ_STICKY,
> + .flags = CPUFREQ_STICKY |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> .verify = bL_cpufreq_verify_policy,
> .target = bL_cpufreq_set_target,
> .get = bL_cpufreq_get,
> .init = bL_cpufreq_init,
> .exit = bL_cpufreq_exit,
> - .have_governor_per_policy = true,
> .attr = bL_cpufreq_attr,
> };
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 267ae99..23e0a40 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -92,7 +92,7 @@ static DEFINE_MUTEX(cpufreq_governor_mutex);
>
> bool have_governor_per_policy(void)
> {
> - return cpufreq_driver->have_governor_per_policy;
> + return !!(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY);
> }
> EXPORT_SYMBOL_GPL(have_governor_per_policy);
>
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index 88cd39f..b5f2b86 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -191,7 +191,10 @@ struct common_dbs_data {
> struct attribute_group *attr_group_gov_sys; /* one governor - system */
> struct attribute_group *attr_group_gov_pol; /* one governor - policy */
>
> - /* Common data for platforms that don't set have_governor_per_policy */
> + /*
> + * Common data for platforms that don't set
> + * CPUFREQ_HAVE_GOVERNOR_PER_POLICY
> + */
> struct dbs_data *gdbs_data;
>
> struct cpu_dbs_common_info *(*get_cpu_cdbs)(int cpu);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 9321059..6c9c365 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -194,13 +194,6 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
> struct cpufreq_driver {
> char name[CPUFREQ_NAME_LEN];
> u8 flags;
> - /*
> - * This should be set by platforms having multiple clock-domains, i.e.
> - * supporting multiple policies. With this sysfs directories of governor
> - * would be created in cpu/cpu<num>/cpufreq/ directory and so they can
> - * use the same governor with different tunables for different clusters.
> - */
> - bool have_governor_per_policy;
>
> /* needed by all drivers */
> int (*init) (struct cpufreq_policy *policy);
> @@ -234,6 +227,14 @@ struct cpufreq_driver {
> #define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> speed mismatches */
>
> +/*
> + * This should be set by platforms having multiple clock-domains, i.e.
> + * supporting multiple policies. With this sysfs directories of governor would
> + * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
> + * governor with different tunables for different clusters.
> + */
> +#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
> +
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RESEND 11/11] cpufreq: add new routine cpufreq_verify_within_cpu_limits()
2013-10-02 8:43 [PATCH RESEND 00/11] CPUFreq: Cleanups/fixes for v3.13 Viresh Kumar
` (9 preceding siblings ...)
2013-10-02 8:43 ` [PATCH RESEND 10/11] cpufreq: use cpufreq_driver->flags to mark CPUFREQ_HAVE_GOVERNOR_PER_POLICY Viresh Kumar
@ 2013-10-02 8:43 ` Viresh Kumar
2013-10-02 17:40 ` Dirk Brandewie
10 siblings, 1 reply; 19+ messages in thread
From: Viresh Kumar @ 2013-10-02 8:43 UTC (permalink / raw)
To: rjw; +Cc: cpufreq, linux-pm, Viresh Kumar
Most of the users of cpufreq_verify_within_limits() calls it for limiting with
min/max from policy->cpuinfo. We can make that code simple by introducing
another routine which will do this for them automatically.
This patch adds another routine cpufreq_verify_within_cpu_limits() and updates
others to use it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq-nforce2.c | 4 +---
drivers/cpufreq/davinci-cpufreq.c | 4 +---
drivers/cpufreq/freq_table.c | 6 ++----
drivers/cpufreq/integrator-cpufreq.c | 9 ++-------
drivers/cpufreq/intel_pstate.c | 4 +---
drivers/cpufreq/longrun.c | 4 +---
drivers/cpufreq/pcc-cpufreq.c | 3 +--
drivers/cpufreq/sh-cpufreq.c | 7 ++-----
drivers/cpufreq/unicore2-cpufreq.c | 4 +---
include/linux/cpufreq.h | 7 +++++++
10 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index b83d45f6..56c964c 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -303,9 +303,7 @@ static int nforce2_verify(struct cpufreq_policy *policy)
if (policy->min < (fsb_pol_max * fid * 100))
policy->max = (fsb_pol_max + 1) * fid * 100;
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
return 0;
}
diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index f67196e..ba03e6f 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -50,9 +50,7 @@ static int davinci_verify_speed(struct cpufreq_policy *policy)
if (policy->cpu)
return -EINVAL;
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
-
+ cpufreq_verify_within_cpu_limits(policy);
policy->min = clk_round_rate(armclk, policy->min * 1000) / 1000;
policy->max = clk_round_rate(armclk, policy->max * 1000) / 1000;
cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 10f3cfb..b9336ed 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -60,8 +60,7 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
pr_debug("request for verification of policy (%u - %u kHz) for cpu %u\n",
policy->min, policy->max, policy->cpu);
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
if (freq == CPUFREQ_ENTRY_INVALID)
@@ -77,8 +76,7 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
if (!found) {
policy->max = next_larger;
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
}
pr_debug("verification lead to (%u - %u kHz) for cpu %u\n",
diff --git a/drivers/cpufreq/integrator-cpufreq.c b/drivers/cpufreq/integrator-cpufreq.c
index f7c99df..8152a9b 100644
--- a/drivers/cpufreq/integrator-cpufreq.c
+++ b/drivers/cpufreq/integrator-cpufreq.c
@@ -59,9 +59,7 @@ static int integrator_verify_policy(struct cpufreq_policy *policy)
{
struct icst_vco vco;
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
vco = icst_hz_to_vco(&cclk_params, policy->max * 1000);
policy->max = icst_hz(&cclk_params, vco) / 1000;
@@ -69,10 +67,7 @@ static int integrator_verify_policy(struct cpufreq_policy *policy)
vco = icst_hz_to_vco(&cclk_params, policy->min * 1000);
policy->min = icst_hz(&cclk_params, vco) / 1000;
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
-
+ cpufreq_verify_within_cpu_limits(policy);
return 0;
}
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 32b3479..22b8d54 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -614,9 +614,7 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
{
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
if ((policy->policy != CPUFREQ_POLICY_POWERSAVE) &&
(policy->policy != CPUFREQ_POLICY_PERFORMANCE))
diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c
index 5aa0316..074971b 100644
--- a/drivers/cpufreq/longrun.c
+++ b/drivers/cpufreq/longrun.c
@@ -129,9 +129,7 @@ static int longrun_verify_policy(struct cpufreq_policy *policy)
return -EINVAL;
policy->cpu = 0;
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
if ((policy->policy != CPUFREQ_POLICY_POWERSAVE) &&
(policy->policy != CPUFREQ_POLICY_PERFORMANCE))
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index 70438de..160cc1e 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -111,8 +111,7 @@ static struct pcc_cpu __percpu *pcc_cpu_info;
static int pcc_cpufreq_verify(struct cpufreq_policy *policy)
{
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
return 0;
}
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 1362e88..f1fb944 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -87,15 +87,12 @@ static int sh_cpufreq_verify(struct cpufreq_policy *policy)
if (freq_table)
return cpufreq_frequency_table_verify(policy, freq_table);
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
+ cpufreq_verify_within_cpu_limits(policy);
policy->min = (clk_round_rate(cpuclk, 1) + 500) / 1000;
policy->max = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
- cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
- policy->cpuinfo.max_freq);
-
+ cpufreq_verify_within_cpu_limits(policy);
return 0;
}
diff --git a/drivers/cpufreq/unicore2-cpufreq.c b/drivers/cpufreq/unicore2-cpufreq.c
index b225f04..14e6d31 100644
--- a/drivers/cpufreq/unicore2-cpufreq.c
+++ b/drivers/cpufreq/unicore2-cpufreq.c
@@ -29,9 +29,7 @@ static int ucv2_verify_speed(struct cpufreq_policy *policy)
if (policy->cpu)
return -EINVAL;
- cpufreq_verify_within_limits(policy,
- policy->cpuinfo.min_freq, policy->cpuinfo.max_freq);
-
+ cpufreq_verify_within_cpu_limits(policy);
return 0;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6c9c365..ddc6332 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -256,6 +256,13 @@ static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
return;
}
+static inline void
+cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy)
+{
+ cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
+ policy->cpuinfo.max_freq);
+}
+
/*********************************************************************
* CPUFREQ NOTIFIER INTERFACE *
*********************************************************************/
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RESEND 11/11] cpufreq: add new routine cpufreq_verify_within_cpu_limits()
2013-10-02 8:43 ` [PATCH RESEND 11/11] cpufreq: add new routine cpufreq_verify_within_cpu_limits() Viresh Kumar
@ 2013-10-02 17:40 ` Dirk Brandewie
0 siblings, 0 replies; 19+ messages in thread
From: Dirk Brandewie @ 2013-10-02 17:40 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, cpufreq, linux-pm, dirk.brandewie
On 10/02/2013 01:43 AM, Viresh Kumar wrote:
> Most of the users of cpufreq_verify_within_limits() calls it for limiting with
> min/max from policy->cpuinfo. We can make that code simple by introducing
> another routine which will do this for them automatically.
>
> This patch adds another routine cpufreq_verify_within_cpu_limits() and updates
> others to use it.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq-nforce2.c | 4 +---
> drivers/cpufreq/davinci-cpufreq.c | 4 +---
> drivers/cpufreq/freq_table.c | 6 ++----
> drivers/cpufreq/integrator-cpufreq.c | 9 ++-------
> drivers/cpufreq/intel_pstate.c | 4 +---
> drivers/cpufreq/longrun.c | 4 +---
> drivers/cpufreq/pcc-cpufreq.c | 3 +--
> drivers/cpufreq/sh-cpufreq.c | 7 ++-----
> drivers/cpufreq/unicore2-cpufreq.c | 4 +---
> include/linux/cpufreq.h | 7 +++++++
> 10 files changed, 19 insertions(+), 33 deletions(-)
>
For intel_pstate portion
Acked-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
> index b83d45f6..56c964c 100644
> --- a/drivers/cpufreq/cpufreq-nforce2.c
> +++ b/drivers/cpufreq/cpufreq-nforce2.c
> @@ -303,9 +303,7 @@ static int nforce2_verify(struct cpufreq_policy *policy)
> if (policy->min < (fsb_pol_max * fid * 100))
> policy->max = (fsb_pol_max + 1) * fid * 100;
>
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
> return 0;
> }
>
> diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
> index f67196e..ba03e6f 100644
> --- a/drivers/cpufreq/davinci-cpufreq.c
> +++ b/drivers/cpufreq/davinci-cpufreq.c
> @@ -50,9 +50,7 @@ static int davinci_verify_speed(struct cpufreq_policy *policy)
> if (policy->cpu)
> return -EINVAL;
>
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> -
> + cpufreq_verify_within_cpu_limits(policy);
> policy->min = clk_round_rate(armclk, policy->min * 1000) / 1000;
> policy->max = clk_round_rate(armclk, policy->max * 1000) / 1000;
> cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 10f3cfb..b9336ed 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -60,8 +60,7 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
> pr_debug("request for verification of policy (%u - %u kHz) for cpu %u\n",
> policy->min, policy->max, policy->cpu);
>
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
>
> for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
> if (freq == CPUFREQ_ENTRY_INVALID)
> @@ -77,8 +76,7 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
>
> if (!found) {
> policy->max = next_larger;
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
> }
>
> pr_debug("verification lead to (%u - %u kHz) for cpu %u\n",
> diff --git a/drivers/cpufreq/integrator-cpufreq.c b/drivers/cpufreq/integrator-cpufreq.c
> index f7c99df..8152a9b 100644
> --- a/drivers/cpufreq/integrator-cpufreq.c
> +++ b/drivers/cpufreq/integrator-cpufreq.c
> @@ -59,9 +59,7 @@ static int integrator_verify_policy(struct cpufreq_policy *policy)
> {
> struct icst_vco vco;
>
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
>
> vco = icst_hz_to_vco(&cclk_params, policy->max * 1000);
> policy->max = icst_hz(&cclk_params, vco) / 1000;
> @@ -69,10 +67,7 @@ static int integrator_verify_policy(struct cpufreq_policy *policy)
> vco = icst_hz_to_vco(&cclk_params, policy->min * 1000);
> policy->min = icst_hz(&cclk_params, vco) / 1000;
>
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> -
> + cpufreq_verify_within_cpu_limits(policy);
> return 0;
> }
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 32b3479..22b8d54 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -614,9 +614,7 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>
> static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> {
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
>
> if ((policy->policy != CPUFREQ_POLICY_POWERSAVE) &&
> (policy->policy != CPUFREQ_POLICY_PERFORMANCE))
> diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c
> index 5aa0316..074971b 100644
> --- a/drivers/cpufreq/longrun.c
> +++ b/drivers/cpufreq/longrun.c
> @@ -129,9 +129,7 @@ static int longrun_verify_policy(struct cpufreq_policy *policy)
> return -EINVAL;
>
> policy->cpu = 0;
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
>
> if ((policy->policy != CPUFREQ_POLICY_POWERSAVE) &&
> (policy->policy != CPUFREQ_POLICY_PERFORMANCE))
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> index 70438de..160cc1e 100644
> --- a/drivers/cpufreq/pcc-cpufreq.c
> +++ b/drivers/cpufreq/pcc-cpufreq.c
> @@ -111,8 +111,7 @@ static struct pcc_cpu __percpu *pcc_cpu_info;
>
> static int pcc_cpufreq_verify(struct cpufreq_policy *policy)
> {
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
> return 0;
> }
>
> diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
> index 1362e88..f1fb944 100644
> --- a/drivers/cpufreq/sh-cpufreq.c
> +++ b/drivers/cpufreq/sh-cpufreq.c
> @@ -87,15 +87,12 @@ static int sh_cpufreq_verify(struct cpufreq_policy *policy)
> if (freq_table)
> return cpufreq_frequency_table_verify(policy, freq_table);
>
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> + cpufreq_verify_within_cpu_limits(policy);
>
> policy->min = (clk_round_rate(cpuclk, 1) + 500) / 1000;
> policy->max = (clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
>
> - cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> - policy->cpuinfo.max_freq);
> -
> + cpufreq_verify_within_cpu_limits(policy);
> return 0;
> }
>
> diff --git a/drivers/cpufreq/unicore2-cpufreq.c b/drivers/cpufreq/unicore2-cpufreq.c
> index b225f04..14e6d31 100644
> --- a/drivers/cpufreq/unicore2-cpufreq.c
> +++ b/drivers/cpufreq/unicore2-cpufreq.c
> @@ -29,9 +29,7 @@ static int ucv2_verify_speed(struct cpufreq_policy *policy)
> if (policy->cpu)
> return -EINVAL;
>
> - cpufreq_verify_within_limits(policy,
> - policy->cpuinfo.min_freq, policy->cpuinfo.max_freq);
> -
> + cpufreq_verify_within_cpu_limits(policy);
> return 0;
> }
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 6c9c365..ddc6332 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -256,6 +256,13 @@ static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
> return;
> }
>
> +static inline void
> +cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy)
> +{
> + cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> + policy->cpuinfo.max_freq);
> +}
> +
> /*********************************************************************
> * CPUFREQ NOTIFIER INTERFACE *
> *********************************************************************/
>
^ permalink raw reply [flat|nested] 19+ messages in thread