netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vdpasim: allow to enable a vq repeatedly
@ 2022-05-19 14:31 Eugenio Pérez
  2022-05-19 14:48 ` Stefano Garzarella
  0 siblings, 1 reply; 3+ messages in thread
From: Eugenio Pérez @ 2022-05-19 14:31 UTC (permalink / raw)
  To: mst, jasowang
  Cc: linux-kernel, sgarzare, gdawar, lingshan.zhu, kvm, lulu, netdev,
	lvivier, eli, virtualization, parav

Code must be resilient to enable a queue many times.

At the moment the queue is resetting so it's definitely not the expected
behavior.

Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Cc: stable@vger.kernel.org
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index ddbe142af09a..b53cd00ad161 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -355,9 +355,10 @@ static void vdpasim_set_vq_ready(struct vdpa_device *vdpa, u16 idx, bool ready)
 	struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
 
 	spin_lock(&vdpasim->lock);
-	vq->ready = ready;
-	if (vq->ready)
+	if (!vq->ready) {
+		vq->ready = ready;
 		vdpasim_queue_ready(vdpasim, idx);
+	}
 	spin_unlock(&vdpasim->lock);
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] vdpasim: allow to enable a vq repeatedly
  2022-05-19 14:31 [PATCH] vdpasim: allow to enable a vq repeatedly Eugenio Pérez
@ 2022-05-19 14:48 ` Stefano Garzarella
  2022-05-19 14:57   ` Eugenio Perez Martin
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2022-05-19 14:48 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: mst, jasowang, linux-kernel, gdawar, lingshan.zhu, kvm, lulu,
	netdev, lvivier, eli, virtualization, parav

On Thu, May 19, 2022 at 04:31:45PM +0200, Eugenio Pérez wrote:
>Code must be resilient to enable a queue many times.
>
>At the moment the queue is resetting so it's definitely not the expected
>behavior.
>
>Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
>Cc: stable@vger.kernel.org
>Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>---
> drivers/vdpa/vdpa_sim/vdpa_sim.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>index ddbe142af09a..b53cd00ad161 100644
>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>@@ -355,9 +355,10 @@ static void vdpasim_set_vq_ready(struct vdpa_device *vdpa, u16 idx, bool ready)
> 	struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
>
> 	spin_lock(&vdpasim->lock);
>-	vq->ready = ready;
>-	if (vq->ready)
>+	if (!vq->ready) {
>+		vq->ready = ready;
> 		vdpasim_queue_ready(vdpasim, idx);
>+	}

But this way the first time vq->ready is set to true, then it will never 
be set back to false.

Should we leave the assignment out of the block?
Maybe after the if block to avoid the problem we are fixing.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vdpasim: allow to enable a vq repeatedly
  2022-05-19 14:48 ` Stefano Garzarella
@ 2022-05-19 14:57   ` Eugenio Perez Martin
  0 siblings, 0 replies; 3+ messages in thread
From: Eugenio Perez Martin @ 2022-05-19 14:57 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Michael Tsirkin, Jason Wang, linux-kernel, Gautam Dawar,
	Zhu Lingshan, kvm list, Cindy Lu, netdev, Laurent Vivier,
	Eli Cohen, virtualization, Parav Pandit

On Thu, May 19, 2022 at 4:48 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Thu, May 19, 2022 at 04:31:45PM +0200, Eugenio Pérez wrote:
> >Code must be resilient to enable a queue many times.
> >
> >At the moment the queue is resetting so it's definitely not the expected
> >behavior.
> >
> >Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >---
> > drivers/vdpa/vdpa_sim/vdpa_sim.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >index ddbe142af09a..b53cd00ad161 100644
> >--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >@@ -355,9 +355,10 @@ static void vdpasim_set_vq_ready(struct vdpa_device *vdpa, u16 idx, bool ready)
> >       struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
> >
> >       spin_lock(&vdpasim->lock);
> >-      vq->ready = ready;
> >-      if (vq->ready)
> >+      if (!vq->ready) {
> >+              vq->ready = ready;
> >               vdpasim_queue_ready(vdpasim, idx);
> >+      }
>
> But this way the first time vq->ready is set to true, then it will never
> be set back to false.
>

You're right, I had in mind to reset the flow before enabling as the
only possibility.

Sending v2 with that part fixed, thanks!

> Should we leave the assignment out of the block?
> Maybe after the if block to avoid the problem we are fixing.
>
> Thanks,
> Stefano
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-19 14:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 14:31 [PATCH] vdpasim: allow to enable a vq repeatedly Eugenio Pérez
2022-05-19 14:48 ` Stefano Garzarella
2022-05-19 14:57   ` Eugenio Perez Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).