Re: [PATCH 1/3] drm: Balance error path for GEM handle allocation

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jan 04, 2016 at 10:10:59AM +0000, Chris Wilson wrote:
> The current error path for failure when establishing a handle for a GEM
> object is unbalance, e.g. we call object_close() without calling first
> object_open(). Use the typical onion structure to only undo what has
> been set up prior to the error.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_gem.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2e10bba4468b..a08176debc0e 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -343,27 +343,32 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>  	spin_unlock(&file_priv->table_lock);
>  	idr_preload_end();
>  	mutex_unlock(&dev->object_name_lock);
> -	if (ret < 0) {
> -		drm_gem_object_handle_unreference_unlocked(obj);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto err_unref;
> +
>  	*handlep = ret;
>  
>  	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
> -	if (ret) {
> -		drm_gem_handle_delete(file_priv, *handlep);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_remove;
>  
>  	if (dev->driver->gem_open_object) {
>  		ret = dev->driver->gem_open_object(obj, file_priv);
> -		if (ret) {
> -			drm_gem_handle_delete(file_priv, *handlep);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err_revoke;
>  	}
>  
>  	return 0;
> +
> +err_revoke:
> +	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
> +err_remove:
> +	spin_lock(&file_priv->table_lock);
> +	idr_remove(&file_priv->object_idr, *handlep);
> +	spin_unlock(&file_priv->table_lock);
> +err_unref:
> +	drm_gem_object_handle_unreference_unlocked(obj);
> +	return ret;

First I misread this as drm_gem_object_unreference_unlocked()
and though we'd leak the handle_count++, but it's
drm_gem_object_handle_unreference_unlocked() which does the
handle_count-- we need.

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

>  }
>  
>  /**
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel