Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
> 
> For consistency with other reference counting APIs in the kernel, add
> drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> mode 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.
> 

drm code looks good and is 

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

> A semantic patch is provided that can be used to convert all drivers to
> the new helpers.

I'm not convinced we need to commit the cocci patch. I think including it in
your cover letter and then following up with a follow on series to actually make
the change is sufficient (See: ickle's s/fence/dma_fence/ series).

Sean

> 
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_atomic.c             | 14 +++++------
>  drivers/gpu/drm/drm_mode_object.c        | 26 ++++++++++----------
>  drivers/gpu/drm/drm_property.c           |  6 ++---
>  include/drm/drm_mode_object.h            | 36 ++++++++++++++++++++++-----
>  scripts/coccinelle/api/drm-get-put.cocci | 42 ++++++++++++++++++++++++++++++++
>  5 files changed, 95 insertions(+), 29 deletions(-)
>  create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a5673107db26..2bb0a759e8ec 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2122,13 +2122,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  		}
>  
>  		if (!obj->properties) {
> -			drm_mode_object_unreference(obj);
> +			drm_mode_object_put(obj);
>  			ret = -ENOENT;
>  			goto out;
>  		}
>  
>  		if (get_user(count_props, count_props_ptr + copied_objs)) {
> -			drm_mode_object_unreference(obj);
> +			drm_mode_object_put(obj);
>  			ret = -EFAULT;
>  			goto out;
>  		}
> @@ -2141,14 +2141,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  			struct drm_property *prop;
>  
>  			if (get_user(prop_id, props_ptr + copied_props)) {
> -				drm_mode_object_unreference(obj);
> +				drm_mode_object_put(obj);
>  				ret = -EFAULT;
>  				goto out;
>  			}
>  
>  			prop = drm_mode_obj_find_prop_id(obj, prop_id);
>  			if (!prop) {
> -				drm_mode_object_unreference(obj);
> +				drm_mode_object_put(obj);
>  				ret = -ENOENT;
>  				goto out;
>  			}
> @@ -2156,14 +2156,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  			if (copy_from_user(&prop_value,
>  					   prop_values_ptr + copied_props,
>  					   sizeof(prop_value))) {
> -				drm_mode_object_unreference(obj);
> +				drm_mode_object_put(obj);
>  				ret = -EFAULT;
>  				goto out;
>  			}
>  
>  			ret = atomic_set_prop(state, obj, prop, prop_value);
>  			if (ret) {
> -				drm_mode_object_unreference(obj);
> +				drm_mode_object_put(obj);
>  				goto out;
>  			}
>  
> @@ -2176,7 +2176,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  			plane_mask |= (1 << drm_plane_index(plane));
>  			plane->old_fb = plane->fb;
>  		}
> -		drm_mode_object_unreference(obj);
> +		drm_mode_object_put(obj);
>  	}
>  
>  	ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 3b405dbf1b8d..da9a9adbcc98 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -133,7 +133,7 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
>   *
>   * This function is used to look up a modeset object. It will acquire a
>   * reference for reference counted objects. This reference must be dropped again
> - * by callind drm_mode_object_unreference().
> + * by callind drm_mode_object_put().
>   */
>  struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>  		uint32_t id, uint32_t type)
> @@ -146,38 +146,38 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_mode_object_find);
>  
>  /**
> - * drm_mode_object_unreference - decr the object refcnt
> - * @obj: mode_object
> + * drm_mode_object_put - release a mode object reference
> + * @obj: DRM mode object
>   *
>   * This function decrements the object's refcount if it is a refcounted modeset
>   * object. It is a no-op on any other object. This is used to drop references
> - * acquired with drm_mode_object_reference().
> + * acquired with drm_mode_object_get().
>   */
> -void drm_mode_object_unreference(struct drm_mode_object *obj)
> +void drm_mode_object_put(struct drm_mode_object *obj)
>  {
>  	if (obj->free_cb) {
>  		DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
>  		kref_put(&obj->refcount, obj->free_cb);
>  	}
>  }
> -EXPORT_SYMBOL(drm_mode_object_unreference);
> +EXPORT_SYMBOL(drm_mode_object_put);
>  
>  /**
> - * drm_mode_object_reference - incr the object refcnt
> - * @obj: mode_object
> + * drm_mode_object_get - acquire a mode object reference
> + * @obj: DRM mode object
>   *
>   * This function increments the object's refcount if it is a refcounted modeset
>   * object. It is a no-op on any other object. References should be dropped again
> - * by calling drm_mode_object_unreference().
> + * by calling drm_mode_object_put().
>   */
> -void drm_mode_object_reference(struct drm_mode_object *obj)
> +void drm_mode_object_get(struct drm_mode_object *obj)
>  {
>  	if (obj->free_cb) {
>  		DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
>  		kref_get(&obj->refcount);
>  	}
>  }
> -EXPORT_SYMBOL(drm_mode_object_reference);
> +EXPORT_SYMBOL(drm_mode_object_get);
>  
>  /**
>   * drm_object_attach_property - attach a property to a modeset object
> @@ -363,7 +363,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
>  			&arg->count_props);
>  
>  out_unref:
> -	drm_mode_object_unreference(obj);
> +	drm_mode_object_put(obj);
>  out:
>  	drm_modeset_unlock_all(dev);
>  	return ret;
> @@ -428,7 +428,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
>  	drm_property_change_valid_put(property, ref);
>  
>  out_unref:
> -	drm_mode_object_unreference(arg_obj);
> +	drm_mode_object_put(arg_obj);
>  out:
>  	drm_modeset_unlock_all(dev);
>  	return ret;
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 411e470369c0..15af0d42e8be 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -597,7 +597,7 @@ void drm_property_unreference_blob(struct drm_property_blob *blob)
>  	if (!blob)
>  		return;
>  
> -	drm_mode_object_unreference(&blob->base);
> +	drm_mode_object_put(&blob->base);
>  }
>  EXPORT_SYMBOL(drm_property_unreference_blob);
>  
> @@ -625,7 +625,7 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
>   */
>  struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
>  {
> -	drm_mode_object_reference(&blob->base);
> +	drm_mode_object_get(&blob->base);
>  	return blob;
>  }
>  EXPORT_SYMBOL(drm_property_reference_blob);
> @@ -906,7 +906,7 @@ void drm_property_change_valid_put(struct drm_property *property,
>  		return;
>  
>  	if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> -		drm_mode_object_unreference(ref);
> +		drm_mode_object_put(ref);
>  	} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
>  		drm_property_unreference_blob(obj_to_blob(ref));
>  }
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index 2c017adf6d74..a767b4a30a6d 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -45,10 +45,10 @@ struct drm_device;
>   *   drm_object_attach_property() before the object is visible to userspace.
>   *
>   * - For objects with dynamic lifetimes (as indicated by a non-NULL @free_cb) it
> - *   provides reference counting through drm_mode_object_reference() and
> - *   drm_mode_object_unreference(). This is used by &drm_framebuffer,
> - *   &drm_connector and &drm_property_blob. These objects provide specialized
> - *   reference counting wrappers.
> + *   provides reference counting through drm_mode_object_get() and
> + *   drm_mode_object_put(). This is used by &drm_framebuffer, &drm_connector
> + *   and &drm_property_blob. These objects provide specialized reference
> + *   counting wrappers.
>   */
>  struct drm_mode_object {
>  	uint32_t id;
> @@ -114,8 +114,32 @@ struct drm_object_properties {
>  
>  struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>  					     uint32_t id, uint32_t type);
> -void drm_mode_object_reference(struct drm_mode_object *obj);
> -void drm_mode_object_unreference(struct drm_mode_object *obj);
> +void drm_mode_object_get(struct drm_mode_object *obj);
> +void drm_mode_object_put(struct drm_mode_object *obj);
> +
> +/**
> + * drm_mode_object_reference - acquire a mode object reference
> + * @obj: DRM mode object
> + *
> + * This is a compatibility alias for drm_mode_object_get() and should not be
> + * used by new code.
> + */
> +static inline void drm_mode_object_reference(struct drm_mode_object *obj)
> +{
> +	drm_mode_object_get(obj);
> +}
> +
> +/**
> + * drm_mode_object_unreference - release a mode object reference
> + * @obj: DRM mode object
> + *
> + * This is a compatibility alias for drm_mode_object_put() and should not be
> + * used by new code.
> + */
> +static inline void drm_mode_object_unreference(struct drm_mode_object *obj)
> +{
> +	drm_mode_object_put(obj);
> +}
>  
>  int drm_object_property_set_value(struct drm_mode_object *obj,
>  				  struct drm_property *property,
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> new file mode 100644
> index 000000000000..a3742447c981
> --- /dev/null
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -0,0 +1,42 @@
> +///
> +/// Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
> +/// drm_*_unreference() helpers.
> +///
> +// Confidence: High
> +// Copyright: (C) 2017 NVIDIA Corporation
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual patch
> +virtual report
> +
> +@depends on patch@
> +expression object;
> +@@
> +
> +(
> +- drm_mode_object_reference(object)
> ++ drm_mode_object_get(object)
> +|
> +- drm_mode_object_unreference(object)
> ++ drm_mode_object_put(object)
> +)
> +
> +@r depends on report@
> +expression object;
> +position p;
> +@@
> +
> +(
> +drm_mode_object_unreference@p(object)
> +|
> +drm_mode_object_reference@p(object)
> +)
> +
> +@script:python depends on report@
> +object << r.object;
> +p << r.p;
> +@@
> +
> +msg="WARNING: use get/put helpers to reference and dereference %s" % (object)
> +coccilib.report.print_report(p[0], msg)
> -- 
> 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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux