Re: [PATCH 2/2] drm/i915: clean up shrinker_release_pages

Matthew Auld <matthew.auld@xxxxxxxxx> · Wed, 15 Dec 2021 16:08:33 +0000

On 15/12/2021 15:55, Tvrtko Ursulin wrote:

On 15/12/2021 11:07, Matthew Auld wrote:
Add some proper flags for the different modes, and shorten the name to
something more snappy.

Looks good to me - but since it touches TTM I leave for Thomas to approve.

Regards,

Tvrtko

P.S. I hope writing the patch means you thought it is an improvement as 
well, rather than feeling I was asking for it to be done.

Yes, I do see both patches as an improvement :)


Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
---
  .../gpu/drm/i915/gem/i915_gem_object_types.h  | 23 ++++++++++++++++---
  drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |  8 +++----
  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  | 16 +++++++++----
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c       | 10 ++++----
  4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 00c844caeabd..6f446cca4322 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -57,9 +57,26 @@ struct drm_i915_gem_object_ops {
      void (*put_pages)(struct drm_i915_gem_object *obj,
                struct sg_table *pages);
      int (*truncate)(struct drm_i915_gem_object *obj);
-    int (*shrinker_release_pages)(struct drm_i915_gem_object *obj,
-                      bool no_gpu_wait,
-                      bool should_writeback);
+    /**
+     * shrink - Perform further backend specific actions to facilate
+     * shrinking.
+     * @obj: The gem object
+     * @flags: Extra flags to control shrinking behaviour in the backend
+     *
+     * Possible values for @flags:
+     *
+     * I915_GEM_OBJECT_SHRINK_WRITEBACK - Try to perform writeback of 
the
+     * backing pages, if supported.
+     *
+     * I915_GEM_OBJECT_SHRINK_NO_GPU_WAIT - Don't wait for the object to
+     * idle.  Active objects can be considered later. The TTM backend 
for
+     * example might have aync migrations going on, which don't use any
+     * i915_vma to track the active GTT binding, and hence having an 
unbound
+     * object might not be enough.
+     */
+#define I915_GEM_OBJECT_SHRINK_WRITEBACK   BIT(0)
+#define I915_GEM_OBJECT_SHRINK_NO_GPU_WAIT BIT(1)
+    int (*shrink)(struct drm_i915_gem_object *obj, unsigned int flags);
      int (*pread)(struct drm_i915_gem_object *obj,
               const struct drm_i915_gem_pread *arg);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 7fdf4fa10b0e..6c57b0a79c8a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -331,9 +331,7 @@ shmem_writeback(struct drm_i915_gem_object *obj)
      __shmem_writeback(obj->base.size, obj->base.filp->f_mapping);
  }
-static int shmem_shrinker_release_pages(struct drm_i915_gem_object *obj,
-                    bool no_gpu_wait,
-                    bool writeback)
+static int shmem_shrink(struct drm_i915_gem_object *obj, unsigned int 
flags)
  {
      switch (obj->mm.madv) {
      case I915_MADV_DONTNEED:
@@ -342,7 +340,7 @@ static int shmem_shrinker_release_pages(struct 
drm_i915_gem_object *obj,
          return 0;
      }
-    if (writeback)
+    if (flags & I915_GEM_OBJECT_SHRINK_WRITEBACK)
          shmem_writeback(obj);
      return 0;
@@ -520,7 +518,7 @@ const struct drm_i915_gem_object_ops 
i915_gem_shmem_ops = {
      .get_pages = shmem_get_pages,
      .put_pages = shmem_put_pages,
      .truncate = shmem_truncate,
-    .shrinker_release_pages = shmem_shrinker_release_pages,
+    .shrink = shmem_shrink,
      .pwrite = shmem_pwrite,
      .pread = shmem_pread,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index fd54e05521f6..968ca0fdd57b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -57,10 +57,18 @@ static bool unsafe_drop_pages(struct 
drm_i915_gem_object *obj,
  static int try_to_writeback(struct drm_i915_gem_object *obj, 
unsigned int flags)
  {
-    if (obj->ops->shrinker_release_pages)
-        return obj->ops->shrinker_release_pages(obj,
-                            !(flags & I915_SHRINK_ACTIVE),
-                            flags & I915_SHRINK_WRITEBACK);
+    if (obj->ops->shrink) {
+        unsigned int shrink_flags = 0;
+
+        if (!(flags & I915_SHRINK_ACTIVE))
+            shrink_flags |= I915_GEM_OBJECT_SHRINK_NO_GPU_WAIT;
+
+        if (flags & I915_SHRINK_WRITEBACK)
+            shrink_flags |= I915_GEM_OBJECT_SHRINK_WRITEBACK;
+
+        return obj->ops->shrink(obj, shrink_flags);
+    }
+
      return 0;
  }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 923cc7ad8d70..21277f3c64e7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -424,16 +424,14 @@ int i915_ttm_purge(struct drm_i915_gem_object *obj)
      return 0;
  }
-static int i915_ttm_shrinker_release_pages(struct drm_i915_gem_object 
*obj,
-                       bool no_wait_gpu,
-                       bool should_writeback)
+static int i915_ttm_shrink(struct drm_i915_gem_object *obj, unsigned 
int flags)
  {
      struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
      struct i915_ttm_tt *i915_tt =
          container_of(bo->ttm, typeof(*i915_tt), ttm);
      struct ttm_operation_ctx ctx = {
          .interruptible = true,
-        .no_wait_gpu = no_wait_gpu,
+        .no_wait_gpu = flags & I915_GEM_OBJECT_SHRINK_NO_GPU_WAIT,
      };
      struct ttm_placement place = {};
      int ret;
@@ -467,7 +465,7 @@ static int i915_ttm_shrinker_release_pages(struct 
drm_i915_gem_object *obj,
          return ret;
      }
-    if (should_writeback)
+    if (flags & I915_GEM_OBJECT_SHRINK_WRITEBACK)
          __shmem_writeback(obj->base.size, i915_tt->filp->f_mapping);
      return 0;
@@ -953,7 +951,7 @@ static const struct drm_i915_gem_object_ops 
i915_gem_ttm_obj_ops = {
      .get_pages = i915_ttm_get_pages,
      .put_pages = i915_ttm_put_pages,
      .truncate = i915_ttm_purge,
-    .shrinker_release_pages = i915_ttm_shrinker_release_pages,
+    .shrink = i915_ttm_shrink,
      .adjust_lru = i915_ttm_adjust_lru,
      .delayed_free = i915_ttm_delayed_free,







