[PATCH 2/2] virtio: Use ida to allocate virtio index

[PATCH 2/2] virtio: Use ida to allocate virtio index

[Asias He]

Current index allocation in virtio is based on a monotonically
increasing variable "index". This means we'll run out of numbers
after a while. E.g. someone crazy doing this in host side.

while(1) {
	hot-plug a virtio device
	hot-unplug the virito devcie
}

Signed-off-by: Asias He <asias@redhat.com>
---
 drivers/virtio/virtio.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 984c501..f355807 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -2,9 +2,10 @@
 #include <linux/spinlock.h>
 #include <linux/virtio_config.h>
 #include <linux/module.h>
+#include <linux/idr.h>
 
 /* Unique numbering for virtio devices. */
-static unsigned int dev_index;
+static DEFINE_IDA(virtio_index_ida);
 
 static ssize_t device_show(struct device *_d,
 			   struct device_attribute *attr, char *buf)
@@ -193,7 +194,11 @@ int register_virtio_device(struct virtio_device *dev)
 	dev->dev.bus = &virtio_bus;
 
 	/* Assign a unique device index and hence name. */
-	dev->index = dev_index++;
+	err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL);
+	if (err < 0)
+		goto out;
+
+	dev->index = err;
 	dev_set_name(&dev->dev, "virtio%u", dev->index);
 
 	/* We always start by resetting the device, in case a previous
@@ -208,6 +213,7 @@ int register_virtio_device(struct virtio_device *dev)
 	/* device_register() causes the bus infrastructure to look for a
 	 * matching driver. */
 	err = device_register(&dev->dev);
+out:
 	if (err)
 		add_status(dev, VIRTIO_CONFIG_S_FAILED);
 	return err;
@@ -217,6 +223,7 @@ EXPORT_SYMBOL_GPL(register_virtio_device);
 void unregister_virtio_device(struct virtio_device *dev)
 {
 	device_unregister(&dev->dev);
+	ida_simple_remove(&virtio_index_ida, dev->index);
 }
 EXPORT_SYMBOL_GPL(unregister_virtio_device);
 
-- 
1.7.10

Re: [PATCH 2/2] virtio: Use ida to allocate virtio index

[Rusty Russell]

On Thu,  3 May 2012 10:20:51 +0800, Asias He <asias@redhat.com> wrote:
> Current index allocation in virtio is based on a monotonically
> increasing variable "index". This means we'll run out of numbers
> after a while. E.g. someone crazy doing this in host side.
> 
> while(1) {
> 	hot-plug a virtio device
> 	hot-unplug the virito devcie
> }
> 
> Signed-off-by: Asias He <asias@redhat.com>

Applied.  Nice and simple.

Thanks,
Rusty.

> ---
>  drivers/virtio/virtio.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 984c501..f355807 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -2,9 +2,10 @@
>  #include <linux/spinlock.h>
>  #include <linux/virtio_config.h>
>  #include <linux/module.h>
> +#include <linux/idr.h>
>  
>  /* Unique numbering for virtio devices. */
> -static unsigned int dev_index;
> +static DEFINE_IDA(virtio_index_ida);
>  
>  static ssize_t device_show(struct device *_d,
>  			   struct device_attribute *attr, char *buf)
> @@ -193,7 +194,11 @@ int register_virtio_device(struct virtio_device *dev)
>  	dev->dev.bus = &virtio_bus;
>  
>  	/* Assign a unique device index and hence name. */
> -	dev->index = dev_index++;
> +	err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL);
> +	if (err < 0)
> +		goto out;
> +
> +	dev->index = err;
>  	dev_set_name(&dev->dev, "virtio%u", dev->index);
>  
>  	/* We always start by resetting the device, in case a previous
> @@ -208,6 +213,7 @@ int register_virtio_device(struct virtio_device *dev)
>  	/* device_register() causes the bus infrastructure to look for a
>  	 * matching driver. */
>  	err = device_register(&dev->dev);
> +out:
>  	if (err)
>  		add_status(dev, VIRTIO_CONFIG_S_FAILED);
>  	return err;
> @@ -217,6 +223,7 @@ EXPORT_SYMBOL_GPL(register_virtio_device);
>  void unregister_virtio_device(struct virtio_device *dev)
>  {
>  	device_unregister(&dev->dev);
> +	ida_simple_remove(&virtio_index_ida, dev->index);
>  }
>  EXPORT_SYMBOL_GPL(unregister_virtio_device);
>  
> -- 
> 1.7.10
>