* [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector
@ 2011-10-05 5:38 Krishna Kumar
2011-10-05 10:48 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Krishna Kumar @ 2011-10-05 5:38 UTC (permalink / raw)
To: rusty, mst; +Cc: netdev, linux-kernel, Krishna Kumar, virtualization
For the MSI but non-per_vq_vector case, the config/change vq
also gets added to the list of vqs that need to process the
MSI interrupt. This is not needed as config has it's own
handler (vp_config_changed). In any case, vring_interrupt()
finds nothing needs to be done on this vq.
I tested this patch by testing the "Fallback:" and "Finally
fall back" cases in vp_find_vqs(). Please review.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
drivers/virtio/virtio_pci.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff -ruNp org/drivers/virtio/virtio_pci.c new/drivers/virtio/virtio_pci.c
--- org/drivers/virtio/virtio_pci.c 2011-10-03 09:10:11.000000000 +0530
+++ new/drivers/virtio/virtio_pci.c 2011-10-04 19:16:34.000000000 +0530
@@ -415,9 +415,13 @@ static struct virtqueue *setup_vq(struct
}
}
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_add(&info->node, &vp_dev->virtqueues);
- spin_unlock_irqrestore(&vp_dev->lock, flags);
+ if (callback) {
+ spin_lock_irqsave(&vp_dev->lock, flags);
+ list_add(&info->node, &vp_dev->virtqueues);
+ spin_unlock_irqrestore(&vp_dev->lock, flags);
+ } else {
+ INIT_LIST_HEAD(&info->node);
+ }
return vq;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector
2011-10-05 5:38 [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector Krishna Kumar
@ 2011-10-05 10:48 ` Michael S. Tsirkin
2011-10-06 0:31 ` Rusty Russell
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2011-10-05 10:48 UTC (permalink / raw)
To: Krishna Kumar; +Cc: rusty, netdev, linux-kernel, virtualization
On Wed, Oct 05, 2011 at 11:08:59AM +0530, Krishna Kumar wrote:
> For the MSI but non-per_vq_vector case, the config/change vq
> also gets added to the list of vqs that need to process the
> MSI interrupt. This is not needed as config has it's own
> handler (vp_config_changed). In any case, vring_interrupt()
> finds nothing needs to be done on this vq.
>
> I tested this patch by testing the "Fallback:" and "Finally
> fall back" cases in vp_find_vqs(). Please review.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
(note: this is not a bugfix so not 3.1 material).
> ---
> drivers/virtio/virtio_pci.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff -ruNp org/drivers/virtio/virtio_pci.c new/drivers/virtio/virtio_pci.c
> --- org/drivers/virtio/virtio_pci.c 2011-10-03 09:10:11.000000000 +0530
> +++ new/drivers/virtio/virtio_pci.c 2011-10-04 19:16:34.000000000 +0530
> @@ -415,9 +415,13 @@ static struct virtqueue *setup_vq(struct
> }
> }
>
> - spin_lock_irqsave(&vp_dev->lock, flags);
> - list_add(&info->node, &vp_dev->virtqueues);
> - spin_unlock_irqrestore(&vp_dev->lock, flags);
> + if (callback) {
> + spin_lock_irqsave(&vp_dev->lock, flags);
> + list_add(&info->node, &vp_dev->virtqueues);
> + spin_unlock_irqrestore(&vp_dev->lock, flags);
> + } else {
> + INIT_LIST_HEAD(&info->node);
> + }
>
> return vq;
Some further enhancement suggestions for this shared case:
- we don't really need a lock, do we? and how about replacing vq list
with an array?
- vring_interrupt calls
if (!more_used(vq))
outside any lock.
This looks scary - don't we need a read barrier somewhere?
--
MST
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector
2011-10-05 10:48 ` Michael S. Tsirkin
@ 2011-10-06 0:31 ` Rusty Russell
0 siblings, 0 replies; 3+ messages in thread
From: Rusty Russell @ 2011-10-06 0:31 UTC (permalink / raw)
To: Michael S. Tsirkin, Krishna Kumar; +Cc: netdev, linux-kernel, virtualization
On Wed, 5 Oct 2011 08:48:23 -0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Oct 05, 2011 at 11:08:59AM +0530, Krishna Kumar wrote:
> > For the MSI but non-per_vq_vector case, the config/change vq
> > also gets added to the list of vqs that need to process the
> > MSI interrupt. This is not needed as config has it's own
> > handler (vp_config_changed). In any case, vring_interrupt()
> > finds nothing needs to be done on this vq.
> >
> > I tested this patch by testing the "Fallback:" and "Finally
> > fall back" cases in vp_find_vqs(). Please review.
> >
> > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> (note: this is not a bugfix so not 3.1 material).
Thanks, applied.
Cheers,
Rusty.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-06 0:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-05 5:38 [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector Krishna Kumar
2011-10-05 10:48 ` Michael S. Tsirkin
2011-10-06 0:31 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).