From: "Michael S. Tsirkin" <mst@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
qemu-devel@nongnu.org, Tiwei Bie <tiwei.bie@intel.com>
Subject: Re: [PATCH] virtio: signal after wrapping packed used_idx
Date: Tue, 30 Nov 2021 18:40:49 -0500 [thread overview]
Message-ID: <20211130184031-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20211130134510.267382-1-stefanha@redhat.com>
On Tue, Nov 30, 2021 at 01:45:10PM +0000, Stefan Hajnoczi wrote:
> Packed Virtqueues wrap used_idx instead of letting it run freely like
> Split Virtqueues do. If the used ring wraps more than once there is no
> way to compare vq->signalled_used and vq->used_idx in
> virtio_packed_should_notify() since they are modulo vq->vring.num.
>
> This causes the device to stop sending used buffer notifications when
> when virtio_packed_should_notify() is called less than once each time
> around the used ring.
>
> It is possible to trigger this with virtio-blk's dataplane
> notify_guest_bh() irq coalescing optimization. The call to
> virtio_notify_irqfd() (and virtio_packed_should_notify()) is deferred to
> a BH. If the guest driver is polling it can complete and submit more
> requests before the BH executes, causing the used ring to wrap more than
> once. The result is that the virtio-blk device ceases to raise
> interrupts and I/O hangs.
>
> Cc: Tiwei Bie <tiwei.bie@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Makes sense. Fixes tag?
> ---
> Smarter solutions welcome, but I think notifying once per vq->vring.num
> is acceptable.
> ---
> hw/virtio/virtio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ea7c079fb0..f7851c2750 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -885,6 +885,7 @@ static void virtqueue_packed_flush(VirtQueue *vq, unsigned int count)
> if (vq->used_idx >= vq->vring.num) {
> vq->used_idx -= vq->vring.num;
> vq->used_wrap_counter ^= 1;
> + vq->signalled_used_valid = false;
> }
> }
>
> --
> 2.33.1
next prev parent reply other threads:[~2021-11-30 23:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-30 13:45 [PATCH] virtio: signal after wrapping packed used_idx Stefan Hajnoczi
2021-11-30 23:40 ` Michael S. Tsirkin [this message]
2021-12-01 9:58 ` Stefan Hajnoczi
2021-12-02 3:11 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211130184031-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=jasowang@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=tiwei.bie@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.