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
>