linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/virtio: fix ring free check
@ 2020-02-06 11:14 Gerd Hoffmann
  2020-02-06 18:27 ` Chia-I Wu
  0 siblings, 1 reply; 3+ messages in thread
From: Gerd Hoffmann @ 2020-02-06 11:14 UTC (permalink / raw)
  To: dri-devel
  Cc: gurchetansingh, olvaffe, Gerd Hoffmann, David Airlie,
	Daniel Vetter, open list:VIRTIO GPU DRIVER, open list

If the virtio device supports indirect ring descriptors we need only one
ring entry for the whole command.  Take that into account when checking
whenever the virtqueue has enough free entries for our command.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_vq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 41e475fbd67b..a2ec09dba530 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -328,7 +328,8 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
 {
 	struct virtqueue *vq = vgdev->ctrlq.vq;
 	bool notify = false;
-	int ret;
+	bool indirect;
+	int vqcnt, ret;
 
 again:
 	spin_lock(&vgdev->ctrlq.qlock);
@@ -341,9 +342,11 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
 		return;
 	}
 
-	if (vq->num_free < elemcnt) {
+	indirect = virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC);
+	vqcnt = indirect ? 1 : elemcnt;
+	if (vq->num_free < vqcnt) {
 		spin_unlock(&vgdev->ctrlq.qlock);
-		wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
+		wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= vqcnt);
 		goto again;
 	}
 
-- 
2.18.1


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

* Re: [PATCH] drm/virtio: fix ring free check
  2020-02-06 11:14 [PATCH] drm/virtio: fix ring free check Gerd Hoffmann
@ 2020-02-06 18:27 ` Chia-I Wu
  2020-02-07  6:46   ` Gerd Hoffmann
  0 siblings, 1 reply; 3+ messages in thread
From: Chia-I Wu @ 2020-02-06 18:27 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: ML dri-devel, Gurchetan Singh, David Airlie, Daniel Vetter,
	open list:VIRTIO GPU DRIVER, open list

On Thu, Feb 6, 2020 at 3:14 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> If the virtio device supports indirect ring descriptors we need only one
> ring entry for the whole command.  Take that into account when checking
> whenever the virtqueue has enough free entries for our command.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_vq.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 41e475fbd67b..a2ec09dba530 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -328,7 +328,8 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
>  {
>         struct virtqueue *vq = vgdev->ctrlq.vq;
>         bool notify = false;
> -       int ret;
> +       bool indirect;
> +       int vqcnt, ret;
>
>  again:
>         spin_lock(&vgdev->ctrlq.qlock);
> @@ -341,9 +342,11 @@ static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
>                 return;
>         }
>
> -       if (vq->num_free < elemcnt) {
> +       indirect = virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC);
> +       vqcnt = indirect ? 1 : elemcnt;
Is the feature dynamic and require the lock held?  If not, the result
can be cached and the fixup can happen before grabbing the lock

  if (vgdev->has_indirect_desc)
    elemcnt = 1;

Either way, patch is

  Reviewed-by: Chia-I Wu <olvaffe@gmail.com>


> +       if (vq->num_free < vqcnt) {
>                 spin_unlock(&vgdev->ctrlq.qlock);
> -               wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
> +               wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= vqcnt);
>                 goto again;
>         }
>
> --
> 2.18.1
>

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

* Re: [PATCH] drm/virtio: fix ring free check
  2020-02-06 18:27 ` Chia-I Wu
@ 2020-02-07  6:46   ` Gerd Hoffmann
  0 siblings, 0 replies; 3+ messages in thread
From: Gerd Hoffmann @ 2020-02-07  6:46 UTC (permalink / raw)
  To: Chia-I Wu
  Cc: ML dri-devel, Gurchetan Singh, David Airlie, Daniel Vetter,
	open list:VIRTIO GPU DRIVER, open list

  Hi,

> > +       indirect = virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC);
> > +       vqcnt = indirect ? 1 : elemcnt;
> Is the feature dynamic and require the lock held?  If not, the result
> can be cached and the fixup can happen before grabbing the lock

Not dynamic, so yes, caching makes sense.

cheers,
  Gerd


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

end of thread, other threads:[~2020-02-07  6:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06 11:14 [PATCH] drm/virtio: fix ring free check Gerd Hoffmann
2020-02-06 18:27 ` Chia-I Wu
2020-02-07  6:46   ` Gerd Hoffmann

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).