Message ID | 20170420213359.31300-11-laurent.pinchart@ideasonboard.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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() - unless the double drm_dev_ref() wasn't an accident... -Chris
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.
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; } /* -----------------------------------------------------------------------------
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(-)