Skip to contentSkip to conversation

Migrate Pages in lieu of discard

RFC PATCH Superseded Dave Hansen 5 participants 22 messages Oct 7, 2020 – Oct 29, 2020

Subsystems (12)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover Migrate Pages in lieu of discard

Changes since (automigrate-20200818):
* Fall back to normal reclaim when demotion fails

The full series is also available here:

	https://github.com/hansendc/linux/tree/automigrate-20201007

I really just want folks to look at:

[RFC][PATCH 5/9] mm/migrate: demote pages during reclaim

I've reworked that so that it can both use the high-level migration API, and fall back to normal reclaim if migration fails. I think that gives us the best of both worlds.

I'm posting the series in case folks want to run the whole thing.

--

We're starting to see systems with more and more kinds of memory such as Intel's implementation of persistent memory.

Let's say you have a system with some DRAM and some persistent memory. Today, once DRAM fills up, reclaim will start and some of the DRAM contents will be thrown out. Allocations will, at some point, start falling over to the slower persistent memory.

That has two nasty properties. First, the newer allocations can end up in the slower persistent memory. Second, reclaimed data in DRAM are just discarded even if there are gobs of space in persistent memory that could be used.

This set implements a solution to these problems. At the end of the reclaim process in shrink_page_list() just before the last page refcount is dropped, the page is migrated to persistent memory instead of being dropped.

While I've talked about a DRAM/PMEM pairing, this approach would function in any environment where memory tiers exist.

This is not perfect. It "strands" pages in slower memory and never brings them back to fast DRAM. Other things need to be built to promote hot pages back to DRAM.

This is also all based on an upstream mechanism that allows persistent memory to be onlined and used as if it were volatile:

	http://lkml.kernel.org/r/[email protected]

== Open Issues ==

* For cpusets and memory policies that restrict allocations to PMEM, is it OK to demote to PMEM? Do we need a cgroup- level API to opt-in or opt-out of these migrations?

--

Changes since (https://lwn.net/Articles/824830/):
* Use higher-level migrate_pages() API approach from Yang Shi's earlier patches.
* made sure to actually check node_reclaim_mode's new bit
* disabled migration entirely before introducing RECLAIM_MIGRATE
* Replace GFP_NOWAIT with explicit __GFP_KSWAPD_RECLAIM and comment why we want that.
* Comment on effects of that keep multiple source nodes from sharing target nodes

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>
#1 lore raw

RFC PATCH v4 1/9 mm/numa: node demotion data structure and lookup

From: Dave Hansen <[email protected]>

Prepare for the kernel to auto-migrate pages to other memory nodes with a user defined node migration table. This allows creating single migration target for each NUMA node to enable the kernel to do NUMA page migrations instead of simply reclaiming colder pages. A node with no target is a "terminal node", so reclaim acts normally there. The migration target does not fundamentally _need_ to be a single node, but this implementation starts there to limit complexity.

If you consider the migration path as a graph, cycles (loops) in the graph are disallowed. This avoids wasting resources by constantly migrating (A->B, B->A, A->B ...). The expectation is that cycles will never be allowed.

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

--

changes in July 2020:
- Remove loop from next_demotion_node() and get_online_mems(). This means that the node returned by next_demotion_node() might now be offline, but the worst case is that the allocation fails. That's fine since it is transient.
---

 b/mm/migrate.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)
mm/migrate.c renamed from mm/migrate.c~0006-node-Define-and-export-memory-migration-path +16 -0
diff -puN mm/migrate.c~0006-node-Define-and-export-memory-migration-path mm/migrate.c
@@ -1161,6 +1161,22 @@ out:
 	return rc;
 }
 
+static int node_demotion[MAX_NUMNODES] = {[0 ...  MAX_NUMNODES - 1] = NUMA_NO_NODE};
+
+/**
+ * next_demotion_node() - Get the next node in the demotion path
+ * @node: The starting node to lookup the next node
+ *
+ * @returns: node id for next memory node in the demotion path hierarchy
+ * from @node; NUMA_NO_NODE if @node is terminal.  This does not keep
+ * @node online or guarantee that it *continues* to be the next demotion
+ * target.
+ */
+int next_demotion_node(int node)
+{
+	return node_demotion[node];
+}
+
 /*
  * Obtain the lock on page, remove all ptes and migrate the page
  * to the newly allocated page in newpage.

_

#2 lore raw

RFC PATCH v4 2/9 mm/numa: automatically generate node migration order

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

When memory fills up on a node, memory contents can be automatically migrated to another node. The biggest problems are knowing when to migrate and to where the migration should be targeted.

The most straightforward way to generate the "to where" list would be to follow the page allocator fallback lists. Those lists already tell us if memory is full where to look next. It would also be logical to move memory in that order.

But, the allocator fallback lists have a fatal flaw: most nodes appear in all the lists. This would potentially lead to migration cycles (A->B, B->A, A->B, ...).

Instead of using the allocator fallback lists directly, keep a separate node migration ordering. But, reuse the same data used to generate page allocator fallback in the first place:
find_next_best_node().

This means that the firmware data used to populate node distances essentially dictates the ordering for now. It should also be architecture-neutral since all NUMA architectures have a working find_next_best_node().

The protocol for node_demotion[] access and writing is not standard. It has no specific locking and is intended to be read locklessly. Readers must take care to avoid observing changes that appear incoherent. This was done so that node_demotion[] locking has no chance of becoming a bottleneck on large systems with lots of CPUs in direct reclaim.

This code is unused for now. It will be called later in the series.

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/mm/internal.h   |    1 
 b/mm/migrate.c    |  137 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 b/mm/page_alloc.c |    2 
 3 files changed, 138 insertions(+), 2 deletions(-)
mm/internal.h renamed from mm/internal.h~auto-setup-default-migration-path-from-firmware +1 -0
diff -puN mm/internal.h~auto-setup-default-migration-path-from-firmware mm/internal.h
@@ -203,6 +203,7 @@ extern int user_min_free_kbytes;
 
 extern void zone_pcp_update(struct zone *zone);
 extern void zone_pcp_reset(struct zone *zone);
+extern int find_next_best_node(int node, nodemask_t *used_node_mask);
 
 #if defined CONFIG_COMPACTION || defined CONFIG_CMA
 
mm/migrate.c renamed from mm/migrate.c~auto-setup-default-migration-path-from-firmware +136 -1

152 lines not on this page · raw message

mm/page_alloc.c renamed from mm/page_alloc.c~auto-setup-default-migration-path-from-firmware +1 -1
diff -puN mm/page_alloc.c~auto-setup-default-migration-path-from-firmware mm/page_alloc.c
@@ -5632,7 +5632,7 @@ static int node_load[MAX_NUMNODES];
  *
  * Return: node id of the found node or %NUMA_NO_NODE if no node is found.
  */
-static int find_next_best_node(int node, nodemask_t *used_node_mask)
+int find_next_best_node(int node, nodemask_t *used_node_mask)
 {
 	int n, val;
 	int min_val = INT_MAX;

_

#3 lore raw

RFC PATCH v4 3/9 mm/migrate: update migration order during on hotplug events

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

Reclaim-based migration is attempting to optimize data placement in memory based on the system topology. If the system changes, so must the migration ordering.

The implementation here is pretty simple and entirely unoptimized. On any memory or CPU hotplug events, assume that a node was added or removed and recalculate all migration targets. This ensures that the node_demotion[] array is always ready to be used in case the new reclaim mode is enabled.

This recalculation is far from optimal, most glaringly that it does not even attempt to figure out if nodes are actually coming or going. But, given the expected paucity of hotplug events, this should be fine.

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/mm/migrate.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
mm/migrate.c renamed from mm/migrate.c~enable-numa-demotion +93 -0
diff -puN mm/migrate.c~enable-numa-demotion mm/migrate.c
@@ -49,6 +49,7 @@
 #include <linux/sched/mm.h>
 #include <linux/ptrace.h>
 #include <linux/oom.h>
+#include <linux/memory.h>
 
 #include <asm/tlbflush.h>
 
@@ -3241,9 +3242,101 @@ again:
 		goto again;
 }
 
+/*
+ * For callers that do not hold get_online_mems() already.
+ */
 void set_migration_target_nodes(void)
 {
 	get_online_mems();
 	__set_migration_target_nodes();
 	put_online_mems();
 }
