[PATCH 0/2] virtio-serial: set up vqs on demand

[PATCH 0/2] virtio-serial: set up vqs on demand

[zanghongyong]

From: Hongyong Zang <zanghongyong@huawei.com>

Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all 
io_ports are used.
These patches create vqs of port0 and control port when probing the device, then 
create io-vqs when called add_port().

Hongyong Zang (2):
  virtio-pci: add setup_vqs flag in vp_try_to_find_vqs
  virtio-serial: setup_port_vq when adding port

 drivers/char/virtio_console.c |   65 ++++++++++++++++++++++++++++++++++++++--
 drivers/virtio/virtio_pci.c   |   17 ++++++++--
 2 files changed, 74 insertions(+), 8 deletions(-)

[PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs

[zanghongyong]

From: Hongyong Zang <zanghongyong@huawei.com>

changes in vp_try_to_find_vqs:
Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and
controls; add_port() calls it to set up vqs of io_port.
it will not create virtqueue if the name is null.

Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
---
 drivers/virtio/virtio_pci.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index baabb79..1f98c36 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev)
 	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
 		info = vq->priv;
 		if (vp_dev->per_vq_vectors &&
-			info->msix_vector != VIRTIO_MSI_NO_VECTOR)
+			info->msix_vector != VIRTIO_MSI_NO_VECTOR) {
 			free_irq(vp_dev->msix_entries[info->msix_vector].vector,
 				 vq);
+			vp_dev->msix_used_vectors--;
+		}
 		vp_del_vq(vq);
 	}
 	vp_dev->per_vq_vectors = false;
@@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	u16 msix_vec;
-	int i, err, nvectors, allocated_vectors;
+	int i, err, nvectors;
+
+	if (vp_dev->msix_used_vectors)
+		goto setup_vqs;
 
 	if (!use_msix) {
 		/* Old style: one normal interrupt for change and all vqs. */
@@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 	}
 
 	vp_dev->per_vq_vectors = per_vq_vectors;
-	allocated_vectors = vp_dev->msix_used_vectors;
+
+setup_vqs:
 	for (i = 0; i < nvqs; ++i) {
+		if (names[i] == NULL)
+			continue;
+
 		if (!callbacks[i] || !vp_dev->msix_enabled)
 			msix_vec = VIRTIO_MSI_NO_VECTOR;
 		else if (vp_dev->per_vq_vectors)
-			msix_vec = allocated_vectors++;
+			msix_vec = vp_dev->msix_used_vectors++;
 		else
 			msix_vec = VP_MSIX_VQ_VECTOR;
 		vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
-- 
1.7.1

[PATCH 2/2] virtio-serial: setup_port_vq when adding port

[zanghongyong]

From: Hongyong Zang <zanghongyong@huawei.com>

Add setup_port_vq(). Create the io ports' vqs when add_port.

Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
---
 drivers/char/virtio_console.c |   65 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8e3c46d..2e5187e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port)
 		kill_fasync(&port->async_queue, SIGIO, POLL_OUT);
 }
 
+static void in_intr(struct virtqueue *vq);
+static void out_intr(struct virtqueue *vq);
+
+static int setup_port_vq(struct ports_device *portdev,  u32 id)
+{
+	int err, vq_num;
+	vq_callback_t **io_callbacks;
+	char **io_names;
+	struct virtqueue **vqs;
+	u32 i,j,nr_ports,nr_queues;
+
+	err = 0;
+	vq_num = (id + 1) * 2;
+	nr_ports = portdev->config.max_nr_ports;
+	nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
+
+	vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
+	io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL);
+	io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL);
+	if (!vqs || !io_callbacks || !io_names) {
+		err = -ENOMEM;
+		goto free;
+	}
+
+	for (i = 0, j = 0; i <= nr_ports; i++) {
+		io_callbacks[j] = in_intr;
+		io_callbacks[j + 1] = out_intr;
+		io_names[j] = NULL;
+		io_names[j + 1] = NULL;
+		j += 2;
+	}
+	io_names[vq_num] = "serial-input";
+	io_names[vq_num + 1] = "serial-output";
+	err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
+				io_callbacks,
+				(const char **)io_names);
+	if (err)
+		goto free;
+	portdev->in_vqs[id] = vqs[vq_num];
+	portdev->out_vqs[id] = vqs[vq_num + 1];
+
+free:
+	kfree(io_names);
+	kfree(io_callbacks);
+	kfree(vqs);
+
+	return err;
+}
+
 static int add_port(struct ports_device *portdev, u32 id)
 {
 	char debugfs_name[16];
@@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id)
 
 	port->outvq_full = false;
 
