On 11/8/21 09:08, Thomas Hellström wrote:
On 11/5/21 16:18, Matthew Auld wrote:
On 05/11/2021 13:03, Thomas Hellström wrote:
Gem-TTM objects that are backed by shmem might have populated
page-vectors without having the Gem pages set. Those objects
aren't moved to the correct shrinker / purge list by the
gem_madvise. Furthermore they are purged directly on
MADV_DONTNEED rather than waiting for the shrinker to do that.
For such objects, identified by having the
_SELF_MANAGED_SHRINK_LIST set, make sure they end up on the
correct list and defer purging to the shrinker.
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d0e642c82064..da972c8d45b1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1005,7 +1005,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev,
void *data,
obj->ops->adjust_lru(obj);
}
- if (i915_gem_object_has_pages(obj)) {
+ if (i915_gem_object_has_pages(obj) ||
+ i915_gem_object_has_self_managed_shrink_list(obj)) {
Makes sense.
unsigned long flags;
spin_lock_irqsave(&i915->mm.obj_lock, flags);
@@ -1024,7 +1025,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev,
void *data,
/* if the object is no longer attached, discard its backing
storage */
if (obj->mm.madv == I915_MADV_DONTNEED &&
- !i915_gem_object_has_pages(obj))
+ !i915_gem_object_has_pages(obj) &&
+ !i915_gem_object_has_self_managed_shrink_list(obj))
i915_gem_object_truncate(obj);
And it looks like this also matches the workings of lmem, where under
memory pressure we also just purge such objects, instead of moving
them, making sure to keep them first in the LRU?
One thing is to maybe immediately discard already swapped-out objects
here, since the shrinker will be oblivious to them, and they sort of
just linger in swap until the object is destroyed?
This might be a bit ugly if we want to avoid exposing even more gem
object ops.
Could we perhaps for the truncate callback only truncate swapped-out
objects if we have a self-managed shrinker list? That will match all
the current call-sites AFAICT since truncate is never called from the
shrinker with the self-managed shrinker list...
Or actually, Thinking of it, if we don't have GEM pages, even if there
are CPU PTEs set up, I can't see why we shouldn't purge this object
immediately. I'll just revert that last hunk.
/Thomas
/Thomas
args->retained = obj->mm.madv != __I915_MADV_PURGED;
