[PATCH 0/8] RFC: Introduce ida_simple interfaces and use them.

[PATCH 0/8] RFC: Introduce ida_simple interfaces and use them.

[Jonathan Cameron]

Hi All,

Firstly sorry for the huge cc list, but this series does get about.

This series came out of two separate lkml threads:

https://lkml.org/lkml/2011/7/13/64
http://thread.gmane.org/gmane.linux.kernel/1148513/focus=74236

Basically Rusty and I both got annoyed with yet more instances
of the same cut and past ida allocation code for the very simple
case of wanting a unique id for some device. Tejun helpfully
joined the two threads up.

Anyhow, Rusty proposed more or less the first patch here (with a name
change requested by Tejun applied.)

The others patches are some of the more obvious looking cases for
applying this code.  My original reason was for IIO but those patches will
have to wait a little while for some prior changes to merge.
For reference, it saves about 40 lines there so I'm happy ;)

There are a number of other cases in tree that can be cleaned up
in a second series once these are sorted.

There are a couple of cases in here where I've carefully mangled
the error codes to keep consistent with the existing code.  Please
can people who know those subsystems well decide on whether the
mangling is necessary.

All comments welcome. I expect I've broken at least one driver
doing this so please take a close look.

Rusty, Tejun. I kept your sign of an ack for the first patch. Could
you quickly verify that's fine?

Thanks,

Jonathan

Jonathan Cameron (7):
  hwmon: convert idr to ida and use ida_simple interface.
  hwmon: ibmaem: convert idr to ida and use ida_simple_get
  [SCSI] use ida_simple_get and ida_simple remove in place of
    boilerplate code.
  drm/vmwgfx: use ida_simple_get for id allocation.
  [SCSI] osduld: use ida_simple_get to handle id.
  w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.
  rtc: class idr converted to ida and ida_simple_get used.

Rusty Russell (1):
  ida: Simplified functions for id allocation.

 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |   34 +++----------
 drivers/hwmon/hwmon.c                         |   32 +++---------
 drivers/hwmon/ibmaem.c                        |   47 +++--------------
 drivers/rtc/class.c                           |   32 +++---------
 drivers/scsi/osd/osd_uld.c                    |   22 +++-----
 drivers/scsi/sd.c                             |   32 ++++--------
 drivers/w1/slaves/w1_ds2760.c                 |   48 ++---------------
 drivers/w1/slaves/w1_ds2780.c                 |   48 ++---------------
 include/linux/idr.h                           |    4 ++
 lib/idr.c                                     |   67 +++++++++++++++++++++++++
 10 files changed, 134 insertions(+), 232 deletions(-)

-- 
1.7.3.4

[PATCH 1/8] ida: Simplified functions for id allocation.

[Jonathan Cameron]

From: Rusty Russell <rusty@rustcorp.com.au>

The current hyper-optimized functions are overkill if you simply want
to allocate an id for a device.  Create versions which use an internal
lock.

Thanks to Tejun for feedback.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 include/linux/idr.h |    4 +++
 lib/idr.c           |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/linux/idr.h b/include/linux/idr.h
index 13a801f..255491c 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -146,6 +146,10 @@ void ida_remove(struct ida *ida, int id);
 void ida_destroy(struct ida *ida);
 void ida_init(struct ida *ida);
 
+int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
+		   gfp_t gfp_mask);
+void ida_simple_remove(struct ida *ida, unsigned int id);
+
 void __init idr_init_cache(void);
 
 #endif /* __IDR_H__ */
diff --git a/lib/idr.c b/lib/idr.c
index e15502e..db040ce 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -34,8 +34,10 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/idr.h>
+#include <linux/spinlock.h>
 
 static struct kmem_cache *idr_layer_cache;
+static DEFINE_SPINLOCK(simple_ida_lock);
 
 static struct idr_layer *get_from_free_list(struct idr *idp)
 {
@@ -926,6 +928,71 @@ void ida_destroy(struct ida *ida)
 EXPORT_SYMBOL(ida_destroy);
 
 /**
+ * ida_simple_get - get a new id.
+ * @ida: the (initialized) ida.
+ * @start: the minimum id (inclusive, < 0x8000000)
+ * @end: the maximum id (exclusive, < 0x8000000 or 0)
+ * @gfp_mask: memory allocation flags
+ *
+ * Allocates an id in the range start <= id < end, or returns -ENOSPC.
+ * On memory allocation failure, returns -ENOMEM.
+ *
+ * Use ida_simple_remove() to get rid of an id.
+ */
+int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
+		   gfp_t gfp_mask)
+{
+	int ret, id;
+	unsigned int max;
+
+	BUG_ON((int)start < 0);
+	BUG_ON((int)end < 0);
+
+	if (end == 0)
+		max = 0x80000000;
+	else {
+		BUG_ON(end < start);
+		max = end - 1;
+	}
+
+again:
+	if (!ida_pre_get(ida, gfp_mask))
+		return -ENOMEM;
+
+	spin_lock(&simple_ida_lock);
+	ret = ida_get_new_above(ida, start, &id);
+	if (!ret) {
+		if (id > max) {
+			ida_remove(ida, id);
+			ret = -ENOSPC;
+		} else {
+			ret = id;
+		}
+	}
+	spin_unlock(&simple_ida_lock);
+
+	if (unlikely(ret == -EAGAIN))
+		goto again;
+
+	return ret;
+}
+EXPORT_SYMBOL(ida_simple_get);
+
+/**
+ * ida_simple_remove - remove an allocated id.
+ * @ida: the (initialized) ida.
+ * @id: the id returned by ida_simple_get.
+ */
+void ida_simple_remove(struct ida *ida, unsigned int id)
+{
+	BUG_ON((int)id < 0);
+	spin_lock(&simple_ida_lock);
+	ida_remove(ida, id);
+	spin_unlock(&simple_ida_lock);
+}
+EXPORT_SYMBOL(ida_simple_remove);
+
+/**
  * ida_init - initialize ida handle
  * @ida:	ida handle
  *
-- 
1.7.3.4

[PATCH 2/8] hwmon: convert idr to ida and use ida_simple interface.

[Jonathan Cameron]

hwmon was using an idr with a NULL pointer, so convert to an
ida which then allows use of Rusty's ida_simple_get.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/hwmon/hwmon.c |   32 ++++++++------------------------
 1 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index a61e781..6460487 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -27,8 +27,7 @@
 
 static struct class *hwmon_class;
 
-static DEFINE_IDR(hwmon_idr);
-static DEFINE_SPINLOCK(idr_lock);
+static DEFINE_IDA(hwmon_ida);
 
 /**
  * hwmon_device_register - register w/ hwmon
@@ -42,30 +41,17 @@ static DEFINE_SPINLOCK(idr_lock);
 struct device *hwmon_device_register(struct device *dev)
 {
 	struct device *hwdev;
-	int id, err;
-
-again:
-	if (unlikely(idr_pre_get(&hwmon_idr, GFP_KERNEL) == 0))
-		return ERR_PTR(-ENOMEM);
-
-	spin_lock(&idr_lock);
-	err = idr_get_new(&hwmon_idr, NULL, &id);
-	spin_unlock(&idr_lock);
+	int id;
 
-	if (unlikely(err == -EAGAIN))
-		goto again;
-	else if (unlikely(err))
-		return ERR_PTR(err);
+	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
+	if (id < 0)
+		return ERR_PTR(id);
 
-	id = id & MAX_ID_MASK;
 	hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
 			      HWMON_ID_FORMAT, id);
 
-	if (IS_ERR(hwdev)) {
-		spin_lock(&idr_lock);
-		idr_remove(&hwmon_idr, id);
-		spin_unlock(&idr_lock);
-	}
+	if (IS_ERR(hwdev))
+		ida_simple_remove(&hwmon_ida, id);
 
 	return hwdev;
 }
@@ -81,9 +67,7 @@ void hwmon_device_unregister(struct device *dev)
 
 	if (likely(sscanf(dev_name(dev), HWMON_ID_FORMAT, &id) == 1)) {
 		device_unregister(dev);
-		spin_lock(&idr_lock);
-		idr_remove(&hwmon_idr, id);
-		spin_unlock(&idr_lock);
+		ida_simple_remove(&hwmon_ida, id);
 	} else
 		dev_dbg(dev->parent,
 			"hwmon_device_unregister() failed: bad class ID!\n");
-- 
1.7.3.4

Re: [PATCH 2/8] hwmon: convert idr to ida and use ida_simple interface.

[Guenter Roeck]

On Fri, Jul 22, 2011 at 12:41:11PM -0400, Jonathan Cameron wrote:
> hwmon was using an idr with a NULL pointer, so convert to an
> ida which then allows use of Rusty's ida_simple_get.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Looks ok to me.

Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>

> ---
>  drivers/hwmon/hwmon.c |   32 ++++++++------------------------
>  1 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index a61e781..6460487 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -27,8 +27,7 @@
>  
>  static struct class *hwmon_class;
>  
> -static DEFINE_IDR(hwmon_idr);
> -static DEFINE_SPINLOCK(idr_lock);
> +static DEFINE_IDA(hwmon_ida);
>  
>  /**
>   * hwmon_device_register - register w/ hwmon
> @@ -42,30 +41,17 @@ static DEFINE_SPINLOCK(idr_lock);
>  struct device *hwmon_device_register(struct device *dev)
>  {
>  	struct device *hwdev;
> -	int id, err;
> -
> -again:
> -	if (unlikely(idr_pre_get(&hwmon_idr, GFP_KERNEL) == 0))
> -		return ERR_PTR(-ENOMEM);
> -
> -	spin_lock(&idr_lock);
> -	err = idr_get_new(&hwmon_idr, NULL, &id);
> -	spin_unlock(&idr_lock);
> +	int id;
>  
> -	if (unlikely(err == -EAGAIN))
> -		goto again;
> -	else if (unlikely(err))
> -		return ERR_PTR(err);
> +	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
> +	if (id < 0)
> +		return ERR_PTR(id);
>  
> -	id = id & MAX_ID_MASK;
>  	hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
>  			      HWMON_ID_FORMAT, id);
>  
> -	if (IS_ERR(hwdev)) {
> -		spin_lock(&idr_lock);
> -		idr_remove(&hwmon_idr, id);
> -		spin_unlock(&idr_lock);
> -	}
> +	if (IS_ERR(hwdev))
> +		ida_simple_remove(&hwmon_ida, id);
>  
>  	return hwdev;
>  }
> @@ -81,9 +67,7 @@ void hwmon_device_unregister(struct device *dev)
>  
>  	if (likely(sscanf(dev_name(dev), HWMON_ID_FORMAT, &id) == 1)) {
>  		device_unregister(dev);
> -		spin_lock(&idr_lock);
> -		idr_remove(&hwmon_idr, id);
> -		spin_unlock(&idr_lock);
> +		ida_simple_remove(&hwmon_ida, id);
>  	} else
>  		dev_dbg(dev->parent,
>  			"hwmon_device_unregister() failed: bad class ID!\n");
> -- 
> 1.7.3.4
> 

[PATCH 3/8] hwmon: ibmaem: convert idr to ida and use ida_simple_get

[Jonathan Cameron]

A straightforward looking use of idr for a device id.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
Note I have even build tested this one so will definitely
want an Ack from someone who has.

 drivers/hwmon/ibmaem.c |   47 ++++++++---------------------------------------
 1 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/ibmaem.c b/drivers/hwmon/ibmaem.c
index 1a409c5..058e0ae 100644
--- a/drivers/hwmon/ibmaem.c
+++ b/drivers/hwmon/ibmaem.c
@@ -88,8 +88,7 @@
 #define AEM_MIN_POWER_INTERVAL	200
 #define UJ_PER_MJ		1000L
 
-static DEFINE_IDR(aem_idr);
-static DEFINE_SPINLOCK(aem_idr_lock);
+static DEFINE_IDA(aem_ida);
 
 static struct platform_driver aem_driver = {
 	.driver = {
@@ -356,38 +355,6 @@ static void aem_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
 	complete(&data->read_complete);
 }
 
-/* ID functions */
-
-/* Obtain an id */
-static int aem_idr_get(int *id)
-{
-	int i, err;
-
-again:
-	if (unlikely(!idr_pre_get(&aem_idr, GFP_KERNEL)))
-		return -ENOMEM;
-
-	spin_lock(&aem_idr_lock);
-	err = idr_get_new(&aem_idr, NULL, &i);
-	spin_unlock(&aem_idr_lock);
-
-	if (unlikely(err == -EAGAIN))
-		goto again;
-	else if (unlikely(err))
-		return err;
-
-	*id = i & MAX_ID_MASK;
-	return 0;
-}
-
-/* Release an object ID */
-static void aem_idr_put(int id)
-{
-	spin_lock(&aem_idr_lock);
-	idr_remove(&aem_idr, id);
-	spin_unlock(&aem_idr_lock);
-}
-
 /* Sensor support functions */
 
 /* Read a sensor value */
@@ -525,7 +492,7 @@ static void aem_delete(struct aem_data *data)
 	ipmi_destroy_user(data->ipmi.user);
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 	kfree(data);
 }
 
@@ -582,7 +549,8 @@ static int aem_init_aem1_inst(struct aem_ipmi_data *probe, u8 module_handle)
 		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
 
 	/* Create sub-device for this fw instance */
-	if (aem_idr_get(&data->id))
+	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
+	if (data->id < 0)
 		goto id_err;
 
 	data->pdev = platform_device_alloc(DRVNAME, data->id);
@@ -633,7 +601,7 @@ ipmi_err:
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
 dev_err:
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 id_err:
 	kfree(data);
 
@@ -715,7 +683,8 @@ static int aem_init_aem2_inst(struct aem_ipmi_data *probe,
 		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
 
 	/* Create sub-device for this fw instance */
-	if (aem_idr_get(&data->id))
+	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
+	if (data->id < 0)
 		goto id_err;
 
 	data->pdev = platform_device_alloc(DRVNAME, data->id);
@@ -766,7 +735,7 @@ ipmi_err:
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
 dev_err:
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 id_err:
 	kfree(data);
 
-- 
1.7.3.4

Re: [PATCH 3/8] hwmon: ibmaem: convert idr to ida and use ida_simple_get

[Darrick J. Wong]

On Fri, Jul 22, 2011 at 05:41:12PM +0100, Jonathan Cameron wrote:
> A straightforward looking use of idr for a device id.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
> Note I have even build tested this one so will definitely
> want an Ack from someone who has.

Looks ok and seems to build and run ok, so:

Acked-by: Darrick J. Wong <djwong@us.ibm.com>
> 
>  drivers/hwmon/ibmaem.c |   47 ++++++++---------------------------------------
>  1 files changed, 8 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/hwmon/ibmaem.c b/drivers/hwmon/ibmaem.c
> index 1a409c5..058e0ae 100644
> --- a/drivers/hwmon/ibmaem.c
> +++ b/drivers/hwmon/ibmaem.c
> @@ -88,8 +88,7 @@
>  #define AEM_MIN_POWER_INTERVAL	200
>  #define UJ_PER_MJ		1000L
> 
> -static DEFINE_IDR(aem_idr);
> -static DEFINE_SPINLOCK(aem_idr_lock);
> +static DEFINE_IDA(aem_ida);
> 
>  static struct platform_driver aem_driver = {
>  	.driver = {
> @@ -356,38 +355,6 @@ static void aem_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
>  	complete(&data->read_complete);
>  }
> 
> -/* ID functions */
> -
> -/* Obtain an id */
> -static int aem_idr_get(int *id)
> -{
> -	int i, err;
> -
> -again:
> -	if (unlikely(!idr_pre_get(&aem_idr, GFP_KERNEL)))
> -		return -ENOMEM;
> -
> -	spin_lock(&aem_idr_lock);
> -	err = idr_get_new(&aem_idr, NULL, &i);
> -	spin_unlock(&aem_idr_lock);
> -
> -	if (unlikely(err == -EAGAIN))
> -		goto again;
> -	else if (unlikely(err))
> -		return err;
> -
> -	*id = i & MAX_ID_MASK;
> -	return 0;
> -}
> -
> -/* Release an object ID */
> -static void aem_idr_put(int id)
> -{
> -	spin_lock(&aem_idr_lock);
> -	idr_remove(&aem_idr, id);
> -	spin_unlock(&aem_idr_lock);
> -}
> -
>  /* Sensor support functions */
> 
>  /* Read a sensor value */
> @@ -525,7 +492,7 @@ static void aem_delete(struct aem_data *data)
>  	ipmi_destroy_user(data->ipmi.user);
>  	platform_set_drvdata(data->pdev, NULL);
>  	platform_device_unregister(data->pdev);
> -	aem_idr_put(data->id);
> +	ida_simple_remove(&aem_ida, data->id);
>  	kfree(data);
>  }
> 
> @@ -582,7 +549,8 @@ static int aem_init_aem1_inst(struct aem_ipmi_data *probe, u8 module_handle)
>  		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
> 
>  	/* Create sub-device for this fw instance */
> -	if (aem_idr_get(&data->id))
> +	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
> +	if (data->id < 0)
>  		goto id_err;
> 
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
> @@ -633,7 +601,7 @@ ipmi_err:
>  	platform_set_drvdata(data->pdev, NULL);
>  	platform_device_unregister(data->pdev);
>  dev_err:
> -	aem_idr_put(data->id);
> +	ida_simple_remove(&aem_ida, data->id);
>  id_err:
>  	kfree(data);
> 
> @@ -715,7 +683,8 @@ static int aem_init_aem2_inst(struct aem_ipmi_data *probe,
>  		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
> 
>  	/* Create sub-device for this fw instance */
> -	if (aem_idr_get(&data->id))
> +	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
> +	if (data->id < 0)
>  		goto id_err;
> 
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
> @@ -766,7 +735,7 @@ ipmi_err:
>  	platform_set_drvdata(data->pdev, NULL);
>  	platform_device_unregister(data->pdev);
>  dev_err:
> -	aem_idr_put(data->id);
> +	ida_simple_remove(&aem_ida, data->id);
>  id_err:
>  	kfree(data);
> 
> -- 
> 1.7.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] [SCSI] use ida_simple_get and ida_simple remove in place of boilerplate code.

[Jonathan Cameron]

Some mangling of errors was necessary to maintain current interface.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/scsi/sd.c |   32 ++++++++++----------------------
 1 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 953773c..c228b3b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -111,7 +111,6 @@ static void scsi_disk_release(struct device *cdev);
 static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
 static void sd_print_result(struct scsi_disk *, int);
 
-static DEFINE_SPINLOCK(sd_index_lock);
 static DEFINE_IDA(sd_index_ida);
 
 /* This semaphore is used to mediate the 0->1 reference get in the
@@ -2580,22 +2579,15 @@ static int sd_probe(struct device *dev)
 	if (!gd)
 		goto out_free;
 
-	do {
-		if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
-			goto out_put;
-
-		spin_lock(&sd_index_lock);
-		error = ida_get_new(&sd_index_ida, &index);
-		spin_unlock(&sd_index_lock);
-	} while (error == -EAGAIN);
-
-	if (error)
+	index = ida_simple_get(&sd_index_ida, 0, SD_MAX_DISKS, GFP_KERNEL);
+	if (index < 0) {
+		error = index;
+		if (error == -ENOSPC) {
+			sdev_printk(KERN_WARNING, sdp,
+				    "SCSI disk (sd) name space exhausted.\n");
+			error = -ENODEV;
+		}
 		goto out_put;
-
-	if (index >= SD_MAX_DISKS) {
-		error = -ENODEV;
-		sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name space exhausted.\n");
-		goto out_free_index;
 	}
 
 	error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
@@ -2633,9 +2625,7 @@ static int sd_probe(struct device *dev)
 	return 0;
 
  out_free_index:
-	spin_lock(&sd_index_lock);
-	ida_remove(&sd_index_ida, index);
-	spin_unlock(&sd_index_lock);
+	ida_simple_remove(&sd_index_ida, index);
  out_put:
 	put_disk(gd);
  out_free:
@@ -2691,9 +2681,7 @@ static void scsi_disk_release(struct device *dev)
 	struct scsi_disk *sdkp = to_scsi_disk(dev);
 	struct gendisk *disk = sdkp->disk;
 	
-	spin_lock(&sd_index_lock);
-	ida_remove(&sd_index_ida, sdkp->index);
-	spin_unlock(&sd_index_lock);
+	ida_simple_remove(&sd_index_ida, sdkp->index);
 
 	disk->private_data = NULL;
 	put_disk(disk);
-- 
1.7.3.4

[PATCH 5/8] drm/vmwgfx: use ida_simple_get for id allocation.

[Jonathan Cameron]

Some messing with error codes to return 0 on out id's and match
current situation.  Is this necessary? Looks a touch 'interesting'.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |   34 ++++++-------------------
 1 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index ac6e0d1..d51577b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -37,7 +37,6 @@
 #include <linux/kernel.h>
 
 struct vmwgfx_gmrid_man {
-	spinlock_t lock;
 	struct ida gmr_ida;
 	uint32_t max_gmr_ids;
 };
@@ -49,34 +48,20 @@ static int vmw_gmrid_man_get_node(struct ttm_mem_type_manager *man,
 {
 	struct vmwgfx_gmrid_man *gman =
 		(struct vmwgfx_gmrid_man *)man->priv;
-	int ret;
 	int id;
 
 	mem->mm_node = NULL;
 
-	do {
-		if (unlikely(ida_pre_get(&gman->gmr_ida, GFP_KERNEL) == 0))
-			return -ENOMEM;
-
-		spin_lock(&gman->lock);
-		ret = ida_get_new(&gman->gmr_ida, &id);
-
-		if (unlikely(ret == 0 && id >= gman->max_gmr_ids)) {
-			ida_remove(&gman->gmr_ida, id);
-			spin_unlock(&gman->lock);
+	id = ida_simple_get(&gman->gmr_ida, 0, gman->max_gmr_ids, GFP_KERNEL);
+	if (id < 0) {
+		if (id == -ENOSPC)
 			return 0;
-		}
-
-		spin_unlock(&gman->lock);
-
-	} while (ret == -EAGAIN);
-
-	if (likely(ret == 0)) {
-		mem->mm_node = gman;
-		mem->start = id;
+		return id;
 	}
+	mem->mm_node = gman;
+	mem->start = id;
 
-	return ret;
+	return 0;
 }
 
 static void vmw_gmrid_man_put_node(struct ttm_mem_type_manager *man,
@@ -86,9 +71,7 @@ static void vmw_gmrid_man_put_node(struct ttm_mem_type_manager *man,
 		(struct vmwgfx_gmrid_man *)man->priv;
 
 	if (mem->mm_node) {
-		spin_lock(&gman->lock);
-		ida_remove(&gman->gmr_ida, mem->start);
-		spin_unlock(&gman->lock);
+		ida_simple_remove(&gman->gmr_ida, mem->start);
 		mem->mm_node = NULL;
 	}
 }
@@ -102,7 +85,6 @@ static int vmw_gmrid_man_init(struct ttm_mem_type_manager *man,
 	if (unlikely(gman == NULL))
 		return -ENOMEM;
 
-	spin_lock_init(&gman->lock);
 	ida_init(&gman->gmr_ida);
 	gman->max_gmr_ids = p_size;
 	man->priv = (void *) gman;
-- 
1.7.3.4

[PATCH 6/8] [SCSI] osduld: use ida_simple_get to handle id.

[Jonathan Cameron]

This does involve additional use of the spin lock in idr.c.
Is this an issue?

Also, some error mangling was needed to keep the interface
the same.  Does this matter or can we return -ENOSPC instead
of -EBUSY?

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/scsi/osd/osd_uld.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index b31a8e3..fa849bd 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -387,7 +387,7 @@ static void __remove(struct device *dev)
 
 	if (oud->disk)
 		put_disk(oud->disk);
-	ida_remove(&osd_minor_ida, oud->minor);
+	ida_simple_remove(&osd_minor_ida, oud->minor);
 
 	kfree(oud);
 }
@@ -403,18 +403,12 @@ static int osd_probe(struct device *dev)
 	if (scsi_device->type != TYPE_OSD)
 		return -ENODEV;
 
-	do {
-		if (!ida_pre_get(&osd_minor_ida, GFP_KERNEL))
-			return -ENODEV;
-
-		error = ida_get_new(&osd_minor_ida, &minor);
-	} while (error == -EAGAIN);
-
-	if (error)
-		return error;
-	if (minor >= SCSI_OSD_MAX_MINOR) {
-		error = -EBUSY;
-		goto err_retract_minor;
+	minor = ida_simple_get(&osd_minor_ida, 0,
+			       SCSI_OSD_MAX_MINOR, GFP_KERNEL);
+	if (minor < 0) {
+		if (minor == -ENOSPC)
+			return -EBUSY;
+		return minor;
 	}
 
 	error = -ENOMEM;
@@ -491,7 +485,7 @@ err_free_osd:
 	dev_set_drvdata(dev, NULL);
 	kfree(oud);
 err_retract_minor:
-	ida_remove(&osd_minor_ida, minor);
+	ida_simple_remove(&osd_minor_ida, minor);
 	return error;
 }
 
-- 
1.7.3.4

Re: [osd-dev] [PATCH 6/8] [SCSI] osduld: use ida_simple_get to handle id.

[Boaz Harrosh]

On 07/22/2011 09:41 AM, Jonathan Cameron wrote:
> This does involve additional use of the spin lock in idr.c.
> Is this an issue?
> 

Actually it looks like a bug fix. I had a TODO: to add one.

> Also, some error mangling was needed to keep the interface
> the same.  Does this matter or can we return -ENOSPC instead
> of -EBUSY?
> 

Na. -ENOSPC is just fine. All the osd Application just check for
"any error" not a specific one.

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Ack-by: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  drivers/scsi/osd/osd_uld.c |   22 ++++++++--------------
>  1 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index b31a8e3..fa849bd 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -387,7 +387,7 @@ static void __remove(struct device *dev)
>  
>  	if (oud->disk)
>  		put_disk(oud->disk);
> -	ida_remove(&osd_minor_ida, oud->minor);
> +	ida_simple_remove(&osd_minor_ida, oud->minor);
>  
>  	kfree(oud);
>  }
> @@ -403,18 +403,12 @@ static int osd_probe(struct device *dev)
>  	if (scsi_device->type != TYPE_OSD)
>  		return -ENODEV;
>  
> -	do {
> -		if (!ida_pre_get(&osd_minor_ida, GFP_KERNEL))
> -			return -ENODEV;
> -
> -		error = ida_get_new(&osd_minor_ida, &minor);
> -	} while (error == -EAGAIN);
> -
> -	if (error)
> -		return error;
> -	if (minor >= SCSI_OSD_MAX_MINOR) {
> -		error = -EBUSY;
> -		goto err_retract_minor;
> +	minor = ida_simple_get(&osd_minor_ida, 0,
> +			       SCSI_OSD_MAX_MINOR, GFP_KERNEL);
> +	if (minor < 0) {
> +		if (minor == -ENOSPC)
> +			return -EBUSY;

Just drop the translation is fine as well

> +		return minor;
>  	}
>  
>  	error = -ENOMEM;
> @@ -491,7 +485,7 @@ err_free_osd:
>  	dev_set_drvdata(dev, NULL);
>  	kfree(oud);
>  err_retract_minor:
> -	ida_remove(&osd_minor_ida, minor);
> +	ida_simple_remove(&osd_minor_ida, minor);
>  	return error;
>  }
>  

Re: [osd-dev] [PATCH 6/8] [SCSI] osduld: use ida_simple_get to handle id.

[Jonathan Cameron]

On 07/22/11 22:47, Boaz Harrosh wrote:
> On 07/22/2011 09:41 AM, Jonathan Cameron wrote:
>> This does involve additional use of the spin lock in idr.c.
>> Is this an issue?
>>
> 
> Actually it looks like a bug fix. I had a TODO: to add one.
> 
>> Also, some error mangling was needed to keep the interface
>> the same.  Does this matter or can we return -ENOSPC instead
>> of -EBUSY?
>>
> 
> Na. -ENOSPC is just fine. All the osd Application just check for
> "any error" not a specific one.
Cool, I've scrapped the error mangling and added your acked-by
+ removed questions from commit message as you've answered them!

Thanks,

> 
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Ack-by: Boaz Harrosh <bharrosh@panasas.com>
> 
>> ---
>>  drivers/scsi/osd/osd_uld.c |   22 ++++++++--------------
>>  1 files changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
>> index b31a8e3..fa849bd 100644
>> --- a/drivers/scsi/osd/osd_uld.c
>> +++ b/drivers/scsi/osd/osd_uld.c
>> @@ -387,7 +387,7 @@ static void __remove(struct device *dev)
>>  
>>  	if (oud->disk)
>>  		put_disk(oud->disk);
>> -	ida_remove(&osd_minor_ida, oud->minor);
>> +	ida_simple_remove(&osd_minor_ida, oud->minor);
>>  
>>  	kfree(oud);
>>  }
>> @@ -403,18 +403,12 @@ static int osd_probe(struct device *dev)
>>  	if (scsi_device->type != TYPE_OSD)
>>  		return -ENODEV;
>>  
>> -	do {
>> -		if (!ida_pre_get(&osd_minor_ida, GFP_KERNEL))
>> -			return -ENODEV;
>> -
>> -		error = ida_get_new(&osd_minor_ida, &minor);
>> -	} while (error == -EAGAIN);
>> -
>> -	if (error)
>> -		return error;
>> -	if (minor >= SCSI_OSD_MAX_MINOR) {
>> -		error = -EBUSY;
>> -		goto err_retract_minor;
>> +	minor = ida_simple_get(&osd_minor_ida, 0,
>> +			       SCSI_OSD_MAX_MINOR, GFP_KERNEL);
>> +	if (minor < 0) {
>> +		if (minor == -ENOSPC)
>> +			return -EBUSY;
> 
> Just drop the translation is fine as well
> 
>> +		return minor;
>>  	}
>>  
>>  	error = -ENOMEM;
>> @@ -491,7 +485,7 @@ err_free_osd:
>>  	dev_set_drvdata(dev, NULL);
>>  	kfree(oud);
>>  err_retract_minor:
>> -	ida_remove(&osd_minor_ida, minor);
>> +	ida_simple_remove(&osd_minor_ida, minor);
>>  	return error;
>>  }
>>  
> 
> 

[PATCH 7/8] w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.

[Jonathan Cameron]

Straight forward.  As an aside, the ida_init calls are not needed as far
as I can see needed. (DEFINE_IDA does the same already).

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/w1/slaves/w1_ds2760.c |   48 +++++-----------------------------------
 drivers/w1/slaves/w1_ds2780.c |   48 +++++-----------------------------------
 2 files changed, 12 insertions(+), 84 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2760.c b/drivers/w1/slaves/w1_ds2760.c
index 483d451..5754c9a 100644
--- a/drivers/w1/slaves/w1_ds2760.c
+++ b/drivers/w1/slaves/w1_ds2760.c
@@ -114,43 +114,7 @@ static struct bin_attribute w1_ds2760_bin_attr = {
 	.read = w1_ds2760_read_bin,
 };
 
-static DEFINE_IDR(bat_idr);
-static DEFINE_MUTEX(bat_idr_lock);
-
-static int new_bat_id(void)
-{
-	int ret;
-
-	while (1) {
-		int id;
-
-		ret = idr_pre_get(&bat_idr, GFP_KERNEL);
-		if (ret == 0)
-			return -ENOMEM;
-
-		mutex_lock(&bat_idr_lock);
-		ret = idr_get_new(&bat_idr, NULL, &id);
-		mutex_unlock(&bat_idr_lock);
-
-		if (ret == 0) {
-			ret = id & MAX_ID_MASK;
-			break;
-		} else if (ret == -EAGAIN) {
-			continue;
-		} else {
-			break;
-		}
-	}
-
-	return ret;
-}
-
-static void release_bat_id(int id)
-{
-	mutex_lock(&bat_idr_lock);
-	idr_remove(&bat_idr, id);
-	mutex_unlock(&bat_idr_lock);
-}
+static DEFINE_IDA(bat_ida);
 
 static int w1_ds2760_add_slave(struct w1_slave *sl)
 {
@@ -158,7 +122,7 @@ static int w1_ds2760_add_slave(struct w1_slave *sl)
 	int id;
 	struct platform_device *pdev;
 
-	id = new_bat_id();
+	id = ida_simple_get(&bat_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		ret = id;
 		goto noid;
@@ -187,7 +151,7 @@ bin_attr_failed:
 pdev_add_failed:
 	platform_device_unregister(pdev);
 pdev_alloc_failed:
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 noid:
 success:
 	return ret;
@@ -199,7 +163,7 @@ static void w1_ds2760_remove_slave(struct w1_slave *sl)
 	int id = pdev->id;
 
 	platform_device_unregister(pdev);
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 	sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2760_bin_attr);
 }
 
@@ -217,14 +181,14 @@ static int __init w1_ds2760_init(void)
 {
 	printk(KERN_INFO "1-Wire driver for the DS2760 battery monitor "
 	       " chip  - (c) 2004-2005, Szabolcs Gyurko\n");
-	idr_init(&bat_idr);
+	ida_init(&bat_ida);
 	return w1_register_family(&w1_ds2760_family);
 }
 
 static void __exit w1_ds2760_exit(void)
 {
 	w1_unregister_family(&w1_ds2760_family);
-	idr_destroy(&bat_idr);
+	ida_destroy(&bat_ida);
 }
 
 EXPORT_SYMBOL(w1_ds2760_read);
diff --git a/drivers/w1/slaves/w1_ds2780.c b/drivers/w1/slaves/w1_ds2780.c
index 274c8f3..a134b38 100644
--- a/drivers/w1/slaves/w1_ds2780.c
+++ b/drivers/w1/slaves/w1_ds2780.c
@@ -99,43 +99,7 @@ static struct bin_attribute w1_ds2780_bin_attr = {
 	.read = w1_ds2780_read_bin,
 };
 
-static DEFINE_IDR(bat_idr);
-static DEFINE_MUTEX(bat_idr_lock);
-
-static int new_bat_id(void)
-{
-	int ret;
-
-	while (1) {
-		int id;
-
-		ret = idr_pre_get(&bat_idr, GFP_KERNEL);
-		if (ret == 0)
-			return -ENOMEM;
-
-		mutex_lock(&bat_idr_lock);
-		ret = idr_get_new(&bat_idr, NULL, &id);
-		mutex_unlock(&bat_idr_lock);
-
-		if (ret == 0) {
-			ret = id & MAX_ID_MASK;
-			break;
-		} else if (ret == -EAGAIN) {
-			continue;
-		} else {
-			break;
-		}
-	}
-
-	return ret;
-}
-
-static void release_bat_id(int id)
-{
-	mutex_lock(&bat_idr_lock);
-	idr_remove(&bat_idr, id);
-	mutex_unlock(&bat_idr_lock);
-}
+static DEFINE_IDA(bat_ida);
 
 static int w1_ds2780_add_slave(struct w1_slave *sl)
 {
@@ -143,7 +107,7 @@ static int w1_ds2780_add_slave(struct w1_slave *sl)
 	int id;
 	struct platform_device *pdev;
 
-	id = new_bat_id();
+	id = ida_simple_get(&bat_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		ret = id;
 		goto noid;
@@ -172,7 +136,7 @@ bin_attr_failed:
 pdev_add_failed:
 	platform_device_unregister(pdev);
 pdev_alloc_failed:
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 noid:
 	return ret;
 }
@@ -183,7 +147,7 @@ static void w1_ds2780_remove_slave(struct w1_slave *sl)
 	int id = pdev->id;
 
 	platform_device_unregister(pdev);
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 	sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2780_bin_attr);
 }
 
@@ -199,14 +163,14 @@ static struct w1_family w1_ds2780_family = {
 
 static int __init w1_ds2780_init(void)
 {
-	idr_init(&bat_idr);
+	ida_init(&bat_ida);
 	return w1_register_family(&w1_ds2780_family);
 }
 
 static void __exit w1_ds2780_exit(void)
 {
 	w1_unregister_family(&w1_ds2780_family);
-	idr_destroy(&bat_idr);
+	ida_destroy(&bat_ida);
 }
 
 module_init(w1_ds2780_init);
-- 
1.7.3.4

[PATCH 8/8] rtc: class idr converted to ida and ida_simple_get used.

[Jonathan Cameron]

This is the one use of an ida that doesn't retry on receiving -EAGAIN.
I'm assuming do so will cause no harm and may help on a rare occasion.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/rtc/class.c |   32 +++++++++-----------------------
 1 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 4194e59..68ae46f 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -21,16 +21,13 @@
 #include "rtc-core.h"
 
 
-static DEFINE_IDR(rtc_idr);
-static DEFINE_MUTEX(idr_lock);
+static DEFINE_IDA(rtc_ida);
 struct class *rtc_class;
 
 static void rtc_device_release(struct device *dev)
 {
 	struct rtc_device *rtc = to_rtc_device(dev);
-	mutex_lock(&idr_lock);
-	idr_remove(&rtc_idr, rtc->id);
-	mutex_unlock(&idr_lock);
+	ida_simple_remove(&rtc_ida, rtc->id);
 	kfree(rtc);
 }
 
@@ -115,25 +112,16 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 	struct rtc_wkalrm alrm;
 	int id, err;
 
-	if (idr_pre_get(&rtc_idr, GFP_KERNEL) == 0) {
-		err = -ENOMEM;
+	id = ida_simple_get(&rtc_ida, 0, 0, GFP_KERNEL);
+	if (id < 0) {
+		err = id;
 		goto exit;
 	}
 
-
-	mutex_lock(&idr_lock);
-	err = idr_get_new(&rtc_idr, NULL, &id);
-	mutex_unlock(&idr_lock);
-
-	if (err < 0)
-		goto exit;
-
-	id = id & MAX_ID_MASK;
-
 	rtc = kzalloc(sizeof(struct rtc_device), GFP_KERNEL);
 	if (rtc == NULL) {
 		err = -ENOMEM;
-		goto exit_idr;
+		goto exit_ida;
 	}
 
 	rtc->id = id;
@@ -191,10 +179,8 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 exit_kfree:
 	kfree(rtc);
 
-exit_idr:
-	mutex_lock(&idr_lock);
-	idr_remove(&rtc_idr, id);
-	mutex_unlock(&idr_lock);
+exit_ida:
+	ida_simple_remove(&rtc_ida, id);
 
 exit:
 	dev_err(dev, "rtc core: unable to register %s, err = %d\n",
@@ -245,7 +231,7 @@ static void __exit rtc_exit(void)
 {
 	rtc_dev_exit();
 	class_destroy(rtc_class);
-	idr_destroy(&rtc_idr);
+	ida_destroy(&rtc_ida);
 }
 
 subsys_initcall(rtc_init);
-- 
1.7.3.4

Re: [PATCH 8/8] rtc: class idr converted to ida and ida_simple_get used.

[Tejun Heo]

On Fri, Jul 22, 2011 at 05:41:17PM +0100, Jonathan Cameron wrote:
> This is the one use of an ida that doesn't retry on receiving -EAGAIN.
> I'm assuming do so will cause no harm and may help on a rare occasion.
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

And all conversions seem correct to me.  Please feel free to add

Reviewed-by: Tejun Heo <tj@kernel.org>

Thank you.

-- 
tejun

[PATCH 0/8] RFC: Introduce ida_simple interfaces and use them.

[Jonathan Cameron]

Hi All,

Firstly sorry for the huge cc list, but this series does get about.

This series came out of two separate lkml threads:

https://lkml.org/lkml/2011/7/13/64
http://thread.gmane.org/gmane.linux.kernel/1148513/focus=74236

Basically Rusty and I both got annoyed with yet more instances
of the same cut and past ida allocation code for the very simple
case of wanting a unique id for some device. Tejun helpfully
joined the two threads up.

Anyhow, Rusty proposed more or less the first patch here (with a name
change requested by Tejun applied.)

The others patches are some of the more obvious looking cases for
applying this code.  My original reason was for IIO but those patches will
have to wait a little while for some prior changes to merge.
For reference, it saves about 40 lines there so I'm happy ;)

There are a number of other cases in tree that can be cleaned up
in a second series once these are sorted.

There are a couple of cases in here where I've carefully mangled
the error codes to keep consistent with the existing code.  Please
can people who know those subsystems well decide on whether the
mangling is necessary.

All comments welcome. I expect I've broken at least one driver
doing this so please take a close look.

Rusty, Tejun. I kept your sign of an ack for the first patch. Could
you quickly verify that's fine?

Thanks,

Jonathan

Jonathan Cameron (7):
  hwmon: convert idr to ida and use ida_simple interface.
  hwmon: ibmaem: convert idr to ida and use ida_simple_get
  [SCSI] use ida_simple_get and ida_simple remove in place of
    boilerplate code.
  drm/vmwgfx: use ida_simple_get for id allocation.
  [SCSI] osduld: use ida_simple_get to handle id.
  w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.
  rtc: class idr converted to ida and ida_simple_get used.

Rusty Russell (1):
  ida: Simplified functions for id allocation.

 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |   34 +++----------
 drivers/hwmon/hwmon.c                         |   32 +++---------
 drivers/hwmon/ibmaem.c                        |   47 +++--------------
 drivers/rtc/class.c                           |   32 +++---------
 drivers/scsi/osd/osd_uld.c                    |   22 +++-----
 drivers/scsi/sd.c                             |   32 ++++--------
 drivers/w1/slaves/w1_ds2760.c                 |   48 ++---------------
 drivers/w1/slaves/w1_ds2780.c                 |   48 ++---------------
 include/linux/idr.h                           |    4 ++
 lib/idr.c                                     |   67 +++++++++++++++++++++++++
 10 files changed, 134 insertions(+), 232 deletions(-)

-- 
1.7.3.4

[PATCH 1/8] ida: Simplified functions for id allocation.

[Jonathan Cameron]

From: Rusty Russell <rusty@rustcorp.com.au>

The current hyper-optimized functions are overkill if you simply want
to allocate an id for a device.  Create versions which use an internal
lock.

Thanks to Tejun for feedback.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 include/linux/idr.h |    4 +++
 lib/idr.c           |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/linux/idr.h b/include/linux/idr.h
index 13a801f..255491c 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -146,6 +146,10 @@ void ida_remove(struct ida *ida, int id);
 void ida_destroy(struct ida *ida);
 void ida_init(struct ida *ida);
 
+int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
+		   gfp_t gfp_mask);
+void ida_simple_remove(struct ida *ida, unsigned int id);
+
 void __init idr_init_cache(void);
 
 #endif /* __IDR_H__ */
diff --git a/lib/idr.c b/lib/idr.c
index e15502e..db040ce 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -34,8 +34,10 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/idr.h>
+#include <linux/spinlock.h>
 
 static struct kmem_cache *idr_layer_cache;
+static DEFINE_SPINLOCK(simple_ida_lock);
 
 static struct idr_layer *get_from_free_list(struct idr *idp)
 {
@@ -926,6 +928,71 @@ void ida_destroy(struct ida *ida)
 EXPORT_SYMBOL(ida_destroy);
 
 /**
+ * ida_simple_get - get a new id.
+ * @ida: the (initialized) ida.
+ * @start: the minimum id (inclusive, < 0x8000000)
+ * @end: the maximum id (exclusive, < 0x8000000 or 0)
+ * @gfp_mask: memory allocation flags
+ *
+ * Allocates an id in the range start <= id < end, or returns -ENOSPC.
+ * On memory allocation failure, returns -ENOMEM.
+ *
+ * Use ida_simple_remove() to get rid of an id.
+ */
+int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
+		   gfp_t gfp_mask)
+{
+	int ret, id;
+	unsigned int max;
+
+	BUG_ON((int)start < 0);
+	BUG_ON((int)end < 0);
+
+	if (end == 0)
+		max = 0x80000000;
+	else {
+		BUG_ON(end < start);
+		max = end - 1;
+	}
+
+again:
+	if (!ida_pre_get(ida, gfp_mask))
+		return -ENOMEM;
+
+	spin_lock(&simple_ida_lock);
+	ret = ida_get_new_above(ida, start, &id);
+	if (!ret) {
+		if (id > max) {
+			ida_remove(ida, id);
+			ret = -ENOSPC;
+		} else {
+			ret = id;
+		}
+	}
+	spin_unlock(&simple_ida_lock);
+
+	if (unlikely(ret == -EAGAIN))
+		goto again;
+
+	return ret;
+}
+EXPORT_SYMBOL(ida_simple_get);
+
+/**
+ * ida_simple_remove - remove an allocated id.
+ * @ida: the (initialized) ida.
+ * @id: the id returned by ida_simple_get.
+ */
+void ida_simple_remove(struct ida *ida, unsigned int id)
+{
+	BUG_ON((int)id < 0);
+	spin_lock(&simple_ida_lock);
+	ida_remove(ida, id);
+	spin_unlock(&simple_ida_lock);
+}
+EXPORT_SYMBOL(ida_simple_remove);
+
+/**
  * ida_init - initialize ida handle
  * @ida:	ida handle
  *
-- 
1.7.3.4

Re: [PATCH 1/8] ida: Simplified functions for id allocation.

[Tejun Heo]

On Fri, Jul 22, 2011 at 05:41:19PM +0100, Jonathan Cameron wrote:
> From: Rusty Russell <rusty@rustcorp.com.au>
> 
> The current hyper-optimized functions are overkill if you simply want
> to allocate an id for a device.  Create versions which use an internal
> lock.
> 
> Thanks to Tejun for feedback.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Acked-by: Tejun Heo <tj@kernel.org>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

Yeap, still looks good to me.

Thank you.

-- 
tejun

[PATCH 2/8] hwmon: convert idr to ida and use ida_simple interface.

[Jonathan Cameron]

hwmon was using an idr with a NULL pointer, so convert to an
ida which then allows use of Rusty's ida_simple_get.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/hwmon/hwmon.c |   32 ++++++++------------------------
 1 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index a61e781..6460487 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -27,8 +27,7 @@
 
 static struct class *hwmon_class;
 
-static DEFINE_IDR(hwmon_idr);
-static DEFINE_SPINLOCK(idr_lock);
+static DEFINE_IDA(hwmon_ida);
 
 /**
  * hwmon_device_register - register w/ hwmon
@@ -42,30 +41,17 @@ static DEFINE_SPINLOCK(idr_lock);
 struct device *hwmon_device_register(struct device *dev)
 {
 	struct device *hwdev;
-	int id, err;
-
-again:
-	if (unlikely(idr_pre_get(&hwmon_idr, GFP_KERNEL) == 0))
-		return ERR_PTR(-ENOMEM);
-
-	spin_lock(&idr_lock);
-	err = idr_get_new(&hwmon_idr, NULL, &id);
-	spin_unlock(&idr_lock);
+	int id;
 
-	if (unlikely(err == -EAGAIN))
-		goto again;
-	else if (unlikely(err))
-		return ERR_PTR(err);
+	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
+	if (id < 0)
+		return ERR_PTR(id);
 
-	id = id & MAX_ID_MASK;
 	hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
 			      HWMON_ID_FORMAT, id);
 
-	if (IS_ERR(hwdev)) {
-		spin_lock(&idr_lock);
-		idr_remove(&hwmon_idr, id);
-		spin_unlock(&idr_lock);
-	}
+	if (IS_ERR(hwdev))
+		ida_simple_remove(&hwmon_ida, id);
 
 	return hwdev;
 }
@@ -81,9 +67,7 @@ void hwmon_device_unregister(struct device *dev)
 
 	if (likely(sscanf(dev_name(dev), HWMON_ID_FORMAT, &id) == 1)) {
 		device_unregister(dev);
-		spin_lock(&idr_lock);
-		idr_remove(&hwmon_idr, id);
-		spin_unlock(&idr_lock);
+		ida_simple_remove(&hwmon_ida, id);
 	} else
 		dev_dbg(dev->parent,
 			"hwmon_device_unregister() failed: bad class ID!\n");
-- 
1.7.3.4

[PATCH 3/8] hwmon: ibmaem: convert idr to ida and use ida_simple_get

[Jonathan Cameron]

A straightforward looking use of idr for a device id.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
Note I have even build tested this one so will definitely
want an Ack from someone who has.

 drivers/hwmon/ibmaem.c |   47 ++++++++---------------------------------------
 1 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/ibmaem.c b/drivers/hwmon/ibmaem.c
index 1a409c5..058e0ae 100644
--- a/drivers/hwmon/ibmaem.c
+++ b/drivers/hwmon/ibmaem.c
@@ -88,8 +88,7 @@
 #define AEM_MIN_POWER_INTERVAL	200
 #define UJ_PER_MJ		1000L
 
-static DEFINE_IDR(aem_idr);
-static DEFINE_SPINLOCK(aem_idr_lock);
+static DEFINE_IDA(aem_ida);
 
 static struct platform_driver aem_driver = {
 	.driver = {
@@ -356,38 +355,6 @@ static void aem_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
 	complete(&data->read_complete);
 }
 
-/* ID functions */
-
-/* Obtain an id */
-static int aem_idr_get(int *id)
-{
-	int i, err;
-
-again:
-	if (unlikely(!idr_pre_get(&aem_idr, GFP_KERNEL)))
-		return -ENOMEM;
-
-	spin_lock(&aem_idr_lock);
-	err = idr_get_new(&aem_idr, NULL, &i);
-	spin_unlock(&aem_idr_lock);
-
-	if (unlikely(err == -EAGAIN))
-		goto again;
-	else if (unlikely(err))
-		return err;
-
-	*id = i & MAX_ID_MASK;
-	return 0;
-}
-
-/* Release an object ID */
-static void aem_idr_put(int id)
-{
-	spin_lock(&aem_idr_lock);
-	idr_remove(&aem_idr, id);
-	spin_unlock(&aem_idr_lock);
-}
-
 /* Sensor support functions */
 
 /* Read a sensor value */
@@ -525,7 +492,7 @@ static void aem_delete(struct aem_data *data)
 	ipmi_destroy_user(data->ipmi.user);
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 	kfree(data);
 }
 