+	if (!portdev->in_vqs[port->id] && !portdev->out_vqs[port->id]) {
+		spin_lock(&portdev->ports_lock);
+		err = setup_port_vq(portdev, port->id);
+		spin_unlock(&portdev->ports_lock);
+		if (err)
+			goto free_port;
+	}
+
 	port->in_vq = portdev->in_vqs[port->id];
 	port->out_vq = portdev->out_vqs[port->id];
 
@@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev)
 			j += 2;
 			io_callbacks[j] = in_intr;
 			io_callbacks[j + 1] = out_intr;
-			io_names[j] = "input";
-			io_names[j + 1] = "output";
+			io_names[j] = NULL;
+			io_names[j + 1] = NULL;
 		}
 	}
 	/* Find the queues. */
@@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev)
 
 		for (i = 1; i < nr_ports; i++) {
 			j += 2;
-			portdev->in_vqs[i] = vqs[j];
-			portdev->out_vqs[i] = vqs[j + 1];
+			portdev->in_vqs[i] = NULL;
+			portdev->out_vqs[i] = NULL;
 		}
 	}
 	kfree(io_names);
-- 
1.7.1

Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port

[Amit Shah]

Hi,

Sorry for the late reply.

On (Thu) 12 Jan 2012 [09:20:07], zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> Add setup_port_vq(). Create the io ports' vqs when add_port.

Can you describe the changes in more detail, please?

> Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
> ---
>  drivers/char/virtio_console.c |   65 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 8e3c46d..2e5187e 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port)
>  		kill_fasync(&port->async_queue, SIGIO, POLL_OUT);
>  }
>  
> +static void in_intr(struct virtqueue *vq);
> +static void out_intr(struct virtqueue *vq);
> +
> +static int setup_port_vq(struct ports_device *portdev,  u32 id)
> +{
> +	int err, vq_num;
> +	vq_callback_t **io_callbacks;
> +	char **io_names;
> +	struct virtqueue **vqs;
> +	u32 i,j,nr_ports,nr_queues;
> +
> +	err = 0;
> +	vq_num = (id + 1) * 2;
> +	nr_ports = portdev->config.max_nr_ports;
> +	nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
> +
> +	vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
> +	io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL);
> +	io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL);
> +	if (!vqs || !io_callbacks || !io_names) {
> +		err = -ENOMEM;
> +		goto free;
> +	}
> +
> +	for (i = 0, j = 0; i <= nr_ports; i++) {
> +		io_callbacks[j] = in_intr;
> +		io_callbacks[j + 1] = out_intr;
> +		io_names[j] = NULL;
> +		io_names[j + 1] = NULL;
> +		j += 2;
> +	}
> +	io_names[vq_num] = "serial-input";
> +	io_names[vq_num + 1] = "serial-output";
> +	err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
> +				io_callbacks,
> +				(const char **)io_names);
> +	if (err)
> +		goto free;
> +	portdev->in_vqs[id] = vqs[vq_num];
> +	portdev->out_vqs[id] = vqs[vq_num + 1];

I don't think this approach will work fine for port hot-plug /
hot-unplug cases at all.  For example, I first start qemu with one
port, at id 1.  Then I add a port at id 5, then at 2, then at 10.
Will that be fine?

> +
> +free:
> +	kfree(io_names);
> +	kfree(io_callbacks);
> +	kfree(vqs);
> +
> +	return err;
> +}
> +
>  static int add_port(struct ports_device *portdev, u32 id)
>  {
>  	char debugfs_name[16];
> @@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id)
>  
>  	port->outvq_full = false;
>  
> +	if (!portdev->in_vqs[port->id] && !portdev->out_vqs[port->id]) {
> +		spin_lock(&portdev->ports_lock);
> +		err = setup_port_vq(portdev, port->id);
> +		spin_unlock(&portdev->ports_lock);
> +		if (err)
> +			goto free_port;
> +	}
> +
>  	port->in_vq = portdev->in_vqs[port->id];
>  	port->out_vq = portdev->out_vqs[port->id];
>  
> @@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev)
>  			j += 2;
>  			io_callbacks[j] = in_intr;
>  			io_callbacks[j + 1] = out_intr;
> -			io_names[j] = "input";
> -			io_names[j + 1] = "output";
> +			io_names[j] = NULL;
> +			io_names[j + 1] = NULL;
>  		}
>  	}
>  	/* Find the queues. */
> @@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev)
>  
>  		for (i = 1; i < nr_ports; i++) {
>  			j += 2;
> -			portdev->in_vqs[i] = vqs[j];
> -			portdev->out_vqs[i] = vqs[j + 1];
> +			portdev->in_vqs[i] = NULL;
> +			portdev->out_vqs[i] = NULL;
>  		}
>  	}
>  	kfree(io_names);

