On Mon, Nov 06, 2017 at 11:48:33AM +0000, Chris Wilson wrote:
> Silence smatch by demonstrating that guc->stage_desc_pool is allocated
> following a successful guc_stage_desc_pool_create()
> 
> drivers/gpu/drm/i915/i915_guc_submission.c:1293 i915_guc_submission_init() 
> error: we previously assumed 'guc->stage_desc_pool' could be null (see line 
> 1261
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index d14c1342f09d..6f2548114bd2 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1265,10 +1265,18 @@ int i915_guc_submission_init(struct drm_i915_private 
> *dev_priv)
>       if (ret)
>               return ret;
>  
> +     /*
> +      * Keep static analysers happy, let them know that we allocated the
> +      * vma after testing that it didn't exist earlier.
> +      */
> +     GEM_BUG_ON(!guc->stage_desc_pool);
> +
>       ret = guc_shared_data_create(guc);
>       if (ret)
>               goto err_stage_desc_pool;
>  
> +     GEM_BUG_ON(!guc->shared_data);
> +
>       ret = intel_guc_log_create(guc);
>       if (ret < 0)
>               goto err_shared_data;
> @@ -1277,10 +1285,14 @@ int i915_guc_submission_init(struct drm_i915_private 
> *dev_priv)
>       if (ret)
>               goto err_log;
>  
> +     GEM_BUG_ON(!guc->preempt_wq);
> +
>       ret = guc_ads_create(guc);
>       if (ret < 0)
>               goto err_wq;
>  
> +     GEM_BUG_ON(!guc->ads_vma);
> +
>       return 0;
>  

Nitpick: I would place GEM_BUG_ONs right after "if(ret)" without
any separation line to emphase that those statements are related.

Also I hope that one day we will call submission_init just once and
than early check of stage_desc_pool will not be needed any more.

Reviewed-by: Michal Wajdeczko <michal.wajdec...@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to