@@ -582,7 +549,8 @@ static int aem_init_aem1_inst(struct aem_ipmi_data *probe, u8 module_handle)
 		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
 
 	/* Create sub-device for this fw instance */
-	if (aem_idr_get(&data->id))
+	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
+	if (data->id < 0)
 		goto id_err;
 
 	data->pdev = platform_device_alloc(DRVNAME, data->id);
@@ -633,7 +601,7 @@ ipmi_err:
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
 dev_err:
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 id_err:
 	kfree(data);
 
@@ -715,7 +683,8 @@ static int aem_init_aem2_inst(struct aem_ipmi_data *probe,
 		data->power_period[i] = AEM_DEFAULT_POWER_INTERVAL;
 
 	/* Create sub-device for this fw instance */
-	if (aem_idr_get(&data->id))
+	data->id = ida_simple_get(&aem_ida, 0, 0, GFP_KERNEL);
+	if (data->id < 0)
 		goto id_err;
 
 	data->pdev = platform_device_alloc(DRVNAME, data->id);
@@ -766,7 +735,7 @@ ipmi_err:
 	platform_set_drvdata(data->pdev, NULL);
 	platform_device_unregister(data->pdev);
 dev_err:
-	aem_idr_put(data->id);
+	ida_simple_remove(&aem_ida, data->id);
 id_err:
 	kfree(data);
 
-- 
1.7.3.4

[PATCH 4/8] [SCSI] use ida_simple_get and ida_simple remove in place of boilerplate code.

[Jonathan Cameron]

Some mangling of errors was necessary to maintain current interface.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/scsi/sd.c |   32 ++++++++++----------------------
 1 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 953773c..c228b3b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -111,7 +111,6 @@ static void scsi_disk_release(struct device *cdev);
 static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
 static void sd_print_result(struct scsi_disk *, int);
 
-static DEFINE_SPINLOCK(sd_index_lock);
 static DEFINE_IDA(sd_index_ida);
 
 /* This semaphore is used to mediate the 0->1 reference get in the
@@ -2580,22 +2579,15 @@ static int sd_probe(struct device *dev)
 	if (!gd)
 		goto out_free;
 
-	do {
-		if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
-			goto out_put;
-
-		spin_lock(&sd_index_lock);
-		error = ida_get_new(&sd_index_ida, &index);
-		spin_unlock(&sd_index_lock);
-	} while (error == -EAGAIN);
-
-	if (error)
+	index = ida_simple_get(&sd_index_ida, 0, SD_MAX_DISKS, GFP_KERNEL);
+	if (index < 0) {
+		error = index;
+		if (error == -ENOSPC) {
+			sdev_printk(KERN_WARNING, sdp,
+				    "SCSI disk (sd) name space exhausted.\n");
+			error = -ENODEV;
+		}
 		goto out_put;
-
-	if (index >= SD_MAX_DISKS) {
-		error = -ENODEV;
-		sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name space exhausted.\n");
-		goto out_free_index;
 	}
 
 	error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
@@ -2633,9 +2625,7 @@ static int sd_probe(struct device *dev)
 	return 0;
 
  out_free_index:
-	spin_lock(&sd_index_lock);
-	ida_remove(&sd_index_ida, index);
-	spin_unlock(&sd_index_lock);
+	ida_simple_remove(&sd_index_ida, index);
  out_put:
 	put_disk(gd);
  out_free:
@@ -2691,9 +2681,7 @@ static void scsi_disk_release(struct device *dev)
 	struct scsi_disk *sdkp = to_scsi_disk(dev);
 	struct gendisk *disk = sdkp->disk;
 	
-	spin_lock(&sd_index_lock);
-	ida_remove(&sd_index_ida, sdkp->index);
-	spin_unlock(&sd_index_lock);
+	ida_simple_remove(&sd_index_ida, sdkp->index);
 
 	disk->private_data = NULL;
 	put_disk(disk);
-- 
1.7.3.4

[PATCH 5/8] drm/vmwgfx: use ida_simple_get for id allocation.

[Jonathan Cameron]

Some messing with error codes to return 0 on out id's and match
current situation.  Is this necessary? Looks a touch 'interesting'.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |   34 ++++++-------------------
 1 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index ac6e0d1..d51577b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -37,7 +37,6 @@
 #include <linux/kernel.h>
 
 struct vmwgfx_gmrid_man {
-	spinlock_t lock;
 	struct ida gmr_ida;
 	uint32_t max_gmr_ids;
 };
@@ -49,34 +48,20 @@ static int vmw_gmrid_man_get_node(struct ttm_mem_type_manager *man,
 {
 	struct vmwgfx_gmrid_man *gman =
 		(struct vmwgfx_gmrid_man *)man->priv;
-	int ret;
 	int id;
 
 	mem->mm_node = NULL;
 
-	do {
-		if (unlikely(ida_pre_get(&gman->gmr_ida, GFP_KERNEL) == 0))
-			return -ENOMEM;
-
-		spin_lock(&gman->lock);
-		ret = ida_get_new(&gman->gmr_ida, &id);
-
-		if (unlikely(ret == 0 && id >= gman->max_gmr_ids)) {
-			ida_remove(&gman->gmr_ida, id);
-			spin_unlock(&gman->lock);
+	id = ida_simple_get(&gman->gmr_ida, 0, gman->max_gmr_ids, GFP_KERNEL);
+	if (id < 0) {
+		if (id == -ENOSPC)
 			return 0;
-		}
-
-		spin_unlock(&gman->lock);
-
-	} while (ret == -EAGAIN);
-
-	if (likely(ret == 0)) {
-		mem->mm_node = gman;
-		mem->start = id;
+		return id;
 	}
+	mem->mm_node = gman;
+	mem->start = id;
 
-	return ret;
+	return 0;
 }
 
 static void vmw_gmrid_man_put_node(struct ttm_mem_type_manager *man,
@@ -86,9 +71,7 @@ static void vmw_gmrid_man_put_node(struct ttm_mem_type_manager *man,
 		(struct vmwgfx_gmrid_man *)man->priv;
 
 	if (mem->mm_node) {
-		spin_lock(&gman->lock);
-		ida_remove(&gman->gmr_ida, mem->start);
-		spin_unlock(&gman->lock);
+		ida_simple_remove(&gman->gmr_ida, mem->start);
 		mem->mm_node = NULL;
 	}
 }
@@ -102,7 +85,6 @@ static int vmw_gmrid_man_init(struct ttm_mem_type_manager *man,
 	if (unlikely(gman == NULL))
 		return -ENOMEM;
 
-	spin_lock_init(&gman->lock);
 	ida_init(&gman->gmr_ida);
 	gman->max_gmr_ids = p_size;
 	man->priv = (void *) gman;
-- 
1.7.3.4

[PATCH 6/8] [SCSI] osduld: use ida_simple_get to handle id.

[Jonathan Cameron]

This does involve additional use of the spin lock in idr.c.
Is this an issue?

Also, some error mangling was needed to keep the interface
the same.  Does this matter or can we return -ENOSPC instead
of -EBUSY?

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/scsi/osd/osd_uld.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index b31a8e3..fa849bd 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -387,7 +387,7 @@ static void __remove(struct device *dev)
 
 	if (oud->disk)
 		put_disk(oud->disk);
-	ida_remove(&osd_minor_ida, oud->minor);
+	ida_simple_remove(&osd_minor_ida, oud->minor);
 
 	kfree(oud);
 }
@@ -403,18 +403,12 @@ static int osd_probe(struct device *dev)
 	if (scsi_device->type != TYPE_OSD)
 		return -ENODEV;
 
-	do {
-		if (!ida_pre_get(&osd_minor_ida, GFP_KERNEL))
-			return -ENODEV;
-
-		error = ida_get_new(&osd_minor_ida, &minor);
-	} while (error == -EAGAIN);
-
-	if (error)
-		return error;
-	if (minor >= SCSI_OSD_MAX_MINOR) {
-		error = -EBUSY;
-		goto err_retract_minor;
+	minor = ida_simple_get(&osd_minor_ida, 0,
+			       SCSI_OSD_MAX_MINOR, GFP_KERNEL);
+	if (minor < 0) {
+		if (minor == -ENOSPC)
+			return -EBUSY;
+		return minor;
 	}
 
 	error = -ENOMEM;
@@ -491,7 +485,7 @@ err_free_osd:
 	dev_set_drvdata(dev, NULL);
 	kfree(oud);
 err_retract_minor:
-	ida_remove(&osd_minor_ida, minor);
+	ida_simple_remove(&osd_minor_ida, minor);
 	return error;
 }
 
-- 
1.7.3.4

[PATCH 7/8] w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.

[Jonathan Cameron]

Straight forward.  As an aside, the ida_init calls are not needed as far
as I can see needed. (DEFINE_IDA does the same already).

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/w1/slaves/w1_ds2760.c |   48 +++++-----------------------------------
 drivers/w1/slaves/w1_ds2780.c |   48 +++++-----------------------------------
 2 files changed, 12 insertions(+), 84 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2760.c b/drivers/w1/slaves/w1_ds2760.c
index 483d451..5754c9a 100644
--- a/drivers/w1/slaves/w1_ds2760.c
+++ b/drivers/w1/slaves/w1_ds2760.c
@@ -114,43 +114,7 @@ static struct bin_attribute w1_ds2760_bin_attr = {
 	.read = w1_ds2760_read_bin,
 };
 
-static DEFINE_IDR(bat_idr);
-static DEFINE_MUTEX(bat_idr_lock);
-
-static int new_bat_id(void)
-{
-	int ret;
-
-	while (1) {
-		int id;
-
-		ret = idr_pre_get(&bat_idr, GFP_KERNEL);
-		if (ret == 0)
-			return -ENOMEM;
-
-		mutex_lock(&bat_idr_lock);
-		ret = idr_get_new(&bat_idr, NULL, &id);
-		mutex_unlock(&bat_idr_lock);
-
-		if (ret == 0) {
-			ret = id & MAX_ID_MASK;
-			break;
-		} else if (ret == -EAGAIN) {
-			continue;
-		} else {
-			break;
-		}
-	}
-
-	return ret;
-}
-
-static void release_bat_id(int id)
-{
-	mutex_lock(&bat_idr_lock);
-	idr_remove(&bat_idr, id);
-	mutex_unlock(&bat_idr_lock);
-}
+static DEFINE_IDA(bat_ida);
 
 static int w1_ds2760_add_slave(struct w1_slave *sl)
 {
@@ -158,7 +122,7 @@ static int w1_ds2760_add_slave(struct w1_slave *sl)
 	int id;
 	struct platform_device *pdev;
 
-	id = new_bat_id();
+	id = ida_simple_get(&bat_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		ret = id;
 		goto noid;
@@ -187,7 +151,7 @@ bin_attr_failed:
 pdev_add_failed:
 	platform_device_unregister(pdev);
 pdev_alloc_failed:
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 noid:
 success:
 	return ret;
@@ -199,7 +163,7 @@ static void w1_ds2760_remove_slave(struct w1_slave *sl)
 	int id = pdev->id;
 
 	platform_device_unregister(pdev);
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 	sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2760_bin_attr);
 }
 
@@ -217,14 +181,14 @@ static int __init w1_ds2760_init(void)
 {
 	printk(KERN_INFO "1-Wire driver for the DS2760 battery monitor "
 	       " chip  - (c) 2004-2005, Szabolcs Gyurko\n");
-	idr_init(&bat_idr);
+	ida_init(&bat_ida);
 	return w1_register_family(&w1_ds2760_family);
 }
 
 static void __exit w1_ds2760_exit(void)
 {
 	w1_unregister_family(&w1_ds2760_family);
-	idr_destroy(&bat_idr);
+	ida_destroy(&bat_ida);
 }
 
 EXPORT_SYMBOL(w1_ds2760_read);
diff --git a/drivers/w1/slaves/w1_ds2780.c b/drivers/w1/slaves/w1_ds2780.c
index 274c8f3..a134b38 100644
--- a/drivers/w1/slaves/w1_ds2780.c
+++ b/drivers/w1/slaves/w1_ds2780.c
@@ -99,43 +99,7 @@ static struct bin_attribute w1_ds2780_bin_attr = {
 	.read = w1_ds2780_read_bin,
 };
 
-static DEFINE_IDR(bat_idr);
-static DEFINE_MUTEX(bat_idr_lock);
-
-static int new_bat_id(void)
-{
-	int ret;
-
-	while (1) {
-		int id;
-
-		ret = idr_pre_get(&bat_idr, GFP_KERNEL);
-		if (ret == 0)
-			return -ENOMEM;
-
-		mutex_lock(&bat_idr_lock);
-		ret = idr_get_new(&bat_idr, NULL, &id);
-		mutex_unlock(&bat_idr_lock);
-
-		if (ret == 0) {
-			ret = id & MAX_ID_MASK;
-			break;
-		} else if (ret == -EAGAIN) {
-			continue;
-		} else {
-			break;
-		}
-	}
-
-	return ret;
-}
-
-static void release_bat_id(int id)
-{
-	mutex_lock(&bat_idr_lock);
-	idr_remove(&bat_idr, id);
-	mutex_unlock(&bat_idr_lock);
-}
+static DEFINE_IDA(bat_ida);
 
 static int w1_ds2780_add_slave(struct w1_slave *sl)
 {
@@ -143,7 +107,7 @@ static int w1_ds2780_add_slave(struct w1_slave *sl)
 	int id;
 	struct platform_device *pdev;
 
-	id = new_bat_id();
+	id = ida_simple_get(&bat_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		ret = id;
 		goto noid;
@@ -172,7 +136,7 @@ bin_attr_failed:
 pdev_add_failed:
 	platform_device_unregister(pdev);
 pdev_alloc_failed:
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 noid:
 	return ret;
 }
@@ -183,7 +147,7 @@ static void w1_ds2780_remove_slave(struct w1_slave *sl)
 	int id = pdev->id;
 
 	platform_device_unregister(pdev);
-	release_bat_id(id);
+	ida_simple_remove(&bat_ida, id);
 	sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2780_bin_attr);
 }
 
@@ -199,14 +163,14 @@ static struct w1_family w1_ds2780_family = {
 
 static int __init w1_ds2780_init(void)
 {
-	idr_init(&bat_idr);
+	ida_init(&bat_ida);
 	return w1_register_family(&w1_ds2780_family);
 }
 
 static void __exit w1_ds2780_exit(void)
 {
 	w1_unregister_family(&w1_ds2780_family);
-	idr_destroy(&bat_idr);
+	ida_destroy(&bat_ida);
 }
 
 module_init(w1_ds2780_init);
-- 
1.7.3.4

Re: [PATCH 7/8] w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.

[Barnes, Clifton A.]

On Fri 7/22/2011 12:41 PM, Jonathan Cameron wrote:

> Straight forward.  As an aside, the ida_init calls are not needed as far
> as I can see needed. (DEFINE_IDA does the same already).

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Checked out ok here for ds2780.

Acked-by: Clifton Barnes <cabarnes@indesign-llc.com>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH 8/8] rtc: class idr converted to ida and ida_simple_get used.

[Jonathan Cameron]

This is the one use of an ida that doesn't retry on receiving -EAGAIN.
I'm assuming do so will cause no harm and may help on a rare occasion.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/rtc/class.c |   32 +++++++++-----------------------
 1 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 4194e59..68ae46f 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -21,16 +21,13 @@
 #include "rtc-core.h"
 
 
-static DEFINE_IDR(rtc_idr);
-static DEFINE_MUTEX(idr_lock);
+static DEFINE_IDA(rtc_ida);
 struct class *rtc_class;
 
 static void rtc_device_release(struct device *dev)
 {
 	struct rtc_device *rtc = to_rtc_device(dev);
-	mutex_lock(&idr_lock);
-	idr_remove(&rtc_idr, rtc->id);
-	mutex_unlock(&idr_lock);
+	ida_simple_remove(&rtc_ida, rtc->id);
 	kfree(rtc);
 }
 
@@ -115,25 +112,16 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 	struct rtc_wkalrm alrm;
 	int id, err;
 
-	if (idr_pre_get(&rtc_idr, GFP_KERNEL) == 0) {
-		err = -ENOMEM;
+	id = ida_simple_get(&rtc_ida, 0, 0, GFP_KERNEL);
+	if (id < 0) {
+		err = id;
 		goto exit;
 	}
 
-
-	mutex_lock(&idr_lock);
-	err = idr_get_new(&rtc_idr, NULL, &id);
-	mutex_unlock(&idr_lock);
-
-	if (err < 0)
-		goto exit;
-
-	id = id & MAX_ID_MASK;
-
 	rtc = kzalloc(sizeof(struct rtc_device), GFP_KERNEL);
 	if (rtc == NULL) {
 		err = -ENOMEM;
-		goto exit_idr;
+		goto exit_ida;
 	}
 
 	rtc->id = id;
@@ -191,10 +179,8 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 exit_kfree:
 	kfree(rtc);
 
-exit_idr:
-	mutex_lock(&idr_lock);
-	idr_remove(&rtc_idr, id);
-	mutex_unlock(&idr_lock);
+exit_ida:
+	ida_simple_remove(&rtc_ida, id);
 
 exit:
 	dev_err(dev, "rtc core: unable to register %s, err = %d\n",
@@ -245,7 +231,7 @@ static void __exit rtc_exit(void)
 {
 	rtc_dev_exit();
 	class_destroy(rtc_class);
-	idr_destroy(&rtc_idr);
+	ida_destroy(&rtc_ida);
 }
 
 subsys_initcall(rtc_init);
-- 
1.7.3.4

Re: [PATCH 0/8] RFC: Introduce ida_simple interfaces and use them.

[Jonathan Cameron]

Gah,

Sorry all, I have no idea why this double sent.

Jonathan
> Hi All,
> 
> Firstly sorry for the huge cc list, but this series does get about.
> 
> This series came out of two separate lkml threads:
> 
> https://lkml.org/lkml/2011/7/13/64
> http://thread.gmane.org/gmane.linux.kernel/1148513/focus=74236
> 
> Basically Rusty and I both got annoyed with yet more instances
> of the same cut and past ida allocation code for the very simple
> case of wanting a unique id for some device. Tejun helpfully
> joined the two threads up.
> 
> Anyhow, Rusty proposed more or less the first patch here (with a name
> change requested by Tejun applied.)
> 
> The others patches are some of the more obvious looking cases for
> applying this code.  My original reason was for IIO but those patches will
> have to wait a little while for some prior changes to merge.
> For reference, it saves about 40 lines there so I'm happy ;)
> 
> There are a number of other cases in tree that can be cleaned up
> in a second series once these are sorted.
> 
> There are a couple of cases in here where I've carefully mangled
> the error codes to keep consistent with the existing code.  Please
> can people who know those subsystems well decide on whether the
> mangling is necessary.
> 
> All comments welcome. I expect I've broken at least one driver
> doing this so please take a close look.
> 
> Rusty, Tejun. I kept your sign of an ack for the first patch. Could
> you quickly verify that's fine?
> 
> Thanks,
> 
> Jonathan
> 
> Jonathan Cameron (7):
>   hwmon: convert idr to ida and use ida_simple interface.
>   hwmon: ibmaem: convert idr to ida and use ida_simple_get
>   [SCSI] use ida_simple_get and ida_simple remove in place of
>     boilerplate code.
>   drm/vmwgfx: use ida_simple_get for id allocation.
>   [SCSI] osduld: use ida_simple_get to handle id.
>   w1: ds2760 and ds2780, use ida for id and ida_simple_get to get it.
>   rtc: class idr converted to ida and ida_simple_get used.
> 
> Rusty Russell (1):
>   ida: Simplified functions for id allocation.
> 
>  drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |   34 +++----------
>  drivers/hwmon/hwmon.c                         |   32 +++---------
>  drivers/hwmon/ibmaem.c                        |   47 +++--------------
>  drivers/rtc/class.c                           |   32 +++---------
>  drivers/scsi/osd/osd_uld.c                    |   22 +++-----
>  drivers/scsi/sd.c                             |   32 ++++--------
>  drivers/w1/slaves/w1_ds2760.c                 |   48 ++---------------
>  drivers/w1/slaves/w1_ds2780.c                 |   48 ++---------------
>  include/linux/idr.h                           |    4 ++
>  lib/idr.c                                     |   67 +++++++++++++++++++++++++
>  10 files changed, 134 insertions(+), 232 deletions(-)
>