On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
> 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).
Yeah, if you do a large-scale refactor anyway, I think it's best to just
store the cocci in the commit message. I think storing the cocci is ok if
you have thousands of hits among lots of subsystems, and it's clear it's
going to take at least a few release cycles or maybe even years to clean
it all up. drm is luckily not yet that big :-)
I'll drop this while applying if no one minds ...
-Daniel
>
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
