Gerd Hoffmann
2019-Sep-04 07:48 UTC
[PATCH] drm/virtio: fix command submission with objects but without fence.
Only call virtio_gpu_array_add_fence if we actually have a fence.
Fixes: da758d51968a ("drm/virtio: rework virtio_gpu_execbuffer_ioctl
fencing")
Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 595fa6ec2d58..7fd2851f7b97 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -339,11 +339,12 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
goto again;
}
- if (fence)
+ if (fence) {
virtio_gpu_fence_emit(vgdev, hdr, fence);
- if (vbuf->objs) {
- virtio_gpu_array_add_fence(vbuf->objs, &fence->f);
- virtio_gpu_array_unlock_resv(vbuf->objs);
+ if (vbuf->objs) {
+ virtio_gpu_array_add_fence(vbuf->objs, &fence->f);
+ virtio_gpu_array_unlock_resv(vbuf->objs);
+ }
}
notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
spin_unlock(&vgdev->ctrlq.qlock);
--
2.18.1
Chia-I Wu
2019-Sep-04 23:10 UTC
[PATCH] drm/virtio: fix command submission with objects but without fence.
On Wed, Sep 4, 2019 at 12:48 AM Gerd Hoffmann <kraxel at redhat.com> wrote:> > Only call virtio_gpu_array_add_fence if we actually have a fence. > > Fixes: da758d51968a ("drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing") > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_vq.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 595fa6ec2d58..7fd2851f7b97 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -339,11 +339,12 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev, > goto again; > } > > - if (fence) > + if (fence) { > virtio_gpu_fence_emit(vgdev, hdr, fence); > - if (vbuf->objs) { > - virtio_gpu_array_add_fence(vbuf->objs, &fence->f); > - virtio_gpu_array_unlock_resv(vbuf->objs); > + if (vbuf->objs) { > + virtio_gpu_array_add_fence(vbuf->objs, &fence->f); > + virtio_gpu_array_unlock_resv(vbuf->objs); > + }This leaks when fence == NULL and vbuf->objs != NULL (which can really happen IIRC... not at my desk to check).> } > notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf); > spin_unlock(&vgdev->ctrlq.qlock); > -- > 2.18.1 >
Gerd Hoffmann
2019-Sep-05 05:23 UTC
[PATCH] drm/virtio: fix command submission with objects but without fence.
On Wed, Sep 04, 2019 at 04:10:30PM -0700, Chia-I Wu wrote:> On Wed, Sep 4, 2019 at 12:48 AM Gerd Hoffmann <kraxel at redhat.com> wrote: > > > > Only call virtio_gpu_array_add_fence if we actually have a fence. > > > > Fixes: da758d51968a ("drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing") > > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> > > --- > > drivers/gpu/drm/virtio/virtgpu_vq.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c > > index 595fa6ec2d58..7fd2851f7b97 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > > @@ -339,11 +339,12 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev, > > goto again; > > } > > > > - if (fence) > > + if (fence) { > > virtio_gpu_fence_emit(vgdev, hdr, fence); > > - if (vbuf->objs) { > > - virtio_gpu_array_add_fence(vbuf->objs, &fence->f); > > - virtio_gpu_array_unlock_resv(vbuf->objs); > > + if (vbuf->objs) { > > + virtio_gpu_array_add_fence(vbuf->objs, &fence->f); > > + virtio_gpu_array_unlock_resv(vbuf->objs); > > + } > This leaks when fence == NULL and vbuf->objs != NULL (which can really > happen IIRC... not at my desk to check).Yes, it can happen, for example when flushing dumb buffers. But I don't think we leak in this case. The code paths which don't need a fence also do not call virtio_gpu_array_lock_resv(), so things are balanced. The actual release of the objs happens in virtio_gpu_dequeue_ctrl_func() via virtio_gpu_array_put_free_delayed(). cheers, Gerd