From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eduardo Valentin Subject: Re: [PATCH 16/26] cpu_cooling: Drop useless locking around idr_alloc/idr_remove Date: Tue, 2 Dec 2014 19:05:35 -0400 Message-ID: <20141202230533.GB3432@developer> References: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tsOsTdHNUZQcU9Ye" Return-path: Received: from mail-qc0-f179.google.com ([209.85.216.179]:59338 "EHLO mail-qc0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932722AbaLBXFl (ORCPT ); Tue, 2 Dec 2014 18:05:41 -0500 Received: by mail-qc0-f179.google.com with SMTP id c9so10083629qcz.24 for ; Tue, 02 Dec 2014 15:05:40 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Viresh Kumar Cc: linux-pm@vger.kernel.org, linaro-kernel@lists.linaro.org, rui.zhang@intel.com --tsOsTdHNUZQcU9Ye Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 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. >=20 > Signed-off-by: Viresh Kumar > --- > @Eduardo: Same is true for thermal-core as well ? Probably not ? > --- > drivers/thermal/cpu_cooling.c | 45 ++++---------------------------------= ------ > 1 file changed, 4 insertions(+), 41 deletions(-) >=20 > 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; > =20 > -/** > - * 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 =3D idr_alloc(idr, NULL, 0, 0, GFP_KERNEL); > - mutex_unlock(&cooling_cpufreq_lock); > - if (unlikely(ret < 0)) > - return ret; > - *id =3D 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= */ > =20 > 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 =3D 0; > =20 > cpufreq_dev =3D kzalloc(sizeof(*cool_dev), GFP_KERNEL); > if (!cpufreq_dev) > @@ -438,8 +401,8 @@ __cpufreq_cooling_register(struct device_node *np, > =20 > cpumask_copy(&cpufreq_dev->allowed_cpus, clip_cpus); > =20 > - ret =3D get_idr(&cpufreq_idr, &cpufreq_dev->id); > - if (ret) { > + cpufreq_dev->id =3D idr_alloc(&cpufreq_idr, NULL, 0, 0, GFP_KERNEL); > + if (unlikely(cpufreq_dev->id < 0)) { > cool_dev =3D ERR_PTR(cpufreq_dev->id); > goto free_cdev; > } > @@ -467,7 +430,7 @@ __cpufreq_cooling_register(struct device_node *np, > return cool_dev; > =20 > remove_idr: > - release_idr(&cpufreq_idr, cpufreq_dev->id); > + idr_remove(&cpufreq_idr, cpufreq_dev->id); > free_cdev: > kfree(cpufreq_dev); > =20 > @@ -540,7 +503,7 @@ void cpufreq_cooling_unregister(struct thermal_coolin= g_device *cdev) > mutex_unlock(&cooling_cpufreq_lock); > =20 > 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); > --=20 > 2.0.3.693.g996b0fd >=20 --tsOsTdHNUZQcU9Ye Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBAgAGBQJUfkW0AAoJEMLUO4d9pOJW+pwH/jiL4IOf6xRg4UzABeUuCDb5 Rt5Tr3K0ao6nWL0bMvx7trDYfbti9Yw00kFuuTYGsev6ykL7sEoytz9RAS7SQVt5 xYHZhmCH1K50dGofrbvq0GDLlFknBR2Kg8Tx6LJRKUlk0KJrDgTQglcRJvlVhwtc ZG7tk+AkNwschR0SRyLNOfnpkHY2CNw1aK/jSgcnTuSep6TH4993ideVrjvUGxL7 kLEyPTf54CUrXvWwTynhXhlv/Yx9uu30GQA1gMo+b61OUxs0j7bd3WI5uw0hjjB0 /vHeBdpO1BI/QfLbtNZHXfB1hu1EH2+QOJWZ+Lab67tFIwVDNLMBEcaPiK0yflk= =OmaP -----END PGP SIGNATURE----- --tsOsTdHNUZQcU9Ye--