On Fri, Nov 28, 2014 at 03:14:10PM +0530, Viresh Kumar wrote:
Locking around idr_alloc/idr_remove looks rather pointless as there is no race it is trying to fix. Remove it.
Can you please elaborate on how the idr's are going to be serialize without the lock?
get_idr() and release_idr() aren't much useful now, so get rid of them as well.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
@Eduardo: Same is true for thermal-core as well ?
Probably not ?
drivers/thermal/cpu_cooling.c | 45 ++++--------------------------------------- 1 file changed, 4 insertions(+), 41 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 032cba3..ca8f1bb 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -73,42 +73,6 @@ static unsigned int cpufreq_dev_count; #define NOTIFY_INVALID NULL static struct cpufreq_cooling_device *notify_device; -/**
- get_idr - function to get a unique id.
- @idr: struct idr * handle used to create a id.
- @id: int * value generated by this function.
- This function will populate @id with an unique
- id, using the idr API.
- Return: 0 on success, an error code on failure.
- */
-static int get_idr(struct idr *idr, int *id) -{
- int ret;
- mutex_lock(&cooling_cpufreq_lock);
- ret = idr_alloc(idr, NULL, 0, 0, GFP_KERNEL);
- mutex_unlock(&cooling_cpufreq_lock);
- if (unlikely(ret < 0))
return ret;- *id = ret;
- return 0;
-}
-/**
- release_idr - function to free the unique id.
- @idr: struct idr * handle used for creating the id.
- @id: int value representing the unique id.
- */
-static void release_idr(struct idr *idr, int id) -{
- mutex_lock(&cooling_cpufreq_lock);
- idr_remove(idr, id);
- mutex_unlock(&cooling_cpufreq_lock);
-}
/* Below code defines functions to be used for cpufreq as cooling device */ enum cpufreq_cooling_property { @@ -430,7 +394,6 @@ __cpufreq_cooling_register(struct device_node *np, struct thermal_cooling_device *cool_dev; struct cpufreq_cooling_device *cpufreq_dev; char dev_name[THERMAL_NAME_LENGTH];
- int ret = 0;
cpufreq_dev = kzalloc(sizeof(*cool_dev), GFP_KERNEL); if (!cpufreq_dev) @@ -438,8 +401,8 @@ __cpufreq_cooling_register(struct device_node *np, cpumask_copy(&cpufreq_dev->allowed_cpus, clip_cpus);
- ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
- if (ret) {
- cpufreq_dev->id = idr_alloc(&cpufreq_idr, NULL, 0, 0, GFP_KERNEL);
- if (unlikely(cpufreq_dev->id < 0)) { cool_dev = ERR_PTR(cpufreq_dev->id); goto free_cdev; }
@@ -467,7 +430,7 @@ __cpufreq_cooling_register(struct device_node *np, return cool_dev; remove_idr:
- release_idr(&cpufreq_idr, cpufreq_dev->id);
- idr_remove(&cpufreq_idr, cpufreq_dev->id);
free_cdev: kfree(cpufreq_dev); @@ -540,7 +503,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) mutex_unlock(&cooling_cpufreq_lock); thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
- release_idr(&cpufreq_idr, cpufreq_dev->id);
- idr_remove(&cpufreq_idr, cpufreq_dev->id); kfree(cpufreq_dev);
} EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister); -- 2.0.3.693.g996b0fd