So a queue once created will not be removed unless the module is
removed or the device is removed.  That seems reasonable, port
hot-unplug will keep queues around, as is the case now.

		Amit

Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs

[Amit Shah]

Michael, Rusty, any comments?

On (Thu) 12 Jan 2012 [09:20:06], zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> changes in vp_try_to_find_vqs:
> Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and
> controls; add_port() calls it to set up vqs of io_port.
> it will not create virtqueue if the name is null.
> 
> Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
> ---
>  drivers/virtio/virtio_pci.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index baabb79..1f98c36 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev)
>  	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
>  		info = vq->priv;
>  		if (vp_dev->per_vq_vectors &&
> -			info->msix_vector != VIRTIO_MSI_NO_VECTOR)
> +			info->msix_vector != VIRTIO_MSI_NO_VECTOR) {
>  			free_irq(vp_dev->msix_entries[info->msix_vector].vector,
>  				 vq);
> +			vp_dev->msix_used_vectors--;
> +		}
>  		vp_del_vq(vq);
>  	}
>  	vp_dev->per_vq_vectors = false;
> @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	u16 msix_vec;
> -	int i, err, nvectors, allocated_vectors;
> +	int i, err, nvectors;
> +
> +	if (vp_dev->msix_used_vectors)
> +		goto setup_vqs;
>  
>  	if (!use_msix) {
>  		/* Old style: one normal interrupt for change and all vqs. */
> @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  	}
>  
>  	vp_dev->per_vq_vectors = per_vq_vectors;
> -	allocated_vectors = vp_dev->msix_used_vectors;
> +
> +setup_vqs:
>  	for (i = 0; i < nvqs; ++i) {
> +		if (names[i] == NULL)
> +			continue;
> +
>  		if (!callbacks[i] || !vp_dev->msix_enabled)
>  			msix_vec = VIRTIO_MSI_NO_VECTOR;
>  		else if (vp_dev->per_vq_vectors)
> -			msix_vec = allocated_vectors++;
> +			msix_vec = vp_dev->msix_used_vectors++;
>  		else
>  			msix_vec = VP_MSIX_VQ_VECTOR;
>  		vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
> -- 
> 1.7.1
> 

		Amit

Re: [PATCH 2/2] virtio-serial: setup_port_vq when adding port

[Zang Hongyong]

On 2012/2/1,星期三 16:12, Amit Shah wrote:
> Hi,
>
> Sorry for the late reply.
>
> On (Thu) 12 Jan 2012 [09:20:07], zanghongyong@huawei.com wrote:
>> From: Hongyong Zang<zanghongyong@huawei.com>
>>
>> Add setup_port_vq(). Create the io ports' vqs when add_port.
> Can you describe the changes in more detail, please?

The motivation of this patch is as follows. When we use virtio-serial
for communication between guest and host, we usually use only a few
ports (for example 1 or 2 ports), so there's no need to create max ports
when build a virtio-serial. The patch does the port hot-plug thing without
port hot-unplug.
>
>> Signed-off-by: Hongyong Zang<zanghongyong@huawei.com>
>> ---
>>   drivers/char/virtio_console.c |   65 ++++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 61 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
>> index 8e3c46d..2e5187e 100644
>> --- a/drivers/char/virtio_console.c
>> +++ b/drivers/char/virtio_console.c
>> @@ -1132,6 +1132,55 @@ static void send_sigio_to_port(struct port *port)
>>   		kill_fasync(&port->async_queue, SIGIO, POLL_OUT);
>>   }
>>
>> +static void in_intr(struct virtqueue *vq);
>> +static void out_intr(struct virtqueue *vq);
>> +
>> +static int setup_port_vq(struct ports_device *portdev,  u32 id)
>> +{
>> +	int err, vq_num;
>> +	vq_callback_t **io_callbacks;
>> +	char **io_names;
>> +	struct virtqueue **vqs;
>> +	u32 i,j,nr_ports,nr_queues;
>> +
>> +	err = 0;
>> +	vq_num = (id + 1) * 2;
>> +	nr_ports = portdev->config.max_nr_ports;
>> +	nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
>> +
>> +	vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
>> +	io_callbacks = kmalloc(nr_queues * sizeof(vq_callback_t *), GFP_KERNEL);
>> +	io_names = kmalloc(nr_queues * sizeof(char *), GFP_KERNEL);
>> +	if (!vqs || !io_callbacks || !io_names) {
>> +		err = -ENOMEM;
>> +		goto free;
>> +	}
>> +
>> +	for (i = 0, j = 0; i<= nr_ports; i++) {
>> +		io_callbacks[j] = in_intr;
>> +		io_callbacks[j + 1] = out_intr;
>> +		io_names[j] = NULL;
>> +		io_names[j + 1] = NULL;
>> +		j += 2;
>> +	}
>> +	io_names[vq_num] = "serial-input";
>> +	io_names[vq_num + 1] = "serial-output";
>> +	err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
>> +				io_callbacks,
>> +				(const char **)io_names);
>> +	if (err)
>> +		goto free;
>> +	portdev->in_vqs[id] = vqs[vq_num];
>> +	portdev->out_vqs[id] = vqs[vq_num + 1];
> I don't think this approach will work fine for port hot-plug /
> hot-unplug cases at all.  For example, I first start qemu with one
> port, at id 1.  Then I add a port at id 5, then at 2, then at 10.
> Will that be fine?

