Linux Virtualization - Jan 2013 - [PATCH] virtio_console: Use virtio device index to generate port name

From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>

Use virtio device index for creating unique device port names.
Current index allocation in virtio is based on a monotonically
increasing variable "index". A better handling of this is to
use device index which is allocated by ida.

Signed-off-by: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
---
 drivers/char/virtio_console.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c17b053..031be0b 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -61,9 +61,6 @@ struct ports_driver_data {
 	/* List of all the devices we're handling */
 	struct list_head portdevs;
 
-	/* Number of devices this driver is handling */
-	unsigned int index;
-
 	/*
 	 * This is used to keep track of the number of hvc consoles
 	 * spawned by this driver.  This number is given as the first
@@ -169,9 +166,6 @@ struct ports_device {
 	/* Array of per-port IO virtqueues */
 	struct virtqueue **in_vqs, **out_vqs;
 
-	/* Used for numbering devices for sysfs and debugfs */
-	unsigned int drv_index;
-
 	/* Major number for this device.  Ports will be created as minors. */
 	int chr_major;
 };
@@ -1415,7 +1409,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 	}
 	port->dev = device_create(pdrvdata.class,
&port->portdev->vdev->dev,
 				  devt, port, "vport%up%u",
-				  port->portdev->drv_index, id);
+				  port->portdev->vdev->index, id);
 	if (IS_ERR(port->dev)) {
 		err = PTR_ERR(port->dev);
 		dev_err(&port->portdev->vdev->dev,
@@ -1470,7 +1464,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 		 * inspect a port's state at any time
 		 */
 		sprintf(debugfs_name, "vport%up%u",
-			port->portdev->drv_index, id);
+			port->portdev->vdev->index, id);
 		port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
 							 pdrvdata.debugfs_dir,
 							 port,
@@ -1961,16 +1955,12 @@ static int virtcons_probe(struct virtio_device *vdev)
 	portdev->vdev = vdev;
 	vdev->priv = portdev;
 
-	spin_lock_irq(&pdrvdata_lock);
-	portdev->drv_index = pdrvdata.index++;
-	spin_unlock_irq(&pdrvdata_lock);
-
 	portdev->chr_major = register_chrdev(0, "virtio-portsdev",
 					     &portdev_fops);
 	if (portdev->chr_major < 0) {
 		dev_err(&vdev->dev,
 			"Error %d registering chrdev for device %u\n",
-			portdev->chr_major, portdev->drv_index);
+			portdev->chr_major, vdev->index);
 		err = portdev->chr_major;
 		goto free;
 	}
-- 
1.7.5.4

sjur.brandeland at stericsson.com writes:
> From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
>
> Use virtio device index for creating unique device port names.
> Current index allocation in virtio is based on a monotonically
> increasing variable "index". A better handling of this is to
> use device index which is allocated by ida.
>
> Signed-off-by: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
This seems to make sense, but I'd need Amit's Ack to verify that the
current numbering isn't important for anyone.

Thanks,
Rusty.
> ---
>  drivers/char/virtio_console.c |   16 +++-------------
>  1 files changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index c17b053..031be0b 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -61,9 +61,6 @@ struct ports_driver_data {
>  	/* List of all the devices we're handling */
>  	struct list_head portdevs;
>  
> -	/* Number of devices this driver is handling */
> -	unsigned int index;
> -
>  	/*
>  	 * This is used to keep track of the number of hvc consoles
>  	 * spawned by this driver.  This number is given as the first
> @@ -169,9 +166,6 @@ struct ports_device {
>  	/* Array of per-port IO virtqueues */
>  	struct virtqueue **in_vqs, **out_vqs;
>  
> -	/* Used for numbering devices for sysfs and debugfs */
> -	unsigned int drv_index;
> -
>  	/* Major number for this device.  Ports will be created as minors. */
>  	int chr_major;
>  };
> @@ -1415,7 +1409,7 @@ static int add_port(struct ports_device *portdev, u32
id)
>  	}
>  	port->dev = device_create(pdrvdata.class,
&port->portdev->vdev->dev,
>  				  devt, port, "vport%up%u",
> -				  port->portdev->drv_index, id);
> +				  port->portdev->vdev->index, id);
>  	if (IS_ERR(port->dev)) {
>  		err = PTR_ERR(port->dev);
>  		dev_err(&port->portdev->vdev->dev,
> @@ -1470,7 +1464,7 @@ static int add_port(struct ports_device *portdev, u32
id)
>  		 * inspect a port's state at any time
>  		 */
>  		sprintf(debugfs_name, "vport%up%u",
> -			port->portdev->drv_index, id);
> +			port->portdev->vdev->index, id);
>  		port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
>  							 pdrvdata.debugfs_dir,
>  							 port,
> @@ -1961,16 +1955,12 @@ static int virtcons_probe(struct virtio_device
*vdev)
>  	portdev->vdev = vdev;
>  	vdev->priv = portdev;
>  
> -	spin_lock_irq(&pdrvdata_lock);
> -	portdev->drv_index = pdrvdata.index++;
> -	spin_unlock_irq(&pdrvdata_lock);
> -
>  	portdev->chr_major = register_chrdev(0, "virtio-portsdev",
>  					     &portdev_fops);
>  	if (portdev->chr_major < 0) {
>  		dev_err(&vdev->dev,
>  			"Error %d registering chrdev for device %u\n",
> -			portdev->chr_major, portdev->drv_index);
> +			portdev->chr_major, vdev->index);
>  		err = portdev->chr_major;
>  		goto free;
>  	}
> -- 
> 1.7.5.4

On (Thu) 17 Jan 2013 [13:23:17], sjur.brandeland at stericsson.com
wrote:
> From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
> 
> Use virtio device index for creating unique device port names.
> Current index allocation in virtio is based on a monotonically
> increasing variable "index". A better handling of this is to
> use device index which is allocated by ida.
This should be fine.  I'm wondering, though, if there's something else
you need from the naming scheme (esp. considering the next patch)?

		Amit

On (Thu) 17 Jan 2013 [13:23:17], sjur.brandeland at stericsson.com
wrote:
> From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
> 
> Use virtio device index for creating unique device port names.
> Current index allocation in virtio is based on a monotonically
> increasing variable "index". A better handling of this is to
> use device index which is allocated by ida.
> 
> Signed-off-by: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
Acked-by: Amit Shah <amit.shah at redhat.com>

Rusty, please pick this one up.

Thanks,

		Amit

Amit Shah <amit.shah at redhat.com> writes:
> On (Thu) 17 Jan 2013 [13:23:17], sjur.brandeland at stericsson.com wrote:
>> From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
>> 
>> Use virtio device index for creating unique device port names.
>> Current index allocation in virtio is based on a monotonically
>> increasing variable "index". A better handling of this is to
>> use device index which is allocated by ida.
>> 
>> Signed-off-by: Sjur Br?ndeland <sjur.brandeland at
stericsson.com>
>
> Acked-by: Amit Shah <amit.shah at redhat.com>
>
> Rusty, please pick this one up.
In my virtio-next queue now...

Thanks,
Rusty.