Re: [PATCH 2/3] drm: Only bump object-reference count when adding first handle

Ville Syrjälä
2016-01-04T15:29:58Z

On Mon, Jan 04, 2016 at 10:11:00AM +0000, Chris Wilson wrote:
> We only need a single reference count for all handles (i.e. non-zero
> obj->handle_count) and so can trim a few atomic operations by only
> taking the reference on the first handle and dropping it after the last.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/drm_gem.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index a08176debc0e..ad955d7c99fd 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -220,6 +220,9 @@ 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)
> {
> + struct drm_device *dev = obj->dev;
> + bool final = false;
> +
> if (WARN_ON(obj->handle_count == 0))
> return;
>
> @@ -229,14 +232,16 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
> * checked for a name
> */
>
> - mutex_lock(&obj->dev->object_name_lock);
> + mutex_lock(&dev->object_name_lock);
> if (--obj->handle_count == 0) {
> drm_gem_object_handle_free(obj);
> drm_gem_object_exported_dma_buf_free(obj);
> + final = true;
> }
> - mutex_unlock(&obj->dev->object_name_lock);
> + mutex_unlock(&dev->object_name_lock);
>
> - drm_gem_object_unreference_unlocked(obj);
> + if (final)
> + drm_gem_object_unreference_unlocked(obj);
> }
>
> /**
> @@ -329,6 +334,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> int ret;
>
> WARN_ON(!mutex_is_locked(&dev->object_name_lock));
> + if (obj->handle_count++ == 0)
> + drm_gem_object_reference(obj);

Hmm. Yeah, handle_count is protected by object_name_lock, so this looks
better here rather than within the spinlocked section.

Patch looks sane:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>
> /*
> * Get the user-visible handle using idr. Preload and perform
> @@ -338,10 +345,10 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> spin_lock(&file_priv->table_lock);
>
> ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
> - drm_gem_object_reference(obj);
> - obj->handle_count++;
> +
> spin_unlock(&file_priv->table_lock);
> idr_preload_end();
> +
> mutex_unlock(&dev->object_name_lock);
> if (ret < 0)
> goto err_unref;
> --
> 2.6.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrjälä
Intel OTC