Re: virtio: imply disable_cb on callbacks

Re: virtio: imply disable_cb on callbacks

[Michael S. Tsirkin]

Rusty, the patch "virtio: imply disable_cb on callbacks" is on your tree.
I'd like to figure out how it works: for example:

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -69,6 +69,8 @@ static void blk_done(struct virtqueue *v
        /* In case queue is stopped waiting for more buffers. */
        blk_start_queue(vblk->disk->queue);
        spin_unlock_irqrestore(&vblk->lock, flags);
+
+       vq->vq_ops->enable_cb(vq);
 }

 static bool do_req(struct request_queue *q, struct virtio_blk *vblk,


Since this does not check the return status from enable_cb,
it seems we could loose an interrupt if it arrives
between poll and callback enable?

Same might apply to other devices.

Thanks,

-- 
MST

Re: virtio: imply disable_cb on callbacks

[Rusty Russell]

On Tue, 18 May 2010 09:03:17 pm Michael S. Tsirkin wrote:
> Rusty, the patch "virtio: imply disable_cb on callbacks" is on your tree.
> I'd like to figure out how it works: for example:
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -69,6 +69,8 @@ static void blk_done(struct virtqueue *v
>         /* In case queue is stopped waiting for more buffers. */
>         blk_start_queue(vblk->disk->queue);
>         spin_unlock_irqrestore(&vblk->lock, flags);
> +
> +       vq->vq_ops->enable_cb(vq);
>  }
> 
>  static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
> 
> 
> Since this does not check the return status from enable_cb,
> it seems we could loose an interrupt if it arrives
> between poll and callback enable?

It's been quite a while since I wrote this patch, and never really liked it
enough to polish it.

We would need to enable this *before* reading the queue, AFAICT.

I'll remove it from my series; it's in the wilderness area already :)

Thanks!
Rusty.