[PATCH 121/124] drm/i915: Make GPU pages movable

Chris Wilson chris at chris-wilson.co.uk
Mon Sep 10 21:43:41 UTC 2018


On a long run of more than 2-3 days, physical memory tends to get
fragmented severely, which considerably slows down the system. In such a
scenario, the shrinker is also unable to help as lack of memory is not
the actual problem, since it has been observed that there are enough free
pages of 0 order. This also manifests itself when an indiviual zone in
the mm runs out of pages and if we cannot migrate pages between zones,
the kernel hits an out-of-memory even though there are free pages (and
often all of swap) available.

To address the issue of external fragementation, kernel does a compaction
(which involves migration of pages) but it's efficacy depends upon how
many pages are marked as MOVABLE, as only those pages can be migrated.

Currently the backing pages for GPU buffers are allocated from shmemfs
with GFP_RECLAIMABLE flag, in units of 4KB pages.  In the case of limited
swap space, it may not be possible always to reclaim or swap-out pages of
all the inactive objects, to make way for free space allowing formation
of higher order groups of physically-contiguous pages on compaction.

Just marking the GPU pages as MOVABLE will not suffice, as i915.ko has to
pin the pages if they are in use by GPU, which will prevent their
migration. So the migratepage callback in shmem is also hooked up to get
a notification when kernel initiates the page migration. On the
notification, i915.ko queues a task to unpin the pages (thereby
avoiding any lock recursion) allowing a future migrate to succeed,
and hence mitigate the fragmentation problem.

Originally written by Akash Goel based on my suggestion, but now
gutted to work with fresh locking conflicts.

Testcase: igt/gem_shrink
Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Hugh Dickins <hughd at google.com>
Cc: linux-mm at kvack.org
---
 drivers/gpu/drm/i915/i915_drv.h          |  8 +++
 drivers/gpu/drm/i915/i915_gem.c          | 10 ++-
 drivers/gpu/drm/i915/i915_gem_object.h   | 19 +++++
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 91 ++++++++++++++++++++++++
 4 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8ef626814769..3c781e4fc5db 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -857,6 +857,8 @@ struct intel_l3_parity {
 };
 
 struct i915_gem_mm {
+	struct shmem_dev_info shmem_info;
+
 	/** Memory allocator for GTT stolen memory */
 	struct drm_mm stolen;
 	/** Protects the usage of the GTT stolen memory allocator. This is
@@ -897,6 +899,12 @@ struct i915_gem_mm {
 	 */
 	atomic_t free_count;
 
+	/**
+	 * List of objects which are pending migration.
+	 */
+	struct llist_head migrate_list;
+	struct work_struct migrate_work;
+
 	/**
 	 * Small stash of WC pages
 	 */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ede78ebf3a0c..2888724e43b5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2273,6 +2273,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
 		if (obj->mm.madv == I915_MADV_WILLNEED)
 			mark_page_accessed(page);
 
+		set_page_private(page, 0);
 		put_page(page);
 	}
 	obj->mm.dirty = false;
@@ -2504,6 +2505,8 @@ static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 		}
 		last_pfn = page_to_pfn(page);
 
+		set_page_private(page, (unsigned long)obj);
+
 		/* Check that the i965g/gm workaround works. */
 		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
 	}
@@ -2547,8 +2550,10 @@ static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 err_sg:
 	sg_mark_end(sg);
 err_pages:
-	for_each_sgt_page(page, sgt_iter, st)
+	for_each_sgt_page(page, sgt_iter, st) {
+		set_page_private(page, 0);
 		put_page(page);
+	}
 	sg_free_table(st);
 	kfree(st);
 
@@ -4260,7 +4265,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size)
 	if (ret)
 		goto fail;
 
-	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
+	mask = GFP_HIGHUSER_MOVABLE;
 	if (IS_I965GM(dev_priv) || IS_I965G(dev_priv)) {
 		/* 965gm cannot relocate objects above 4GiB. */
 		mask &= ~__GFP_HIGHMEM;
@@ -4270,6 +4275,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size)
 	mapping = obj->base.filp->f_mapping;
 	mapping_set_gfp_mask(mapping, mask);
 	GEM_BUG_ON(!(mapping_gfp_mask(mapping) & __GFP_RECLAIM));
+	shmem_set_device_ops(mapping, &dev_priv->mm.shmem_info);
 
 	i915_gem_object_init(obj, &i915_gem_object_ops);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 919dc5e2cc66..137a208891f3 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -139,6 +139,7 @@ struct drm_i915_gem_object {
 	 * activity?
 	 */
 #define I915_BO_ACTIVE_REF 0
+#define I915_BO_MIGRATING 1
 
 	/*
 	 * Is the object to be mapped as read-only to the GPU
@@ -401,6 +402,24 @@ i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
 	return obj->active_count;
 }
 
+static inline bool
+i915_gem_object_is_migrating(struct drm_i915_gem_object *obj)
+{
+	return test_bit(I915_BO_MIGRATING, &obj->flags);
+}
+
+static inline bool
+i915_gem_object_set_migrating(struct drm_i915_gem_object *obj)
+{
+	return test_and_set_bit(I915_BO_MIGRATING, &obj->flags);
+}
+
+static inline void
+i915_gem_object_clear_migrating(struct drm_i915_gem_object *obj)
+{
+	clear_bit(I915_BO_MIGRATING, &obj->flags);
+}
+
 static inline bool
 i915_gem_object_has_active_reference(const struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 0176411775bc..bf1e5137e364 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -25,6 +25,7 @@
 #include <linux/oom.h>
 #include <linux/sched/mm.h>
 #include <linux/shmem_fs.h>
+#include <linux/migrate.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
 #include <linux/pci.h>
@@ -493,6 +494,88 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
 	return NOTIFY_DONE;
 }
 
+#if IS_ENABLED(CONFIG_MIGRATION)
+static bool can_isolate_page(struct drm_i915_gem_object *obj)
+{
+	if (i915_gem_object_is_migrating(obj))
+		return false;
+
+	/* Avoid the migration of page if being actively used by GPU */
+	if (i915_gem_object_is_active(obj) ||
+	    i915_gem_object_is_framebuffer(obj))
+		return false;
+
+	/* Skip the migration for a pinned object */
+	if (atomic_read(&obj->mm.pages_pin_count) > obj->bind_count)
+		return false;
+
+	return !READ_ONCE(obj->pin_global);
+}
+
+static void __i915_gem_migrate_worker(struct work_struct *wrk)
+{
+	struct drm_i915_private *i915 =
+		container_of(wrk, typeof(*i915), mm.migrate_work);
+	struct llist_node *migrate_list = llist_del_all(&i915->mm.migrate_list);
+	struct drm_i915_gem_object *obj, *on;
+	intel_wakeref_t wakeref;
+
+	wakeref = intel_runtime_pm_get(i915);
+	llist_for_each_entry_safe(obj, on, migrate_list, freed) {
+		i915_gem_object_clear_migrating(obj);
+
+		mutex_lock(&i915->drm.struct_mutex);
+		if (can_isolate_page(obj))
+			unsafe_drop_pages(obj);
+		mutex_unlock(&i915->drm.struct_mutex);
+
+		i915_gem_object_put(obj);
+	}
+	intel_runtime_pm_put(i915, wakeref);
+}
+
+static int i915_gem_shrinker_migratepage(struct address_space *mapping,
+					 struct page *newpage,
+					 struct page *page,
+					 enum migrate_mode mode,
+					 void *dev_priv_data)
+{
+	struct drm_i915_private *i915 = dev_priv_data;
+	struct drm_i915_gem_object *obj;
+
+	/*
+	 * Clear the private field of the new target page as it could have a
+	 * stale value in the private field. Otherwise later on if this page
+	 * itself gets migrated, without getting referred by the Driver
+	 * in between, the stale value would cause the i915_migratepage
+	 * function to go for a toss as object pointer is derived from it.
+	 * This should be safe since at the time of migration, private field
+	 * of the new page (which is actually an independent free 4KB page now)
+	 * should be like a don't care for the kernel.
+	 */
+	set_page_private(newpage, 0);
+
+	/*
+	 * Check the page count, if Driver also has a reference then it should
+	 * be more than 2, as shmem will have one reference and one reference
+	 * would have been taken by the migration path itself. So if reference
+	 * is <=2, we can directly invoke the migration function.
+	 */
+	if (!page_private(page) || PageSwapCache(page))
+		return migrate_page(mapping, newpage, page, mode);
+
+	obj = (struct drm_i915_gem_object *)page_private(page);
+	if (can_isolate_page(obj) &&
+	    !i915_gem_object_set_migrating(obj) &&
+	    kref_get_unless_zero(&obj->base.refcount)) {
+		if (llist_add(&obj->freed, &i915->mm.migrate_list))
+			schedule_work(&i915->mm.migrate_work);
+	}
+
+	return -EBUSY;
+}
+#endif
+
 /**
  * i915_gem_shrinker_register - Register the i915 shrinker
  * @i915: i915 device
@@ -512,6 +595,14 @@ void i915_gem_shrinker_register(struct drm_i915_private *i915)
 
 	i915->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
 	WARN_ON(register_vmap_purge_notifier(&i915->mm.vmap_notifier));
+
+#if IS_ENABLED(CONFIG_MIGRATION)
+	init_llist_head(&i915->mm.migrate_list);
+	INIT_WORK(&i915->mm.migrate_work, __i915_gem_migrate_worker);
+
+	i915->mm.shmem_info.dev_private_data = i915;
+	i915->mm.shmem_info.dev_migratepage = i915_gem_shrinker_migratepage;
+#endif
 }
 
 /**
-- 
2.19.0.rc2



More information about the Intel-gfx-trybot mailing list