From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
This (partially) reverts
commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd
Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
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 <chris@xxxxxxxxxxxxxxxxxx>
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 <sean.v.kelley@xxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Sean V Kelley <sean.v.kelley@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> (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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html