On Wed, Feb 08, 2017 at 07:24:07PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> 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@
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
