On Tue, 7 Mar 2023 14:43:58 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> On Thu, Mar 2, 2023 at 7:59 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> >
> > The subsequent reset function will reuse these logic.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > ---
> > drivers/virtio/virtio_ring.c | 58 ++++++++++++++++++++++++------------
> > 1 file changed, 39 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 500cb8563f2b..5ab9136a363e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -2215,6 +2215,43 @@ static int virtqueue_resize_packed(struct virtqueue *_vq, u32 num)
> > return -ENOMEM;
> > }
> >
> > +static int virtqueue_disable_and_recycle(struct virtqueue *_vq,
> > + void (*recycle)(struct virtqueue *vq, void *buf))
> > +{
> > + struct vring_virtqueue *vq = to_vvq(_vq);
> > + struct virtio_device *vdev = vq->vq.vdev;
> > + void *buf;
> > + int err;
> > +
> > + if (!vq->we_own_ring)
> > + return -EPERM;
> > +
> > + if (!vdev->config->disable_vq_and_reset)
> > + return -ENOENT;
> > +
> > + if (!vdev->config->enable_vq_after_reset)
> > + return -ENOENT;
> > +
> > + err = vdev->config->disable_vq_and_reset(_vq);
> > + if (err)
> > + return err;
> > +
> > + while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
> > + recycle(_vq, buf);
> > +
> > + return 0;
> > +}
> > +
> > +static int virtqueue_enable_after_reset(struct virtqueue *_vq)
> > +{
> > + struct vring_virtqueue *vq = to_vvq(_vq);
> > + struct virtio_device *vdev = vq->vq.vdev;
> > +
> > + if (vdev->config->enable_vq_after_reset(_vq))
> > + return -EBUSY;
> > +
> > + return 0;
> > +}
> >
> > /*
> > * Generic functions and exported symbols.
> > @@ -2836,13 +2873,8 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num,
> > void (*recycle)(struct virtqueue *vq, void *buf))
> > {
> > struct vring_virtqueue *vq = to_vvq(_vq);
> > - struct virtio_device *vdev = vq->vq.vdev;
> > - void *buf;
> > int err;
> >
> > - if (!vq->we_own_ring)
> > - return -EPERM;
>
> Any reason to move this but keep the num_max check?
The purpose of this commit is to separate disable/enable logic.
But num_max check is resize-related, so it is still retained in this function.
>
> > -
> > if (num > vq->vq.num_max)
> > return -E2BIG;
> >
> > @@ -2852,28 +2884,16 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num,
> > if ((vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num) == num)
> > return 0;
> >
> > - if (!vdev->config->disable_vq_and_reset)
> > - return -ENOENT;
> > -
> > - if (!vdev->config->enable_vq_after_reset)
> > - return -ENOENT;
> > -
> > - err = vdev->config->disable_vq_and_reset(_vq);
> > + err = virtqueue_disable_and_recycle(_vq, recycle);
> > if (err)
> > return err;
> >
> > - while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
> > - recycle(_vq, buf);
>
> I wonder if it makes sense to factor out recycle() to be used by other
> facilities in the future?
I think that for virtQueue_disable_and_recycle(), it is meaningful just
disabling vq and no recycle. If there is this scene in the future, we only need
to allow virtqueue_disable_and_recycle to accept NULL as reycle.
Thansk.
>
> Thanks
>
>
> > -
> > if (vq->packed_ring)
> > err = virtqueue_resize_packed(_vq, num);
> > else
> > err = virtqueue_resize_split(_vq, num);
> >
> > - if (vdev->config->enable_vq_after_reset(_vq))
> > - return -EBUSY;
> > -
> > - return err;
> > + return virtqueue_enable_after_reset(_vq);
> > }
> > EXPORT_SYMBOL_GPL(virtqueue_resize);
> >
> > --
> > 2.32.0.3.g01195cf9f
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
