Re: [Intel-gfx] [PATCH] drm/i915: Do not invalidate obj->pages under mempressure

[Sean V Kelley <seanvk@posteo.de>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 01/14/2015 04:55 PM, Daniel Vetter wrote:
> On Wed, Jan 14, 2015 at 08:34:31PM +0000, Chris Wilson wrote:
>> This (partially) reverts
>> 
>> commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: Chris
>> Wilson <chris@chris-wilson.co.uk> 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).
>> 
>> Reported-by: Sean V Kelley <sean.v.kelley@intel.com> 
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Sean V
>> Kelley <sean.v.kelley@intel.com> Cc: stable@vger.kernel.org
> 
> Hm, scary. Do we have a testcase for this (might need a
> correctness version for some of the swap thrashing tests maybe). 
> -Daniel

I can put together a test case we use with a media runtime workload
for an EU that we could add to igt.  Also this appears to be something
that has been also encountered on BDW during an Android kernel team
port that from what I can tell was patched locally (internally) and
not shared upstream.  I stumbled upon it only after getting Chris'
help with this patch.

Sean


> 
>> --- drivers/gpu/drm/i915/i915_gem.c | 23 ++--------------------- 
>> 1 file changed, 2 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c index
>> 4d453490596e..b06f051a73de 100644 ---
>> a/drivers/gpu/drm/i915/i915_gem.c +++
>> b/drivers/gpu/drm/i915/i915_gem.c @@ -1947,26 +1947,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) { @@ -2029,7 +2009,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; } -- 2.1.4
>> 
>> _______________________________________________ Intel-gfx mailing
>> list Intel-gfx@lists.freedesktop.org 
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJUtxW9AAoJEGScDsMo8QYOlZkP/3IbGTlfw4aj37nlClMA+JpO
qHDbYi9evALt7NkiE5UcVz+LJwPZ/FLZR+Q8w5m/4NqPDVbGCDFDEovoJx/MKWgA
u6MuUrkuFC5KjGFiYZFuP/+EsXKMblNKRv4xWeWoPWgwlvd9ZYw1rSNVPLqkgIPu
RtvSJAsKdehvbl7Y4VyKDjKn9gUp58qKE9P/setx3u1vc/xbufKRACkdp9QLDzBm
s0HjmgIkXRQr1n7L4tUmZiYB9igZnapjxzyX2FazTQhHQFthUMu7pVJaP6MYLuSJ
Dr9CidL/DC4r4sciaOUhLlA5rD0PJ5AMrS9ZN6wf50gN1JWYYIgdl51TLkqFMjou
EpTv442qzjO+Mi5mY8YMheruDV/voGU7mc/11i3Ng9kSBdkv7kfVhcnSLsJsGCxw
4mI3fHOdSySIG+ZgM5Fw7XmFpXypbcqTZ99qbQkMoyQoXtbn96s1wCWp4j3qHHQ1
hQN6lgU3eb35gafmJ18qtEYouUahtRtB1SHOppGiLMuEaFcqr4hvFoq7fDl7VtQa
RqnoOyDcwUEht7btPLaRcnyd7kUl0fTcwmTbeFLo5hrOuv5Ke08ms9aMfAvk+cR7
mYO9OzxkvUJAgjPn9yCLR70fuDVGQa3rbM31LY6QBEGxfvjG2qPe2eFw5XdBkgTV
iCREv8mvFVotVg0vFAJB
=RERA
-----END PGP SIGNATURE-----