The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
well after initial setup, and the device can read it afterwards.

Ensure that reads and writes to vq->ready are SMP safe so that the
caller can trust that virtqueue kicks and calls behave as expected
immediately after the operation returns.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 73d1d517dc6c..a4963aaf9332 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
+static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
+{
+	/*
+	 * Paired with vduse_vq_set_ready smp_store, as the driver may modify
+	 * it while the VDUSE instance is reading it.
+	 */
+	return smp_load_acquire(&vq->ready);
+}
+
+static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
+{
+	/*
+	 * Paired with vduse_vq_get_ready smp_load, as the driver may modify
+	 * it while the VDUSE instance is reading it.
+	 */
+	smp_store_release(&vq->ready, ready);
+}
+
 static void vduse_dev_reset(struct vduse_dev *dev)
 {
 	int i;
@@ -486,7 +504,7 @@ static void vduse_dev_reset(struct vduse_dev *dev)
 	for (i = 0; i < dev->vq_num; i++) {
 		struct vduse_virtqueue *vq = dev->vqs[i];
 
-		vq->ready = false;
+		vduse_vq_set_ready(vq, false);
 		vq->desc_addr = 0;
 		vq->driver_addr = 0;
 		vq->device_addr = 0;
@@ -529,7 +547,7 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
 static void vduse_vq_kick(struct vduse_virtqueue *vq)
 {
 	spin_lock(&vq->kick_lock);
-	if (!vq->ready)
+	if (!vduse_vq_get_ready(vq))
 		goto unlock;
 
 	if (vq->kickfd)
@@ -598,7 +616,7 @@ static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
 	struct vduse_virtqueue *vq = dev->vqs[idx];
 
-	vq->ready = ready;
+	vduse_vq_set_ready(vq, ready);
 }
 
 static bool vduse_vdpa_get_vq_ready(struct vdpa_device *vdpa, u16 idx)
@@ -606,7 +624,7 @@ static bool vduse_vdpa_get_vq_ready(struct vdpa_device *vdpa, u16 idx)
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
 	struct vduse_virtqueue *vq = dev->vqs[idx];
 
-	return vq->ready;
+	return vduse_vq_get_ready(vq);
 }
 
 static int vduse_vdpa_set_vq_state(struct vdpa_device *vdpa, u16 idx,
@@ -1097,7 +1115,7 @@ static int vduse_kickfd_setup(struct vduse_dev *dev,
 	if (vq->kickfd)
 		eventfd_ctx_put(vq->kickfd);
 	vq->kickfd = ctx;
-	if (vq->ready && vq->kicked && vq->kickfd) {
+	if (vduse_vq_get_ready(vq) && vq->kicked && vq->kickfd) {
 		eventfd_signal(vq->kickfd);
 		vq->kicked = false;
 	}
@@ -1133,7 +1151,7 @@ static void vduse_vq_irq_inject(struct work_struct *work)
 					struct vduse_virtqueue, inject);
 
 	spin_lock_bh(&vq->irq_lock);
-	if (vq->ready && vq->cb.callback)
+	if (vduse_vq_get_ready(vq) && vq->cb.callback)
 		vq->cb.callback(vq->cb.private);
 	spin_unlock_bh(&vq->irq_lock);
 }
@@ -1146,7 +1164,7 @@ static bool vduse_vq_signal_irqfd(struct vduse_virtqueue *vq)
 		return false;
 
 	spin_lock_irq(&vq->irq_lock);
-	if (vq->ready && vq->cb.trigger) {
+	if (vduse_vq_get_ready(vq) && vq->cb.trigger) {
 		eventfd_signal(vq->cb.trigger);
 		signal = true;
 	}
@@ -1500,7 +1518,7 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			vq_info.split.avail_index =
 				vq->state.split.avail_index;
 
-		vq_info.ready = vq->ready;
+		vq_info.ready = vduse_vq_get_ready(vq);
 
 		ret = -EFAULT;
 		if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
-- 
2.52.0

On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> well after initial setup, and the device can read it afterwards.
>
> Ensure that reads and writes to vq->ready are SMP safe so that the
> caller can trust that virtqueue kicks and calls behave as expected
> immediately after the operation returns.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 73d1d517dc6c..a4963aaf9332 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
>         return mask;
>  }
>
> +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> +{
> +       /*
> +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> +        * it while the VDUSE instance is reading it.
> +        */
> +       return smp_load_acquire(&vq->ready);
> +}
> +
> +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> +{
> +       /*
> +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> +        * it while the VDUSE instance is reading it.
> +        */
> +       smp_store_release(&vq->ready, ready);

Assuming this is not used in the datapath, I wonder if we can simply
use vq_lock mutex.

Thanks

On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > well after initial setup, and the device can read it afterwards.
> >
> > Ensure that reads and writes to vq->ready are SMP safe so that the
> > caller can trust that virtqueue kicks and calls behave as expected
> > immediately after the operation returns.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> >  1 file changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 73d1d517dc6c..a4963aaf9332 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> >         return mask;
> >  }
> >
> > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > +{
> > +       /*
> > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > +        * it while the VDUSE instance is reading it.
> > +        */
> > +       return smp_load_acquire(&vq->ready);
> > +}
> > +
> > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > +{
> > +       /*
> > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > +        * it while the VDUSE instance is reading it.
> > +        */
> > +       smp_store_release(&vq->ready, ready);
>
> Assuming this is not used in the datapath, I wonder if we can simply
> use vq_lock mutex.
>

The function vduse_vq_set/get_ready are not in the datapath, but
vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
switch to vq_mutex if you want though, maybe it's even comparable with
the cost of the ioctls or eventfd signaling.

On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > well after initial setup, and the device can read it afterwards.
> > >
> > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > caller can trust that virtqueue kicks and calls behave as expected
> > > immediately after the operation returns.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 73d1d517dc6c..a4963aaf9332 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > >         return mask;
> > >  }
> > >
> > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > +{
> > > +       /*
> > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > +        * it while the VDUSE instance is reading it.
> > > +        */
> > > +       return smp_load_acquire(&vq->ready);
> > > +}
> > > +
> > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > +{
> > > +       /*
> > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > +        * it while the VDUSE instance is reading it.
> > > +        */
> > > +       smp_store_release(&vq->ready, ready);
> >
> > Assuming this is not used in the datapath, I wonder if we can simply
> > use vq_lock mutex.
> >
>
> The function vduse_vq_set/get_ready are not in the datapath, but
> vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> switch to vq_mutex if you want though, maybe it's even comparable with
> the cost of the ioctls or eventfd signaling.

I'd like to use mutex for simplicity.

Thanks

>

On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> <eperezma@redhat.com> wrote:
> >
> > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > >
> > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > well after initial setup, and the device can read it afterwards.
> > > >
> > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > immediately after the operation returns.
> > > >
> > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > ---
> > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > >         return mask;
> > > >  }
> > > >
> > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > +{
> > > > +       /*
> > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > +        * it while the VDUSE instance is reading it.
> > > > +        */
> > > > +       return smp_load_acquire(&vq->ready);
> > > > +}
> > > > +
> > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > +{
> > > > +       /*
> > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > +        * it while the VDUSE instance is reading it.
> > > > +        */
> > > > +       smp_store_release(&vq->ready, ready);
> > >
> > > Assuming this is not used in the datapath, I wonder if we can simply
> > > use vq_lock mutex.
> > >
> >
> > The function vduse_vq_set/get_ready are not in the datapath, but
> > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > switch to vq_mutex if you want though, maybe it's even comparable with
> > the cost of the ioctls or eventfd signaling.
>
> I'd like to use mutex for simplicity.
>

I cannot move it to a mutex, as we need to take it in the critical
sections of the kick_lock and irq_lock spinlocks.

I can move it to a spinlock, but it seems more complicated to me. We
need to make sure we always take these kick_lock and irq_lock in the
same order as the ready_lock, to not create deadlocks, and they always
protect just the ready boolean. But sure, I'll send the spinlock
version for V2.

On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > <eperezma@redhat.com> wrote:
> > >
> > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > >
> > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > well after initial setup, and the device can read it afterwards.
> > > > >
> > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > immediately after the operation returns.
> > > > >
> > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > ---
> > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > >         return mask;
> > > > >  }
> > > > >
> > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > +{
> > > > > +       /*
> > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > +        * it while the VDUSE instance is reading it.
> > > > > +        */
> > > > > +       return smp_load_acquire(&vq->ready);
> > > > > +}
> > > > > +
> > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > +{
> > > > > +       /*
> > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > +        * it while the VDUSE instance is reading it.
> > > > > +        */
> > > > > +       smp_store_release(&vq->ready, ready);
> > > >
> > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > use vq_lock mutex.
> > > >
> > >
> > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > the cost of the ioctls or eventfd signaling.
> >
> > I'd like to use mutex for simplicity.
> >
>
> I cannot move it to a mutex, as we need to take it in the critical
> sections of the kick_lock and irq_lock spinlocks.
>
> I can move it to a spinlock, but it seems more complicated to me. We
> need to make sure we always take these kick_lock and irq_lock in the
> same order as the ready_lock, to not create deadlocks, and they always
> protect just the ready boolean. But sure, I'll send the spinlock
> version for V2.

Thinking about this, I'm not sure I understand the issue.

Maybe you can give me an example of the race.

THanks

>

On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
> <eperezma@redhat.com> wrote:
> >
> > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > > <eperezma@redhat.com> wrote:
> > > >
> > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > >
> > > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > > well after initial setup, and the device can read it afterwards.
> > > > > >
> > > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > > immediately after the operation returns.
> > > > > >
> > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > ---
> > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > > >         return mask;
> > > > > >  }
> > > > > >
> > > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > > +{
> > > > > > +       /*
> > > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > +        */
> > > > > > +       return smp_load_acquire(&vq->ready);
> > > > > > +}
> > > > > > +
> > > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > > +{
> > > > > > +       /*
> > > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > +        */
> > > > > > +       smp_store_release(&vq->ready, ready);
> > > > >
> > > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > > use vq_lock mutex.
> > > > >
> > > >
> > > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > > the cost of the ioctls or eventfd signaling.
> > >
> > > I'd like to use mutex for simplicity.
> > >
> >
> > I cannot move it to a mutex, as we need to take it in the critical
> > sections of the kick_lock and irq_lock spinlocks.
> >
> > I can move it to a spinlock, but it seems more complicated to me. We
> > need to make sure we always take these kick_lock and irq_lock in the
> > same order as the ready_lock, to not create deadlocks, and they always
> > protect just the ready boolean. But sure, I'll send the spinlock
> > version for V2.
>
> Thinking about this, I'm not sure I understand the issue.
>
> Maybe you can give me an example of the race.
>

It's the same issue as you describe with set_group_asid on cpu1 and
dma_map and dma_unmap in cpu1 in this mail [0], but now the functions
are set_vq_ready in the vdpa driver side and VDUSE_VQ_GET_INFO in the
VDUSE instance.

To me, the barriers should be in the driver and the VDUSE instance,
and the VDUSE instance should use the VDUSE_SET_VQ_READY message to
know when a VQ is ready. But I don't expect all the VDUSE instances to
opt-in for that, so the flow without smp barriers is:

[cpu0] .set_vq_ready(vq, true)
[cpu1] VDUSE_VQ_GET_INFO(vq)
[cpu1] check vq->ready, and it races with the set_vq_ready from cpu0.

[0] https://patchew.org/linux/20251113115558.1277981-1-eperezma@redhat.com/20251113115558.1277981-6-eperezma@redhat.com/#CACGkMEtMyte91bbdb0hfUo+YgSpZdjoWe0meKwbDc18CfWdATA@mail.gmail.com

On Tue, Feb 3, 2026 at 6:35 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
> > <eperezma@redhat.com> wrote:
> > >
> > > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > > > <eperezma@redhat.com> wrote:
> > > > >
> > > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > >
> > > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > > >
> > > > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > > > well after initial setup, and the device can read it afterwards.
> > > > > > >
> > > > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > > > immediately after the operation returns.
> > > > > > >
> > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > > ---
> > > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > > > >         return mask;
> > > > > > >  }
> > > > > > >
> > > > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > > > +{
> > > > > > > +       /*
> > > > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > +        */
> > > > > > > +       return smp_load_acquire(&vq->ready);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > > > +{
> > > > > > > +       /*
> > > > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > +        */
> > > > > > > +       smp_store_release(&vq->ready, ready);
> > > > > >
> > > > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > > > use vq_lock mutex.
> > > > > >
> > > > >
> > > > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > > > the cost of the ioctls or eventfd signaling.
> > > >
> > > > I'd like to use mutex for simplicity.
> > > >
> > >
> > > I cannot move it to a mutex, as we need to take it in the critical
> > > sections of the kick_lock and irq_lock spinlocks.
> > >
> > > I can move it to a spinlock, but it seems more complicated to me. We
> > > need to make sure we always take these kick_lock and irq_lock in the
> > > same order as the ready_lock, to not create deadlocks, and they always
> > > protect just the ready boolean. But sure, I'll send the spinlock
> > > version for V2.
> >
> > Thinking about this, I'm not sure I understand the issue.
> >
> > Maybe you can give me an example of the race.
> >
>
> It's the same issue as you describe with set_group_asid on cpu1 and
> dma_map and dma_unmap in cpu1 in this mail [0], but now the functions
> are set_vq_ready in the vdpa driver side and VDUSE_VQ_GET_INFO in the
> VDUSE instance.
>
> To me, the barriers should be in the driver and the VDUSE instance,
> and the VDUSE instance should use the VDUSE_SET_VQ_READY message to
> know when a VQ is ready. But I don't expect all the VDUSE instances to
> opt-in for that, so the flow without smp barriers is:
>
> [cpu0] .set_vq_ready(vq, true)
> [cpu1] VDUSE_VQ_GET_INFO(vq)
> [cpu1] check vq->ready, and it races with the set_vq_ready from cpu0.
>

Are there any side effects of this race? I meant the driver can do
set_vq_ready() at any time. And there would be message for notifying
the usersapce in this series.

> [0] https://patchew.org/linux/20251113115558.1277981-1-eperezma@redhat.com/20251113115558.1277981-6-eperezma@redhat.com/#CACGkMEtMyte91bbdb0hfUo+YgSpZdjoWe0meKwbDc18CfWdATA@mail.gmail.com
>

Thanks

On Wed, Feb 4, 2026 at 3:48 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Feb 3, 2026 at 6:35 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
> > > <eperezma@redhat.com> wrote:
> > > >
> > > > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > > > > <eperezma@redhat.com> wrote:
> > > > > >
> > > > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > > > >
> > > > > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > > > > well after initial setup, and the device can read it afterwards.
> > > > > > > >
> > > > > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > > > > immediately after the operation returns.
> > > > > > > >
> > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > > > ---
> > > > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > > > > >         return mask;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > > > > +{
> > > > > > > > +       /*
> > > > > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > +        */
> > > > > > > > +       return smp_load_acquire(&vq->ready);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > > > > +{
> > > > > > > > +       /*
> > > > > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > +        */
> > > > > > > > +       smp_store_release(&vq->ready, ready);
> > > > > > >
> > > > > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > > > > use vq_lock mutex.
> > > > > > >
> > > > > >
> > > > > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > > > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > > > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > > > > the cost of the ioctls or eventfd signaling.
> > > > >
> > > > > I'd like to use mutex for simplicity.
> > > > >
> > > >
> > > > I cannot move it to a mutex, as we need to take it in the critical
> > > > sections of the kick_lock and irq_lock spinlocks.
> > > >
> > > > I can move it to a spinlock, but it seems more complicated to me. We
> > > > need to make sure we always take these kick_lock and irq_lock in the
> > > > same order as the ready_lock, to not create deadlocks, and they always
> > > > protect just the ready boolean. But sure, I'll send the spinlock
> > > > version for V2.
> > >
> > > Thinking about this, I'm not sure I understand the issue.
> > >
> > > Maybe you can give me an example of the race.
> > >
> >
> > It's the same issue as you describe with set_group_asid on cpu1 and
> > dma_map and dma_unmap in cpu1 in this mail [0], but now the functions
> > are set_vq_ready in the vdpa driver side and VDUSE_VQ_GET_INFO in the
> > VDUSE instance.
> >
> > To me, the barriers should be in the driver and the VDUSE instance,
> > and the VDUSE instance should use the VDUSE_SET_VQ_READY message to
> > know when a VQ is ready. But I don't expect all the VDUSE instances to
> > opt-in for that, so the flow without smp barriers is:
> >
> > [cpu0] .set_vq_ready(vq, true)
> > [cpu1] VDUSE_VQ_GET_INFO(vq)
> > [cpu1] check vq->ready, and it races with the set_vq_ready from cpu0.
> >
>
> Are there any side effects of this race? I meant the driver can do
> set_vq_ready() at any time. And there would be message for notifying
> the usersapce in this series.
>

Actually I'm moving to a semaphore for the moment. SMP barrier is not enough.

The message works for the driver to know that the device vq is ready
and it can send kicks. But there is a race for the device where it has
replied to the message but vq->ready is still not true:

static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
                                        u16 idx, bool ready)
{
        [...]
        r = vduse_dev_msg_sync(dev, &msg);
        // The device could try to call() from here...

        [...]
out:
        // to here
        vduse_vq_set_ready(vq, ready);
}

To be honest, I'd prefer to just set ready = true at
vduse_dev_write_iter before returning success to the VDUSE userland.
This way the synchronization struct is omitted entirely. But let me
know if you prefer otherwise.

Thanks!

On Wed, Feb 4, 2026 at 4:54 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Wed, Feb 4, 2026 at 3:48 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Feb 3, 2026 at 6:35 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> > >
> > > On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
> > > > <eperezma@redhat.com> wrote:
> > > > >
> > > > > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > >
> > > > > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > > > > > <eperezma@redhat.com> wrote:
> > > > > > >
> > > > > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > > > > > well after initial setup, and the device can read it afterwards.
> > > > > > > > >
> > > > > > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > > > > > immediately after the operation returns.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > > > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > > > > > >         return mask;
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > > > > > +{
> > > > > > > > > +       /*
> > > > > > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > > +        */
> > > > > > > > > +       return smp_load_acquire(&vq->ready);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > > > > > +{
> > > > > > > > > +       /*
> > > > > > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > > +        */
> > > > > > > > > +       smp_store_release(&vq->ready, ready);
> > > > > > > >
> > > > > > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > > > > > use vq_lock mutex.
> > > > > > > >
> > > > > > >
> > > > > > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > > > > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > > > > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > > > > > the cost of the ioctls or eventfd signaling.
> > > > > >
> > > > > > I'd like to use mutex for simplicity.
> > > > > >
> > > > >
> > > > > I cannot move it to a mutex, as we need to take it in the critical
> > > > > sections of the kick_lock and irq_lock spinlocks.
> > > > >
> > > > > I can move it to a spinlock, but it seems more complicated to me. We
> > > > > need to make sure we always take these kick_lock and irq_lock in the
> > > > > same order as the ready_lock, to not create deadlocks, and they always
> > > > > protect just the ready boolean. But sure, I'll send the spinlock
> > > > > version for V2.
> > > >
> > > > Thinking about this, I'm not sure I understand the issue.
> > > >
> > > > Maybe you can give me an example of the race.
> > > >
> > >
> > > It's the same issue as you describe with set_group_asid on cpu1 and
> > > dma_map and dma_unmap in cpu1 in this mail [0], but now the functions
> > > are set_vq_ready in the vdpa driver side and VDUSE_VQ_GET_INFO in the
> > > VDUSE instance.
> > >
> > > To me, the barriers should be in the driver and the VDUSE instance,
> > > and the VDUSE instance should use the VDUSE_SET_VQ_READY message to
> > > know when a VQ is ready. But I don't expect all the VDUSE instances to
> > > opt-in for that, so the flow without smp barriers is:
> > >
> > > [cpu0] .set_vq_ready(vq, true)
> > > [cpu1] VDUSE_VQ_GET_INFO(vq)
> > > [cpu1] check vq->ready, and it races with the set_vq_ready from cpu0.
> > >
> >
> > Are there any side effects of this race? I meant the driver can do
> > set_vq_ready() at any time. And there would be message for notifying
> > the usersapce in this series.
> >
>
> Actually I'm moving to a semaphore for the moment. SMP barrier is not enough.
>
> The message works for the driver to know that the device vq is ready
> and it can send kicks. But there is a race for the device where it has
> replied to the message but vq->ready is still not true:
>
> static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
>                                         u16 idx, bool ready)
> {
>         [...]
>         r = vduse_dev_msg_sync(dev, &msg);
>         // The device could try to call() from here...
>
>         [...]
> out:
>         // to here
>         vduse_vq_set_ready(vq, ready);
> }
>
> To be honest, I'd prefer to just set ready = true at
> vduse_dev_write_iter before returning success to the VDUSE userland.

I think you meant:

vduse_vq_set_ready(vq, ready);
r = vduse_dev_msg_sync(dev, &msg);

> This way the synchronization struct is omitted entirely. But let me
> know if you prefer otherwise.

Let's go this way.

Thanks

>
> Thanks!
>

On Thu, Feb 5, 2026 at 5:04 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Feb 4, 2026 at 4:54 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Wed, Feb 4, 2026 at 3:48 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Feb 3, 2026 at 6:35 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> > > >
> > > > On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin
> > > > > <eperezma@redhat.com> wrote:
> > > > > >
> > > > > > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > > >
> > > > > > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin
> > > > > > > <eperezma@redhat.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > > > > > >
> > > > > > > > > > The vduse_vdpa_set_vq_ready can be called in the lifetime of the device
> > > > > > > > > > well after initial setup, and the device can read it afterwards.
> > > > > > > > > >
> > > > > > > > > > Ensure that reads and writes to vq->ready are SMP safe so that the
> > > > > > > > > > caller can trust that virtqueue kicks and calls behave as expected
> > > > > > > > > > immediately after the operation returns.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 34 +++++++++++++++++++++++-------
> > > > > > > > > >  1 file changed, 26 insertions(+), 8 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > > index 73d1d517dc6c..a4963aaf9332 100644
> > > > > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > > > > @@ -460,6 +460,24 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > > > > > > > > >         return mask;
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > > +static bool vduse_vq_get_ready(const struct vduse_virtqueue *vq)
> > > > > > > > > > +{
> > > > > > > > > > +       /*
> > > > > > > > > > +        * Paired with vduse_vq_set_ready smp_store, as the driver may modify
> > > > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > > > +        */
> > > > > > > > > > +       return smp_load_acquire(&vq->ready);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void vduse_vq_set_ready(struct vduse_virtqueue *vq, bool ready)
> > > > > > > > > > +{
> > > > > > > > > > +       /*
> > > > > > > > > > +        * Paired with vduse_vq_get_ready smp_load, as the driver may modify
> > > > > > > > > > +        * it while the VDUSE instance is reading it.
> > > > > > > > > > +        */
> > > > > > > > > > +       smp_store_release(&vq->ready, ready);
> > > > > > > > >
> > > > > > > > > Assuming this is not used in the datapath, I wonder if we can simply
> > > > > > > > > use vq_lock mutex.
> > > > > > > > >
> > > > > > > >
> > > > > > > > The function vduse_vq_set/get_ready are not in the datapath, but
> > > > > > > > vduse_vq_kick and vduse_vq_signal_irqfd are.  I'm ok if you want to
> > > > > > > > switch to vq_mutex if you want though, maybe it's even comparable with
> > > > > > > > the cost of the ioctls or eventfd signaling.
> > > > > > >
> > > > > > > I'd like to use mutex for simplicity.
> > > > > > >
> > > > > >
> > > > > > I cannot move it to a mutex, as we need to take it in the critical
> > > > > > sections of the kick_lock and irq_lock spinlocks.
> > > > > >
> > > > > > I can move it to a spinlock, but it seems more complicated to me. We
> > > > > > need to make sure we always take these kick_lock and irq_lock in the
> > > > > > same order as the ready_lock, to not create deadlocks, and they always
> > > > > > protect just the ready boolean. But sure, I'll send the spinlock
> > > > > > version for V2.
> > > > >
> > > > > Thinking about this, I'm not sure I understand the issue.
> > > > >
> > > > > Maybe you can give me an example of the race.
> > > > >
> > > >
> > > > It's the same issue as you describe with set_group_asid on cpu1 and
> > > > dma_map and dma_unmap in cpu1 in this mail [0], but now the functions
> > > > are set_vq_ready in the vdpa driver side and VDUSE_VQ_GET_INFO in the
> > > > VDUSE instance.
> > > >
> > > > To me, the barriers should be in the driver and the VDUSE instance,
> > > > and the VDUSE instance should use the VDUSE_SET_VQ_READY message to
> > > > know when a VQ is ready. But I don't expect all the VDUSE instances to
> > > > opt-in for that, so the flow without smp barriers is:
> > > >
> > > > [cpu0] .set_vq_ready(vq, true)
> > > > [cpu1] VDUSE_VQ_GET_INFO(vq)
> > > > [cpu1] check vq->ready, and it races with the set_vq_ready from cpu0.
> > > >
> > >
> > > Are there any side effects of this race? I meant the driver can do
> > > set_vq_ready() at any time. And there would be message for notifying
> > > the usersapce in this series.
> > >
> >
> > Actually I'm moving to a semaphore for the moment. SMP barrier is not enough.
> >
> > The message works for the driver to know that the device vq is ready
> > and it can send kicks. But there is a race for the device where it has
> > replied to the message but vq->ready is still not true:
> >
> > static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
> >                                         u16 idx, bool ready)
> > {
> >         [...]
> >         r = vduse_dev_msg_sync(dev, &msg);
> >         // The device could try to call() from here...
> >
> >         [...]
> > out:
> >         // to here
> >         vduse_vq_set_ready(vq, ready);
> > }
> >
> > To be honest, I'd prefer to just set ready = true at
> > vduse_dev_write_iter before returning success to the VDUSE userland.
>
> I think you meant:
>
> vduse_vq_set_ready(vq, ready);

Since this function's error path is to break the device, I think it
can work, yes. I'm sending the V2 with this.

Thanks!

> r = vduse_dev_msg_sync(dev, &msg);
>
> > This way the synchronization struct is omitted entirely. But let me
> > know if you prefer otherwise.
>
> Let's go this way.
>
> Thanks
>
> >
> > Thanks!
> >
>
>