Re: [PATCH] [v2] drm/i915/ppgtt: Never return a NULL context

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Ben Widawsky <benjamin.widawsky@intel.com> writes:

> It makes all the code which calls into this function way too confusing.
>
> v2: Fix destroy IOCTL as well
>
> v3: Clarify the other two callers of i915_gem_context_get() to never
> check for NULL. (Mika)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72903
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  drivers/gpu/drm/i915/i915_gem_context.c    | 12 +++++++++---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index ebe0f67..44dddc00 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -526,10 +526,16 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>  struct i915_hw_context *
>  i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
>  {
> +	struct i915_hw_context *ctx;
> +
>  	if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev))
>  		return file_priv->private_default_ctx;
>  
> -	return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
> +	ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
> +	if (!ctx)
> +		return ERR_PTR(-ENOENT);
> +
> +	return ctx;
>  }
>  
>  static inline int
> @@ -776,9 +782,9 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  		return ret;
>  
>  	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> -	if (!ctx) {
> +	if (IS_ERR(ctx)) {
>  		mutex_unlock(&dev->struct_mutex);
> -		return -ENOENT;
> +		return PTR_ERR(ctx);
>  	}
>  
>  	idr_remove(&ctx->file_priv->context_idr, ctx->id);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index bbff8f9..c30a6ee 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -916,7 +916,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>  		return ERR_PTR(-EINVAL);
>  
>  	ctx = i915_gem_context_get(file->driver_priv, ctx_id);
> -	if (IS_ERR_OR_NULL(ctx))
> +	if (IS_ERR(ctx))
>  		return ctx;
>  
>  	hs = &ctx->hang_stats;
> @@ -1126,7 +1126,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	}
>  
>  	ctx = i915_gem_validate_context(dev, file, ring, ctx_id);
> -	if (IS_ERR_OR_NULL(ctx)) {
> +	if (IS_ERR(ctx)) {
>  		mutex_unlock(&dev->struct_mutex);
>  		ret = PTR_ERR(ctx);
>  		goto pre_mutex_err;
> -- 
> 1.8.5.2