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.hellst...@linux.intel.com>
---
  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;

Reply via email to