yes.
In this case, the virtio-serial will create id 1's queue when probe the
device. Then, add port id 5, it will create the id 5's queue and the queues
of id 2-4 still be null.
we find the right ioport by vq_num, and only the io_name[vq_num] and
io_name[vq_num+1] are not null. So after portdev->vdev->config->find_vqs(),
the id 5's queues are created(see the changes in virtio-pci), the others 
are
still null.
>
>> +
>> +free:
>> +	kfree(io_names);
>> +	kfree(io_callbacks);
>> +	kfree(vqs);
>> +
>> +	return err;
>> +}
>> +
>>   static int add_port(struct ports_device *portdev, u32 id)
>>   {
>>   	char debugfs_name[16];
>> @@ -1163,6 +1212,14 @@ static int add_port(struct ports_device *portdev, u32 id)
>>
>>   	port->outvq_full = false;
>>
>> +	if (!portdev->in_vqs[port->id]&&  !portdev->out_vqs[port->id]) {
>> +		spin_lock(&portdev->ports_lock);
>> +		err = setup_port_vq(portdev, port->id);
>> +		spin_unlock(&portdev->ports_lock);
>> +		if (err)
>> +			goto free_port;
>> +	}
>> +
>>   	port->in_vq = portdev->in_vqs[port->id];
>>   	port->out_vq = portdev->out_vqs[port->id];
>>
>> @@ -1614,8 +1671,8 @@ static int init_vqs(struct ports_device *portdev)
>>   			j += 2;
>>   			io_callbacks[j] = in_intr;
>>   			io_callbacks[j + 1] = out_intr;
>> -			io_names[j] = "input";
>> -			io_names[j + 1] = "output";
>> +			io_names[j] = NULL;
>> +			io_names[j + 1] = NULL;
>>   		}
>>   	}
>>   	/* Find the queues. */
>> @@ -1635,8 +1692,8 @@ static int init_vqs(struct ports_device *portdev)
>>
>>   		for (i = 1; i<  nr_ports; i++) {
>>   			j += 2;
>> -			portdev->in_vqs[i] = vqs[j];
>> -			portdev->out_vqs[i] = vqs[j + 1];
>> +			portdev->in_vqs[i] = NULL;
>> +			portdev->out_vqs[i] = NULL;
>>   		}
>>   	}
>>   	kfree(io_names);
> So a queue once created will not be removed unless the module is
> removed or the device is removed.  That seems reasonable, port
> hot-unplug will keep queues around, as is the case now.

As we use the virtio-serial with only a few io-ports, the maxport queues 
may
waste more memory. So we try to create queues when port hot-plug, as for
port hot-unplug there's no change.

>
> 		Amit
>
> .
>


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/2] virtio-serial: set up vqs on demand

[Michael S. Tsirkin]

On Thu, Jan 12, 2012 at 09:20:05AM +0800, zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all 
> io_ports are used.
> These patches create vqs of port0 and control port when probing the device, then 
> create io-vqs when called add_port().
> 
> Hongyong Zang (2):
>   virtio-pci: add setup_vqs flag in vp_try_to_find_vqs
>   virtio-serial: setup_port_vq when adding port
> 
>  drivers/char/virtio_console.c |   65 ++++++++++++++++++++++++++++++++++++++--
>  drivers/virtio/virtio_pci.c   |   17 ++++++++--
>  2 files changed, 74 insertions(+), 8 deletions(-)

This wasn't Cc'd to the right list, look at MAINTAINERS
please.

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs

[Michael S. Tsirkin]

On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> changes in vp_try_to_find_vqs:
> Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and
> controls; add_port() calls it to set up vqs of io_port.
> it will not create virtqueue if the name is null.
> 
> Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>

This looks like a fragile hack, to me.

virtio spec also implied that VQ initialization is done
before status is set to OK (during probe) and
devices might have relied on that. So if we want to change
that, I think we need a feature bit.

Besides, what about documentation? non pci transports?

> ---
>  drivers/virtio/virtio_pci.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index baabb79..1f98c36 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev)
>  	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
>  		info = vq->priv;
>  		if (vp_dev->per_vq_vectors &&
> -			info->msix_vector != VIRTIO_MSI_NO_VECTOR)
> +			info->msix_vector != VIRTIO_MSI_NO_VECTOR) {
>  			free_irq(vp_dev->msix_entries[info->msix_vector].vector,
>  				 vq);
> +			vp_dev->msix_used_vectors--;
> +		}
>  		vp_del_vq(vq);
>  	}
>  	vp_dev->per_vq_vectors = false;
> @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	u16 msix_vec;
> -	int i, err, nvectors, allocated_vectors;
> +	int i, err, nvectors;
> +
> +	if (vp_dev->msix_used_vectors)
> +		goto setup_vqs;

Please don't abuse goto in this way.

>  
>  	if (!use_msix) {
>  		/* Old style: one normal interrupt for change and all vqs. */
> @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  	}
>  
>  	vp_dev->per_vq_vectors = per_vq_vectors;
> -	allocated_vectors = vp_dev->msix_used_vectors;
> +
> +setup_vqs:
>  	for (i = 0; i < nvqs; ++i) {
> +		if (names[i] == NULL)

if (![names[i])

> +			continue;
> +
>  		if (!callbacks[i] || !vp_dev->msix_enabled)
>  			msix_vec = VIRTIO_MSI_NO_VECTOR;
>  		else if (vp_dev->per_vq_vectors)
> -			msix_vec = allocated_vectors++;
> +			msix_vec = vp_dev->msix_used_vectors++;
>  		else
>  			msix_vec = VP_MSIX_VQ_VECTOR;
>  		vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs

[Rusty Russell]

On Wed, 1 Feb 2012 11:54:09 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote:
> > From: Hongyong Zang <zanghongyong@huawei.com>
> > 
> > changes in vp_try_to_find_vqs:
> > Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and
> > controls; add_port() calls it to set up vqs of io_port.
> > it will not create virtqueue if the name is null.
> > 
> > Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>
> 
> This looks like a fragile hack, to me.
> 
> virtio spec also implied that VQ initialization is done
> before status is set to OK (during probe) and
> devices might have relied on that. So if we want to change
> that, I think we need a feature bit.

If virtio_serial doesn't use those virtqueues, I think it's OK.

I think I'd rather have vp_find_vqs take an offset, however.

> Besides, what about documentation? non pci transports?

I think this is a device-specific issue, but re-calling find_vqs will
have to be audited.  Which adding an offset arg should lead to anyway.

Thanks,
Rusty.

Re: [PATCH 0/2] virtio-serial: set up vqs on demand

[Amit Shah]

On (Thu) 12 Jan 2012 [09:20:05], zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> Virtio-serial set up (max_ports+1)*2 vqs when device probes, but may not all 
> io_ports are used.
> These patches create vqs of port0 and control port when probing the device, then 
> create io-vqs when called add_port().

Hi,

Can you resurrect this series?  I think last time we were waiting for
a v2, but looks like it got lost.

Let me know if you're not interested anymore, and I can do something
about it.

Thanks,

		Amit