[PATCH 0/2] Fix lock errors in VDUSE suspend feature

[PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Eugenio Pérez]

Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
take and release into guards, so they are not lost after a return.

This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/
---
It would be great if these can be squashed.

Eugenio Pérez (2):
  vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
  vduse: not take the device semaphore while holding vq spinlock

 drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

--
2.54.0

[PATCH 1/2] vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work

[Eugenio Pérez]

If dev->suspended the function returns not relasing the semaphore, so
the only solution to be able to use the device is to reset again from
userspace.  Convert the semaphore functions to the equivalent guard() to
avoid it.

Fixes: 6c141c034c1b ("vduse: Add suspend")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0500da043761..86bd3116eda7 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1281,21 +1281,16 @@ static int vduse_dev_queue_irq_work(struct vduse_dev *dev,
 {
 	int ret = -EINVAL;
 
-	down_read(&dev->rwsem);
-	if (dev->suspended)
+	guard(rwsem_read)(&dev->rwsem);
+	if (dev->suspended || !(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
 		return ret;
 
-	if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
-		goto unlock;
-
 	ret = 0;
 	if (irq_effective_cpu == IRQ_UNBOUND)
 		queue_work(vduse_irq_wq, irq_work);
 	else
 		queue_work_on(irq_effective_cpu,
 			      vduse_irq_bound_wq, irq_work);
-unlock:
-	up_read(&dev->rwsem);
 
 	return ret;
 }
-- 
2.54.0

[PATCH 2/2] vduse: not take the device semaphore while holding vq spinlock

[Eugenio Pérez]

It's an invalid lock pattern, so take them reversely.

This also makes the lock ordering coherent with vduse_dev_reset.

Fixes: e4a249d15eb2 ("vduse: Fix error around jumping over a __cleanup() variable")
Fixes: 6c141c034c1b ("vduse: Add suspend")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 86bd3116eda7..2c1cafea6536 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -561,14 +561,14 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,

 static void vduse_vq_kick(struct vduse_virtqueue *vq)
 {
-	guard(spinlock)(&vq->kick_lock);
-	if (!vq->ready)
-		return;
-
 	guard(rwsem_read)(&vq->dev->rwsem);
 	if (vq->dev->suspended)
 		return;

+	guard(spinlock)(&vq->kick_lock);
+	if (!vq->ready)
+		return;
+
 	if (vq->kickfd)
 		eventfd_signal(vq->kickfd);
 	else
--
2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Michael S. Tsirkin]

On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> take and release into guards, so they are not lost after a return.
> 
> This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/


And     vduse: Fix error around jumping over a __cleanup() variable
    
does it obliviate that?

> ---
> It would be great if these can be squashed.
> 
> Eugenio Pérez (2):
>   vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
>   vduse: not take the device semaphore while holding vq spinlock
> 
>  drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> --
> 2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Michael S. Tsirkin]

On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> take and release into guards, so they are not lost after a return.
> 
> This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/
> ---
> It would be great if these can be squashed.


if you want that then just send v4.

> 
> Eugenio Pérez (2):
>   vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
>   vduse: not take the device semaphore while holding vq spinlock
> 
>  drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> --
> 2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Michael S. Tsirkin]

On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> take and release into guards, so they are not lost after a return.
> 
> This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/

does not apply there.

> ---
> It would be great if these can be squashed.
> 
> Eugenio Pérez (2):
>   vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
>   vduse: not take the device semaphore while holding vq spinlock
> 
>  drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> --
> 2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Michael S. Tsirkin]

On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> take and release into guards, so they are not lost after a return.
> 
> This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/

You mean
20260610-vduse_vq_kick-fix-guard-usage-v1-1-0ce02c08006e@kernel.org
actually

> ---
> It would be great if these can be squashed.
> 
> Eugenio Pérez (2):
>   vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
>   vduse: not take the device semaphore while holding vq spinlock
> 
>  drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> --
> 2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Michael S. Tsirkin]

On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> take and release into guards, so they are not lost after a return.
> 
> This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/
> ---
> It would be great if these can be squashed.

ok i figured out the confusion. squashed


> 
> Eugenio Pérez (2):
>   vduse: fix not releasing taken semaphore in vduse_dev_queue_irq_work
>   vduse: not take the device semaphore while holding vq spinlock
> 
>  drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> --
> 2.54.0

Re: [PATCH 0/2] Fix lock errors in VDUSE suspend feature

[Eugenio Perez Martin]

On Thu, Jun 11, 2026 at 4:53 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jun 11, 2026 at 03:38:04PM +0200, Eugenio Pérez wrote:
> > Fix wrong ordering at taking semaphore after spinlock and convert the spinlock
> > take and release into guards, so they are not lost after a return.
> >
> > This series goes on top of https://lore.kernel.org/lkml/20260610083452.477759-1-eperezma@redhat.com/
> > ---
> > It would be great if these can be squashed.
>
> ok i figured out the confusion. squashed
>

Yes, you're right; this is on top of Nathan's. Thanks for both merging
and figuring it out!