From: Chris Wilson <[email protected]> This (partially) reverts
commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: Chris Wilson <[email protected]> Date: Tue Mar 25 13:23:06 2014 +0000 drm/i915: Invalidate our pages under memory pressure It appears given the right workload, that pages which are swapped out more than once are incorrectly invalidated and discarded. I had presumed that the swapin would mark the pages dirty again and so preserve them against the next cycle of invalidation - that appears to be false, and leads to memory corruption (even leak of stale pages to userspace). v2: Do a more throughrought revert and als get rid of the hunk in gem_free_objects which we've tried to patch up already in commit 340fbd8ca1c7d6006a6b6afe716c10007bbfde85 Author: Chris Wilson <[email protected]> Date: Thu May 22 09:16:52 2014 +0100 drm/i915: Only discard backing storage on releasing the last ref This means this patch also fully reverts this fixup. Apparently this is just too tricky. Reported-by: Sean V Kelley <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Cc: Sean V Kelley <[email protected]> Cc: [email protected] Cc: Chris Wilson <[email protected]> Cc: Jani Nikula <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> (v2) --- drivers/gpu/drm/i915/i915_gem.c | 49 ++--------------------------------------- 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 36f1093e3c63..39e2af9b5fef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1946,26 +1946,6 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj) obj->madv = __I915_MADV_PURGED; } -/* Try to discard unwanted pages */ -static void -i915_gem_object_invalidate(struct drm_i915_gem_object *obj) -{ - struct address_space *mapping; - - switch (obj->madv) { - case I915_MADV_DONTNEED: - i915_gem_object_truncate(obj); - case __I915_MADV_PURGED: - return; - } - - if (obj->base.filp == NULL) - return; - - mapping = file_inode(obj->base.filp)->i_mapping, - invalidate_mapping_pages(mapping, 0, (loff_t)-1); -} - static void i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj) { @@ -2028,7 +2008,8 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj) ops->put_pages(obj); obj->pages = NULL; - i915_gem_object_invalidate(obj); + if (i915_gem_object_is_purgeable(obj)) + i915_gem_object_truncate(obj); return 0; } @@ -4458,30 +4439,6 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev, return obj; } -static bool discard_backing_storage(struct drm_i915_gem_object *obj) -{ - /* If we are the last user of the backing storage (be it shmemfs - * pages or stolen etc), we know that the pages are going to be - * immediately released. In this case, we can then skip copying - * back the contents from the GPU. - */ - - if (obj->madv != I915_MADV_WILLNEED) - return false; - - if (obj->base.filp == NULL) - return true; - - /* At first glance, this looks racy, but then again so would be - * userspace racing mmap against close. However, the first external - * reference to the filp can only be obtained through the - * i915_gem_mmap_ioctl() which safeguards us against the user - * acquiring such a reference whilst we are in the middle of - * freeing the object. - */ - return atomic_long_read(&obj->base.filp->f_count) == 1; -} - void i915_gem_free_object(struct drm_gem_object *gem_obj) { struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); @@ -4524,8 +4481,6 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (WARN_ON(obj->pages_pin_count)) obj->pages_pin_count = 0; - if (discard_backing_storage(obj)) - obj->madv = I915_MADV_DONTNEED; i915_gem_object_put_pages(obj); i915_gem_object_free_mmap_offset(obj); -- 2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
