[PATCH v2 5/7] drm: Introduce drm_gem_object_{get,put}()

From: Thierry Reding treding@nvidia.com

For consistency with other reference counting APIs in the kernel, add drm_gem_object_get() and drm_gem_object_put(), as well as an unlocked variant of the latter, to reference count GEM buffer objects.

Compatibility aliases are added to keep existing code working. To help speed up the transition, all the instances of the old functions in the DRM core are already replaced in this commit.

The existing semantic patch for the DRM subsystem-wide conversion is extended to account for these new helpers.

Reviewed-by: Sean Paul seanpaul@chromium.org Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Thierry Reding treding@nvidia.com --- Documentation/gpu/drm-mm.rst | 14 +++--- drivers/gpu/drm/drm_fb_cma_helper.c | 16 +++---- drivers/gpu/drm/drm_gem.c | 44 +++++++++--------- drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++-- drivers/gpu/drm/drm_prime.c | 10 ++-- include/drm/drm_gem.h | 80 +++++++++++++++++++++++++------- scripts/coccinelle/api/drm-get-put.cocci | 20 ++++++++ 7 files changed, 130 insertions(+), 64 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index f5760b140f13..fd35998acefc 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -183,14 +183,12 @@ GEM Objects Lifetime --------------------

All GEM objects are reference-counted by the GEM core. References can be -acquired and release by :c:func:`calling -drm_gem_object_reference()` and -:c:func:`drm_gem_object_unreference()` respectively. The caller -must hold the :c:type:`struct drm_device <drm_device>` -struct_mutex lock when calling -:c:func:`drm_gem_object_reference()`. As a convenience, GEM -provides :c:func:`drm_gem_object_unreference_unlocked()` -functions that can be called without holding the lock. +acquired and release by :c:func:`calling drm_gem_object_get()` and +:c:func:`drm_gem_object_put()` respectively. The caller must hold the +:c:type:`struct drm_device <drm_device>` struct_mutex lock when calling +:c:func:`drm_gem_object_get()`. As a convenience, GEM provides +:c:func:`drm_gem_object_put_unlocked()` functions that can be called without +holding the lock.

When the last reference to a GEM object is released the GEM core calls the :c:type:`struct drm_driver <drm_driver>` gem_free_object diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 596fabf18c3e..eccc07d20925 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -102,7 +102,7 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb)

for (i = 0; i < 4; i++) { if (fb_cma->obj[i]) - drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base); + drm_gem_object_put_unlocked(&fb_cma->obj[i]->base); }

drm_framebuffer_cleanup(fb); @@ -190,7 +190,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, if (!obj) { dev_err(dev->dev, "Failed to lookup GEM object\n"); ret = -ENXIO; - goto err_gem_object_unreference; + goto err_gem_object_put; }

min_size = (height - 1) * mode_cmd->pitches[i] @@ -198,9 +198,9 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, + mode_cmd->offsets[i];

if (obj->size < min_size) { - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); ret = -EINVAL; - goto err_gem_object_unreference; + goto err_gem_object_put; } objs[i] = to_drm_gem_cma_obj(obj); } @@ -208,14 +208,14 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs); if (IS_ERR(fb_cma)) { ret = PTR_ERR(fb_cma); - goto err_gem_object_unreference; + goto err_gem_object_put; }

return &fb_cma->fb;

-err_gem_object_unreference: +err_gem_object_put: for (i--; i >= 0; i--) - drm_gem_object_unreference_unlocked(&objs[i]->base); + drm_gem_object_put_unlocked(&objs[i]->base); return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs); @@ -477,7 +477,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper, err_fb_info_destroy: drm_fb_helper_release_fbi(helper); err_gem_free_object: - drm_gem_object_unreference_unlocked(&obj->base); + drm_gem_object_put_unlocked(&obj->base); return ret; }

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bc93de308673..b1e28c944637 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -218,7 +218,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj) }

static void -drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; bool final = false; @@ -241,7 +241,7 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) mutex_unlock(&dev->object_name_lock);

