Drop the atomic shrink_pin stuff, and just have make_{un}shrinkable
update the shrinker visible lists immediately. This at least simplifies
the next patch, and does make the behaviour more obvious. The potential
downside is that make_unshrinkable now grabs a global lock even when the
object itself is no longer shrinkable(transitioning from purgeable <->
shrinkable doesn't seem to be a thing), for example in the ppGTT
insertion paths we should now be careful not to needlessly call
make_unshrinkable multiple times. Outside of that there is some fallout
in intel_context which relies on nesting calls to shrink_pin.

Signed-off-by: Matthew Auld <matthew.a...@intel.com>
Cc: Thomas Hellström <thomas.hellst...@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  9 ----
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 +-
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 16 +-----
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  | 52 +++++++++++++------
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  1 -
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c          |  1 -
 drivers/gpu/drm/i915/gt/intel_context.c       |  9 +---
 7 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 6fb9afb65034..e8265a432fcb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -305,15 +305,6 @@ static void i915_gem_free_object(struct drm_gem_object 
*gem_obj)
         */
        atomic_inc(&i915->mm.free_count);
 
-       /*
-        * This serializes freeing with the shrinker. Since the free
-        * is delayed, first by RCU then by the workqueue, we want the
-        * shrinker to be able to free pages of unreferenced objects,
-        * or else we may oom whilst there are plenty of deferred
-        * freed objects.
-        */
-       i915_gem_object_make_unshrinkable(obj);
-
        /*
         * Since we require blocking on struct_mutex to unbind the freed
         * object from the GPU before releasing resources back to the
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 f0fb17be2f7a..e4f8a6774da8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -461,7 +461,6 @@ struct drm_i915_gem_object {
                 * instead go through the pin/unpin interfaces.
                 */
                atomic_t pages_pin_count;
-               atomic_t shrink_pin;
 
                /**
                 * Priority list of potential placements for this object.
@@ -522,7 +521,7 @@ struct drm_i915_gem_object {
                struct i915_gem_object_page_iter get_dma_page;
 
                /**
-                * Element within i915->mm.unbound_list or i915->mm.bound_list,
+                * Element within i915->mm.shrink_list or i915->mm.purge_list,
                 * locked by i915->mm.obj_lock.
                 */
                struct list_head link;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 8eb1c3a6fc9c..f0df1394d7f6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -64,28 +64,16 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object 
*obj,
                GEM_BUG_ON(i915_gem_object_has_tiling_quirk(obj));
                i915_gem_object_set_tiling_quirk(obj);
                GEM_BUG_ON(!list_empty(&obj->mm.link));
-               atomic_inc(&obj->mm.shrink_pin);
                shrinkable = false;
        }
 
        if (shrinkable) {
-               struct list_head *list;
-               unsigned long flags;
-
                assert_object_held(obj);
-               spin_lock_irqsave(&i915->mm.obj_lock, flags);
-
-               i915->mm.shrink_count++;
-               i915->mm.shrink_memory += obj->base.size;
 
                if (obj->mm.madv != I915_MADV_WILLNEED)
-                       list = &i915->mm.purge_list;
+                       i915_gem_object_make_purgeable(obj);
                else
-                       list = &i915->mm.shrink_list;
-               list_add_tail(&obj->mm.link, list);
-
-               atomic_set(&obj->mm.shrink_pin, 0);
-               spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
+                       i915_gem_object_make_shrinkable(obj);
        }
 }
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index cc80bd23d323..0440696f786a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -460,23 +460,26 @@ void i915_gem_shrinker_taints_mutex(struct 
drm_i915_private *i915,
 
 #define obj_to_i915(obj__) to_i915((obj__)->base.dev)
 
+/**
+ * i915_gem_object_make_unshrinkable - Hide the object from the shrinker. By
+ * default all object types that support shrinking(see IS_SHRINKABLE), will 
also
+ * make the object visible to the shrinker after allocating the system memory
+ * pages.
+ * @obj: The GEM object.
+ *
+ * This is typically used for special kernel internal objects that can't be
+ * easily processed by the shrinker, like if they are perma-pinned.
+ */
 void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object *obj)
 {
        struct drm_i915_private *i915 = obj_to_i915(obj);
        unsigned long flags;
 
-       /*
-        * We can only be called while the pages are pinned or when
-        * the pages are released. If pinned, we should only be called
-        * from a single caller under controlled conditions; and on release
-        * only one caller may release us. Neither the two may cross.
-        */
-       if (atomic_add_unless(&obj->mm.shrink_pin, 1, 0))
+       if (!i915_gem_object_is_shrinkable(obj))
                return;
 
        spin_lock_irqsave(&i915->mm.obj_lock, flags);
-       if (!atomic_fetch_inc(&obj->mm.shrink_pin) &&
-           !list_empty(&obj->mm.link)) {
+       if (!list_empty(&obj->mm.link)) {
                list_del_init(&obj->mm.link);
                i915->mm.shrink_count--;
                i915->mm.shrink_memory -= obj->base.size;
@@ -494,28 +497,45 @@ static void __i915_gem_object_make_shrinkable(struct 
drm_i915_gem_object *obj,
        if (!i915_gem_object_is_shrinkable(obj))
                return;
 
-       if (atomic_add_unless(&obj->mm.shrink_pin, -1, 1))
-               return;
-
        spin_lock_irqsave(&i915->mm.obj_lock, flags);
+
        GEM_BUG_ON(!kref_read(&obj->base.refcount));
-       if (atomic_dec_and_test(&obj->mm.shrink_pin)) {
-               GEM_BUG_ON(!list_empty(&obj->mm.link));
 
-               list_add_tail(&obj->mm.link, head);
+       if (list_empty(&obj->mm.link)) {
                i915->mm.shrink_count++;
                i915->mm.shrink_memory += obj->base.size;
-
+               list_add_tail(&obj->mm.link, head);
+       } else {
+               list_move_tail(&obj->mm.link, head);
        }
+
        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.
+ *
+ * Should only be called on objects which have backing pages.
+ */
 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_purgeable - Move the object to the tail of the 
purgeable
+ * list. Used with DONTNEED objects. Unlike with shrinkable objects, the
+ * shrinker will attempt to discard the backing pages, instead of trying to 
swap
+ * them out.
+ * @obj: The GEM object.
+ *
+ * Should only be called on objects which have backing pages.
+ */
 void i915_gem_object_make_purgeable(struct drm_i915_gem_object *obj)
 {
        __i915_gem_object_make_shrinkable(obj,
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 890191f286e3..baea9770200a 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -185,7 +185,6 @@ static void gen6_alloc_va_range(struct i915_address_space 
*vm,
 
                        pt = stash->pt[0];
                        __i915_gem_object_pin_pages(pt->base);
-                       i915_gem_object_make_unshrinkable(pt->base);
 
                        fill32_px(pt, vm->scratch[0]->encode);
 
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 037a9a6e4889..8af2f709571c 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -301,7 +301,6 @@ static void __gen8_ppgtt_alloc(struct i915_address_space * 
const vm,
 
                        pt = stash->pt[!!lvl];
                        __i915_gem_object_pin_pages(pt->base);
-                       i915_gem_object_make_unshrinkable(pt->base);
 
                        fill_px(pt, vm->scratch[lvl]->encode);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index ff637147b1a9..1b7dc57e6ec1 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -111,11 +111,6 @@ static int __context_pin_state(struct i915_vma *vma, 
struct i915_gem_ww_ctx *ww)
        if (err)
                goto err_unpin;
 
-       /*
-        * And mark it as a globally pinned object to let the shrinker know
-        * it cannot reclaim the object until we release it.
-        */
-       i915_vma_make_unshrinkable(vma);
        vma->obj->mm.dirty = true;
 
        return 0;
@@ -127,7 +122,6 @@ static int __context_pin_state(struct i915_vma *vma, struct 
i915_gem_ww_ctx *ww)
 
 static void __context_unpin_state(struct i915_vma *vma)
 {
-       i915_vma_make_shrinkable(vma);
        i915_active_release(&vma->active);
        __i915_vma_unpin(vma);
 }
@@ -180,7 +174,6 @@ static int intel_context_pre_pin(struct intel_context *ce,
        if (err)
                goto err_timeline;
 
-
        return 0;
 
 err_timeline:
@@ -338,6 +331,8 @@ static void __intel_context_retire(struct i915_active 
*active)
 
        set_bit(CONTEXT_VALID_BIT, &ce->flags);
        intel_context_post_unpin(ce);
+       if (ce->state)
+               i915_vma_make_shrinkable(ce->state);
        intel_context_put(ce);
 }
 
-- 
2.26.3

Reply via email to