[PATCH 0/5] Modify ida_* users to use ida_simple_*

[PATCH 0/5] Modify ida_* users to use ida_simple_*

[Lee Duncan]

The ida index management routines are used in several
driver modules to manage allocation and release of
index values. Reviewing the way in which the
ida routines were called, together with the small
number of such clients, led to the belief that
these users should all be able to share a simple
built-in lock in the ida module by calling the
ida_simple_*() functions instead of the non-simple
versions. This means that ida does all the
required locking so that clients don't have to
manage that.

This will greatly simplify the client calling code,
and if there is any problem with these clients
sharing a "simple" lock, the ida code can be
transparently expanded to allocate a lock per client,
without having to change any of the clients again.

NOTE: this patch series replaces an earlier attempt
to create a new set of ida helper functions titled:

  "Create and use ida and idr helper routines"

Another set will soon be sent out soon to (1) add idr
helper functions, (2) modify clients to use them,
and (3) update SCSI host_no to use them.

Lee Duncan (5):
  SCSI: sd: simplify ida usage
  block: rsxx: core: simplify ida usage
  block: nvme-core: simplify ida usage
  block: mtip32xx: simplify ida usage
  base: soc: siplify ida usage

 drivers/base/soc.c                | 21 +++++----------------
 drivers/block/mtip32xx/mtip32xx.c | 26 ++++++--------------------
 drivers/block/nvme-core.c         | 16 ++++------------
 drivers/block/rsxx/core.c         | 20 ++++----------------
 drivers/scsi/sd.c                 | 22 +++++-----------------
 5 files changed, 24 insertions(+), 81 deletions(-)

-- 
2.1.4

[PATCH 1/5] SCSI: sd: simplify ida usage

[Lee Duncan]

