Re: [PATCH v2 10/10] drm: omapdrm: Take GEM obj and DRM dev references when exporting dmabuf

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Chris,

On Friday 21 Apr 2017 00:08:21 Chris Wilson wrote:
> On Fri, Apr 21, 2017 at 12:33:59AM +0300, Laurent Pinchart wrote:
> > To ensure that neither the GEM object nor the DRM device goes away while
> > a GEM object exported through dma-buf is still accessible, take a
> > reference to both the GEM object and the DRM device at export time. The
> > dma-buf release handler already releases the GEM object (which resulted
> > in a refcount underflow) and now also releases the DRM device.
> > 
> > Fixes: 6ad11bc3a0b8 ("staging: drm/omap: dmabuf/prime support")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> > b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index
> > 79dfded9613c..88ad723b3044 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> > @@ -75,10 +75,11 @@ static void omap_gem_unmap_dma_buf(struct
> > dma_buf_attachment *attachment,> 
> >  static void omap_gem_dmabuf_release(struct dma_buf *buffer)
> >  {
> >  
> >  	struct drm_gem_object *obj = buffer->priv;
> > 
> > -	/* release reference that was taken when dmabuf was exported
> > -	 * in omap_gem_prime_set()..
> > -	 */
> > +	struct drm_device *dev = obj->dev;
> > +
> > +	/* Release references taken by omap_gem_prime_export(). */
> > 
> >  	drm_gem_object_unreference_unlocked(obj);
> > 
> > +	drm_dev_ref(dev);
> > 
> >  }
> > 
> > @@ -184,13 +185,21 @@ struct dma_buf *omap_gem_prime_export(struct
> > drm_device *dev,> 
> >  		struct drm_gem_object *obj, int flags)
> >  
> >  {
> >  
> >  	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > 
> > +	struct dma_buf *dma_buf;
> > 
> >  	exp_info.ops = &omap_dmabuf_ops;
> >  	exp_info.size = obj->size;
> >  	exp_info.flags = flags;
> >  	exp_info.priv = obj;
> > 
> > -	return dma_buf_export(&exp_info);
> > +	dma_buf = dma_buf_export(&exp_info);
> > +	if (IS_ERR(dma_buf))
> > +		return dma_buf;
> > +
> > +	drm_dev_ref(dev);
> > +	drm_gem_object_reference(obj);
> > +
> > +	return dma_buf;
> > 
> >  }
> 
> Please see drm_gem_dmabuf_export() and drm_gem_dmabuf_release()

Good point, I'll replace the custom implementation by those functions. I had 
noticed drm_gem_dmabuf_release() but for some reason failed to see that 
drm_gem_dmabuf_export() was exported.

> - unless the double drm_dev_ref() wasn't an accident...

Oops :-/ Thank you for catching that.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel