On Thu, Mar 12, 2026 at 5:03 AM Jason Wang <[email protected]> wrote: > > On Wed, Mar 11, 2026 at 3:10 AM Eugenio Pérez <[email protected]> wrote: > > > > Implement suspend operation for vduse devices, so vhost-vdpa will offer > > that backend feature and userspace can effectively suspend the device. > > > > This is a must before get virtqueue indexes (base) for live migration, > > since the device could modify them after userland gets them. > > As discussed in the pervious version, let's explain why and the plan > to support resume. >
Is it ok to explain it just in the patch message or do you prefer something like a /* TODO: resume op */ before the suspend callback? > > > > Signed-off-by: Eugenio Pérez <[email protected]> > > --- > > This patch depends on > > https://lore.kernel.org/lkml/[email protected] > > > > v2: > > * Take the rwsem only before the actual kick, not in vduse_vdpa_kick_vq. > > This assures that we're not in a critical section. > > --- > > drivers/vdpa/vdpa_user/vduse_dev.c | 86 +++++++++++++++++++++++++++++- > > include/uapi/linux/vduse.h | 4 ++ > > 2 files changed, 88 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c > > b/drivers/vdpa/vdpa_user/vduse_dev.c > > index 4f642b95a7cb..f56b1e3eb82d 100644 > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > @@ -54,7 +54,8 @@ > > #define IRQ_UNBOUND -1 > > > > /* Supported VDUSE features */ > > -static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY); > > +static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY) | > > + BIT_U64(VDUSE_F_SUSPEND); > > > > /* > > * VDUSE instance have not asked the vduse API version, so assume 0. > > @@ -85,6 +86,7 @@ struct vduse_virtqueue { > > int irq_effective_cpu; > > struct cpumask irq_affinity; > > struct kobject kobj; > > + struct vduse_dev *dev; > > }; > > > > struct vduse_dev; > > @@ -134,6 +136,7 @@ struct vduse_dev { > > int minor; > > bool broken; > > bool connected; > > + bool suspended; > > u64 api_version; > > u64 device_features; > > u64 driver_features; > > @@ -480,6 +483,7 @@ static void vduse_dev_reset(struct vduse_dev *dev) > > > > down_write(&dev->rwsem); > > > > + dev->suspended = false; > > dev->status = 0; > > dev->driver_features = 0; > > dev->generation++; > > @@ -538,6 +542,10 @@ static void vduse_vq_kick(struct vduse_virtqueue *vq) > > if (!vq->ready) > > goto unlock; > > > > + guard(rwsem_read)(&vq->dev->rwsem); > > + if (vq->dev->suspended) > > + return; > > Any reason for this? E.g We don't do this for other transports. > It's just a hardening but I'm happy to remove it. > Everything else looks good. > > Thanks >
