From 754a25442705c4f90e0d05f1a7bd303ffe700ca9 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 21 Mar 2019 16:19:08 +0000 Subject: [PATCH] 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 Cc: Joonas Lahtinen Cc: Matthew Auld Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20190321161908.8007-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 72374e952e4b..638ae277ff03 100644 --- 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. */ -- 2.30.2