From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chia-I Wu Subject: Re: [PATCH] drm/virtio: fix command submission with objects but without fence. Date: Thu, 5 Sep 2019 10:14:20 -0700 Message-ID: References: <20190904074828.32502-1-kraxel@redhat.com> <20190905052340.gfwmzkqwcpxtvzvu@sirius.home.kraxel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190905052340.gfwmzkqwcpxtvzvu@sirius.home.kraxel.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Gerd Hoffmann Cc: David Airlie , open list , Daniel Vetter , ML dri-devel , "open list:VIRTIO GPU DRIVER" List-Id: virtualization@lists.linuxfoundation.org On Wed, Sep 4, 2019 at 10:23 PM Gerd Hoffmann wrote: > > 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 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 > > > --- > > > 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(). I misread and thought this was in virtio_gpu_dequeue_ctrl_func. Sorry :( Reviewed-by: Chia-I Wu > > cheers, > Gerd >