On Thu, Feb 5, 2026 at 5:04 AM Jason Wang <[email protected]> wrote: > > 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);
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! > > > >