if (final) - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); }

/* @@ -262,7 +262,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, file_priv);

- drm_gem_object_handle_unreference_unlocked(obj); + drm_gem_object_handle_put_unlocked(obj);

return 0; } @@ -352,7 +352,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,

WARN_ON(!mutex_is_locked(&dev->object_name_lock)); if (obj->handle_count++ == 0) - drm_gem_object_reference(obj); + drm_gem_object_get(obj);

/* * Get the user-visible handle using idr. Preload and perform @@ -392,7 +392,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, idr_remove(&file_priv->object_idr, handle); spin_unlock(&file_priv->table_lock); err_unref: - drm_gem_object_handle_unreference_unlocked(obj); + drm_gem_object_handle_put_unlocked(obj); return ret; }

@@ -606,7 +606,7 @@ drm_gem_object_lookup(struct drm_file *filp, u32 handle) /* Check if we currently have a reference on the object */ obj = idr_find(&filp->object_idr, handle); if (obj) - drm_gem_object_reference(obj); + drm_gem_object_get(obj);

spin_unlock(&filp->table_lock);

@@ -683,7 +683,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,

err: mutex_unlock(&dev->object_name_lock); - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); return ret; }

@@ -713,7 +713,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data, mutex_lock(&dev->object_name_lock); obj = idr_find(&dev->object_name_idr, (int) args->name); if (obj) { - drm_gem_object_reference(obj); + drm_gem_object_get(obj); } else { mutex_unlock(&dev->object_name_lock); return -ENOENT; @@ -721,7 +721,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,

/* drm_gem_handle_create_tail unlocks dev->object_name_lock. */ ret = drm_gem_handle_create_tail(file_priv, obj, &handle); - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); if (ret) return ret;

@@ -809,16 +809,16 @@ drm_gem_object_free(struct kref *kref) EXPORT_SYMBOL(drm_gem_object_free);

/** - * drm_gem_object_unreference_unlocked - release a GEM BO reference + * drm_gem_object_put_unlocked - drop a GEM buffer object reference * @obj: GEM buffer object * * This releases a reference to @obj. Callers must not hold the * &drm_device.struct_mutex lock when calling this function. * - * See also __drm_gem_object_unreference(). + * See also __drm_gem_object_put(). */ void -drm_gem_object_unreference_unlocked(struct drm_gem_object *obj) +drm_gem_object_put_unlocked(struct drm_gem_object *obj) { struct drm_device *dev;

@@ -834,10 +834,10 @@ drm_gem_object_unreference_unlocked(struct drm_gem_object *obj) &dev->struct_mutex)) mutex_unlock(&dev->struct_mutex); } -EXPORT_SYMBOL(drm_gem_object_unreference_unlocked); +EXPORT_SYMBOL(drm_gem_object_put_unlocked);

/** - * drm_gem_object_unreference - release a GEM BO reference + * drm_gem_object_put - release a GEM buffer object reference * @obj: GEM buffer object * * This releases a reference to @obj. Callers must hold the @@ -845,10 +845,10 @@ EXPORT_SYMBOL(drm_gem_object_unreference_unlocked); * driver doesn't use &drm_device.struct_mutex for anything. * * For drivers not encumbered with legacy locking use - * drm_gem_object_unreference_unlocked() instead. + * drm_gem_object_put_unlocked() instead. */ void -drm_gem_object_unreference(struct drm_gem_object *obj) +drm_gem_object_put(struct drm_gem_object *obj) { if (obj) { WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex)); @@ -856,7 +856,7 @@ drm_gem_object_unreference(struct drm_gem_object *obj) kref_put(&obj->refcount, drm_gem_object_free); } } -EXPORT_SYMBOL(drm_gem_object_unreference); +EXPORT_SYMBOL(drm_gem_object_put);

