Skip to contentSkip to conversation

virtio: imply disable_cb on callbacks

DISCUSSION Michael S. Tsirkin 2 participants 3 messages May 18, 2010 – May 19, 2010

Subsystems (4)
KVM Block layer virtio core virtio block and SCSI drivers
lore
Share & tools
lore

Michael S. Tsirkin

author maintainer
#0 lore raw

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

drivers/block/virtio_blk.c +2 -0
@@ -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,

Signature · 1 line
MST
#1 lore raw
Quoted 17 lines · 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:

drivers/block/virtio_blk.c +2 -0
@@ -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,
Quoted 3 lines

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.

#2 lore raw
In reply to Michael S. Tsirkin
Quoted 17 lines · 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:

drivers/block/virtio_blk.c +2 -0
@@ -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,
Quoted 3 lines

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.