+
+/*
+ * React to hotplug events that might affect the migration targes
+ * like events that online or offline NUMA nodes.
+ *
+ * The ordering is also currently dependent on which nodes have
+ * CPUs.  That means we need CPU on/offline notification too.
+ */
+static int migration_online_cpu(unsigned int cpu)
+{
+	set_migration_target_nodes();
+	return 0;
+}
+
+static int migration_offline_cpu(unsigned int cpu)
+{
+	set_migration_target_nodes();
+	return 0;
+}
+
+/*
+ * This leaves migrate-on-reclaim transiently disabled
+ * between the MEM_GOING_OFFLINE and MEM_OFFLINE events.
+ * This runs reclaim-based micgration is enabled or not.
+ * This ensures that the user can turn reclaim-based
+ * migration at any time without needing to recalcuate
+ * migration targets.
+ *
+ * These callbacks already hold get_online_mems().  That
+ * is why __set_migration_target_nodes() can be used as
+ * opposed to set_migration_target_nodes().
+ */
+#if defined(CONFIG_MEMORY_HOTPLUG)
+static int __meminit migrate_on_reclaim_callback(struct notifier_block *self,
+						 unsigned long action, void *arg)
+{
+	switch (action) {
+	case MEM_GOING_OFFLINE:
+		/*
+		 * Make sure there are not transient states where
+		 * an offline node is a migration target.  This
+		 * will leave migration disabled until the offline
+		 * completes and the MEM_OFFLINE case below runs.
+		 */
+		disable_all_migrate_targets();
+		break;
+	case MEM_OFFLINE:
+	case MEM_ONLINE:
+		/*
+		 * Recalculate the target nodes once the node
+		 * reaches its final state (online or offline).
+		 */
+		__set_migration_target_nodes();
+		break;
+	case MEM_CANCEL_OFFLINE:
+		/*
+		 * MEM_GOING_OFFLINE disabled all the migration
+		 * targets.  Reenable them.
+		 */
+		__set_migration_target_nodes();
+		break;
+	case MEM_GOING_ONLINE:
+	case MEM_CANCEL_ONLINE:
+		break;
+	}
+
+	return notifier_from_errno(0);
+}
+
+static int __init migrate_on_reclaim_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "migrate on reclaim",
+				migration_online_cpu,
+				migration_offline_cpu);
+	/*
+	 * In the unlikely case that this fails, the automatic
+	 * migration targets may become suboptimal for nodes
+	 * where N_CPU changes.  With such a small impact in a
+	 * rare case, do not bother trying to do anything special.
+	 */
+	WARN_ON(ret < 0);
+
+	hotplug_memory_notifier(migrate_on_reclaim_callback, 100);
+	return 0;
+}
+late_initcall(migrate_on_reclaim_init);
+#endif /* CONFIG_MEMORY_HOTPLUG */

_

#4 lore raw

Re: [RFC][PATCH 3/9] mm/migrate: update migration order during on hotplug events

Quoted 12 lines · Dave Hansen

From: Dave Hansen <[email protected]>

Reclaim-based migration is attempting to optimize data placement in memory based on the system topology. If the system changes, so must the migration ordering.

The implementation here is pretty simple and entirely unoptimized. On any memory or CPU hotplug events, assume that a node was added or removed and recalculate all migration targets. This ensures that the node_demotion[] array is always ready to be used in case the new reclaim mode is enabled.

Quoted 4 lines

This recalculation is far from optimal, most glaringly that it does not even attempt to figure out if nodes are actually coming or going. But, given the expected paucity of hotplug events, this should be fine.

Hi Dave,

I am still going through all the details, but just wanted to comment early on this one. Could not you hook into __try_online_node/try_offline_node?

In there we check whether a node should be brought up or removed due to lack of cpus and memory. That is being checked during hot-remove operations.

We also have node_states_check_changes_{offline,online} and their pair node_states_{set,clear}_node, that checks during online/offline stages which states should be removed from the node, but that is only wrt. memory (I guess we would only be interested in N_MEMORY).

Thanks

#5 lore raw

RFC PATCH v4 4/9 mm/migrate: make migrate_pages() return nr_succeeded

In reply to Dave Hansen

From: Yang Shi <[email protected]>

The migrate_pages() returns the number of pages that were not migrated, or an error code. When returning an error code, there is no way to know how many pages were migrated or not migrated.

In the following patch, migrate_pages() is used to demote pages to PMEM node, we need account how many pages are reclaimed (demoted) since page reclaim behavior depends on this. Add *nr_succeeded parameter to make migrate_pages() return how many pages are demoted successfully for all cases.

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/include/linux/migrate.h |    5 +++--
 b/mm/compaction.c         |    3 ++-
 b/mm/gup.c                |    4 +++-
 b/mm/memory-failure.c     |    7 +++++--
 b/mm/memory_hotplug.c     |    4 +++-
 b/mm/mempolicy.c          |    7 +++++--
 b/mm/migrate.c            |   16 +++++++++-------
 b/mm/page_alloc.c         |    9 ++++++---
 8 files changed, 36 insertions(+), 19 deletions(-)
include/linux/migrate.h renamed from include/linux/migrate.h~migrate_pages-add-success-return +3 -2
diff -puN include/linux/migrate.h~migrate_pages-add-success-return include/linux/migrate.h
@@ -40,7 +40,8 @@ extern int migrate_page(struct address_s
 			struct page *newpage, struct page *page,
 			enum migrate_mode mode);
 extern int migrate_pages(struct list_head *l, new_page_t new, free_page_t free,
-		unsigned long private, enum migrate_mode mode, int reason);
+		unsigned long private, enum migrate_mode mode, int reason,
+		unsigned int *nr_succeeded);
 extern struct page *alloc_migration_target(struct page *page, unsigned long private);
 extern int isolate_movable_page(struct page *page, isolate_mode_t mode);
 extern void putback_movable_page(struct page *page);
@@ -58,7 +59,7 @@ extern int migrate_page_move_mapping(str
 static inline void putback_movable_pages(struct list_head *l) {}
 static inline int migrate_pages(struct list_head *l, new_page_t new,
 		free_page_t free, unsigned long private, enum migrate_mode mode,
-		int reason)
+		int reason, unsigned int *nr_succeeded)
 	{ return -ENOSYS; }
 static inline struct page *alloc_migration_target(struct page *page,
 		unsigned long private)
mm/compaction.c renamed from mm/compaction.c~migrate_pages-add-success-return +2 -1
diff -puN mm/compaction.c~migrate_pages-add-success-return mm/compaction.c
@@ -2196,6 +2196,7 @@ compact_zone(struct compact_control *cc,
 	unsigned long last_migrated_pfn;
 	const bool sync = cc->mode != MIGRATE_ASYNC;
 	bool update_cached;
+	unsigned int nr_succeeded = 0;
 
 	/*
 	 * These counters track activities during zone compaction.  Initialize
@@ -2314,7 +2315,7 @@ compact_zone(struct compact_control *cc,
 
 		err = migrate_pages(&cc->migratepages, compaction_alloc,
 				compaction_free, (unsigned long)cc, cc->mode,
-				MR_COMPACTION);
+				MR_COMPACTION, &nr_succeeded);
 
 		trace_mm_compaction_migratepages(cc->nr_migratepages, err,
 							&cc->migratepages);
mm/gup.c renamed from mm/gup.c~migrate_pages-add-success-return +3 -1
diff -puN mm/gup.c~migrate_pages-add-success-return mm/gup.c
@@ -1586,6 +1586,7 @@ static long check_and_migrate_cma_pages(
 	unsigned long step;
 	bool drain_allow = true;
 	bool migrate_allow = true;
+	unsigned int nr_succeeded = 0;
 	LIST_HEAD(cma_page_list);
 	long ret = nr_pages;
 	struct migration_target_control mtc = {
@@ -1638,7 +1639,8 @@ check_again:
 			put_page(pages[i]);
 
 		if (migrate_pages(&cma_page_list, alloc_migration_target, NULL,
-			(unsigned long)&mtc, MIGRATE_SYNC, MR_CONTIG_RANGE)) {
+			(unsigned long)&mtc, MIGRATE_SYNC, MR_CONTIG_RANGE,
+				  &nr_succeeded)) {
 			/*
 			 * some of the pages failed migration. Do get_user_pages
 			 * without migration.
mm/memory-failure.c renamed from mm/memory-failure.c~migrate_pages-add-success-return +5 -2
diff -puN mm/memory-failure.c~migrate_pages-add-success-return mm/memory-failure.c
@@ -1724,6 +1724,7 @@ static int soft_offline_huge_page(struct
 	int ret;
 	unsigned long pfn = page_to_pfn(page);
 	struct page *hpage = compound_head(page);
+	unsigned int nr_succeeded = 0;
 	LIST_HEAD(pagelist);
 
 	/*
@@ -1751,7 +1752,7 @@ static int soft_offline_huge_page(struct
 	}
 
 	ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
-				MIGRATE_SYNC, MR_MEMORY_FAILURE);
+				MIGRATE_SYNC, MR_MEMORY_FAILURE, &nr_succeeded);
 	if (ret) {
 		pr_info("soft offline: %#lx: hugepage migration failed %d, type %lx (%pGp)\n",
 			pfn, ret, page->flags, &page->flags);
@@ -1782,6 +1783,7 @@ static int __soft_offline_page(struct pa
 {
 	int ret;
 	unsigned long pfn = page_to_pfn(page);
+	unsigned int nr_succeeded = 0;
 
 	/*
 	 * Check PageHWPoison again inside page lock because PageHWPoison
@@ -1841,7 +1843,8 @@ static int __soft_offline_page(struct pa
 						page_is_file_lru(page));
 		list_add(&page->lru, &pagelist);
 		ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
-					MIGRATE_SYNC, MR_MEMORY_FAILURE);
+					MIGRATE_SYNC, MR_MEMORY_FAILURE,
+					&nr_succeeded);
 		if (ret) {
 			if (!list_empty(&pagelist))
 				putback_movable_pages(&pagelist);
mm/memory_hotplug.c renamed from mm/memory_hotplug.c~migrate_pages-add-success-return +3 -1
diff -puN mm/memory_hotplug.c~migrate_pages-add-success-return mm/memory_hotplug.c
@@ -1301,6 +1301,7 @@ do_migrate_range(unsigned long start_pfn
 	unsigned long pfn;
 	struct page *page, *head;
 	int ret = 0;
+	unsigned int nr_succeeded = 0;
 	LIST_HEAD(source);
 
 	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
@@ -1356,7 +1357,8 @@ do_migrate_range(unsigned long start_pfn
 	if (!list_empty(&source)) {
 		/* Allocate a new page from the nearest neighbor node */
 		ret = migrate_pages(&source, new_node_page, NULL, 0,
-					MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+					MIGRATE_SYNC, MR_MEMORY_HOTPLUG,
+					&nr_succeeded);
 		if (ret) {
 			list_for_each_entry(page, &source, lru) {
 				pr_warn("migrating pfn %lx failed ret:%d ",
mm/mempolicy.c renamed from mm/mempolicy.c~migrate_pages-add-success-return +5 -2
diff -puN mm/mempolicy.c~migrate_pages-add-success-return mm/mempolicy.c
@@ -1072,6 +1072,7 @@ static int migrate_page_add(struct page
 static int migrate_to_node(struct mm_struct *mm, int source, int dest,
 			   int flags)
 {
+	unsigned int nr_succeeded = 0;
 	nodemask_t nmask;
 	LIST_HEAD(pagelist);
 	int err = 0;
@@ -1094,7 +1095,7 @@ static int migrate_to_node(struct mm_str
 
 	if (!list_empty(&pagelist)) {
 		err = migrate_pages(&pagelist, alloc_migration_target, NULL,
-				(unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL);
+				(unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL, &nr_succeeded);
 		if (err)
 			putback_movable_pages(&pagelist);
 	}
@@ -1271,6 +1272,7 @@ static long do_mbind(unsigned long start
 		     nodemask_t *nmask, unsigned long flags)
 {
 	struct mm_struct *mm = current->mm;
+	unsigned int nr_succeeded = 0;
 	struct mempolicy *new;
 	unsigned long end;
 	int err;
@@ -1352,7 +1354,8 @@ static long do_mbind(unsigned long start
 		if (!list_empty(&pagelist)) {
 			WARN_ON_ONCE(flags & MPOL_MF_LAZY);
 			nr_failed = migrate_pages(&pagelist, new_page, NULL,
-				start, MIGRATE_SYNC, MR_MEMPOLICY_MBIND);
+				start, MIGRATE_SYNC, MR_MEMPOLICY_MBIND,
+				&nr_succeeded);
 			if (nr_failed)
 				putback_movable_pages(&pagelist);
 		}
mm/migrate.c renamed from mm/migrate.c~migrate_pages-add-success-return +9 -7
diff -puN mm/migrate.c~migrate_pages-add-success-return mm/migrate.c
@@ -1433,6 +1433,7 @@ out:
  * @mode:		The migration mode that specifies the constraints for
  *			page migration, if any.
  * @reason:		The reason for page migration.
+ * @nr_succeeded:	The number of pages migrated successfully.
  *
  * The function returns after 10 attempts or if no pages are movable any more
  * because the list has become empty or no retryable pages exist any more.
@@ -1443,12 +1444,11 @@ out:
  */
 int migrate_pages(struct list_head *from, new_page_t get_new_page,
 		free_page_t put_new_page, unsigned long private,
-		enum migrate_mode mode, int reason)
+		enum migrate_mode mode, int reason, unsigned int *nr_succeeded)
 {
 	int retry = 1;
 	int thp_retry = 1;
 	int nr_failed = 0;
-	int nr_succeeded = 0;
 	int nr_thp_succeeded = 0;
 	int nr_thp_failed = 0;
 	int nr_thp_split = 0;
@@ -1529,7 +1529,7 @@ retry:
 					nr_succeeded += nr_subpages;
 					break;
 				}
-				nr_succeeded++;
+				(*nr_succeeded)++;
 				break;
 			default:
 				/*
@@ -1552,12 +1552,12 @@ retry:
 	nr_thp_failed += thp_retry;
 	rc = nr_failed;
 out:
-	count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded);
+	count_vm_events(PGMIGRATE_SUCCESS, *nr_succeeded);
 	count_vm_events(PGMIGRATE_FAIL, nr_failed);
 	count_vm_events(THP_MIGRATION_SUCCESS, nr_thp_succeeded);
 	count_vm_events(THP_MIGRATION_FAIL, nr_thp_failed);
 	count_vm_events(THP_MIGRATION_SPLIT, nr_thp_split);
-	trace_mm_migrate_pages(nr_succeeded, nr_failed, nr_thp_succeeded,
+	trace_mm_migrate_pages(*nr_succeeded, nr_failed, nr_thp_succeeded,
 			       nr_thp_failed, nr_thp_split, mode, reason);
 
 	if (!swapwrite)
@@ -1625,6 +1625,7 @@ static int store_status(int __user *stat
 static int do_move_pages_to_node(struct mm_struct *mm,
 		struct list_head *pagelist, int node)
 {
+	unsigned int nr_succeeded = 0;
 	int err;
 	struct migration_target_control mtc = {
 		.nid = node,
@@ -1632,7 +1633,7 @@ static int do_move_pages_to_node(struct
 	};
 
 	err = migrate_pages(pagelist, alloc_migration_target, NULL,
-			(unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL);
+			(unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL, &nr_succeeded);
 	if (err)
 		putback_movable_pages(pagelist);
 	return err;
@@ -2090,6 +2091,7 @@ int migrate_misplaced_page(struct page *
 	pg_data_t *pgdat = NODE_DATA(node);
 	int isolated;
 	int nr_remaining;
+	unsigned int nr_succeeded = 0;
 	LIST_HEAD(migratepages);
 
 	/*
@@ -2114,7 +2116,7 @@ int migrate_misplaced_page(struct page *
 	list_add(&page->lru, &migratepages);
 	nr_remaining = migrate_pages(&migratepages, alloc_misplaced_dst_page,
 				     NULL, node, MIGRATE_ASYNC,
-				     MR_NUMA_MISPLACED);
+				     MR_NUMA_MISPLACED, &nr_succeeded);
 	if (nr_remaining) {
 		if (!list_empty(&migratepages)) {
 			list_del(&page->lru);
mm/page_alloc.c renamed from mm/page_alloc.c~migrate_pages-add-success-return +6 -3
diff -puN mm/page_alloc.c~migrate_pages-add-success-return mm/page_alloc.c
@@ -8346,7 +8346,8 @@ static unsigned long pfn_max_align_up(un
 
 /* [start, end) must belong to a single zone. */
 static int __alloc_contig_migrate_range(struct compact_control *cc,
-					unsigned long start, unsigned long end)
+					unsigned long start, unsigned long end,
+					unsigned int *nr_succeeded)
 {
 	/* This function is based on compact_zone() from compaction.c. */
 	unsigned int nr_reclaimed;
@@ -8384,7 +8385,8 @@ static int __alloc_contig_migrate_range(
 		cc->nr_migratepages -= nr_reclaimed;
 
 		ret = migrate_pages(&cc->migratepages, alloc_migration_target,
-				NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE);
+				NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE,
+				nr_succeeded);
 	}
 	if (ret < 0) {
 		putback_movable_pages(&cc->migratepages);
@@ -8420,6 +8422,7 @@ int alloc_contig_range(unsigned long sta
 	unsigned long outer_start, outer_end;
 	unsigned int order;
 	int ret = 0;
+	unsigned int nr_succeeded = 0;
 
 	struct compact_control cc = {
 		.nr_migratepages = 0,
@@ -8472,7 +8475,7 @@ int alloc_contig_range(unsigned long sta
 	 * allocated.  So, if we fall through be sure to clear ret so that
 	 * -EBUSY is not accidentally used or returned to caller.
 	 */
-	ret = __alloc_contig_migrate_range(&cc, start, end);
+	ret = __alloc_contig_migrate_range(&cc, start, end, &nr_succeeded);
 	if (ret && ret != -EBUSY)
 		goto done;
 	ret =0;

_

#6 lore raw

RFC PATCH v4 5/9 mm/migrate: demote pages during reclaim

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

This is mostly derived from a patch from Yang Shi:

	https://lore.kernel.org/linux-mm/[email protected]/

Add code to the reclaim path (shrink_page_list()) to "demote" data to another NUMA node instead of discarding the data. This always avoids the cost of I/O needed to read the page back in and sometimes avoids the writeout cost when the pagee is dirty.

A second pass through shrink_page_list() will be made if any demotions fail. This essentally falls back to normal reclaim behavior in the case that demotions fail. Previous versions of this patch may have simply failed to reclaim pages which were eligible for demotion but were unable to be demoted in practice.

Note: This just adds the start of infratructure for migration. It is actually disabled next to the FIXME in migrate_demote_page_ok().

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>

--

changes from 20200730:
* Add another pass through shrink_page_list() when demotion fails.
---

 b/include/linux/migrate.h |    2 
 b/mm/vmscan.c             |   97 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +2 -0
diff -puN include/linux/migrate.h~demote-with-migrate_pages include/linux/migrate.h
@@ -27,6 +27,7 @@ enum migrate_reason {
 	MR_MEMPOLICY_MBIND,
 	MR_NUMA_MISPLACED,
 	MR_CONTIG_RANGE,
+	MR_DEMOTION,
 	MR_TYPES
 };
 
@@ -196,6 +197,7 @@ struct migrate_vma {
 int migrate_vma_setup(struct migrate_vma *args);
 void migrate_vma_pages(struct migrate_vma *migrate);
 void migrate_vma_finalize(struct migrate_vma *migrate);
+int next_demotion_node(int node);
 
 #endif /* CONFIG_MIGRATION */
 
mm/vmscan.c renamed from mm/vmscan.c~demote-with-migrate_pages +97 -0

139 lines not on this page · raw message

_

maintainer
#7 lore raw

Re: [RFC][PATCH 5/9] mm/migrate: demote pages during reclaim

Quoted 5 lines · Dave Hansen
Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>

I am still going through all the details, but just my thoughts on things that caught my eye:

include/linux/migrate.h enum migrate_reason { · line 31

Quoted 8 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +1 -0
@@ -27,6 +27,7 @@ enum migrate_reason {
 	MR_MEMPOLICY_MBIND,
 	MR_NUMA_MISPLACED,
 	MR_CONTIG_RANGE,
+	MR_DEMOTION,
 	MR_TYPES

I think you also need to add it under include/trace/events/migrate.h, so mm_migrate_pages event can know about it.

include/linux/migrate.h enum migrate_reason { · line 32

Quoted 1 line
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +1 -0
+bool migrate_demote_page_ok(struct page *page, struct scan_control *sc)

Make it static?
Also, scan_control seems to be unused here.

include/linux/migrate.h enum migrate_reason {

Quoted 4 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +4 -0
+{
+	int next_nid = next_demotion_node(page_to_nid(page));
+
+	VM_BUG_ON_PAGE(!PageLocked(page), page);

Right after the call to migrate_demote_page_ok, we call unlock_page which already has this check in place.
I know that this is only to be on the safe side and we do not loss anything, but just my thoughts.

include/linux/migrate.h

Quoted 12 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +12 -0
+static struct page *alloc_demote_page(struct page *page, unsigned long node)
+{
+	/*
+	 * Try to fail quickly if memory on the target node is not
+	 * available.  Leaving out __GFP_IO and __GFP_FS helps with
+	 * this.  If the desintation node is full, we want kswapd to
+	 * run there so that its pages will get reclaimed and future
+	 * migration attempts may succeed.
+	 */
+	gfp_t flags = (__GFP_HIGHMEM | __GFP_MOVABLE | __GFP_NORETRY |
+		       __GFP_NOMEMALLOC | __GFP_NOWARN | __GFP_THISNODE |
+		       __GFP_KSWAPD_RECLAIM);

I think it would be nicer to have this as a real GFP_ thingy defined. e.g: GFP_DEMOTION

include/linux/migrate.h

Quoted 2 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +2 -0
+	/* HugeTLB pages should not be on the LRU */
+	WARN_ON_ONCE(PageHuge(page));

I am not sure about this one.
This could only happen if the page, which now it is in another list, ends up in the buddy system. That is quite unlikely bth.
And nevertheless, this is only a warning, which means that if this scenario gets to happen, we will be allocating a single page to satisfy a higher-order page, and I am not sure about the situation we will end up with.

Quoted 4 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +4 -0
+
+	if (PageTransHuge(page)) {
+		struct page *thp;
+

include/linux/migrate.h

Quoted 10 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +10 -0
+		flags |= __GFP_COMP;
+
+		thp = alloc_pages_node(node, flags, HPAGE_PMD_ORDER);
+		if (!thp)
+			return NULL;
+		prep_transhuge_page(thp);
+		return thp;
+	}
+
+	return __alloc_pages_node(node, flags, 0);

Would make sense to transform this in some sort of new_demotion_page, which actually calls alloc_migration_target with the right stuff in place? And then pass a struct migration_target_control so alloc_migration_target does the right thing.
alloc_migration_target also takes care of calling prep_transhuge_page when needed.
e.g:

static struct page *new_demotion_node(struct page *page, unsigned long private)

{
        struct migration_target_control mtc = {
                .nid = private,
                .gfp_mask = GFP_DEMOTION,
        };

        if (PageTransHuge(page))
                mtc.gfp_mask |= __GFP_COMP;

        return alloc_migration_target(page, (unsigned long)&mtc);
}

The only thing I see is that alloc_migration_target seems to "override" the gfp_mask and does ORs GFP_TRANSHUGE for THP pages, which includes __GFP_DIRECT_RECLAIM (not appreciated in this case).
But maybe this can be worked around by checking if gfp_mask == GFP_DEMOTION, and if so, just keep the mask as it is.

include/linux/migrate.h

Quoted 8 lines
include/linux/migrate.h renamed from include/linux/migrate.h~demote-with-migrate_pages +8 -0
+
+	if (list_empty(demote_pages))
+		return 0;
+
+	/* Demotion ignores all cpuset and mempolicy settings */
+	err = migrate_pages(demote_pages, alloc_demote_page, NULL,
+			    target_nid, MIGRATE_ASYNC, MR_DEMOTION,
+			    &nr_succeeded);

As I said, instead of alloc_demote_page, use a new_demote_page and make alloc_migration_target handle the allocations and prep thp pages.

Signature · 2 lines
Oscar Salvador
SUSE L3
#8 lore raw

Re: [RFC][PATCH 5/9] mm/migrate: demote pages during reclaim

Quoted 11 lines · Oscar Salvador
Quoted 5 lines · Dave Hansen
Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>

I am still going through all the details, but just my thoughts on things that caught my eye:

Quoted 11 lines

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 enum migrate_reason { · line 31

Quoted 8 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +1 -0
@@ -27,6 +27,7 @@ enum migrate_reason {
      MR_MEMPOLICY_MBIND,
      MR_NUMA_MISPLACED,
      MR_CONTIG_RANGE,
+     MR_DEMOTION,
      MR_TYPES

I think you also need to add it under include/trace/events/migrate.h, so mm_migrate_pages event can know about it.

Agree.

Quoted 35 lines

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 enum migrate_reason { · line 32

Quoted 2 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +1 -0
+bool migrate_demote_page_ok(struct page *page, struct scan_control *sc)

Make it static?
Also, scan_control seems to be unused here.

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 enum migrate_reason {

Quoted 4 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +4 -0
+{
+     int next_nid = next_demotion_node(page_to_nid(page));
+
+     VM_BUG_ON_PAGE(!PageLocked(page), page);

Right after the call to migrate_demote_page_ok, we call unlock_page which already has this check in place.
I know that this is only to be on the safe side and we do not loss anything, but just my thoughts.

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700

Quoted 12 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +12 -0
+static struct page *alloc_demote_page(struct page *page, unsigned long node)
+{
+     /*
+      * Try to fail quickly if memory on the target node is not
+      * available.  Leaving out __GFP_IO and __GFP_FS helps with
+      * this.  If the desintation node is full, we want kswapd to
+      * run there so that its pages will get reclaimed and future
+      * migration attempts may succeed.
+      */
+     gfp_t flags = (__GFP_HIGHMEM | __GFP_MOVABLE | __GFP_NORETRY |
+                    __GFP_NOMEMALLOC | __GFP_NOWARN | __GFP_THISNODE |
+                    __GFP_KSWAPD_RECLAIM);

I think it would be nicer to have this as a real GFP_ thingy defined. e.g: GFP_DEMOTION

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700

Quoted 2 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +2 -0
+     /* HugeTLB pages should not be on the LRU */
+     WARN_ON_ONCE(PageHuge(page));
Quoted 6 lines

I am not sure about this one.
This could only happen if the page, which now it is in another list, ends up in the buddy system. That is quite unlikely bth.
And nevertheless, this is only a warning, which means that if this scenario gets to happen, we will be allocating a single page to satisfy a higher-order page, and I am not sure about the situation we will end up with.

IMHO, we should use BUG_ON instead of WARN_ON or we should just back off if we see hugetlb page in this path and print out some warning.

Quoted 37 lines
Quoted 5 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +4 -0
+
+     if (PageTransHuge(page)) {
+             struct page *thp;
+

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700

Quoted 10 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +10 -0
+             flags |= __GFP_COMP;
+
+             thp = alloc_pages_node(node, flags, HPAGE_PMD_ORDER);
+             if (!thp)
+                     return NULL;
+             prep_transhuge_page(thp);
+             return thp;
+     }
+
+     return __alloc_pages_node(node, flags, 0);

Would make sense to transform this in some sort of new_demotion_page, which actually calls alloc_migration_target with the right stuff in place? And then pass a struct migration_target_control so alloc_migration_target does the right thing.
alloc_migration_target also takes care of calling prep_transhuge_page when needed.
e.g:

static struct page *new_demotion_node(struct page *page, unsigned long private)

{
        struct migration_target_control mtc = {
                .nid = private,
                .gfp_mask = GFP_DEMOTION,
        };

        if (PageTransHuge(page))
                mtc.gfp_mask |= __GFP_COMP;

        return alloc_migration_target(page, (unsigned long)&mtc);
}
Quoted 5 lines

The only thing I see is that alloc_migration_target seems to "override" the gfp_mask and does ORs GFP_TRANSHUGE for THP pages, which includes __GFP_DIRECT_RECLAIM (not appreciated in this case).
But maybe this can be worked around by checking if gfp_mask == GFP_DEMOTION, and if so, just keep the mask as it is.

Makes sense to me.

Quoted 18 lines

include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700

Quoted 9 lines
include/linux/migrate.h 2020-10-07 09:15:31.034642442 -0700 renamed from include/linux/migrate.h~demote-with-migrate_pages 2020-10-07 09:15:31.028642442 -0700 +8 -0
+
+     if (list_empty(demote_pages))
+             return 0;
+
+     /* Demotion ignores all cpuset and mempolicy settings */
+     err = migrate_pages(demote_pages, alloc_demote_page, NULL,
+                         target_nid, MIGRATE_ASYNC, MR_DEMOTION,
+                         &nr_succeeded);

As I said, instead of alloc_demote_page, use a new_demote_page and make alloc_migration_target handle the allocations and prep thp pages.

--
Oscar Salvador
SUSE L3

#9 lore raw

RFC PATCH v4 6/9 mm/vmscan: add page demotion counter

In reply to Dave Hansen

From: Yang Shi <[email protected]>

Account the number of demoted pages into reclaim_state->nr_demoted.

Add pgdemote_kswapd and pgdemote_direct VM counters showed in /proc/vmstat.

[ daveh:
- __count_vm_events() a bit, and made them look at the THP size directly rather than getting data from migrate_pages()
]

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/include/linux/vm_event_item.h |    2 ++
 b/mm/vmscan.c                   |    6 ++++++
 b/mm/vmstat.c                   |    2 ++
 3 files changed, 10 insertions(+)
include/linux/vm_event_item.h renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter +2 -0
diff -puN include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter include/linux/vm_event_item.h
@@ -33,6 +33,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
 		PGREUSE,
 		PGSTEAL_KSWAPD,
 		PGSTEAL_DIRECT,
+		PGDEMOTE_KSWAPD,
+		PGDEMOTE_DIRECT,
 		PGSCAN_KSWAPD,
 		PGSCAN_DIRECT,
 		PGSCAN_DIRECT_THROTTLE,
mm/vmscan.c renamed from mm/vmscan.c~mm-vmscan-add-page-demotion-counter +6 -0
diff -puN mm/vmscan.c~mm-vmscan-add-page-demotion-counter mm/vmscan.c
@@ -147,6 +147,7 @@ struct scan_control {
 		unsigned int immediate;
 		unsigned int file_taken;
 		unsigned int taken;
+		unsigned int demoted;
 	} nr;
 
 	/* for recording the reclaimed slab by now */
@@ -1134,6 +1135,11 @@ static unsigned int demote_page_list(str
 			    target_nid, MIGRATE_ASYNC, MR_DEMOTION,
 			    &nr_succeeded);
 
+	if (current_is_kswapd())
+		__count_vm_events(PGDEMOTE_KSWAPD, nr_succeeded);
+	else
+		__count_vm_events(PGDEMOTE_DIRECT, nr_succeeded);
+
 	return nr_succeeded;
 }
 
mm/vmstat.c renamed from mm/vmstat.c~mm-vmscan-add-page-demotion-counter +2 -0
diff -puN mm/vmstat.c~mm-vmscan-add-page-demotion-counter mm/vmstat.c
@@ -1244,6 +1244,8 @@ const char * const vmstat_text[] = {
 	"pgreuse",
 	"pgsteal_kswapd",
 	"pgsteal_direct",
+	"pgdemote_kswapd",
+	"pgdemote_direct",
 	"pgscan_kswapd",
 	"pgscan_direct",
 	"pgscan_direct_throttle",

_

#10 lore raw

Re: [RFC][PATCH 6/9] mm/vmscan: add page demotion counter

Quoted 3 lines · Dave Hansen

From: Yang Shi <[email protected]>

Account the number of demoted pages into reclaim_state->nr_demoted.

It appears that you don't add "nr_demoted" into struct reclaim_state.

Quoted 6 lines

Add pgdemote_kswapd and pgdemote_direct VM counters showed in /proc/vmstat.

[ daveh:
- __count_vm_events() a bit, and made them look at the THP size directly rather than getting data from migrate_pages()

It appears that we get the data from migrate_pages() now.

Quoted 31 lines

]

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/include/linux/vm_event_item.h |    2 ++
 b/mm/vmscan.c                   |    6 ++++++
 b/mm/vmstat.c                   |    2 ++
 3 files changed, 10 insertions(+)

diff -puN include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter include/linux/vm_event_item.h

include/linux/vm_event_item.h renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter +2 -0
@@ -33,6 +33,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
 		PGREUSE,
 		PGSTEAL_KSWAPD,
 		PGSTEAL_DIRECT,
+		PGDEMOTE_KSWAPD,
+		PGDEMOTE_DIRECT,
 		PGSCAN_KSWAPD,
 		PGSCAN_DIRECT,
 		PGSCAN_DIRECT_THROTTLE,

diff -puN mm/vmscan.c~mm-vmscan-add-page-demotion-counter mm/vmscan.c

mm/vmscan.c struct scan_control { · line 150

Quoted 4 lines
mm/vmscan.c renamed from mm/vmscan.c~mm-vmscan-add-page-demotion-counter +1 -0
@@ -147,6 +147,7 @@ struct scan_control {
 		unsigned int immediate;
 		unsigned int file_taken;
 		unsigned int taken;
+		unsigned int demoted;

It appears that this newly added field isn't used in the patch.

Quoted 27 lines
mm/vmscan.c renamed from mm/vmscan.c~mm-vmscan-add-page-demotion-counter +5 -0
 	} nr;
 
 	/* for recording the reclaimed slab by now */
@@ -1134,6 +1135,11 @@ static unsigned int demote_page_list(str
 			    target_nid, MIGRATE_ASYNC, MR_DEMOTION,
 			    &nr_succeeded);
 
+	if (current_is_kswapd())
+		__count_vm_events(PGDEMOTE_KSWAPD, nr_succeeded);
+	else
+		__count_vm_events(PGDEMOTE_DIRECT, nr_succeeded);
+
 	return nr_succeeded;
 }
 

diff -puN mm/vmstat.c~mm-vmscan-add-page-demotion-counter mm/vmstat.c

mm/vmstat.c renamed from mm/vmstat.c~mm-vmscan-add-page-demotion-counter +2 -0
@@ -1244,6 +1244,8 @@ const char * const vmstat_text[] = {
 	"pgreuse",
 	"pgsteal_kswapd",
 	"pgsteal_direct",
+	"pgdemote_kswapd",
+	"pgdemote_direct",
 	"pgscan_kswapd",
 	"pgscan_direct",
 	"pgscan_direct_throttle",
Quoted 1 line

_

Best Regards,
Huang, Ying

#11 lore raw

Re: [RFC][PATCH 6/9] mm/vmscan: add page demotion counter

Quoted 47 lines · Ying
Quoted 3 lines · Dave Hansen

From: Yang Shi <[email protected]>

Account the number of demoted pages into reclaim_state->nr_demoted.

It appears that you don't add "nr_demoted" into struct reclaim_state.

Quoted 6 lines

Add pgdemote_kswapd and pgdemote_direct VM counters showed in /proc/vmstat.

[ daveh:
- __count_vm_events() a bit, and made them look at the THP size directly rather than getting data from migrate_pages()

It appears that we get the data from migrate_pages() now.

Quoted 29 lines

]

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/include/linux/vm_event_item.h |    2 ++
 b/mm/vmscan.c                   |    6 ++++++
 b/mm/vmstat.c                   |    2 ++
 3 files changed, 10 insertions(+)

diff -puN include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter include/linux/vm_event_item.h

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +2 -0
@@ -33,6 +33,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
              PGREUSE,
              PGSTEAL_KSWAPD,
              PGSTEAL_DIRECT,
+             PGDEMOTE_KSWAPD,
+             PGDEMOTE_DIRECT,
              PGSCAN_KSWAPD,
              PGSCAN_DIRECT,
              PGSCAN_DIRECT_THROTTLE,

diff -puN mm/vmscan.c~mm-vmscan-add-page-demotion-counter mm/vmscan.c
--- a/mm/vmscan.c~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.173642439 -0700

Quoted 8 lines

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 struct scan_control { · line 150

Quoted 6 lines

+++ b/mm/vmscan.c 2020-10-07 09:15:32.180642439 -0700

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +1 -0
@@ -147,6 +147,7 @@ struct scan_control {
              unsigned int immediate;
              unsigned int file_taken;
              unsigned int taken;
+             unsigned int demoted;

It appears that this newly added field isn't used in the patch.

My original patch tracked nr_demoted in reclaim_stat as well, but it seems Dave dropped that part. If Dave thinks it is not necessary to keep tracking nr_demoted in reclaim_stat, then that field should be dropped.

Quoted 33 lines
Quoted 28 lines
include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +5 -0
      } nr;
 
      /* for recording the reclaimed slab by now */
@@ -1134,6 +1135,11 @@ static unsigned int demote_page_list(str
                          target_nid, MIGRATE_ASYNC, MR_DEMOTION,
                          &nr_succeeded);
 
+     if (current_is_kswapd())
+             __count_vm_events(PGDEMOTE_KSWAPD, nr_succeeded);
+     else
+             __count_vm_events(PGDEMOTE_DIRECT, nr_succeeded);
+
      return nr_succeeded;
 }
 

diff -puN mm/vmstat.c~mm-vmscan-add-page-demotion-counter mm/vmstat.c

mm/vmstat.c 2020-10-07 09:15:32.181642439 -0700 renamed from mm/vmstat.c~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.175642439 -0700 +2 -0
@@ -1244,6 +1244,8 @@ const char * const vmstat_text[] = {
      "pgreuse",
      "pgsteal_kswapd",
      "pgsteal_direct",
+     "pgdemote_kswapd",
+     "pgdemote_direct",
      "pgscan_kswapd",
      "pgscan_direct",
      "pgscan_direct_throttle",
Quoted 1 line

_

Best Regards,
Huang, Ying

#12 lore raw

Re: [RFC][PATCH 6/9] mm/vmscan: add page demotion counter

Quoted 57 lines · Yang Shi
Quoted 47 lines · Ying
Quoted 3 lines · Dave Hansen

From: Yang Shi <[email protected]>

Account the number of demoted pages into reclaim_state->nr_demoted.

It appears that you don't add "nr_demoted" into struct reclaim_state.

Quoted 6 lines

Add pgdemote_kswapd and pgdemote_direct VM counters showed in /proc/vmstat.

[ daveh:
- __count_vm_events() a bit, and made them look at the THP size directly rather than getting data from migrate_pages()

It appears that we get the data from migrate_pages() now.

Quoted 29 lines

]

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/include/linux/vm_event_item.h |    2 ++
 b/mm/vmscan.c                   |    6 ++++++
 b/mm/vmstat.c                   |    2 ++
 3 files changed, 10 insertions(+)

diff -puN include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter include/linux/vm_event_item.h

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +2 -0
@@ -33,6 +33,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
              PGREUSE,
              PGSTEAL_KSWAPD,
              PGSTEAL_DIRECT,
+             PGDEMOTE_KSWAPD,
+             PGDEMOTE_DIRECT,
              PGSCAN_KSWAPD,
              PGSCAN_DIRECT,
              PGSCAN_DIRECT_THROTTLE,

diff -puN mm/vmscan.c~mm-vmscan-add-page-demotion-counter mm/vmscan.c
--- a/mm/vmscan.c~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.173642439 -0700

Quoted 8 lines

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 struct scan_control { · line 150

Quoted 6 lines

+++ b/mm/vmscan.c 2020-10-07 09:15:32.180642439 -0700

include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +1 -0
@@ -147,6 +147,7 @@ struct scan_control {
              unsigned int immediate;
              unsigned int file_taken;
              unsigned int taken;
+             unsigned int demoted;

It appears that this newly added field isn't used in the patch.

Quoted 4 lines

My original patch tracked nr_demoted in reclaim_stat as well, but it seems Dave dropped that part. If Dave thinks it is not necessary to keep tracking nr_demoted in reclaim_stat, then that field should be dropped.

Personally, I prefer to collect nr_demoted in struct reclaim_stat and call counting functions in shrink_inactive_list() as other reclaiming statistics.

Best Regards,
Huang, Ying

Quoted 33 lines
Quoted 33 lines
Quoted 28 lines
include/linux/vm_event_item.h 2020-10-07 09:15:32.179642439 -0700 renamed from include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.171642439 -0700 +5 -0
      } nr;
 
      /* for recording the reclaimed slab by now */
@@ -1134,6 +1135,11 @@ static unsigned int demote_page_list(str
                          target_nid, MIGRATE_ASYNC, MR_DEMOTION,
                          &nr_succeeded);
 
+     if (current_is_kswapd())
+             __count_vm_events(PGDEMOTE_KSWAPD, nr_succeeded);
+     else
+             __count_vm_events(PGDEMOTE_DIRECT, nr_succeeded);
+
      return nr_succeeded;
 }
 

diff -puN mm/vmstat.c~mm-vmscan-add-page-demotion-counter mm/vmstat.c

mm/vmstat.c 2020-10-07 09:15:32.181642439 -0700 renamed from mm/vmstat.c~mm-vmscan-add-page-demotion-counter 2020-10-07 09:15:32.175642439 -0700 +2 -0
@@ -1244,6 +1244,8 @@ const char * const vmstat_text[] = {
      "pgreuse",
      "pgsteal_kswapd",
      "pgsteal_direct",
+     "pgdemote_kswapd",
+     "pgdemote_direct",
      "pgscan_kswapd",
      "pgscan_direct",
      "pgscan_direct_throttle",
Quoted 1 line

_

Best Regards,
Huang, Ying

#13 lore raw

RFC PATCH v4 7/9 mm/vmscan: Consider anonymous pages without swap

In reply to Dave Hansen

From: Keith Busch <[email protected]>

Age and reclaim anonymous pages if a migration path is available. The node has other recourses for inactive anonymous pages beyond swap,

#Signed-off-by: Keith Busch <[email protected]>

Cc Keith Busch <[email protected]>
[vishal: fixup the migration->demotion rename]
Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

--

Changes from Dave 06/2020:
* rename reclaim_anon_pages()->can_reclaim_anon_pages()

Note: Keith's Intel SoB is commented out because he is no longer at Intel and his @intel.com mail will bouncee
---

 b/include/linux/node.h |    9 +++++++++
 b/mm/vmscan.c          |   33 ++++++++++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)
include/linux/node.h renamed from include/linux/node.h~0009-mm-vmscan-Consider-anonymous-pages-without-swap +9 -0
diff -puN include/linux/node.h~0009-mm-vmscan-Consider-anonymous-pages-without-swap include/linux/node.h
@@ -180,4 +180,13 @@ static inline void register_hugetlbfs_wi
 
 #define to_node(device) container_of(device, struct node, dev)
 
+#ifdef CONFIG_MIGRATION
+extern int next_demotion_node(int node);
+#else
+static inline int next_demotion_node(int node)
+{
+	return NUMA_NO_NODE;
+}
+#endif
+
 #endif /* _LINUX_NODE_H_ */
mm/vmscan.c renamed from mm/vmscan.c~0009-mm-vmscan-Consider-anonymous-pages-without-swap +28 -5
diff -puN mm/vmscan.c~0009-mm-vmscan-Consider-anonymous-pages-without-swap mm/vmscan.c
@@ -290,6 +290,26 @@ static bool writeback_throttling_sane(st
 }
 #endif
 
+static inline bool can_reclaim_anon_pages(struct mem_cgroup *memcg,
+					  int node_id)
+{
+	/* Always age anon pages when we have swap */
+	if (memcg == NULL) {
+		if (get_nr_swap_pages() > 0)
+			return true;
+	} else {
+		if (mem_cgroup_get_nr_swap_pages(memcg) > 0)
+			return true;
+	}
+
+	/* Also age anon pages if we can auto-migrate them */
+	if (next_demotion_node(node_id) >= 0)
+		return true;
+
+	/* No way to reclaim anon pages */
+	return false;
+}
+
 /*
  * This misses isolated pages which are not accounted for to save counters.
  * As the data only determines if reclaim or compaction continues, it is
@@ -301,7 +321,7 @@ unsigned long zone_reclaimable_pages(str
 
 	nr = zone_page_state_snapshot(zone, NR_ZONE_INACTIVE_FILE) +
 		zone_page_state_snapshot(zone, NR_ZONE_ACTIVE_FILE);
-	if (get_nr_swap_pages() > 0)
+	if (can_reclaim_anon_pages(NULL, zone_to_nid(zone)))
 		nr += zone_page_state_snapshot(zone, NR_ZONE_INACTIVE_ANON) +
 			zone_page_state_snapshot(zone, NR_ZONE_ACTIVE_ANON);
 
@@ -2337,6 +2357,7 @@ enum scan_balance {
 static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 			   unsigned long *nr)
 {
+	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	unsigned long anon_cost, file_cost, total_cost;
 	int swappiness = mem_cgroup_swappiness(memcg);
@@ -2347,7 +2368,7 @@ static void get_scan_count(struct lruvec
 	enum lru_list lru;
 
 	/* If we have no swap space, do not bother scanning anon pages. */
-	if (!sc->may_swap || mem_cgroup_get_nr_swap_pages(memcg) <= 0) {
+	if (!sc->may_swap || !can_reclaim_anon_pages(memcg, pgdat->node_id)) {
 		scan_balance = SCAN_FILE;
 		goto out;
 	}
@@ -2631,7 +2652,9 @@ static void shrink_lruvec(struct lruvec
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.
 	 */
-	if (total_swap_pages && inactive_is_low(lruvec, LRU_INACTIVE_ANON))
+	if (can_reclaim_anon_pages(lruvec_memcg(lruvec),
+			       lruvec_pgdat(lruvec)->node_id) &&
+	    inactive_is_low(lruvec, LRU_INACTIVE_ANON))
 		shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
 				   sc, LRU_ACTIVE_ANON);
 }
@@ -2701,7 +2724,7 @@ static inline bool should_continue_recla
 	 */
 	pages_for_compaction = compact_gap(sc->order);
 	inactive_lru_pages = node_page_state(pgdat, NR_INACTIVE_FILE);
-	if (get_nr_swap_pages() > 0)
+	if (can_reclaim_anon_pages(NULL, pgdat->node_id))
 		inactive_lru_pages += node_page_state(pgdat, NR_INACTIVE_ANON);
 
 	return inactive_lru_pages > pages_for_compaction;
@@ -3460,7 +3483,7 @@ static void age_active_anon(struct pglis
 	struct mem_cgroup *memcg;
 	struct lruvec *lruvec;
 
-	if (!total_swap_pages)
+	if (!can_reclaim_anon_pages(NULL, pgdat->node_id))
 		return;
 
 	lruvec = mem_cgroup_lruvec(NULL, pgdat);

_

maintainer
#14 lore raw

Re: [RFC][PATCH 7/9] mm/vmscan: Consider anonymous pages without swap

Quoted 7 lines · Dave Hansen

From: Keith Busch <[email protected]>

Age and reclaim anonymous pages if a migration path is available. The node has other recourses for inactive anonymous pages beyond swap,

#Signed-off-by: Keith Busch <[email protected]>

Quoted 9 lines
Cc Keith Busch <[email protected]>
[vishal: fixup the migration->demotion rename]
Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

I have a question regarding this one.

It seems that we do have places where we read total_swap_pages directly and other places where we use get_nr_swap_pages.
One seems to give the total number of swap pages, while the other gives the number of free swap pages.

With this patch, we will use always the atomic version get_nr_swap_pages from now on.
Is that ok? I guess so, but it might warrant a mention in the changelog?

E.g: age_active_anon seems to base one of its decisions on whether we have swap (it seems it does not care if swap space is available).

Signature · 2 lines
Oscar Salvador
SUSE L3
#15 lore raw

Re: [RFC][PATCH 7/9] mm/vmscan: Consider anonymous pages without swap

Quoted 3 lines · Oscar Salvador

With this patch, we will use always the atomic version get_nr_swap_pages from now on. Is that ok? I guess so, but it might warrant a mention in the changelog?

I _think_ it's OK. But, you're right that it's a potential behavior change that's not mentioned in the changelog.

I'll mention it in the changelog and see if I can dream up any other practical implications from this change.

Thanks for taking a look!

#16 lore raw

Re: [RFC][PATCH 7/9] mm/vmscan: Consider anonymous pages without swap

Quoted 11 lines · Dave Hansen
Quoted 3 lines · Oscar Salvador

With this patch, we will use always the atomic version get_nr_swap_pages from now on. Is that ok? I guess so, but it might warrant a mention in the changelog?

I _think_ it's OK. But, you're right that it's a potential behavior change that's not mentioned in the changelog.

I'll mention it in the changelog and see if I can dream up any other practical implications from this change.

IMHO, we don't have to modify those two places at all. They are used to rebalance the anon lru active/inactive ratio even if we did not try to evict anon pages at all, so "total_swap_pages" is used instead of checking swappiness and available swap space.

The changes may result in imbalanced anon lru.

Quoted 3 lines

Thanks for taking a look!

maintainer
#17 lore raw

Re: [RFC][PATCH 7/9] mm/vmscan: Consider anonymous pages without swap

Quoted 6 lines · Yang Shi

IMHO, we don't have to modify those two places at all. They are used to rebalance the anon lru active/inactive ratio even if we did not try to evict anon pages at all, so "total_swap_pages" is used instead of checking swappiness and available swap space.

The changes may result in imbalanced anon lru.

I might be missing something, so bear with me.

It is true that since we are only rebalancing the lists, we do not need to check for swap space yet, but here we are also adding a new end-point where we can migrate to in case of memory pressure.

So in case we can demote pages, it makes sense to proceed with the aging and rebalancing regardless of whether we have swap in place, right?

But maybe the right procedure would be to perform some sort of the following check in those two places:

	if (total_swap_pages || can_migrate_to_demote_node)
		- proceed_with_rebalancing_or_aging

--
Oscar Salvador
SUSE L3

#18 lore raw

Re: [RFC][PATCH 7/9] mm/vmscan: Consider anonymous pages without swap

Quoted 9 lines · osalvador
Quoted 6 lines · Yang Shi

IMHO, we don't have to modify those two places at all. They are used to rebalance the anon lru active/inactive ratio even if we did not try to evict anon pages at all, so "total_swap_pages" is used instead of checking swappiness and available swap space.

The changes may result in imbalanced anon lru.

Quoted 8 lines

I might be missing something, so bear with me.

It is true that since we are only rebalancing the lists, we do not need to check for swap space yet, but here we are also adding a new end-point where we can migrate to in case of memory pressure.

So in case we can demote pages, it makes sense to proceed with the aging and rebalancing regardless of whether we have swap in place, right?

Yes, makes sense. I missed that point.

Quoted 6 lines

But maybe the right procedure would be to perform some sort of the following check in those two places:

if (total_swap_pages || can_migrate_to_demote_node)
- proceed_with_rebalancing_or_aging

Looks sane to me.

Quoted 4 lines

--
Oscar Salvador
SUSE L3

#19 lore raw

RFC PATCH v4 8/9 mm/vmscan: never demote for memcg reclaim

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

Global reclaim aims to reduce the amount of memory used on a given node or set of nodes. Migrating pages to another node serves this purpose.

memcg reclaim is different. Its goal is to reduce the total memory consumption of the entire memcg, across all nodes. Migration does not assist memcg reclaim because it just moves page contents between nodes rather than actually reducing memory consumption.

Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/mm/vmscan.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)
mm/vmscan.c renamed from mm/vmscan.c~never-demote-for-memcg-reclaim +25 -8
diff -puN mm/vmscan.c~never-demote-for-memcg-reclaim mm/vmscan.c
@@ -291,8 +291,11 @@ static bool writeback_throttling_sane(st
 #endif
 
 static inline bool can_reclaim_anon_pages(struct mem_cgroup *memcg,
-					  int node_id)
+					  int node_id,
+					  struct scan_control *sc)
 {
+	bool in_cgroup_reclaim = false;
+
 	/* Always age anon pages when we have swap */
 	if (memcg == NULL) {
 		if (get_nr_swap_pages() > 0)
@@ -302,8 +305,18 @@ static inline bool can_reclaim_anon_page
 			return true;
 	}
 
-	/* Also age anon pages if we can auto-migrate them */
-	if (next_demotion_node(node_id) >= 0)
+	/* Can only be in memcg reclaim in paths with valid 'sc': */
+	if (sc && cgroup_reclaim(sc))
+		in_cgroup_reclaim = true;
+
+	/*
+	 * Also age anon pages if we can auto-migrate them.
+	 *
+	 * Migrating a page does not reduce comsumption of a
+	 * memcg so should not be performed when in memcg
+	 * reclaim.
+	 */
+	if (!in_cgroup_reclaim && (next_demotion_node(node_id) >= 0))
 		return true;
 
 	/* No way to reclaim anon pages */
@@ -321,7 +334,7 @@ unsigned long zone_reclaimable_pages(str
 
 	nr = zone_page_state_snapshot(zone, NR_ZONE_INACTIVE_FILE) +
 		zone_page_state_snapshot(zone, NR_ZONE_ACTIVE_FILE);
-	if (can_reclaim_anon_pages(NULL, zone_to_nid(zone)))
+	if (can_reclaim_anon_pages(NULL, zone_to_nid(zone), NULL))
 		nr += zone_page_state_snapshot(zone, NR_ZONE_INACTIVE_ANON) +
 			zone_page_state_snapshot(zone, NR_ZONE_ACTIVE_ANON);
 
@@ -1064,6 +1077,10 @@ bool migrate_demote_page_ok(struct page
 	VM_BUG_ON_PAGE(PageHuge(page), page);
 	VM_BUG_ON_PAGE(PageLRU(page), page);
 
+	/* It is pointless to do demotion in memcg reclaim */
+	if (cgroup_reclaim(sc))
+		return false;
+
 	if (next_nid == NUMA_NO_NODE)
 		return false;
 	if (PageTransHuge(page) && !thp_migration_supported())
@@ -2368,7 +2385,7 @@ static void get_scan_count(struct lruvec
 	enum lru_list lru;
 
 	/* If we have no swap space, do not bother scanning anon pages. */
-	if (!sc->may_swap || !can_reclaim_anon_pages(memcg, pgdat->node_id)) {
+	if (!sc->may_swap || !can_reclaim_anon_pages(memcg, pgdat->node_id, sc)) {
 		scan_balance = SCAN_FILE;
 		goto out;
 	}
@@ -2653,7 +2670,7 @@ static void shrink_lruvec(struct lruvec
 	 * rebalance the anon lru active/inactive ratio.
 	 */
 	if (can_reclaim_anon_pages(lruvec_memcg(lruvec),
-			       lruvec_pgdat(lruvec)->node_id) &&
+			       lruvec_pgdat(lruvec)->node_id, sc) &&
 	    inactive_is_low(lruvec, LRU_INACTIVE_ANON))
 		shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
 				   sc, LRU_ACTIVE_ANON);
@@ -2724,7 +2741,7 @@ static inline bool should_continue_recla
 	 */
 	pages_for_compaction = compact_gap(sc->order);
 	inactive_lru_pages = node_page_state(pgdat, NR_INACTIVE_FILE);
-	if (can_reclaim_anon_pages(NULL, pgdat->node_id))
+	if (can_reclaim_anon_pages(NULL, pgdat->node_id, sc))
 		inactive_lru_pages += node_page_state(pgdat, NR_INACTIVE_ANON);
 
 	return inactive_lru_pages > pages_for_compaction;
@@ -3483,7 +3500,7 @@ static void age_active_anon(struct pglis
 	struct mem_cgroup *memcg;
 	struct lruvec *lruvec;
 
-	if (!can_reclaim_anon_pages(NULL, pgdat->node_id))
+	if (!can_reclaim_anon_pages(NULL, pgdat->node_id, sc))
 		return;
 
 	lruvec = mem_cgroup_lruvec(NULL, pgdat);

_

#20 lore raw

RFC PATCH v4 9/9 mm/migrate: new zone_reclaim_mode to enable reclaim migration

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

Some method is obviously needed to enable reclaim-based migration.

Just like traditional autonuma, there will be some workloads that will benefit like workloads with more "static" configurations where hot pages stay hot and cold pages stay cold. If pages come and go from the hot and cold sets, the benefits of this approach will be more limited.

The benefits are truly workload-based and *not* hardware-based. We do not believe that there is a viable threshold where certain hardware configurations should have this mechanism enabled while others do not.

To be conservative, earlier work defaulted to disable reclaim- based migration and did not include a mechanism to enable it. This propses extending the existing "zone_reclaim_mode" (now now really node_reclaim_mode) as a method to enable it.

We are open to any alternative that allows end users to enable this mechanism or disable it it workload harm is detected (just like traditional autonuma).

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>

---

 b/Documentation/admin-guide/sysctl/vm.rst |    9 +++++++++
 b/include/linux/swap.h                    |    3 ++-
 b/include/uapi/linux/mempolicy.h          |    1 +
 b/mm/vmscan.c                             |    6 ++++--
 4 files changed, 16 insertions(+), 3 deletions(-)
Documentation/admin-guide/sysctl/vm.rst renamed from Documentation/admin-guide/sysctl/vm.rst~RECLAIM_MIGRATE +9 -0
diff -puN Documentation/admin-guide/sysctl/vm.rst~RECLAIM_MIGRATE Documentation/admin-guide/sysctl/vm.rst
@@ -969,6 +969,7 @@ This is value OR'ed together of
 1	Zone reclaim on
 2	Zone reclaim writes dirty pages out
 4	Zone reclaim swaps pages
+8	Zone reclaim migrates pages
 =	===================================
 
 zone_reclaim_mode is disabled by default.  For file servers or workloads
@@ -993,3 +994,11 @@ of other processes running on other node
 Allowing regular swap effectively restricts allocations to the local
 node unless explicitly overridden by memory policies or cpuset
 configurations.
+
+Page migration during reclaim is intended for systems with tiered memory
+configurations.  These systems have multiple types of memory with varied
+performance characteristics instead of plain NUMA systems where the same
+kind of memory is found at varied distances.  Allowing page migration
+during reclaim enables these systems to migrate pages from fast tiers to
+slow tiers when the fast tier is under pressure.  This migration is
+performed before swap.
include/linux/swap.h renamed from include/linux/swap.h~RECLAIM_MIGRATE +2 -1
diff -puN include/linux/swap.h~RECLAIM_MIGRATE include/linux/swap.h
@@ -385,7 +385,8 @@ extern int sysctl_min_slab_ratio;
 static inline bool node_reclaim_enabled(void)
 {
 	/* Is any node_reclaim_mode bit set? */
-	return node_reclaim_mode & (RECLAIM_ZONE|RECLAIM_WRITE|RECLAIM_UNMAP);
+	return node_reclaim_mode & (RECLAIM_ZONE |RECLAIM_WRITE|
+				    RECLAIM_UNMAP|RECLAIM_MIGRATE);
 }
 
 extern void check_move_unevictable_pages(struct pagevec *pvec);
include/uapi/linux/mempolicy.h renamed from include/uapi/linux/mempolicy.h~RECLAIM_MIGRATE +1 -0
diff -puN include/uapi/linux/mempolicy.h~RECLAIM_MIGRATE include/uapi/linux/mempolicy.h
@@ -69,5 +69,6 @@ enum {
 #define RECLAIM_ZONE	(1<<0)	/* Run shrink_inactive_list on the zone */
 #define RECLAIM_WRITE	(1<<1)	/* Writeout pages during reclaim */
 #define RECLAIM_UNMAP	(1<<2)	/* Unmap pages during reclaim */
+#define RECLAIM_MIGRATE	(1<<3)	/* Migrate to other nodes during reclaim */
 
 #endif /* _UAPI_LINUX_MEMPOLICY_H */
mm/vmscan.c renamed from mm/vmscan.c~RECLAIM_MIGRATE +4 -2
diff -puN mm/vmscan.c~RECLAIM_MIGRATE mm/vmscan.c
@@ -1077,6 +1077,9 @@ bool migrate_demote_page_ok(struct page
 	VM_BUG_ON_PAGE(PageHuge(page), page);
 	VM_BUG_ON_PAGE(PageLRU(page), page);
 
+	if (!(node_reclaim_mode & RECLAIM_MIGRATE))
+		return false;
+
 	/* It is pointless to do demotion in memcg reclaim */
 	if (cgroup_reclaim(sc))
 		return false;
@@ -1086,8 +1089,7 @@ bool migrate_demote_page_ok(struct page
 	if (PageTransHuge(page) && !thp_migration_supported())
 		return false;
 
-	// FIXME: actually enable this later in the series
-	return false;
+	return true;
 }
 
 

_

#21 lore raw
In reply to Dave Hansen
Quoted 9 lines · Dave Hansen

Changes since (automigrate-20200818):
* Fall back to normal reclaim when demotion fails

The full series is also available here:

        https://github.com/hansendc/linux/tree/automigrate-20201007
Quoted 7 lines

I really just want folks to look at:

[RFC][PATCH 5/9] mm/migrate: demote pages during reclaim

I've reworked that so that it can both use the high-level migration API, and fall back to normal reclaim if migration fails. I think that gives us the best of both worlds.

Thanks for doing this. Although I was inclined to think the kswapds on PMEM nodes could make enough space for retrying migration later instead of doing swap right away, this approach might be over-engineering and over-killing. The simple immediate "retry regular reclaim" approach also looks fine to me for the time being. We always could optimize it later with more test results backed by real life workloads.

Quoted 59 lines

I'm posting the series in case folks want to run the whole thing.

--

We're starting to see systems with more and more kinds of memory such as Intel's implementation of persistent memory.

Let's say you have a system with some DRAM and some persistent memory. Today, once DRAM fills up, reclaim will start and some of the DRAM contents will be thrown out. Allocations will, at some point, start falling over to the slower persistent memory.

That has two nasty properties. First, the newer allocations can end up in the slower persistent memory. Second, reclaimed data in DRAM are just discarded even if there are gobs of space in persistent memory that could be used.

This set implements a solution to these problems. At the end of the reclaim process in shrink_page_list() just before the last page refcount is dropped, the page is migrated to persistent memory instead of being dropped.

While I've talked about a DRAM/PMEM pairing, this approach would function in any environment where memory tiers exist.

This is not perfect. It "strands" pages in slower memory and never brings them back to fast DRAM. Other things need to be built to promote hot pages back to DRAM.

This is also all based on an upstream mechanism that allows persistent memory to be onlined and used as if it were volatile:

        http://lkml.kernel.org/r/[email protected]

== Open Issues ==

* For cpusets and memory policies that restrict allocations to PMEM, is it OK to demote to PMEM? Do we need a cgroup- level API to opt-in or opt-out of these migrations?

--

Changes since (https://lwn.net/Articles/824830/):
* Use higher-level migrate_pages() API approach from Yang Shi's earlier patches.
* made sure to actually check node_reclaim_mode's new bit
* disabled migration entirely before introducing RECLAIM_MIGRATE
* Replace GFP_NOWAIT with explicit __GFP_KSWAPD_RECLAIM and comment why we want that.
* Comment on effects of that keep multiple source nodes from sharing target nodes

Cc Yang Shi <[email protected]>
Cc David Rientjes <[email protected]>
Cc Huang Ying <[email protected]>
Cc Dan Williams <[email protected]>
Cc David Hildenbrand <[email protected]>