/** * drm_gem_vm_open - vma->ops->open implementation for GEM @@ -869,7 +869,7 @@ void drm_gem_vm_open(struct vm_area_struct *vma) { struct drm_gem_object *obj = vma->vm_private_data;

- drm_gem_object_reference(obj); + drm_gem_object_get(obj); } EXPORT_SYMBOL(drm_gem_vm_open);

@@ -884,7 +884,7 @@ void drm_gem_vm_close(struct vm_area_struct *vma) { struct drm_gem_object *obj = vma->vm_private_data;

- drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); } EXPORT_SYMBOL(drm_gem_vm_close);

@@ -935,7 +935,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, * (which should happen whether the vma was created by this call, or * by a vm_open due to mremap or partial unmap or whatever). */ - drm_gem_object_reference(obj); + drm_gem_object_get(obj);

return 0; } @@ -992,14 +992,14 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) return -EINVAL;

if (!drm_vma_node_is_allowed(node, priv)) { - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); return -EACCES; }

ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT, vma);

- drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj);

return ret; } diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index f7ba32bfe39b..bb968e76779b 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -121,7 +121,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, return cma_obj;

error: - drm_gem_object_unreference_unlocked(&cma_obj->base); + drm_gem_object_put_unlocked(&cma_obj->base); return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(drm_gem_cma_create); @@ -163,7 +163,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv, */ ret = drm_gem_handle_create(file_priv, gem_obj, handle); /* drop reference from allocate - handle holds it now. */ - drm_gem_object_unreference_unlocked(gem_obj); + drm_gem_object_put_unlocked(gem_obj); if (ret) return ERR_PTR(ret);

@@ -293,7 +293,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,

*offset = drm_vma_node_offset_addr(&gem_obj->vma_node);

- drm_gem_object_unreference_unlocked(gem_obj); + drm_gem_object_put_unlocked(gem_obj);

return 0; } @@ -416,13 +416,13 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file *filp, return -EINVAL;

if (!drm_vma_node_is_allowed(node, priv)) { - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); return -EACCES; }

cma_obj = to_drm_gem_cma_obj(obj);

- drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj);

return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL; } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 25aa4558f1b5..866b294e7c61 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -318,7 +318,7 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, return dma_buf;

drm_dev_ref(dev); - drm_gem_object_reference(exp_info->priv); + drm_gem_object_get(exp_info->priv);

return dma_buf; } @@ -339,7 +339,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf) struct drm_device *dev = obj->dev;

/* drop the reference on the export fd holds */ - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj);

drm_dev_unref(dev); } @@ -585,7 +585,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, fail_put_dmabuf: dma_buf_put(dmabuf); out: - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); out_unlock: mutex_unlock(&file_priv->prime.lock);

@@ -616,7 +616,7 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, * Importing dmabuf exported from out own gem increases * refcount on gem itself instead of f_count of dmabuf. */ - drm_gem_object_reference(obj); + drm_gem_object_get(obj); return obj; } } @@ -704,7 +704,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,

/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */ ret = drm_gem_handle_create_tail(file_priv, obj, handle); - drm_gem_object_unreference_unlocked(obj); + drm_gem_object_put_unlocked(obj); if (ret) goto out_put;

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 449a41b56ffc..3b2a28f7f49f 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -48,9 +48,9 @@ struct drm_gem_object { * * Reference count of this object * - * Please use drm_gem_object_reference() to acquire and - * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked() - * to release a reference to a GEM buffer object. + * Please use drm_gem_object_get() to acquire and drm_gem_object_put() + * or drm_gem_object_put_unlocked() to release a reference to a GEM + * buffer object. */ struct kref refcount;

@@ -187,42 +187,90 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);

/** - * drm_gem_object_reference - acquire a GEM BO reference + * drm_gem_object_get - acquire a GEM buffer object reference * @obj: GEM buffer object * - * This acquires additional reference to @obj. It is illegal to call this - * without already holding a reference. No locks required. + * This function acquires an additional reference to @obj. It is illegal to + * call this without already holding a reference. No locks required. */ -static inline void -drm_gem_object_reference(struct drm_gem_object *obj) +static inline void drm_gem_object_get(struct drm_gem_object *obj) { kref_get(&obj->refcount); }

