Re: [PATCH v6 6/8] virtio_vdpa: setup correct vq size with callbacks get_vq_num_{max, min}

On Mon, Oct 25, 2021 at 2:25 PM Wu Zongyong
[off-list ref] wrote:

On Mon, Oct 25, 2021 at 12:45:44PM +0800, Jason Wang wrote:

quoted

在 2021/10/25 上午10:44, Wu Zongyong 写道:

quoted

On Mon, Oct 25, 2021 at 10:22:30AM +0800, Jason Wang wrote:

quoted

On Fri, Oct 22, 2021 at 10:45 AM Wu Zongyong
[off-list ref] wrote:

quoted

For the devices which implement the get_vq_num_min callback, the driver
should not negotiate with virtqueue size with the backend vdpa device if
the value returned by get_vq_num_min equals to the value returned by
get_vq_num_max.
This is useful for vdpa devices based on legacy virtio specfication.

Signed-off-by: Wu Zongyong <redacted>
---
  drivers/virtio/virtio_vdpa.c | 21 ++++++++++++++++-----
  1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 72eaef2caeb1..e42ace29daa1 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c

@@ -145,7 +145,8 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
         /* Assume split virtqueue, switch to packed if necessary */
         struct vdpa_vq_state state = {0};
         unsigned long flags;
-       u32 align, num;
+       u32 align, max_num, min_num = 0;
+       bool may_reduce_num = true;
         int err;

         if (!name)

@@ -163,22 +164,32 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
         if (!info)
                 return ERR_PTR(-ENOMEM);

-       num = ops->get_vq_num_max(vdpa);
-       if (num == 0) {
+       max_num = ops->get_vq_num_max(vdpa);
+       if (max_num == 0) {
                 err = -ENOENT;
                 goto error_new_virtqueue;
         }

+       if (ops->get_vq_num_min)
+               min_num = ops->get_vq_num_min(vdpa);
+
+       may_reduce_num = (max_num == min_num) ? false : true;
+
         /* Create the vring */
         align = ops->get_vq_align(vdpa);
-       vq = vring_create_virtqueue(index, num, align, vdev,
-                                   true, true, ctx,
+       vq = vring_create_virtqueue(index, max_num, align, vdev,
+                                   true, may_reduce_num, ctx,
                                     virtio_vdpa_notify, callback, name);
         if (!vq) {
                 err = -ENOMEM;
                 goto error_new_virtqueue;
         }

+       if (virtqueue_get_vring_size(vq) < min_num) {
+               err = -EINVAL;
+               goto err_vq;
+       }

I wonder under which case can we hit this?

Thanks

If min_vq_num < max_vq_num, may_reduce_num should be true, then it is
possible to allocate a virtqueue with a small size which value is less
than the min_vq_num since we only set the upper bound for virtqueue size
when creating virtqueue.

Refers to vring_create_virtqueue_split in driver/virtio/virtio_vring.c:

    for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
            queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
                                      &dma_addr,
                                      GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
            if (queue)
                    break;
            if (!may_reduce_num)
                    return NULL;
    }

It looks to me it's better to fix this function instead of checking it in
the caller?

Or we can simply remove that code since this case only exists in theory, and
there is no real usecase for now.

(Adding list back)

Somehow, it can't happen if you stick to a 256 as both min and max.

Another question, can ENI support vring size which is less than 256?

Thanks

quoted

quoted

BTW, I have replied this mail on Nov.18, have you ever received it?

For some reason I dont' get that.

Thanks

quoted

quoted

quoted

+
         /* Setup virtqueue callback */
         cb.callback = virtio_vdpa_virtqueue_cb;
         cb.private = info;
--
2.31.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization