[Intel-gfx] [RFC 1/8] drm/i915: Switch context id allocation directoy to xarray

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-12-19 18:00:12)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Idr internally uses xarray so we can use it directly which simplifies our
> code by removing the need to do external locking.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c   | 51 ++++++++-----------
>  .../gpu/drm/i915/gem/selftests/mock_context.c |  3 +-
>  drivers/gpu/drm/i915/i915_drv.h               |  6 +--
>  3 files changed, 25 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 6618c0c6506c..e5a7c6f02a47 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -780,12 +780,6 @@ void i915_gem_driver_release__contexts(struct drm_i915_private *i915)
>         flush_work(&i915->gem.contexts.free_work);
>  }
>  
> -static int context_idr_cleanup(int id, void *p, void *data)
> -{
> -       context_close(p);
> -       return 0;
> -}
> -
>  static int vm_idr_cleanup(int id, void *p, void *data)
>  {
>         i915_vm_put(p);
> @@ -793,7 +787,8 @@ static int vm_idr_cleanup(int id, void *p, void *data)
>  }
>  
>  static int gem_context_register(struct i915_gem_context *ctx,
> -                               struct drm_i915_file_private *fpriv)
> +                               struct drm_i915_file_private *fpriv,
> +                               u32 *id)
>  {
>         struct i915_address_space *vm;
>         int ret;
> @@ -811,14 +806,10 @@ static int gem_context_register(struct i915_gem_context *ctx,
>                  current->comm, pid_nr(ctx->pid));
>  
>         /* And finally expose ourselves to userspace via the idr */
> -       mutex_lock(&fpriv->context_idr_lock);
> -       ret = idr_alloc(&fpriv->context_idr, ctx, 0, 0, GFP_KERNEL);
> -       mutex_unlock(&fpriv->context_idr_lock);
> -       if (ret >= 0)
> -               goto out;
> +       ret = xa_alloc(&fpriv->context_xa, id, ctx, xa_limit_32b, GFP_KERNEL);
> +       if (ret)
> +               put_pid(fetch_and_zero(&ctx->pid));
>  
> -       put_pid(fetch_and_zero(&ctx->pid));
> -out:
>         return ret;
>  }
>  
> @@ -828,11 +819,11 @@ int i915_gem_context_open(struct drm_i915_private *i915,
>         struct drm_i915_file_private *file_priv = file->driver_priv;
>         struct i915_gem_context *ctx;
>         int err;
> +       u32 id;
>  
> -       mutex_init(&file_priv->context_idr_lock);
> -       mutex_init(&file_priv->vm_idr_lock);
> +       xa_init_flags(&file_priv->context_xa, XA_FLAGS_ALLOC);
>  
> -       idr_init(&file_priv->context_idr);
> +       mutex_init(&file_priv->vm_idr_lock);

I had a double take here, thinking "why is he removing vm_idr..."
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris