Re: [PATCH] virtio-blk: avoid using ioeventfd state in irqfd conditional

[Stefan Hajnoczi]

On Mon, 22 Jan 2024 at 12:27, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Requests that complete in an IOThread use irqfd to notify the guest
> while requests that complete in the main loop thread use the traditional
> qdev irq code path. The reason for this conditional is that the irq code
> path requires the BQL:
>
>   if (s->ioeventfd_started && !s->ioeventfd_disabled) {
>       virtio_notify_irqfd(vdev, req->vq);
>   } else {
>       virtio_notify(vdev, req->vq);
>   }
>
> There is a corner case where the conditional invokes the irq code path
> instead of the irqfd code path:
>
>   static void virtio_blk_stop_ioeventfd(VirtIODevice *vdev)
>   {
>       ...
>       /*
>        * Set ->ioeventfd_started to false before draining so that host 
> notifiers
>        * are not detached/attached anymore.
>        */
>       s->ioeventfd_started = false;
>
>       /* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */
>       blk_drain(s->conf.conf.blk);
>
> During blk_drain() the conditional produces the wrong result because
> ioeventfd_started is false.
>
> Use qemu_in_iothread() instead of checking the ioeventfd state.
>
> Buglink: https://issues.redhat.com/browse/RHEL-15394
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

Ping?

> Based-on: 
> https://repo.or.cz/qemu/kevin.git/shortlog/c14962c3ea6f0998d028142ed14affcb9dfccf28
>
> Stable backport notes: dataplane_started is being renamed to
> ioeventfd_started in the next block pull request. This patch can be
> safely applied to -stable although the variable name has changed and
> git-am will complain.
>
>  hw/block/virtio-blk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 227d83569f..287c31ee3c 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -64,7 +64,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, 
> unsigned char status)
>      iov_discard_undo(&req->inhdr_undo);
>      iov_discard_undo(&req->outhdr_undo);
>      virtqueue_push(req->vq, &req->elem, req->in_len);
> -    if (s->ioeventfd_started && !s->ioeventfd_disabled) {
> +    if (qemu_in_iothread()) {
>          virtio_notify_irqfd(vdev, req->vq);
>      } else {
>          virtio_notify(vdev, req->vq);
> --
> 2.43.0
>
>