[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 02/12] vhost: Move SVQ queue rewind to the destination
From: |
Eugenio Perez Martin |
Subject: |
Re: [RFC PATCH 02/12] vhost: Move SVQ queue rewind to the destination |
Date: |
Mon, 18 Jul 2022 09:20:14 +0200 |
On Mon, Jul 18, 2022 at 7:50 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Sat, Jul 16, 2022 at 7:34 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > Migration with SVQ already migrate the inflight descriptors,
>
> How is this done?
>
Migration between vhost-vdpa with x-svq=on maintain the guest's
visible state in VirtQueues, and they already have all the protocols
to migrate them.
We cannot migrate if we cannot restore the state / inflight in the
destination, but since we need x-svq=on at this point, we can restore
both of them. Extra care will be needed when ASID is introduced.
> > so the
> > destination can perform the work.
> >
> > This makes easier to migrate between backends or to recover them in
> > vhost devices that support set in flight descriptors.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > hw/virtio/vhost-vdpa.c | 24 +++++++++++-------------
> > 1 file changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 18820498b3..4458c8d23e 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -1178,7 +1178,18 @@ static int vhost_vdpa_set_vring_base(struct
> > vhost_dev *dev,
> > struct vhost_vring_state *ring)
> > {
> > struct vhost_vdpa *v = dev->opaque;
> > + VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
> >
> > + /*
> > + * vhost-vdpa devices does not support in-flight requests. Set all of
> > them
> > + * as available.
> > + *
> > + * TODO: This is ok for networking, but other kinds of devices might
> > + * have problems with these retransmissions.
> > + */
> > + while (virtqueue_rewind(vq, 1)) {
> > + continue;
> > + }
> > if (v->shadow_vqs_enabled) {
> > /*
> > * Device vring base was set at device start. SVQ base is handled
> > by
> > @@ -1197,19 +1208,6 @@ static int vhost_vdpa_get_vring_base(struct
> > vhost_dev *dev,
> > int ret;
> >
> > if (v->shadow_vqs_enabled) {
> > - VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
> > -
> > - /*
> > - * Setting base as last used idx, so destination will see as
> > available
> > - * all the entries that the device did not use, including the
> > in-flight
> > - * processing ones.
> > - *
> > - * TODO: This is ok for networking, but other kinds of devices
> > might
> > - * have problems with these retransmissions.
> > - */
> > - while (virtqueue_rewind(vq, 1)) {
> > - continue;
> > - }
>
> I think it's not a good idea to partially revert the code that was
> just introduced in the previous patch (unless it can be backported to
> -stable independently).
>
> We probably need to squash the changes.
>
You definitely have a point. Maybe it's better to remove the "Fixes"
tag? -stable version cannot enable SVQ anyway (it doesn't have the
parameter).
I'll send a v2 when we have all the comments sorted out. Maybe a
better alternative is to delay the x-svq parameter to the top of both
series if both of them go in?
Thanks!
> Thanks
>
> > ring->num = virtio_queue_get_last_avail_idx(dev->vdev,
> > ring->index);
> > return 0;
> > }
> > --
> > 2.31.1
> >
>
- [RFC PATCH 00/12] NIC vhost-vdpa state restore via Shadow CVQ, Eugenio Pérez, 2022/07/16
- [RFC PATCH 01/12] vhost: Get vring base from vq, not svq, Eugenio Pérez, 2022/07/16
- [RFC PATCH 02/12] vhost: Move SVQ queue rewind to the destination, Eugenio Pérez, 2022/07/16
- [RFC PATCH 03/12] vdpa: Small rename of error labels, Eugenio Pérez, 2022/07/16
- [RFC PATCH 04/12] vdpa: delay set_vring_ready after DRIVER_OK, Eugenio Pérez, 2022/07/16
- [RFC PATCH 06/12] vhost: Use opaque data in SVQDescState, Eugenio Pérez, 2022/07/16
- [RFC PATCH 05/12] vhost: stop transfer elem ownership in vhost_handle_guest_kick, Eugenio Pérez, 2022/07/16
- [RFC PATCH 07/12] vhost: Add VhostVDPAStartOp operation, Eugenio Pérez, 2022/07/16