dma_buf may live a long time, longer than the last direct user of the driver. We already hold a reference to the owner module (that prevents the object code from disappearing), but there is no reference to the drm_dev - so the pointers to the driver backend themselves may vanish.

v2: Resist temptation to fix the bug in armada_gem.c not setting the correct flags on the exported dma-buf (it should pass the flags through and not be arbitrarily setting O_RDWR).

Use a common wrapper for exporting the dmabuf and acquiring the reference to the drm_device.

Testcase: igt/vgem_basic/unload Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Petri Latvala petri.latvala@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org Tested-by: Petri Latvala petri.latvala@intel.com --- drivers/gpu/drm/armada/armada_gem.c | 2 +- drivers/gpu/drm/drm_prime.c | 30 +++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/udl/udl_dmabuf.c | 2 +- include/drm/drmP.h | 4 ++++ 6 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index cb8f0347b934..a5e428d27d2f 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -547,7 +547,7 @@ armada_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj, exp_info.flags = O_RDWR; exp_info.priv = obj;

- return dma_buf_export(&exp_info); + return drm_gem_dmabuf_export(dev, &exp_info); }

struct drm_gem_object * diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 80907b34d857..875df8d719fb 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -284,18 +284,46 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, }

/** + * drm_gem_dmabuf_export - dma_buf export implementation for GEM + * @dma_buf: buffer to be exported + * + * This wraps dma_buf_export() for use by generic GEM drivers that are using + * drm_gem_dmabuf_release(). In addition to calling dma_buf_export(), we take + * a reference to the drm_device which is released by drm_gem_dmabuf_release(). + * + * Returns the new dmabuf. + */ +struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, + struct dma_buf_export_info *exp_info) +{ + struct dma_buf *dma_buf; + + dma_buf = dma_buf_export(exp_info); + if (!IS_ERR(dma_buf)) + drm_dev_ref(dev); + + return dma_buf; +} +EXPORT_SYMBOL(drm_gem_dmabuf_export); + +/** * drm_gem_dmabuf_release - dma_buf release implementation for GEM * @dma_buf: buffer to be released * * Generic release function for dma_bufs exported as PRIME buffers. GEM drivers * must use this in their dma_buf ops structure as the release callback. + * drm_gem_dmabuf_release() should be used in conjunction with + * drm_gem_dmabuf_export(). */ void drm_gem_dmabuf_release(struct dma_buf *dma_buf) { struct drm_gem_object *obj = dma_buf->priv; + struct drm_device *dev = obj->dev;

/* drop the reference on the export fd holds */ drm_gem_object_unreference_unlocked(obj); + + drm_dev_unref(dev); } EXPORT_SYMBOL(drm_gem_dmabuf_release);

@@ -412,7 +440,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_device *dev, if (dev->driver->gem_prime_res_obj) exp_info.resv = dev->driver->gem_prime_res_obj(obj);

- return dma_buf_export(&exp_info); + return drm_gem_dmabuf_export(dev, &exp_info); } EXPORT_SYMBOL(drm_gem_prime_export);

diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 10265bb35604..97c9d68b45df 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -283,7 +283,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, return ERR_PTR(ret); }

- dma_buf = dma_buf_export(&exp_info); + dma_buf = drm_gem_dmabuf_export(dev, &exp_info); if (IS_ERR(dma_buf)) return dma_buf;

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index aa60d9909ea2..95e622e31931 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -613,7 +613,7 @@ struct dma_buf *tegra_gem_prime_export(struct drm_device *drm, exp_info.flags = flags; exp_info.priv = gem;

- return dma_buf_export(&exp_info); + return drm_gem_dmabuf_export(drm, &exp_info); }

struct drm_gem_object *tegra_gem_prime_import(struct drm_device *drm, diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c index e2243edd1ce3..ac90ffdb5912 100644 --- a/drivers/gpu/drm/udl/udl_dmabuf.c +++ b/drivers/gpu/drm/udl/udl_dmabuf.c @@ -209,7 +209,7 @@ struct dma_buf *udl_gem_prime_export(struct drm_device *dev, exp_info.flags = flags; exp_info.priv = obj;

- return dma_buf_export(&exp_info); + return drm_gem_dmabuf_export(dev, &exp_info); }

static int udl_prime_create(struct drm_device *dev, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 81fcd553edf7..672644031bd5 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1011,6 +1011,8 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files, } #endif

+struct dma_buf_export_info; + extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj, int flags); @@ -1021,6 +1023,8 @@ extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf); extern int drm_gem_prime_fd_to_handle(struct drm_device *dev, struct drm_file *file_priv, int prime_fd, uint32_t *handle); +struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, + struct dma_buf_export_info *exp_info); extern void drm_gem_dmabuf_release(struct dma_buf *dma_buf);

extern int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,

For the series:

Reviewed-by: Petri Latvala petri.latvala@intel.com

On 10/05/2016 03:21 PM, Chris Wilson wrote:

dma_buf_export() adds a reference to the owning module to the dmabuf (to prevent the driver from being unloaded whilst a third party still refers to the dmabuf). However, drm_gem_prime_export() was passing its own THIS_MODULE (i.e. drm.ko) rather than the driver. Extract the right owner from the device->fops instead.

v2: Use C99 initializers to zero out unset elements of dma_buf_export_info v3: Extract the right module from dev->fops.

Testcase: igt/vgem_basic/unload Reported-by: Petri Latvala petri.latvala@intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Petri Latvala petri.latvala@intel.com Cc: Christian König christian.koenig@amd.com Cc: stable@vger.kernel.org Tested-by: Petri Latvala petri.latvala@intel.com

---

drivers/gpu/drm/drm_prime.c | 17 ++++++++++------- include/drm/drmP.h | 3 ++- 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 57201d68cf61..80907b34d857 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -397,14 +397,17 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {

• using the PRIME helpers.

*/ struct dma_buf *drm_gem_prime_export(struct drm_device *dev,

• 		     struct drm_gem_object *obj, int flags)
  

• 		     struct drm_gem_object *obj,
  

• 		     int flags)
  

  {

• DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
• exp_info.ops = &drm_gem_prime_dmabuf_ops;
• exp_info.size = obj->size;
• exp_info.flags = flags;
• exp_info.priv = obj;

• struct dma_buf_export_info exp_info = {

• .exp_name = KBUILD_MODNAME, /* white lie for debug */
  

• .owner = dev->driver->fops->owner,
  

• .ops = &drm_gem_prime_dmabuf_ops,
  

• .size = obj->size,
  

• .flags = flags,
  

• .priv = obj,
  

• };

  if (dev->driver->gem_prime_res_obj) exp_info.resv = dev->driver->gem_prime_res_obj(obj);

diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 0e99669159c1..81fcd553edf7 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1012,7 +1012,8 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files, #endif

extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,

• struct drm_gem_object *obj, int flags);
  

• 			    struct drm_gem_object *obj,
  

• 			    int flags);
  

  extern int drm_gem_prime_handle_to_fd(struct drm_device *dev, struct drm_file *file_priv, uint32_t handle, uint32_t flags, int *prime_fd);