Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

Michael S. Tsirkin
2009-05-08T12:50:35Z

On Fri, May 08, 2009 at 04:37:06PM +0930, Rusty Russell wrote:
> On Thu, 7 May 2009 11:40:39 pm Michael S. Tsirkin wrote:
> > This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> > and updates all drivers. This is needed for MSI support, because MSI
> > needs to know the total number of vectors upfront.
>
> Hmm, I have a similar need for a dev to vq mapping (debugging stats). How's
> this as a common basis?

This helps. Should I redo mine on top of this?

> Thanks,
> Rusty.
>
> virtio: add names to virtqueue struct, mapping from devices to queues.
>
> Add a linked list of all virtqueues for a virtio device: this helps for
> debugging and is also needed for upcoming interface change.
>
> Also, add a "name" field for clearer debug messages.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Yes, this will simplify supporting find_vqs.

> @@ -303,10 +313,12 @@ struct virtqueue *vring_new_virtqueue(un
> vq->vq.callback = callback;
> vq->vq.vdev = vdev;
> vq->vq.vq_ops = &vring_vq_ops;
> + vq->vq.name = name;
> vq->notify = notify;
> vq->broken = false;
> vq->last_used_idx = 0;
> vq->num_added = 0;
> + list_add_tail(&vq->vq.list, &vdev->vqs);
> #ifdef DEBUG
> vq->in_use = false;
> #endif
> @@ -327,6 +339,7 @@ EXPORT_SYMBOL_GPL(vring_new_virtqueue);
>
> void vring_del_virtqueue(struct virtqueue *vq)
> {
> + list_del(&vq->list);
> kfree(to_vvq(vq));
> }
> EXPORT_SYMBOL_GPL(vring_del_virtqueue);

I note lack of locking here. This is okay in practice as
drivers don't really call find/del vq in parallel,
but making this explicit with find_vqs will be best, yes?

--
MST