Simplify ida index allocation and removal by
using the ida_simple_* helper functions.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/scsi/sd.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3b2fcb4fada0..3d77ac8f0d4c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -118,7 +118,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(const struct scsi_disk *, const char *, 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
@@ -2948,19 +2947,12 @@ 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) {
+	error = ida_simple_get(&sd_index_ida, 0, 0, GFP_KERNEL);
+	if (error < 0) {
 		sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
 		goto out_put;
 	}
+	index = error;
 
 	error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
 	if (error) {
@@ -3001,9 +2993,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:
@@ -3064,9 +3054,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);
 
 	blk_integrity_unregister(disk);
 	disk->private_data = NULL;
-- 
2.1.4

Re: [PATCH 1/5] SCSI: sd: simplify ida usage

[Johannes Thumshirn]

Lee Duncan <lduncan@suse.com> writes:

> Simplify ida index allocation and removal by
> using the ida_simple_* helper functions.
>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/scsi/sd.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 3b2fcb4fada0..3d77ac8f0d4c 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -118,7 +118,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(const struct scsi_disk *, const char *, 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
> @@ -2948,19 +2947,12 @@ 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) {
> +	error = ida_simple_get(&sd_index_ida, 0, 0, GFP_KERNEL);
> +	if (error < 0) {
>  		sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
>  		goto out_put;
>  	}
> +	index = error;
>  
>  	error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
>  	if (error) {
> @@ -3001,9 +2993,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:
> @@ -3064,9 +3054,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);
>  
>  	blk_integrity_unregister(disk);
>  	disk->private_data = NULL;

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/5] block: rsxx: core: simplify ida usage

[Lee Duncan]

Simplify ida index allocation and removal by
using the ida_simple_* helper functions.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/block/rsxx/core.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index d8b2488aaade..d2279a759b2e 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -58,7 +58,6 @@ MODULE_PARM_DESC(sync_start, "On by Default: Driver load will not complete "
 			     "until the card startup has completed.");
 
 static DEFINE_IDA(rsxx_disk_ida);
-static DEFINE_SPINLOCK(rsxx_ida_lock);
 
 /* --------------------Debugfs Setup ------------------- */
 
@@ -774,19 +773,10 @@ static int rsxx_pci_probe(struct pci_dev *dev,
 	card->dev = dev;
 	pci_set_drvdata(dev, card);
 
-	do {
-		if (!ida_pre_get(&rsxx_disk_ida, GFP_KERNEL)) {
-			st = -ENOMEM;
-			goto failed_ida_get;
-		}
-
-		spin_lock(&rsxx_ida_lock);
-		st = ida_get_new(&rsxx_disk_ida, &card->disk_id);
-		spin_unlock(&rsxx_ida_lock);
-	} while (st == -EAGAIN);
-
-	if (st)
+	st = ida_simple_get(&rsxx_disk_ida, 0, 0, GFP_KERNEL);
+	if (st < 0)
 		goto failed_ida_get;
+	card->disk_id = st;
 
 	st = pci_enable_device(dev);
 	if (st)
@@ -987,9 +977,7 @@ failed_request_regions:
 failed_dma_mask:
 	pci_disable_device(dev);
 failed_enable:
-	spin_lock(&rsxx_ida_lock);
-	ida_remove(&rsxx_disk_ida, card->disk_id);
-	spin_unlock(&rsxx_ida_lock);
+	ida_simple_remove(&rsxx_disk_ida, card->disk_id);
 failed_ida_get:
 	kfree(card);
 
-- 
2.1.4

Re: [PATCH 2/5] block: rsxx: core: simplify ida usage

[Johannes Thumshirn]

Lee Duncan <lduncan@suse.com> writes:

> Simplify ida index allocation and removal by
> using the ida_simple_* helper functions.
>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/block/rsxx/core.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
> index d8b2488aaade..d2279a759b2e 100644
> --- a/drivers/block/rsxx/core.c
> +++ b/drivers/block/rsxx/core.c
> @@ -58,7 +58,6 @@ MODULE_PARM_DESC(sync_start, "On by Default: Driver load will not complete "
>  			     "until the card startup has completed.");
>  
>  static DEFINE_IDA(rsxx_disk_ida);
> -static DEFINE_SPINLOCK(rsxx_ida_lock);
>  
>  /* --------------------Debugfs Setup ------------------- */
>  
> @@ -774,19 +773,10 @@ static int rsxx_pci_probe(struct pci_dev *dev,
>  	card->dev = dev;
>  	pci_set_drvdata(dev, card);
>  
> -	do {
> -		if (!ida_pre_get(&rsxx_disk_ida, GFP_KERNEL)) {
> -			st = -ENOMEM;
> -			goto failed_ida_get;
> -		}
> -
> -		spin_lock(&rsxx_ida_lock);
> -		st = ida_get_new(&rsxx_disk_ida, &card->disk_id);
> -		spin_unlock(&rsxx_ida_lock);
> -	} while (st == -EAGAIN);
> -
> -	if (st)
> +	st = ida_simple_get(&rsxx_disk_ida, 0, 0, GFP_KERNEL);
> +	if (st < 0)
>  		goto failed_ida_get;
> +	card->disk_id = st;
>  
>  	st = pci_enable_device(dev);
>  	if (st)
> @@ -987,9 +977,7 @@ failed_request_regions:
>  failed_dma_mask:
>  	pci_disable_device(dev);
>  failed_enable:
> -	spin_lock(&rsxx_ida_lock);
> -	ida_remove(&rsxx_disk_ida, card->disk_id);
> -	spin_unlock(&rsxx_ida_lock);
> +	ida_simple_remove(&rsxx_disk_ida, card->disk_id);
>  failed_ida_get:
>  	kfree(card);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 3/5] block: nvme-core: simplify ida usage

[Lee Duncan]

Simplify ida index allocation and removal by
using the ida_simple_* helper functions.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/block/nvme-core.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index d1d6141920d3..d354a3391e4a 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -2713,18 +2713,10 @@ static DEFINE_IDA(nvme_instance_ida);
 
 static int nvme_set_instance(struct nvme_dev *dev)
 {
-	int instance, error;
+	int instance;
 
-	do {
-		if (!ida_pre_get(&nvme_instance_ida, GFP_KERNEL))
-			return -ENODEV;
-
-		spin_lock(&dev_list_lock);
-		error = ida_get_new(&nvme_instance_ida, &instance);
-		spin_unlock(&dev_list_lock);
-	} while (error == -EAGAIN);
-
-	if (error)
+	instance = ida_simple_get(&nvme_instance_ida, 0, 0, GFP_KERNEL);
+	if (instance < 0)
 		return -ENODEV;
 
 	dev->instance = instance;
@@ -2734,7 +2726,7 @@ static int nvme_set_instance(struct nvme_dev *dev)
 static void nvme_release_instance(struct nvme_dev *dev)
 {
 	spin_lock(&dev_list_lock);
-	ida_remove(&nvme_instance_ida, dev->instance);
+	ida_simple_remove(&nvme_instance_ida, dev->instance);
 	spin_unlock(&dev_list_lock);
 }
 
-- 
2.1.4

Re: [PATCH 3/5] block: nvme-core: simplify ida usage

[Johannes Thumshirn]

Lee Duncan <lduncan@suse.com> writes:

> Simplify ida index allocation and removal by
> using the ida_simple_* helper functions.
>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/block/nvme-core.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index d1d6141920d3..d354a3391e4a 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -2713,18 +2713,10 @@ static DEFINE_IDA(nvme_instance_ida);
>  
>  static int nvme_set_instance(struct nvme_dev *dev)
>  {
> -	int instance, error;
> +	int instance;
>  
> -	do {
> -		if (!ida_pre_get(&nvme_instance_ida, GFP_KERNEL))
> -			return -ENODEV;
> -
> -		spin_lock(&dev_list_lock);
> -		error = ida_get_new(&nvme_instance_ida, &instance);
> -		spin_unlock(&dev_list_lock);
> -	} while (error == -EAGAIN);
> -
> -	if (error)
> +	instance = ida_simple_get(&nvme_instance_ida, 0, 0, GFP_KERNEL);
> +	if (instance < 0)
>  		return -ENODEV;
>  
>  	dev->instance = instance;
> @@ -2734,7 +2726,7 @@ static int nvme_set_instance(struct nvme_dev *dev)
>  static void nvme_release_instance(struct nvme_dev *dev)
>  {
>  	spin_lock(&dev_list_lock);
> -	ida_remove(&nvme_instance_ida, dev->instance);
> +	ida_simple_remove(&nvme_instance_ida, dev->instance);
>  	spin_unlock(&dev_list_lock);
>  }

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 3/5] block: nvme-core: simplify ida usage

[Keith Busch]

On Fri, 2 Oct 2015, Johannes Thumshirn wrote:
> Lee Duncan <lduncan@suse.com> writes:
>> Simplify ida index allocation and removal by
>> using the ida_simple_* helper functions.

Looks good to me. Just one comment:

>>  static void nvme_release_instance(struct nvme_dev *dev)
>>  {
>>  	spin_lock(&dev_list_lock);
>> -	ida_remove(&nvme_instance_ida, dev->instance);
>> +	ida_simple_remove(&nvme_instance_ida, dev->instance);
>>  	spin_unlock(&dev_list_lock);

No harm from taking the nvme spin lock here, but it's not necessary with
the simple interface.

> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

[PATCH 4/5] block: mtip32xx: simplify ida usage

[Lee Duncan]

Simplify ida index allocation and removal by
using the ida_simple_* helper functions

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/block/mtip32xx/mtip32xx.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 4a2ef09e6704..e62d170b0641 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -118,7 +118,6 @@ static struct dentry *dfs_device_status;
 
 static u32 cpu_use[NR_CPUS];
 
-static DEFINE_SPINLOCK(rssd_index_lock);
 static DEFINE_IDA(rssd_index_ida);
 
 static int mtip_block_initialize(struct driver_data *dd);
@@ -3821,17 +3820,10 @@ static int mtip_block_initialize(struct driver_data *dd)
 	}
 
 	/* Generate the disk name, implemented same as in sd.c */
-	do {
-		if (!ida_pre_get(&rssd_index_ida, GFP_KERNEL))
-			goto ida_get_error;
-
-		spin_lock(&rssd_index_lock);
-		rv = ida_get_new(&rssd_index_ida, &index);
-		spin_unlock(&rssd_index_lock);
-	} while (rv == -EAGAIN);
-
-	if (rv)
+	rv = ida_simple_get(&rssd_index_ida, 0, 0, GFP_KERNEL);
+	if (rv < 0)
 		goto ida_get_error;
+	index = rv;
 
 	rv = rssd_disk_name_format("rssd",
 				index,
@@ -3981,9 +3973,7 @@ init_hw_cmds_error:
 block_queue_alloc_init_error:
 	mtip_hw_debugfs_exit(dd);
 disk_index_error:
-	spin_lock(&rssd_index_lock);
-	ida_remove(&rssd_index_ida, index);
-	spin_unlock(&rssd_index_lock);
+	ida_simple_remove(&rssd_index_ida, index);
 
 ida_get_error:
 	put_disk(dd->disk);
@@ -4051,9 +4041,7 @@ static int mtip_block_remove(struct driver_data *dd)
 	}
 	dd->disk  = NULL;
 
-	spin_lock(&rssd_index_lock);
-	ida_remove(&rssd_index_ida, dd->index);
-	spin_unlock(&rssd_index_lock);
+	ida_simple_remove(&rssd_index_ida, dd->index);
 
 	/* De-initialize the protocol layer. */
 	mtip_hw_exit(dd);
@@ -4092,9 +4080,7 @@ static int mtip_block_shutdown(struct driver_data *dd)
 		dd->queue = NULL;
 	}
 
-	spin_lock(&rssd_index_lock);
-	ida_remove(&rssd_index_ida, dd->index);
-	spin_unlock(&rssd_index_lock);
+	ida_simple_remove(&rssd_index_ida, dd->index);
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH 4/5] block: mtip32xx: simplify ida usage

[Johannes Thumshirn]

Lee Duncan <lduncan@suse.com> writes:

> Simplify ida index allocation and removal by
> using the ida_simple_* helper functions
>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/block/mtip32xx/mtip32xx.c | 26 ++++++--------------------
>  1 file changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
> index 4a2ef09e6704..e62d170b0641 100644
> --- a/drivers/block/mtip32xx/mtip32xx.c
> +++ b/drivers/block/mtip32xx/mtip32xx.c
> @@ -118,7 +118,6 @@ static struct dentry *dfs_device_status;
>  
>  static u32 cpu_use[NR_CPUS];
>  
> -static DEFINE_SPINLOCK(rssd_index_lock);
>  static DEFINE_IDA(rssd_index_ida);
>  
>  static int mtip_block_initialize(struct driver_data *dd);
> @@ -3821,17 +3820,10 @@ static int mtip_block_initialize(struct driver_data *dd)
>  	}
>  
>  	/* Generate the disk name, implemented same as in sd.c */
> -	do {
> -		if (!ida_pre_get(&rssd_index_ida, GFP_KERNEL))
> -			goto ida_get_error;
> -
> -		spin_lock(&rssd_index_lock);
> -		rv = ida_get_new(&rssd_index_ida, &index);
> -		spin_unlock(&rssd_index_lock);
> -	} while (rv == -EAGAIN);
> -
> -	if (rv)
> +	rv = ida_simple_get(&rssd_index_ida, 0, 0, GFP_KERNEL);
> +	if (rv < 0)
>  		goto ida_get_error;
> +	index = rv;
>  
>  	rv = rssd_disk_name_format("rssd",
>  				index,
> @@ -3981,9 +3973,7 @@ init_hw_cmds_error:
>  block_queue_alloc_init_error:
>  	mtip_hw_debugfs_exit(dd);
>  disk_index_error:
> -	spin_lock(&rssd_index_lock);
> -	ida_remove(&rssd_index_ida, index);
> -	spin_unlock(&rssd_index_lock);
> +	ida_simple_remove(&rssd_index_ida, index);
>  
>  ida_get_error:
>  	put_disk(dd->disk);
> @@ -4051,9 +4041,7 @@ static int mtip_block_remove(struct driver_data *dd)
>  	}
>  	dd->disk  = NULL;
>  
> -	spin_lock(&rssd_index_lock);
> -	ida_remove(&rssd_index_ida, dd->index);
> -	spin_unlock(&rssd_index_lock);
> +	ida_simple_remove(&rssd_index_ida, dd->index);
>  
>  	/* De-initialize the protocol layer. */
>  	mtip_hw_exit(dd);
> @@ -4092,9 +4080,7 @@ static int mtip_block_shutdown(struct driver_data *dd)
>  		dd->queue = NULL;
>  	}
>  
> -	spin_lock(&rssd_index_lock);
> -	ida_remove(&rssd_index_ida, dd->index);
> -	spin_unlock(&rssd_index_lock);
> +	ida_simple_remove(&rssd_index_ida, dd->index);
>  	return 0;
>  }

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 5/5] base: soc: siplify ida usage

