[Intel-gfx] [PATCH] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

Quoting Chris Wilson (2019-03-21 08:05:32)
> If we are already in the desired write domain of a set-domain ioctl,
> then there is nothing for us to do and we can quickly return back to
> userspace, avoiding any lock contention.
> 
> Secondary aspect of this is that we undo the arbitrary fetching and
> potential flushing of all pages for a set-domain(.write=CPU) call on a
> fresh object -- which was introduced simply because we do the get-pages
> before taking the struct_mutex.
> 
> References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld at gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1a684b7e8c09..e30165536f9a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1484,7 +1484,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>         if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
>                 return -EINVAL;
>  
> -       /* Having something in the write domain implies it's in the read
> +       /*
> +        * Having something in the write domain implies it's in the read
>          * domain, and only that read domain.  Enforce that in the request.
>          */
>         if (write_domain != 0 && read_domains != write_domain)
> @@ -1494,7 +1495,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
>  
> -       /* Try to flush the object off the GPU without holding the lock.
> +       /* Already in the desired target write domain? Nothing for us to! */
> +       if (READ_ONCE(obj->write_domain) == write_domain) {

This should be write_domain && ...
-Chris