Message-Id: <20220711084948.274787-1-ying.huang@intel.com>
Date:   Mon, 11 Jul 2022 16:49:41 +0800
From:   Huang Ying <ying.huang@...el.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        "Huang, Ying" <ying.huang@...el.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Zi Yan <ziy@...dia.com>, Yang Shi <shy828301@...il.com>
Subject: [PATCH -V2 0/7] migrate_pages(): fix several bugs in error path

From: "Huang, Ying" <ying.huang@...el.com>

During review the code of migrate_pages() and build a test program for
it.  Several bugs in error path are identified and fixed in this
series.

Most patches are tested via

- Apply error-inject.patch in Linux kernel
- Compile test-migrate.c (with -lnuma)
- Test with test-migrate.sh

error-inject.patch, test-migrate.c, and test-migrate.sh are as below.
It turns out that error injection is an important tool to fix bugs in
error path.

Changes:

v2:

- Rebased on v5.19-rc5

- Addressed some comments from Baolin, Thanks!

- Added reviewed-by tags

Best Regards,
Huang, Ying

------------------------- error-inject.patch -------------------------
>From 295ea21204f3f025a041fe39c68a2eaec8313c68 Mon Sep 17 00:00:00 2001
From: Huang Ying <ying.huang@...el.com>
Date: Tue, 21 Jun 2022 11:08:30 +0800
Subject: [PATCH] migrate_pages: error inject

---
 mm/migrate.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 399904015d23..87d47064ec6c 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -337,6 +337,42 @@ void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd)
 }
 #endif
 
+#define EI_MP_ENOSYS		0x0001
+#define EI_MP_THP_ENOMEM	0x0002
+#define EI_MP_NP_ENOMEM		0x0004
+#define EI_MP_EAGAIN		0x0008
+#define EI_MP_EOTHER		0x0010
+#define EI_MP_NOSPLIT		0x0020
+#define EI_MP_SPLIT_FAIL	0x0040
+#define EI_MP_EAGAIN_PERM	0x0080
+#define EI_MP_EBUSY		0x0100
+
+static unsigned int ei_migrate_pages;
+
+module_param(ei_migrate_pages, uint, 0644);
+
+static bool ei_thp_migration_supported(void)
+{
+	if (ei_migrate_pages & EI_MP_ENOSYS)
+		return false;
+	else
+		return thp_migration_supported();
+}
+
+static int ei_trylock_page(struct page *page)
+{
+	if (ei_migrate_pages & EI_MP_EAGAIN)
+		return 0;
+	return trylock_page(page);
+}
+
+static int ei_split_huge_page_to_list(struct page *page, struct list_head *list)
+{
+	if (ei_migrate_pages & EI_MP_SPLIT_FAIL)
+		return -EBUSY;
+	return split_huge_page_to_list(page, list);
+}
+
 static int expected_page_refs(struct address_space *mapping, struct page *page)
 {
 	int expected_count = 1;
@@ -368,6 +404,9 @@ int folio_migrate_mapping(struct address_space *mapping,
 		if (folio_ref_count(folio) != expected_count)
 			return -EAGAIN;
 
+		if (ei_migrate_pages & EI_MP_EAGAIN_PERM)
+			return -EAGAIN;
+
 		/* No turning back from here */
 		newfolio->index = folio->index;
 		newfolio->mapping = folio->mapping;
@@ -929,7 +968,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 	struct anon_vma *anon_vma = NULL;
 	bool is_lru = !__PageMovable(page);
 
-	if (!trylock_page(page)) {
+	if (!ei_trylock_page(page)) {
 		if (!force || mode == MIGRATE_ASYNC)
 			goto out;
 
@@ -952,6 +991,11 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 		lock_page(page);
 	}
 
+	if (ei_migrate_pages & EI_MP_EBUSY) {
+		rc = -EBUSY;
+		goto out_unlock;
+	}
+
 	if (PageWriteback(page)) {
 		/*
 		 * Only in the case of a full synchronous migration is it
@@ -1086,7 +1130,7 @@ static int unmap_and_move(new_page_t get_new_page,
 	int rc = MIGRATEPAGE_SUCCESS;
 	struct page *newpage = NULL;
 
-	if (!thp_migration_supported() && PageTransHuge(page))
+	if (!ei_thp_migration_supported() && PageTransHuge(page))
 		return -ENOSYS;
 
 	if (page_count(page) == 1) {
@@ -1102,6 +1146,11 @@ static int unmap_and_move(new_page_t get_new_page,
 		goto out;
 	}
 
+	if ((ei_migrate_pages & EI_MP_THP_ENOMEM) && PageTransHuge(page))
+		return -ENOMEM;
+	if ((ei_migrate_pages & EI_MP_NP_ENOMEM) && !PageTransHuge(page))
+		return -ENOMEM;
+
 	newpage = get_new_page(page, private);
 	if (!newpage)
 		return -ENOMEM;
@@ -1305,7 +1354,7 @@ static inline int try_split_thp(struct page *page, struct list_head *split_pages
 	int rc;
 
 	lock_page(page);
-	rc = split_huge_page_to_list(page, split_pages);
+	rc = ei_split_huge_page_to_list(page, split_pages);
 	unlock_page(page);
 
 	return rc;
@@ -1358,6 +1407,9 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
 	bool nosplit = (reason == MR_NUMA_MISPLACED);
 	bool no_subpage_counting = false;
 
+	if (ei_migrate_pages & EI_MP_NOSPLIT)
+		nosplit = true;
+
 	trace_mm_migrate_pages_start(mode, reason);
 
 thp_subpage_migration:
-- 
2.30.2

------------------------- test-migrate.c -------------------------------------
 #define _GNU_SOURCE

 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>

 #include <fcntl.h>
 #include <sys/uio.h>
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <numaif.h>
 #include <numa.h>

 #ifndef MADV_FREE
 #define MADV_FREE	8		/* free pages only if memory pressure */
 #endif

 #define ONE_MB		(1024 * 1024)
 #define MAP_SIZE	(16 * ONE_MB)
 #define THP_SIZE	(2 * ONE_MB)
 #define THP_MASK	(THP_SIZE - 1)

 #define ERR_EXIT_ON(cond, msg)					\
	 do {							\
		 int __cond_in_macro = (cond);			\
		 if (__cond_in_macro)				\
			 error_exit(__cond_in_macro, (msg));	\
	 } while (0)

 void error_msg(int ret, int nr, int *status, const char *msg)
 {
	 int i;

	 fprintf(stderr, "Error: %s, ret : %d, error: %s\n",
		 msg, ret, strerror(errno));

	 if (!nr)
		 return;
	 fprintf(stderr, "status: ");
	 for (i = 0; i < nr; i++)
		 fprintf(stderr, "%d ", status[i]);
	 fprintf(stderr, "\n");
 }

 void error_exit(int ret, const char *msg)
 {
	 error_msg(ret, 0, NULL, msg);
	 exit(1);
 }

 void *addr_thp;
 void *addr;
 char *pn;
 char *pn1;
 char *pn2;
 char *pn3;
 void *pages[4];
 int status[4];

 void create_map(bool thp)
 {
	 int ret;
	 void *p;

	 p = mmap(NULL, MAP_SIZE, PROT_READ | PROT_WRITE,
		  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
	 ERR_EXIT_ON(p == MAP_FAILED, "mmap");
	 if (thp) {
		 ret = madvise(p, MAP_SIZE, MADV_HUGEPAGE);
		 ERR_EXIT_ON(ret, "advise hugepage");
		 addr_thp = p;
	 } else {
		 addr = p;
	 }
 }

 void prepare()
 {
	 int ret;
	 struct iovec iov;

	 if (addr) {
		 munmap(addr_thp, MAP_SIZE);
		 munmap(addr, MAP_SIZE);
	 }

	 create_map(true);
	 create_map(false);

	 pn = (char *)(((unsigned long)addr_thp + THP_SIZE) & ~THP_MASK);
	 pn1 = pn + THP_SIZE;
	 pages[0] = pn;
	 pages[1] = pn1;
	 *pn = 1;

	 pn2 = (char *)(((unsigned long)addr + THP_SIZE) & ~THP_MASK);
	 pn3 = pn2 + THP_SIZE;
	 pages[2] = pn2;
	 pages[3] = pn3;

	 status[0] = status[1] = status[2] = status[3] = 1024;
 }

 void test_migrate()
 {
	 int ret;
	 int nodes[4] = { 1, 1, 1, 1 };
	 pid_t pid = getpid();

	 prepare();
	 *pn1 = 1;
	 *pn2 = 1;
	 *pn3 = 1;
	 ret = move_pages(pid, 4, pages, nodes, status, MPOL_MF_MOVE_ALL);
	 error_msg(ret, 4, status, "move 4 pages");
 }

 int main(int argc, char *argv[])
 {
	 numa_run_on_node(0);

	 test_migrate();

	 return 0;
 }
--------------------- test-migrate.sh ----------------------------
 #!/bin/bash

 PARAM=/sys/module/migrate/parameters/ei_migrate_pages

 get_vmstat()
 {
	 echo ================= $* ================
	 cat /proc/vmstat | grep -e '\(pgmigrate\|thp_migration\)'
 }

 simple_test()
 {
	 echo $1 > $PARAM
	 shift
	 get_vmstat before $*
	 ./test-migrate
	 get_vmstat after $*
 }

 #define EI_MP_ENOSYS		0x0001
 #define EI_MP_THP_ENOMEM	0x0002
 #define EI_MP_NP_ENOMEM		0x0004
 #define EI_MP_EAGAIN		0x0008
 #define EI_MP_EOTHER		0x0010
 #define EI_MP_NOSPLIT		0x0020
 #define EI_MP_SPLIT_FAIL	0x0040
 #define EI_MP_EAGAIN_PERM	0x0080
 #define EI_MP_EBUSY		0x0100

 simple_test 0x26 ENOMEM
 simple_test 0x81 retry THP subpages
 simple_test 0xc1 ENOSYS
 simple_test 0x101 ENOSYS

Powered by blists - more mailing lists