All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list
@ 2019-05-28 19:50 Chris Wilson
  2019-05-28 19:50 ` [PATCH 2/2] drm/i915: Report all objects with allocated pages to the shrinker Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Chris Wilson @ 2019-05-28 19:50 UTC (permalink / raw)
  To: intel-gfx

Currently the purgeable objects, I915_MADV_DONTNEED, as mixed in the
normal bound/unbound lists. Every shrinker pass starts with an attempt
to purge from this set of unneeded objects, which entails us doing a
walk over both lists looking for any candidates. If there are none, and
since we are shrinking we can reasonably assume that the lists are
full!, this becomes a very slow futile walk.

If we separate out the purgeable objects into own list, this search then
becomes its own phase that is preferentially handled during shrinking.
Instead the cost becomes that we then need to filter the purgeable list
if we want to distinguish between bound and unbound objects.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c   | 14 ++++++++-----
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c    |  4 +---
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 22 +++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h              | 16 ++++++++------
 drivers/gpu/drm/i915/i915_gem.c              | 20 ++++++++++++++++--
 drivers/gpu/drm/i915/i915_vma.c              |  3 ++-
 6 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index cce96e6c6e52..52b73e90c9f4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -462,7 +462,6 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
-	struct list_head *list;
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
@@ -476,10 +475,15 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 	}
 	mutex_unlock(&i915->ggtt.vm.mutex);
 
-	spin_lock(&i915->mm.obj_lock);
-	list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
-	list_move_tail(&obj->mm.link, list);
-	spin_unlock(&i915->mm.obj_lock);
+	if (obj->mm.madv == I915_MADV_WILLNEED) {
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		list = obj->bind_count ?
+			&i915->mm.bound_list : &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
 }
 
 void
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 665f22ebf8e8..19d9ecdb2894 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -80,9 +80,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
 	sg_page_sizes = 0;
 	for (i = 0; i < page_count; i++) {
 		const unsigned int shrink[] = {
-			(I915_SHRINK_BOUND |
-			 I915_SHRINK_UNBOUND |
-			 I915_SHRINK_PURGEABLE),
+			I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
 			0,
 		}, *s = shrink;
 		gfp_t gfp = noreclaim;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index cd42299f019a..6a93e326abf3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -144,6 +144,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 		struct list_head *list;
 		unsigned int bit;
 	} phases[] = {
+		{ &i915->mm.purge_list, ~0u },
 		{ &i915->mm.unbound_list, I915_SHRINK_UNBOUND },
 		{ &i915->mm.bound_list, I915_SHRINK_BOUND },
 		{ NULL, 0 },
@@ -226,10 +227,6 @@ i915_gem_shrink(struct drm_i915_private *i915,
 						       mm.link))) {
 			list_move_tail(&obj->mm.link, &still_in_list);
 
-			if (flags & I915_SHRINK_PURGEABLE &&
-			    obj->mm.madv != I915_MADV_DONTNEED)
-				continue;
-
 			if (flags & I915_SHRINK_VMAPS &&
 			    !is_vmalloc_addr(obj->mm.mapping))
 				continue;
@@ -239,6 +236,10 @@ i915_gem_shrink(struct drm_i915_private *i915,
 			     i915_gem_object_is_framebuffer(obj)))
 				continue;
 
+			if (!(flags & I915_SHRINK_BOUND) &&
+			    READ_ONCE(obj->bind_count))
+				continue;
+
 			if (!can_release_pages(obj))
 				continue;
 
@@ -324,6 +325,11 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 			count += obj->base.size >> PAGE_SHIFT;
 			num_objects++;
 		}
+	list_for_each_entry(obj, &i915->mm.purge_list, mm.link)
+		if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
+			count += obj->base.size >> PAGE_SHIFT;
+			num_objects++;
+		}
 	spin_unlock(&i915->mm.obj_lock);
 
 	/* Update our preferred vmscan batch size for the next pass.
@@ -361,15 +367,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 				&sc->nr_scanned,
 				I915_SHRINK_BOUND |
 				I915_SHRINK_UNBOUND |
-				I915_SHRINK_PURGEABLE |
 				I915_SHRINK_WRITEBACK);
-	if (sc->nr_scanned < sc->nr_to_scan)
-		freed += i915_gem_shrink(i915,
-					 sc->nr_to_scan - sc->nr_scanned,
-					 &sc->nr_scanned,
-					 I915_SHRINK_BOUND |
-					 I915_SHRINK_UNBOUND |
-					 I915_SHRINK_WRITEBACK);
 	if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) {
 		intel_wakeref_t wakeref;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4eb8f6a181c1..fb2e89133e78 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -864,6 +864,10 @@ struct i915_gem_mm {
 	 * not actually have any pages attached.
 	 */
 	struct list_head unbound_list;
+	/**
+	 * List of objects which are purgeable. May be active.
+	 */
+	struct list_head purge_list;
 
 	/** List of all objects in gtt_space, currently mmaped by userspace.
 	 * All objects within this list must also be on bound_list.
@@ -2866,12 +2870,12 @@ unsigned long i915_gem_shrink(struct drm_i915_private *i915,
 			      unsigned long target,
 			      unsigned long *nr_scanned,
 			      unsigned flags);
-#define I915_SHRINK_PURGEABLE	BIT(0)
-#define I915_SHRINK_UNBOUND	BIT(1)
-#define I915_SHRINK_BOUND	BIT(2)
-#define I915_SHRINK_ACTIVE	BIT(3)
-#define I915_SHRINK_VMAPS	BIT(4)
-#define I915_SHRINK_WRITEBACK	BIT(5)
+#define I915_SHRINK_UNBOUND	BIT(0)
+#define I915_SHRINK_BOUND	BIT(1)
+#define I915_SHRINK_ACTIVE	BIT(2)
+#define I915_SHRINK_VMAPS	BIT(3)
+#define I915_SHRINK_WRITEBACK	BIT(4)
+
 unsigned long i915_gem_shrink_all(struct drm_i915_private *i915);
 void i915_gem_shrinker_register(struct drm_i915_private *i915);
 void i915_gem_shrinker_unregister(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e5aafbeb1d19..d98ccbbde53c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1095,7 +1095,7 @@ int
 i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *i915 = to_i915(dev);
 	struct drm_i915_gem_madvise *args = data;
 	struct drm_i915_gem_object *obj;
 	int err;
@@ -1118,7 +1118,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 	if (i915_gem_object_has_pages(obj) &&
 	    i915_gem_object_is_tiled(obj) &&
-	    dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+	    i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
 		if (obj->mm.madv == I915_MADV_WILLNEED) {
 			GEM_BUG_ON(!obj->mm.quirked);
 			__i915_gem_object_unpin_pages(obj);
@@ -1134,6 +1134,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	if (obj->mm.madv != __I915_MADV_PURGED)
 		obj->mm.madv = args->madv;
 
+	if (i915_gem_object_has_pages(obj)) {
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		if (obj->mm.madv != I915_MADV_WILLNEED)
+			list = &i915->mm.purge_list;
+		else if (obj->bind_count)
+			list = &i915->mm.bound_list;
+		else
+			list = &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
+
 	/* if the object is no longer attached, discard its backing storage */
 	if (obj->mm.madv == I915_MADV_DONTNEED &&
 	    !i915_gem_object_has_pages(obj))
@@ -1742,6 +1756,7 @@ static void i915_gem_init__mm(struct drm_i915_private *i915)
 
 	init_llist_head(&i915->mm.free_list);
 
+	INIT_LIST_HEAD(&i915->mm.purge_list);
 	INIT_LIST_HEAD(&i915->mm.unbound_list);
 	INIT_LIST_HEAD(&i915->mm.bound_list);
 	INIT_LIST_HEAD(&i915->mm.fence_list);
@@ -1836,6 +1851,7 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)
 			i915_gem_object_unlock(obj);
 		}
 	}
+	GEM_BUG_ON(!list_empty(&i915->mm.purge_list));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 59a2f6af6103..f640caec4bae 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -717,7 +717,8 @@ i915_vma_remove(struct i915_vma *vma)
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		spin_lock(&i915->mm.obj_lock);
-		if (--obj->bind_count == 0)
+		if (--obj->bind_count == 0 &&
+		    obj->mm.madv == I915_MADV_WILLNEED)
 			list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
 		spin_unlock(&i915->mm.obj_lock);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list
@ 2019-05-30 20:34 Chris Wilson
  2019-05-30 20:49 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/2] " Patchwork
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2019-05-30 20:34 UTC (permalink / raw)
  To: intel-gfx

Currently the purgeable objects, I915_MADV_DONTNEED, are mixed in the
normal bound/unbound lists. Every shrinker pass starts with an attempt
to purge from this set of unneeded objects, which entails us doing a
walk over both lists looking for any candidates. If there are none, and
since we are shrinking we can reasonably assume that the lists are
full!, this becomes a very slow futile walk.

If we separate out the purgeable objects into own list, this search then
becomes its own phase that is preferentially handled during shrinking.
Instead the cost becomes that we then need to filter the purgeable list
if we want to distinguish between bound and unbound objects.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c   | 14 ++++++++-----
 drivers/gpu/drm/i915/gem/i915_gem_object.c   | 11 +++++++++-
 drivers/gpu/drm/i915/gem/i915_gem_pm.c       |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c    |  4 +---
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 22 +++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h              | 16 ++++++++------
 drivers/gpu/drm/i915/i915_gem.c              | 20 ++++++++++++++++--
 drivers/gpu/drm/i915/i915_vma.c              |  3 ++-
 8 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index cce96e6c6e52..52b73e90c9f4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -462,7 +462,6 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
-	struct list_head *list;
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
@@ -476,10 +475,15 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 	}
 	mutex_unlock(&i915->ggtt.vm.mutex);
 
-	spin_lock(&i915->mm.obj_lock);
-	list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
-	list_move_tail(&obj->mm.link, list);
-	spin_unlock(&i915->mm.obj_lock);
+	if (obj->mm.madv == I915_MADV_WILLNEED) {
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		list = obj->bind_count ?
+			&i915->mm.bound_list : &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
 }
 
 void
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index f064876f1214..a42763e4dd5f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -334,9 +334,18 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
 	if (obj->mm.quirked)
 		__i915_gem_object_unpin_pages(obj);
 
-	if (discard_backing_storage(obj))
+	if (discard_backing_storage(obj)) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
 		obj->mm.madv = I915_MADV_DONTNEED;
 
+		if (i915_gem_object_has_pages(obj)) {
+			spin_lock(&i915->mm.obj_lock);
+			list_move_tail(&obj->mm.link, &i915->mm.purge_list);
+			spin_unlock(&i915->mm.obj_lock);
+		}
+	}
+
 	/*
 	 * Before we free the object, make sure any pure RCU-only
 	 * read-side critical sections are complete, e.g.
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 11890e96ed65..89bb6d822f6e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -164,6 +164,7 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
 	struct list_head *phases[] = {
 		&i915->mm.unbound_list,
 		&i915->mm.bound_list,
+		&i915->mm.purge_list,
 		NULL
 	}, **phase;
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 665f22ebf8e8..19d9ecdb2894 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -80,9 +80,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
 	sg_page_sizes = 0;
 	for (i = 0; i < page_count; i++) {
 		const unsigned int shrink[] = {
-			(I915_SHRINK_BOUND |
-			 I915_SHRINK_UNBOUND |
-			 I915_SHRINK_PURGEABLE),
+			I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
 			0,
 		}, *s = shrink;
 		gfp_t gfp = noreclaim;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index cd42299f019a..6a93e326abf3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -144,6 +144,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 		struct list_head *list;
 		unsigned int bit;
 	} phases[] = {
+		{ &i915->mm.purge_list, ~0u },
 		{ &i915->mm.unbound_list, I915_SHRINK_UNBOUND },
 		{ &i915->mm.bound_list, I915_SHRINK_BOUND },
 		{ NULL, 0 },
@@ -226,10 +227,6 @@ i915_gem_shrink(struct drm_i915_private *i915,
 						       mm.link))) {
 			list_move_tail(&obj->mm.link, &still_in_list);
 
-			if (flags & I915_SHRINK_PURGEABLE &&
-			    obj->mm.madv != I915_MADV_DONTNEED)
-				continue;
-
 			if (flags & I915_SHRINK_VMAPS &&
 			    !is_vmalloc_addr(obj->mm.mapping))
 				continue;
@@ -239,6 +236,10 @@ i915_gem_shrink(struct drm_i915_private *i915,
 			     i915_gem_object_is_framebuffer(obj)))
 				continue;
 
+			if (!(flags & I915_SHRINK_BOUND) &&
+			    READ_ONCE(obj->bind_count))
+				continue;
+
 			if (!can_release_pages(obj))
 				continue;
 
@@ -324,6 +325,11 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 			count += obj->base.size >> PAGE_SHIFT;
 			num_objects++;
 		}
+	list_for_each_entry(obj, &i915->mm.purge_list, mm.link)
+		if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
+			count += obj->base.size >> PAGE_SHIFT;
+			num_objects++;
+		}
 	spin_unlock(&i915->mm.obj_lock);
 
 	/* Update our preferred vmscan batch size for the next pass.
@@ -361,15 +367,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 				&sc->nr_scanned,
 				I915_SHRINK_BOUND |
 				I915_SHRINK_UNBOUND |
-				I915_SHRINK_PURGEABLE |
 				I915_SHRINK_WRITEBACK);
-	if (sc->nr_scanned < sc->nr_to_scan)
-		freed += i915_gem_shrink(i915,
-					 sc->nr_to_scan - sc->nr_scanned,
-					 &sc->nr_scanned,
-					 I915_SHRINK_BOUND |
-					 I915_SHRINK_UNBOUND |
-					 I915_SHRINK_WRITEBACK);
 	if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) {
 		intel_wakeref_t wakeref;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4eb8f6a181c1..fb2e89133e78 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -864,6 +864,10 @@ struct i915_gem_mm {
 	 * not actually have any pages attached.
 	 */
 	struct list_head unbound_list;
+	/**
+	 * List of objects which are purgeable. May be active.
+	 */
+	struct list_head purge_list;
 
 	/** List of all objects in gtt_space, currently mmaped by userspace.
 	 * All objects within this list must also be on bound_list.
@@ -2866,12 +2870,12 @@ unsigned long i915_gem_shrink(struct drm_i915_private *i915,
 			      unsigned long target,
 			      unsigned long *nr_scanned,
 			      unsigned flags);
-#define I915_SHRINK_PURGEABLE	BIT(0)
-#define I915_SHRINK_UNBOUND	BIT(1)
-#define I915_SHRINK_BOUND	BIT(2)
-#define I915_SHRINK_ACTIVE	BIT(3)
-#define I915_SHRINK_VMAPS	BIT(4)
-#define I915_SHRINK_WRITEBACK	BIT(5)
+#define I915_SHRINK_UNBOUND	BIT(0)
+#define I915_SHRINK_BOUND	BIT(1)
+#define I915_SHRINK_ACTIVE	BIT(2)
+#define I915_SHRINK_VMAPS	BIT(3)
+#define I915_SHRINK_WRITEBACK	BIT(4)
+
 unsigned long i915_gem_shrink_all(struct drm_i915_private *i915);
 void i915_gem_shrinker_register(struct drm_i915_private *i915);
 void i915_gem_shrinker_unregister(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e5aafbeb1d19..d98ccbbde53c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1095,7 +1095,7 @@ int
 i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *i915 = to_i915(dev);
 	struct drm_i915_gem_madvise *args = data;
 	struct drm_i915_gem_object *obj;
 	int err;
@@ -1118,7 +1118,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 	if (i915_gem_object_has_pages(obj) &&
 	    i915_gem_object_is_tiled(obj) &&
-	    dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+	    i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
 		if (obj->mm.madv == I915_MADV_WILLNEED) {
 			GEM_BUG_ON(!obj->mm.quirked);
 			__i915_gem_object_unpin_pages(obj);
@@ -1134,6 +1134,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	if (obj->mm.madv != __I915_MADV_PURGED)
 		obj->mm.madv = args->madv;
 
+	if (i915_gem_object_has_pages(obj)) {
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		if (obj->mm.madv != I915_MADV_WILLNEED)
+			list = &i915->mm.purge_list;
+		else if (obj->bind_count)
+			list = &i915->mm.bound_list;
+		else
+			list = &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
+
 	/* if the object is no longer attached, discard its backing storage */
 	if (obj->mm.madv == I915_MADV_DONTNEED &&
 	    !i915_gem_object_has_pages(obj))
@@ -1742,6 +1756,7 @@ static void i915_gem_init__mm(struct drm_i915_private *i915)
 
 	init_llist_head(&i915->mm.free_list);
 
+	INIT_LIST_HEAD(&i915->mm.purge_list);
 	INIT_LIST_HEAD(&i915->mm.unbound_list);
 	INIT_LIST_HEAD(&i915->mm.bound_list);
 	INIT_LIST_HEAD(&i915->mm.fence_list);
@@ -1836,6 +1851,7 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)
 			i915_gem_object_unlock(obj);
 		}
 	}
+	GEM_BUG_ON(!list_empty(&i915->mm.purge_list));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 59a2f6af6103..f640caec4bae 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -717,7 +717,8 @@ i915_vma_remove(struct i915_vma *vma)
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		spin_lock(&i915->mm.obj_lock);
-		if (--obj->bind_count == 0)
+		if (--obj->bind_count == 0 &&
+		    obj->mm.madv == I915_MADV_WILLNEED)
 			list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
 		spin_unlock(&i915->mm.obj_lock);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list
@ 2018-05-13  8:21 Chris Wilson
  2018-05-13  8:32 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/2] " Patchwork
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2018-05-13  8:21 UTC (permalink / raw)
  To: intel-gfx

Currently the purgeable objects, I915_MADV_DONTNEED, as mixed in the
normal bound/unbound lists. Every shrinker pass starts with an attempt
to purge from this set of unneeded objects, which entails us doing a
walk over both lists looking for any candidates. If there are none, and
since we are shrinking we can reasonably assume that the lists are
full!, this becomes a very slow futile walk.

If we separate out the purgeable objects into own list, this search then
becomes its own phase that is preferentially handled during shrinking.
Instead the cost becomes that we then need to filter the purgeable list
if we want to distinguish between bound and unbound objects.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 13 ++++---
 drivers/gpu/drm/i915/i915_gem.c          | 49 ++++++++++++++++++------
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 28 +++++++-------
 drivers/gpu/drm/i915/i915_vma.c          |  2 +-
 4 files changed, 60 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 57fb3aa09db0..c7c0cb9636ce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -926,6 +926,10 @@ struct i915_gem_mm {
 	 * not actually have any pages attached.
 	 */
 	struct list_head unbound_list;
+	/**
+	 * List of objects which are purgeable. May be active.
+	 */
+	struct list_head purge_list;
 
 	/** List of all objects in gtt_space, currently mmaped by userspace.
 	 * All objects within this list must also be on bound_list.
@@ -3307,11 +3311,10 @@ unsigned long i915_gem_shrink(struct drm_i915_private *i915,
 			      unsigned long target,
 			      unsigned long *nr_scanned,
 			      unsigned flags);
-#define I915_SHRINK_PURGEABLE 0x1
-#define I915_SHRINK_UNBOUND 0x2
-#define I915_SHRINK_BOUND 0x4
-#define I915_SHRINK_ACTIVE 0x8
-#define I915_SHRINK_VMAPS 0x10
+#define I915_SHRINK_UNBOUND BIT(0)
+#define I915_SHRINK_BOUND BIT(1)
+#define I915_SHRINK_ACTIVE BIT(2)
+#define I915_SHRINK_VMAPS BIT(3)
 unsigned long i915_gem_shrink_all(struct drm_i915_private *i915);
 void i915_gem_shrinker_register(struct drm_i915_private *i915);
 void i915_gem_shrinker_unregister(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89bf5d67cb74..a66579c8520d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1669,8 +1669,6 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 
 static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 {
-	struct drm_i915_private *i915;
-	struct list_head *list;
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
@@ -1685,11 +1683,16 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
 	}
 
-	i915 = to_i915(obj->base.dev);
-	spin_lock(&i915->mm.obj_lock);
-	list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
-	list_move_tail(&obj->mm.link, list);
-	spin_unlock(&i915->mm.obj_lock);
+	if (obj->mm.madv == I915_MADV_WILLNEED) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		list = obj->bind_count ?
+			&i915->mm.bound_list : &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
 }
 
 /**
@@ -2528,7 +2531,7 @@ static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	sg_page_sizes = 0;
 	for (i = 0; i < page_count; i++) {
 		const unsigned int shrink[] = {
-			I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE,
+			I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
 			0,
 		}, *s = shrink;
 		gfp_t gfp = noreclaim;
@@ -4565,7 +4568,7 @@ int
 i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *i915 = to_i915(dev);
 	struct drm_i915_gem_madvise *args = data;
 	struct drm_i915_gem_object *obj;
 	int err;
@@ -4588,7 +4591,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 	if (i915_gem_object_has_pages(obj) &&
 	    i915_gem_object_is_tiled(obj) &&
-	    dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+	    i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
 		if (obj->mm.madv == I915_MADV_WILLNEED) {
 			GEM_BUG_ON(!obj->mm.quirked);
 			__i915_gem_object_unpin_pages(obj);
@@ -4604,6 +4607,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	if (obj->mm.madv != __I915_MADV_PURGED)
 		obj->mm.madv = args->madv;
 
+	if (i915_gem_object_has_pages(obj)) {
+		struct list_head *list;
+
+		spin_lock(&i915->mm.obj_lock);
+		if (obj->mm.madv != I915_MADV_WILLNEED)
+			list = &i915->mm.purge_list;
+		else if (obj->bind_count)
+			list = &i915->mm.bound_list;
+		else
+			list = &i915->mm.unbound_list;
+		list_move_tail(&obj->mm.link, list);
+		spin_unlock(&i915->mm.obj_lock);
+	}
+
 	/* if the object is no longer attached, discard its backing storage */
 	if (obj->mm.madv == I915_MADV_DONTNEED &&
 	    !i915_gem_object_has_pages(obj))
@@ -4922,9 +4939,18 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
 	if (obj->mm.quirked)
 		__i915_gem_object_unpin_pages(obj);
 
-	if (discard_backing_storage(obj))
+	if (discard_backing_storage(obj)) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
 		obj->mm.madv = I915_MADV_DONTNEED;
 
+		if (i915_gem_object_has_pages(obj)) {
+			spin_lock(&i915->mm.obj_lock);
+			list_move_tail(&obj->mm.link, &i915->mm.purge_list);
+			spin_unlock(&i915->mm.obj_lock);
+		}
+	}
+
 	/*
 	 * Before we free the object, make sure any pure RCU-only
 	 * read-side critical sections are complete, e.g.
@@ -5558,6 +5584,7 @@ static void i915_gem_init__mm(struct drm_i915_private *i915)
 
 	init_llist_head(&i915->mm.free_list);
 
+	INIT_LIST_HEAD(&i915->mm.purge_list);
 	INIT_LIST_HEAD(&i915->mm.unbound_list);
 	INIT_LIST_HEAD(&i915->mm.bound_list);
 	INIT_LIST_HEAD(&i915->mm.fence_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 5757fb7c4b5a..ea5a6e2d62d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -151,6 +151,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 		struct list_head *list;
 		unsigned int bit;
 	} phases[] = {
+		{ &i915->mm.purge_list, 0 },
 		{ &i915->mm.unbound_list, I915_SHRINK_UNBOUND },
 		{ &i915->mm.bound_list, I915_SHRINK_BOUND },
 		{ NULL, 0 },
@@ -182,8 +183,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 	 * device just to recover a little memory. If absolutely necessary,
 	 * we will force the wake during oom-notifier.
 	 */
-	if ((flags & I915_SHRINK_BOUND) &&
-	    !intel_runtime_pm_get_if_in_use(i915))
+	if (flags & I915_SHRINK_BOUND && !intel_runtime_pm_get_if_in_use(i915))
 		flags &= ~I915_SHRINK_BOUND;
 
 	/*
@@ -209,7 +209,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 		struct list_head still_in_list;
 		struct drm_i915_gem_object *obj;
 
-		if ((flags & phase->bit) == 0)
+		if (phase->bit && (flags & phase->bit) == 0)
 			continue;
 
 		INIT_LIST_HEAD(&still_in_list);
@@ -228,10 +228,6 @@ i915_gem_shrink(struct drm_i915_private *i915,
 						       mm.link))) {
 			list_move_tail(&obj->mm.link, &still_in_list);
 
-			if (flags & I915_SHRINK_PURGEABLE &&
-			    obj->mm.madv != I915_MADV_DONTNEED)
-				continue;
-
 			if (flags & I915_SHRINK_VMAPS &&
 			    !is_vmalloc_addr(obj->mm.mapping))
 				continue;
@@ -241,6 +237,10 @@ i915_gem_shrink(struct drm_i915_private *i915,
 			     i915_gem_object_is_framebuffer(obj)))
 				continue;
 
+			if (!(flags & I915_SHRINK_BOUND) &&
+			    READ_ONCE(obj->bind_count))
+				continue;
+
 			if (!can_release_pages(obj))
 				continue;
 
@@ -325,6 +325,11 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 			count += obj->base.size >> PAGE_SHIFT;
 			num_objects++;
 		}
+	list_for_each_entry(obj, &i915->mm.purge_list, mm.link)
+		if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
+			count += obj->base.size >> PAGE_SHIFT;
+			num_objects++;
+		}
 	spin_unlock(&i915->mm.obj_lock);
 
 	/* Update our preferred vmscan batch size for the next pass.
@@ -361,14 +366,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 				sc->nr_to_scan,
 				&sc->nr_scanned,
 				I915_SHRINK_BOUND |
-				I915_SHRINK_UNBOUND |
-				I915_SHRINK_PURGEABLE);
-	if (sc->nr_scanned < sc->nr_to_scan)
-		freed += i915_gem_shrink(i915,
-					 sc->nr_to_scan - sc->nr_scanned,
-					 &sc->nr_scanned,
-					 I915_SHRINK_BOUND |
-					 I915_SHRINK_UNBOUND);
+				I915_SHRINK_UNBOUND);
 	if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) {
 		intel_runtime_pm_get(i915);
 		freed += i915_gem_shrink(i915,
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 9324d476e0a7..96039c4ef434 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -624,7 +624,7 @@ i915_vma_remove(struct i915_vma *vma)
 	 * no more VMAs exist.
 	 */
 	spin_lock(&i915->mm.obj_lock);
-	if (--obj->bind_count == 0)
+	if (--obj->bind_count == 0 && obj->mm.madv == I915_MADV_WILLNEED)
 		list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
 	spin_unlock(&i915->mm.obj_lock);
 
-- 
2.17.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-05-30 20:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 19:50 [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list Chris Wilson
2019-05-28 19:50 ` [PATCH 2/2] drm/i915: Report all objects with allocated pages to the shrinker Chris Wilson
2019-05-30 16:10   ` Matthew Auld
2019-05-30 19:10     ` Chris Wilson
2019-05-28 20:24 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Track the purgeable objects on a separate eviction list Patchwork
2019-05-28 20:41 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-29  7:53 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-05-30 15:51 ` [PATCH 1/2] " Matthew Auld
  -- strict thread matches above, loose matches on Subject: below --
2019-05-30 20:34 Chris Wilson
2019-05-30 20:49 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/2] " Patchwork
2018-05-13  8:21 [PATCH 1/2] " Chris Wilson
2018-05-13  8:32 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/2] " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.