Re: [PATCH 3/4] virtio_ring: introduce a per virtqueue waitqueue

From: Michael S. Tsirkin
Date: Mon Dec 26 2022 - 18:35:37 EST


On Mon, Dec 26, 2022 at 03:49:07PM +0800, Jason Wang wrote:
> This patch introduces a per virtqueue waitqueue to allow driver to
> sleep and wait for more used. Two new helpers are introduced to allow
> driver to sleep and wake up.
>
> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> ---
> Changes since V1:
> - check virtqueue_is_broken() as well
> - use more_used() instead of virtqueue_get_buf() to allow caller to
> get buffers afterwards
> ---
> drivers/virtio/virtio_ring.c | 29 +++++++++++++++++++++++++++++
> include/linux/virtio.h | 3 +++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 5cfb2fa8abee..9c83eb945493 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -13,6 +13,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/kmsan.h>
> #include <linux/spinlock.h>
> +#include <linux/wait.h>
> #include <xen/xen.h>
>
> #ifdef DEBUG
> @@ -60,6 +61,7 @@
> "%s:"fmt, (_vq)->vq.name, ##args); \
> /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ \
> WRITE_ONCE((_vq)->broken, true); \
> + wake_up_interruptible(&(_vq)->wq); \
> } while (0)
> #define START_USE(vq)
> #define END_USE(vq)
> @@ -203,6 +205,9 @@ struct vring_virtqueue {
> /* DMA, allocation, and size information */
> bool we_own_ring;
>
> + /* Wait for buffer to be used */
> + wait_queue_head_t wq;
> +
> #ifdef DEBUG
> /* They're supposed to lock for us. */
> unsigned int in_use;
> @@ -2024,6 +2029,8 @@ static struct virtqueue *vring_create_virtqueue_packed(
> if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> vq->weak_barriers = false;
>
> + init_waitqueue_head(&vq->wq);
> +
> err = vring_alloc_state_extra_packed(&vring_packed);
> if (err)
> goto err_state_extra;
> @@ -2517,6 +2524,8 @@ static struct virtqueue *__vring_new_virtqueue(unsigned int index,
> if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> vq->weak_barriers = false;
>
> + init_waitqueue_head(&vq->wq);
> +
> err = vring_alloc_state_extra_split(vring_split);
> if (err) {
> kfree(vq);
> @@ -2654,6 +2663,8 @@ static void vring_free(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
>
> + wake_up_interruptible(&vq->wq);
> +
> if (vq->we_own_ring) {
> if (vq->packed_ring) {
> vring_free_queue(vq->vq.vdev,
> @@ -2863,4 +2874,22 @@ const struct vring *virtqueue_get_vring(struct virtqueue *vq)
> }
> EXPORT_SYMBOL_GPL(virtqueue_get_vring);
>
> +int virtqueue_wait_for_used(struct virtqueue *_vq)
> +{
> + struct vring_virtqueue *vq = to_vvq(_vq);
> +
> + /* TODO: Tweak the timeout. */
> + return wait_event_interruptible_timeout(vq->wq,
> + virtqueue_is_broken(_vq) || more_used(vq), HZ);

There's no good timeout. Let's not even go there, if device goes
bad it should set the need reset bit.


> +}
> +EXPORT_SYMBOL_GPL(virtqueue_wait_for_used);
> +
> +void virtqueue_wake_up(struct virtqueue *_vq)
> +{
> + struct vring_virtqueue *vq = to_vvq(_vq);
> +
> + wake_up_interruptible(&vq->wq);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_wake_up);
> +
> MODULE_LICENSE("GPL");
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index dcab9c7e8784..2eb62c774895 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -72,6 +72,9 @@ void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
> void *virtqueue_get_buf_ctx(struct virtqueue *vq, unsigned int *len,
> void **ctx);
>
> +int virtqueue_wait_for_used(struct virtqueue *vq);
> +void virtqueue_wake_up(struct virtqueue *vq);
> +
> void virtqueue_disable_cb(struct virtqueue *vq);
>
> bool virtqueue_enable_cb(struct virtqueue *vq);
> --
> 2.25.1