On Wed, Feb 4, 2026 at 4:54 PM Eugenio Perez Martin <[email protected]> wrote: > > On Wed, Feb 4, 2026 at 3:48 AM Jason Wang <[email protected]> wrote: > > > > On Tue, Feb 3, 2026 at 6:35 PM Eugenio Perez Martin <[email protected]> > > wrote: > > > > > > On Tue, Feb 3, 2026 at 5:05 AM Jason Wang <[email protected]> wrote: > > > > > > > > On Fri, Jan 30, 2026 at 3:56 PM Eugenio Perez Martin > > > > <[email protected]> wrote: > > > > > > > > > > On Fri, Jan 30, 2026 at 3:18 AM Jason Wang <[email protected]> > > > > > wrote: > > > > > > > > > > > > On Thu, Jan 29, 2026 at 2:21 PM Eugenio Perez Martin > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > On Thu, Jan 29, 2026 at 2:17 AM Jason Wang <[email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Jan 28, 2026 at 8:45 PM Eugenio Pérez > > > > > > > > <[email protected]> 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 <[email protected]> > > > > > > > > > --- > > > > > > > > > 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! >