/** - * __drm_gem_object_unreference - raw function to release a GEM BO reference + * __drm_gem_object_put - raw function to release a GEM buffer object reference * @obj: GEM buffer object * * This function is meant to be used by drivers which are not encumbered with * &drm_device.struct_mutex legacy locking and which are using the * gem_free_object_unlocked callback. It avoids all the locking checks and - * locking overhead of drm_gem_object_unreference() and - * drm_gem_object_unreference_unlocked(). + * locking overhead of drm_gem_object_put() and drm_gem_object_put_unlocked(). * * Drivers should never call this directly in their code. Instead they should - * wrap it up into a ``driver_gem_object_unreference(struct driver_gem_object - * *obj)`` wrapper function, and use that. Shared code should never call this, to + * wrap it up into a ``driver_gem_object_put(struct driver_gem_object *obj)`` + * wrapper function, and use that. Shared code should never call this, to * avoid breaking drivers by accident which still depend upon * &drm_device.struct_mutex locking. */ static inline void -__drm_gem_object_unreference(struct drm_gem_object *obj) +__drm_gem_object_put(struct drm_gem_object *obj) { kref_put(&obj->refcount, drm_gem_object_free); }

-void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj); -void drm_gem_object_unreference(struct drm_gem_object *obj); +void drm_gem_object_put_unlocked(struct drm_gem_object *obj); +void drm_gem_object_put(struct drm_gem_object *obj); + +/** + * drm_gem_object_reference - acquire a GEM buffer object reference + * @obj: GEM buffer object + * + * This is a compatibility alias for drm_gem_object_get() and should not be + * used by new code. + */ +static inline void drm_gem_object_reference(struct drm_gem_object *obj) +{ + drm_gem_object_get(obj); +} + +/** + * __drm_gem_object_unreference - raw function to release a GEM buffer object + * reference + * @obj: GEM buffer object + * + * This is a compatibility alias for __drm_gem_object_put() and should not be + * used by new code. + */ +static inline void __drm_gem_object_unreference(struct drm_gem_object *obj) +{ + __drm_gem_object_put(obj); +} + +/** + * drm_gem_object_unreference_unlocked - release a GEM buffer object reference + * @obj: GEM buffer object + * + * This is a compatibility alias for drm_gem_object_put_unlocked() and should + * not be used by new code. + */ +static inline void +drm_gem_object_unreference_unlocked(struct drm_gem_object *obj) +{ + drm_gem_object_put_unlocked(obj); +} + +/** + * drm_gem_object_unreference - release a GEM buffer object reference + * @obj: GEM buffer object + * + * This is a compatibility alias for drm_gem_object_put() and should not be + * used by new code. + */ +static inline void drm_gem_object_unreference(struct drm_gem_object *obj) +{ + drm_gem_object_put(obj); +}

int drm_gem_handle_create(struct drm_file *file_priv, struct drm_gem_object *obj, diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci index fd298c24a465..24882547b4d1 100644 --- a/scripts/coccinelle/api/drm-get-put.cocci +++ b/scripts/coccinelle/api/drm-get-put.cocci @@ -32,6 +32,18 @@ expression object; | - drm_framebuffer_unreference(object) + drm_framebuffer_put(object) +| +- drm_gem_object_reference(object) ++ drm_gem_object_get(object) +| +- drm_gem_object_unreference(object) ++ drm_gem_object_put(object) +| +- __drm_gem_object_unreference(object) ++ __drm_gem_object_put(object) +| +- drm_gem_object_unreference_unlocked(object) ++ drm_gem_object_put_unlocked(object) )

@r depends on report@ @@ -51,6 +63,14 @@ drm_connector_reference@p(object) drm_framebuffer_unreference@p(object) | drm_framebuffer_reference@p(object) +| +drm_gem_object_unreference@p(object) +| +drm_gem_object_reference@p(object) +| +__drm_gem_object_unreference(object) +| +drm_gem_object_unreference_unlocked(object) )

@script:python depends on report@