Re: [PATCH v5 11/13] drm/i915/ttm: make evicted shmem pages visible to the shrinker

2021-09-29 Thread Thomas Hellström 
On Mon, 2021-09-27 at 12:41 +0100, Matthew Auld wrote:
> We currently just evict lmem objects to system memory when under
> memory
> pressure, and in the next patch we want to use the shmem backend even
> for this case. For this case we lack the usual object mm.pages, which
> effectively hides the pages from the i915-gem shrinker, until we
> actually "attach" the TT to the object, or in the case of lmem-only
> objects it just gets migrated back to lmem when touched again.
> 
> For all cases we can just adjust the i915 shrinker LRU each time we
> also
> adjust the TTM LRU. The two cases we care about are:
> 
>   1) When something is moved by TTM, including when initially
> populating
>  an object. Importantly this covers the case where TTM moves
> something from
>  lmem <-> smem, outside of the normal get_pages() interface,
> which
>  should still ensure the shmem pages underneath are reclaimable.
> 
>   2) When calling into i915_gem_object_unlock(). The unlock should
>  ensure the object is removed from the shinker LRU, if it was
> indeed
>  swapped out, or just purged, when the shrinker drops the object
> lock.
> 
> We can optimise this(if needed) by tracking if the object is already
> visible to the shrinker(protected by the object lock), so we don't
> touch
> the shrinker LRU more than needed.
> 
> v2(Thomas)
>   - Handle managing the shrinker LRU in adjust_lru, where it is
> always
>     safe to touch the object.
> 
> Signed-off-by: Matthew Auld 
> Cc: Thomas Hellström 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_object.h   |  1 +
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 29 +++---
> --
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 28 +++---
> -
>  3 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 1c9a1d8d3434..640dfbf1f01e 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -523,6 +523,7 @@ i915_gem_object_pin_to_display_plane(struct
> drm_i915_gem_object *obj,
>  
>  void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object
> *obj);
>  void i915_gem_object_make_shrinkable(struct drm_i915_gem_object
> *obj);
> +void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object
> *obj);
>  void i915_gem_object_make_purgeable(struct drm_i915_gem_object
> *obj);
>  
>  static inline bool cpu_write_needs_clflush(struct
> drm_i915_gem_object *obj)
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 0440696f786a..4b6b2bb6f180 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -487,13 +487,12 @@ void i915_gem_object_make_unshrinkable(struct
> drm_i915_gem_object *obj)
> spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
>  }
>  
> -static void __i915_gem_object_make_shrinkable(struct
> drm_i915_gem_object *obj,
> - struct list_head *head)
> +static void ___i915_gem_object_make_shrinkable(struct
> drm_i915_gem_object *obj,
> +  struct list_head
> *head)
>  {
> struct drm_i915_private *i915 = obj_to_i915(obj);
> unsigned long flags;
>  
> -   GEM_BUG_ON(!i915_gem_object_has_pages(obj));
> if (!i915_gem_object_is_shrinkable(obj))
> return;
>  
> @@ -512,6 +511,21 @@ static void
> __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj,
> spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
>  }
>  
> +/**
> + * __i915_gem_object_make_shrinkable - Move the object to the tail
> of the
> + * shrinkable list. Objects on this list might be swapped out. Used
> with
> + * WILLNEED objects.
> + * @obj: The GEM object.
> + *
> + * DO NOT USE. This is intended to be called on very special objects
> that don't
> + * yet have mm.pages, but are guaranteed to have potentially
> reclaimable pages
> + * underneath.
> + */
> +void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object
> *obj)
> +{
> +   ___i915_gem_object_make_shrinkable(obj,
> +  &obj_to_i915(obj)-
> >mm.shrink_list);
> +}
>  
>  /**
>   * i915_gem_object_make_shrinkable - Move the object to the tail of
> the
> @@ -523,8 +537,8 @@ static void
> __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj,
>   */
>  void i915_gem_object_make_shrinkable(struct drm_i915_gem_object
> *obj)
>  {
> -   __i915_gem_object_make_shrinkable(obj,
> - &obj_to_i915(obj)-
> >mm.shrink_list);
> +   GEM_BUG_ON(!i915_gem_object_has_pages(obj));
> +   __i915_gem_object_make_shrinkable(obj);
>  }
>  
>  /**
> @@ -538,6 +552,7 @@ void i915_gem_object_make_shrinkable(struct
> drm_i915_gem_object *obj)
>   */
>  void i915_gem_object_make_purgeable(struct drm_

[PATCH v5 11/13] drm/i915/ttm: make evicted shmem pages visible to the shrinker

2021-09-27 Thread Matthew Auld 
We currently just evict lmem objects to system memory when under memory
pressure, and in the next patch we want to use the shmem backend even
for this case. For this case we lack the usual object mm.pages, which
effectively hides the pages from the i915-gem shrinker, until we
actually "attach" the TT to the object, or in the case of lmem-only
objects it just gets migrated back to lmem when touched again.

For all cases we can just adjust the i915 shrinker LRU each time we also
adjust the TTM LRU. The two cases we care about are:

  1) When something is moved by TTM, including when initially populating
 an object. Importantly this covers the case where TTM moves something from
 lmem <-> smem, outside of the normal get_pages() interface, which
 should still ensure the shmem pages underneath are reclaimable.

  2) When calling into i915_gem_object_unlock(). The unlock should
 ensure the object is removed from the shinker LRU, if it was indeed
 swapped out, or just purged, when the shrinker drops the object lock.

We can optimise this(if needed) by tracking if the object is already
visible to the shrinker(protected by the object lock), so we don't touch
the shrinker LRU more than needed.

v2(Thomas)
  - Handle managing the shrinker LRU in adjust_lru, where it is always
safe to touch the object.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
---
 drivers/gpu/drm/i915/gem/i915_gem_object.h   |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 29 +++-
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 28 +++
 3 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 1c9a1d8d3434..640dfbf1f01e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -523,6 +523,7 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 
 void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object *obj);
 void i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj);
+void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj);
 void i915_gem_object_make_purgeable(struct drm_i915_gem_object *obj);
 
 static inline bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 0440696f786a..4b6b2bb6f180 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -487,13 +487,12 @@ void i915_gem_object_make_unshrinkable(struct 
drm_i915_gem_object *obj)
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 }
 
-static void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj,
- struct list_head *head)
+static void ___i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj,
+  struct list_head *head)
 {
struct drm_i915_private *i915 = obj_to_i915(obj);
unsigned long flags;
 
-   GEM_BUG_ON(!i915_gem_object_has_pages(obj));
if (!i915_gem_object_is_shrinkable(obj))
return;
 
@@ -512,6 +511,21 @@ static void __i915_gem_object_make_shrinkable(struct 
drm_i915_gem_object *obj,
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 }
 
+/**
+ * __i915_gem_object_make_shrinkable - Move the object to the tail of the
+ * shrinkable list. Objects on this list might be swapped out. Used with
+ * WILLNEED objects.
+ * @obj: The GEM object.
+ *
+ * DO NOT USE. This is intended to be called on very special objects that don't
+ * yet have mm.pages, but are guaranteed to have potentially reclaimable pages
+ * underneath.
+ */
+void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj)
+{
+   ___i915_gem_object_make_shrinkable(obj,
+  &obj_to_i915(obj)->mm.shrink_list);
+}
 
 /**
  * i915_gem_object_make_shrinkable - Move the object to the tail of the
@@ -523,8 +537,8 @@ static void __i915_gem_object_make_shrinkable(struct 
drm_i915_gem_object *obj,
  */
 void i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj)
 {
-   __i915_gem_object_make_shrinkable(obj,
- &obj_to_i915(obj)->mm.shrink_list);
+   GEM_BUG_ON(!i915_gem_object_has_pages(obj));
+   __i915_gem_object_make_shrinkable(obj);
 }
 
 /**
@@ -538,6 +552,7 @@ void i915_gem_object_make_shrinkable(struct 
drm_i915_gem_object *obj)
  */
 void i915_gem_object_make_purgeable(struct drm_i915_gem_object *obj)
 {
-   __i915_gem_object_make_shrinkable(obj,
- &obj_to_i915(obj)->mm.purge_list);
+   GEM_BUG_ON(!i915_gem_object_has_pages(obj));
+   ___i915_gem_object_make_shrinkable(obj,
+  &obj_to_i915(obj)->mm.purge_list);