[Lee Duncan]

Simplify ida index allocation and removal by
using the ida_simple_* helper functions

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/base/soc.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 39fca01c8fa1..75b98aad6faf 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -16,7 +16,6 @@
 #include <linux/err.h>
 
 static DEFINE_IDA(soc_ida);
-static DEFINE_SPINLOCK(soc_lock);
 
 static ssize_t soc_info_get(struct device *dev,
 			    struct device_attribute *attr,
@@ -122,20 +121,10 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
 	}
 
 	/* Fetch a unique (reclaimable) SOC ID. */
-	do {
-		if (!ida_pre_get(&soc_ida, GFP_KERNEL)) {
-			ret = -ENOMEM;
-			goto out2;
-		}
-
-		spin_lock(&soc_lock);
-		ret = ida_get_new(&soc_ida, &soc_dev->soc_dev_num);
-		spin_unlock(&soc_lock);
-
-	} while (ret == -EAGAIN);
-
-	if (ret)
+	ret = ida_simple_get(&soc_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0)
 		goto out2;
+	soc_dev->soc_dev_num = ret;
 
 	soc_dev->attr = soc_dev_attr;
 	soc_dev->dev.bus = &soc_bus_type;
@@ -151,7 +140,7 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
 	return soc_dev;
 
 out3:
-	ida_remove(&soc_ida, soc_dev->soc_dev_num);
+	ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
 out2:
 	kfree(soc_dev);
 out1:
@@ -161,7 +150,7 @@ out1:
 /* Ensure soc_dev->attr is freed prior to calling soc_device_unregister. */
 void soc_device_unregister(struct soc_device *soc_dev)
 {
-	ida_remove(&soc_ida, soc_dev->soc_dev_num);
+	ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
 
 	device_unregister(&soc_dev->dev);
 }
-- 
2.1.4

Re: [PATCH 5/5] base: soc: siplify ida usage

[Johannes Thumshirn]

Lee Duncan <lduncan@suse.com> writes:

> Simplify ida index allocation and removal by
> using the ida_simple_* helper functions
>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/base/soc.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/base/soc.c b/drivers/base/soc.c
> index 39fca01c8fa1..75b98aad6faf 100644
> --- a/drivers/base/soc.c
> +++ b/drivers/base/soc.c
> @@ -16,7 +16,6 @@
>  #include <linux/err.h>
>  
>  static DEFINE_IDA(soc_ida);
> -static DEFINE_SPINLOCK(soc_lock);
>  
>  static ssize_t soc_info_get(struct device *dev,
>  			    struct device_attribute *attr,
> @@ -122,20 +121,10 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
>  	}
>  
>  	/* Fetch a unique (reclaimable) SOC ID. */
> -	do {
> -		if (!ida_pre_get(&soc_ida, GFP_KERNEL)) {
> -			ret = -ENOMEM;
> -			goto out2;
> -		}
> -
> -		spin_lock(&soc_lock);
> -		ret = ida_get_new(&soc_ida, &soc_dev->soc_dev_num);
> -		spin_unlock(&soc_lock);
> -
> -	} while (ret == -EAGAIN);
> -
> -	if (ret)
> +	ret = ida_simple_get(&soc_ida, 0, 0, GFP_KERNEL);
> +	if (ret < 0)
>  		goto out2;
> +	soc_dev->soc_dev_num = ret;
>  
>  	soc_dev->attr = soc_dev_attr;
>  	soc_dev->dev.bus = &soc_bus_type;
> @@ -151,7 +140,7 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
>  	return soc_dev;
>  
>  out3:
> -	ida_remove(&soc_ida, soc_dev->soc_dev_num);
> +	ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
>  out2:
>  	kfree(soc_dev);
>  out1:
> @@ -161,7 +150,7 @@ out1:
>  /* Ensure soc_dev->attr is freed prior to calling soc_device_unregister. */
>  void soc_device_unregister(struct soc_device *soc_dev)
>  {
> -	ida_remove(&soc_ida, soc_dev->soc_dev_num);
> +	ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
>  
>  	device_unregister(&soc_dev->dev);
>  }

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 0/5] Modify ida_* users to use ida_simple_*

[Tejun Heo]

On Thu, Oct 01, 2015 at 11:59:04AM -0700, Lee Duncan wrote:
> The ida index management routines are used in several
> driver modules to manage allocation and release of
> index values. Reviewing the way in which the
> ida routines were called, together with the small
> number of such clients, led to the belief that
> these users should all be able to share a simple
> built-in lock in the ida module by calling the
> ida_simple_*() functions instead of the non-simple
> versions. This means that ida does all the
> required locking so that clients don't have to
> manage that.

The whole series looks good to me.  Please feel free to add

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

Thanks.

-- 
tejun

Re: [PATCH 0/5] Modify ida_* users to use ida_simple_*

[James Bottomley]

On Mon, 2015-10-05 at 13:44 -0400, Tejun Heo wrote:
> On Thu, Oct 01, 2015 at 11:59:04AM -0700, Lee Duncan wrote:
> > The ida index management routines are used in several
> > driver modules to manage allocation and release of
> > index values. Reviewing the way in which the
> > ida routines were called, together with the small
> > number of such clients, led to the belief that
> > these users should all be able to share a simple
> > built-in lock in the ida module by calling the
> > ida_simple_*() functions instead of the non-simple
> > versions. This means that ida does all the
> > required locking so that clients don't have to
> > manage that.
> 
> The whole series looks good to me.  Please feel free to add

Since they're all independent, they can go via the correct trees without
adverse consequences.  It's probably me: 1/5; Jens 2-4/5; Greg 5/5

James