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