From 9764951e7f517717bc7ecc3f1a9711816646ebf7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 18 Aug 2016 17:16:50 +0100 Subject: [PATCH] drm/i915: Pin the pages first in shmem prepare read/write There is an improbable, but not impossible, case that if we leave the pages unpin as we operate on the object, then somebody via the shrinker may steal the lock (which lock? right now, it is struct_mutex, THE lock) and change the cache domains after we have already inspected them. (Whilst here, avail ourselves of the opportunity to take a couple of steps to make the two functions look more similar.) Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Link: http://patchwork.freedesktop.org/patch/msgid/20160818161718.27187-11-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 48 +++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4c9ecab765f1..c5d5dfe3e0ef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -622,6 +622,12 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj, if (ret) return ret; + ret = i915_gem_object_get_pages(obj); + if (ret) + return ret; + + i915_gem_object_pin_pages(obj); + i915_gem_object_flush_gtt_write_domain(obj); /* If we're not in the cpu read domain, set ourself into the gtt @@ -633,22 +639,20 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj, *needs_clflush = !cpu_cache_is_coherent(obj->base.dev, obj->cache_level); - ret = i915_gem_object_get_pages(obj); - if (ret) - return ret; - - i915_gem_object_pin_pages(obj); - if (*needs_clflush && !static_cpu_has(X86_FEATURE_CLFLUSH)) { ret = i915_gem_object_set_to_cpu_domain(obj, false); - if (ret) { - i915_gem_object_unpin_pages(obj); - return ret; - } + if (ret) + goto err_unpin; + *needs_clflush = 0; } + /* return with the pages pinned */ return 0; + +err_unpin: + i915_gem_object_unpin_pages(obj); + return ret; } int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, @@ -664,6 +668,12 @@ int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, if (ret) return ret; + ret = i915_gem_object_get_pages(obj); + if (ret) + return ret; + + i915_gem_object_pin_pages(obj); + i915_gem_object_flush_gtt_write_domain(obj); /* If we're not in the cpu write domain, set ourself into the @@ -681,18 +691,11 @@ int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, *needs_clflush |= !cpu_cache_is_coherent(obj->base.dev, obj->cache_level); - ret = i915_gem_object_get_pages(obj); - if (ret) - return ret; - - i915_gem_object_pin_pages(obj); - if (*needs_clflush && !static_cpu_has(X86_FEATURE_CLFLUSH)) { ret = i915_gem_object_set_to_cpu_domain(obj, true); - if (ret) { - i915_gem_object_unpin_pages(obj); - return ret; - } + if (ret) + goto err_unpin; + *needs_clflush = 0; } @@ -701,7 +704,12 @@ int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, intel_fb_obj_invalidate(obj, ORIGIN_CPU); obj->dirty = 1; + /* return with the pages pinned */ return 0; + +err_unpin: + i915_gem_object_unpin_pages(obj); + return ret; } /* Per-page copy function for the shmem pread fastpath. -- 2.45.2