drm/i915: Skip object locking around a no-op set-domain ioctl

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. By recognising that the
write_domain is always a subset of the read_domains, and excluding the
no-op case of requiring 0 read_domains in the ioctl, we can infer if the
current write_domain matches the target read_domains, there is nothing
for us to do.

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@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190321161908.8007-2-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 72374e952e4bd0ab9ed11f73f9484590c409a66c..638ae277ff039507c7caa0a5e71014acd458add6 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1484,17 +1484,37 @@ 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)
+       if (write_domain && read_domains != write_domain)
                return -EINVAL;
 
+       if (!read_domains)
+               return 0;
+
        obj = i915_gem_object_lookup(file, args->handle);
        if (!obj)
                return -ENOENT;
 
-       /* Try to flush the object off the GPU without holding the lock.
+       /*
+        * Already in the desired write domain? Nothing for us to do!
+        *
+        * We apply a little bit of cunning here to catch a broader set of
+        * no-ops. If obj->write_domain is set, we must be in the same
+        * obj->read_domains, and only that domain. Therefore, if that
+        * obj->write_domain matches the request read_domains, we are
+        * already in the same read/write domain and can skip the operation,
+        * without having to further check the requested write_domain.
+        */
+       if (READ_ONCE(obj->write_domain) == read_domains) {
+               err = 0;
+               goto out;
+       }
+
+       /*
+        * Try to flush the object off the GPU without holding the lock.
         * We will repeat the flush holding the lock in the normal manner
         * to catch cases where we are gazumped.
         */