Re: [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma()

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 12/09/2017 16:07, Chris Wilson wrote:
> Following the simplification to a single lookup loop in commit
> 170fa29b14fa ("drm/i915: Simplify eb_lookup_vmas()") and commit
> d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr"), we can go
> one setup further and reorder the error paths so that the state of the
> local variable obj is always known to the compiler and doesn't need the
> uninitialized_var markup to squelch a compiler warning.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 7687483ff218..214a850b4b3c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -679,7 +679,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
>   static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   {
>   	struct radix_tree_root *handles_vma = &eb->ctx->handles_vma;
> -	struct drm_i915_gem_object *uninitialized_var(obj);
> +	struct drm_i915_gem_object *obj;
>   	unsigned int i;
>   	int err;
>   
> @@ -725,19 +725,17 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   			goto err_obj;
>   		}
>   
> +		/* transfer ref to ctx */
>   		vma->open_count++;
>   		list_add(&lut->obj_link, &obj->lut_list);
>   		list_add(&lut->ctx_link, &eb->ctx->handles_list);
>   		lut->ctx = eb->ctx;
>   		lut->handle = handle;
>   
> -		/* transfer ref to ctx */
> -		obj = NULL;
> -
>   add_vma:
>   		err = eb_add_vma(eb, i, vma);
>   		if (unlikely(err))
> -			goto err_obj;
> +			goto err_vma;
>   
>   		GEM_BUG_ON(vma != eb->vma[i]);
>   		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
> @@ -766,8 +764,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   	return eb_reserve(eb);
>   
>   err_obj:
> -	if (obj)
> -		i915_gem_object_put(obj);
> +	i915_gem_object_put(obj);
>   err_vma:
>   	eb->vma[i] = NULL;
>   	return err;
> 

Looks good to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx