Re: [PATCH] drm: Don't reference objects in the flink name idr

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Dec 02, 2013 at 05:36:17PM -0800, Kristian Høgsberg wrote:
> There's no reason to keep a reference to objects in the name idr.  Each
> handle to an object has a reference to the object and just before we
> destroy the last handle we take the object out of the name idr.  Thus,
> if an object is in the name idr, there's at least one reference to the
> object.
> 
> Or to put it another way, the name idr reference will never keep the
> object alive.  It just looks like it, which is confusing.
> 
> Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

I expect this to blow up when you race gem_close ioctl calls with flink
open. i-g-t/gem_flink_close tests actually have been written specifically
to exercise these races.
-Daniel

> ---
>  drivers/gpu/drm/drm_gem.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4761ade..3ff39bb 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -175,11 +175,6 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
>  	mutex_unlock(&filp->prime.lock);
>  }
>  
> -static void drm_gem_object_ref_bug(struct kref *list_kref)
> -{
> -	BUG();
> -}
> -
>  /**
>   * Called after the last handle to the object has been closed
>   *
> @@ -195,13 +190,6 @@ static void drm_gem_object_handle_free(struct drm_gem_object *obj)
>  	if (obj->name) {
>  		idr_remove(&dev->object_name_idr, obj->name);
>  		obj->name = 0;
> -		/*
> -		 * The object name held a reference to this object, drop
> -		 * that now.
> -		*
> -		* This cannot be the last reference, since the handle holds one too.
> -		 */
> -		kref_put(&obj->refcount, drm_gem_object_ref_bug);
>  	}
>  }
>  
> @@ -602,9 +590,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
>  			goto err;
>  
>  		obj->name = ret;
> -
> -		/* Allocate a reference for the name table.  */
> -		drm_gem_object_reference(obj);
>  	}
>  
>  	args->name = (uint64_t) obj->name;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch