Jason Wang
2021-Oct-26 04:46 UTC
[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 <wuzongyong at linux.alibaba.com> wrote:> > On Mon, Oct 25, 2021 at 12:45:44PM +0800, Jason Wang wrote: > > > > ? 2021/10/25 ??10:44, Wu Zongyong ??: > > > On Mon, Oct 25, 2021 at 10:22:30AM +0800, Jason Wang wrote: > > > > On Fri, Oct 22, 2021 at 10:45 AM Wu Zongyong > > > > <wuzongyong at linux.alibaba.com> wrote: > > > > > 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 <wuzongyong at linux.alibaba.com> > > > > > --- > > > > > 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> > > > > > > > > > > BTW, I have replied this mail on Nov.18, have you ever received it? > > > > > > For some reason I dont' get that. > > > > Thanks > > > > > > > > > + > > > > > /* Setup virtqueue callback */ > > > > > cb.callback = virtio_vdpa_virtqueue_cb; > > > > > cb.private = info; > > > > > -- > > > > > 2.31.1 > > > > > >