move_pages12: handle errno EBUSY for madvise(..., MADV_SOFT_OFFLINE)

The test#2 is going to simulate the race condition, where move_pages() and soft offline are called on a single hugetlb page concurrently. But, it return EBUSY and report FAIL in soft-offline a moving hugepage as a result sometimes.

The root cause seems a call to page_huge_active return false, then the soft offline action will failed to isolate hugepage with EBUSY return as below call trace:

In Parent:
madvise(..., MADV_SOFT_OFFLINE)
...
soft_offline_page
soft_offline_in_use_page
soft_offline_huge_page
isolate_huge_page
page_huge_active
# return false at here

In Child:
move_pages()
...
do_move_pages
do_move_pages_to_node
add_page_for_migration
isolate_huge_page
# it has already isolated the hugepage

In this patch, I simply regard the returned EBUSY as a normal situation and mask it in error handler. Because move_pages is calling add_page_for_migration to isolate hugepage before do migration, so that's very possible to hit the collision and return EBUSY on the same page.

Error log:
----------
move_pages12.c:235: INFO: Free RAM 8386256 kB move_pages12.c:253: INFO: Increasing 2048kB hugepages pool on node 0 to 4 move_pages12.c:263: INFO: Increasing 2048kB hugepages pool on node 1 to 6 move_pages12.c:179: INFO: Allocating and freeing 4 hugepages on node 0 move_pages12.c:179: INFO: Allocating and freeing 4 hugepages on node 1 move_pages12.c:169: PASS: Bug not reproduced move_pages12.c:81: FAIL: madvise failed: SUCCESS move_pages12.c:81: FAIL: madvise failed: SUCCESS move_pages12.c:143: BROK: mmap((nil),4194304,3,262178,-1,0) failed: ENOMEM move_pages12.c:114: FAIL: move_pages failed: EINVAL

Dmesg:
------

[165435.492170] soft offline: 0x61c00 hugepage failed to isolate
[165435.590252] soft offline: 0x61c00 hugepage failed to isolate
[165435.725493] soft offline: 0x61400 hugepage failed to isolate

Other two fixes in this patch:
* use TERRNO(but not TTERRNO) to catch madvise(..., MADV_SOFT_OFFLINE) errno
* retry mmap when hugepage allocating failed with ENOMEM
* retry numa_move_pages when hitting ENOMEM

Signed-off-by Li Wang <[email protected]>

Cc Mike Kravetz <[email protected]>

Cc Naoya Horiguchi <[email protected]>

Cc Cyril Hrubis <[email protected]>

---

Notes:
Hi Cyril,

Since the SIGBUS problem has been marked as mainline kernel issue, and Mike has worked out a patch[1] to LKML for solving that. So we have no necessary to mask the SIGBUS again for move_page12. Here I take your suggest to retry mmap while hitting ENOMEM in the test looping.

And, after the patch[1] merging in linus kernel. I'll help to note the SIGBUS issue/patch in test#2 code comments via a seperate patch.

[1] https://lkml.org/lkml/2019/8/7/941

 .../kernel/syscalls/move_pages/move_pages12.c | 30 ++++++++++++++-----
 1 file changed, 22 insertions(+), 8 deletions(-)

testcases/kernel/syscalls/move_pages/move_pages12.c +22 -9

index 964b712fb..5d7afad8e 100644
@@ -77,8 +77,8 @@ static void *addr;
 static int do_soft_offline(int tpgs)
 {
 	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
-		if (errno != EINVAL)
-			tst_res(TFAIL | TTERRNO, "madvise failed");
+		if (errno != EINVAL && errno != EBUSY)
+			tst_res(TFAIL | TERRNO, "madvise failed");
 		return errno;
 	}
 	return 0;

@@ -111,6 +111,9 @@ static void do_child(int tpgs)
 		TEST(numa_move_pages(ppid, test_pages,
 			pages, nodes, status, MPOL_MF_MOVE_ALL));
 		if (TST_RET < 0) {
+			if (errno == ENOMEM)
+				continue;
+
 			tst_res(TFAIL | TTERRNO, "move_pages failed");
 			break;
 		}

@@ -121,7 +124,8 @@ static void do_child(int tpgs)
 
 static void do_test(unsigned int n)
 {
-	int i;
+	int i, ret;
+	void *ptr;
 	pid_t cpid = -1;
 	int status;
 	unsigned int twenty_percent = (tst_timeout_remaining() / 5);

@@ -136,18 +140,28 @@ static void do_test(unsigned int n)
 		do_child(tcases[n].tpages);
 
 	for (i = 0; i < LOOPS; i++) {
-		void *ptr;
+		ptr = mmap(NULL, tcases[n].tpages * hpsz,
+				PROT_READ | PROT_WRITE,
+				MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+		if (ptr == MAP_FAILED) {
+			if (i == 0)
+				tst_brk(TBROK | TERRNO, "Cannot allocate hugepage");
+
+			if (errno == ENOMEM) {
+				usleep(1000);
+				continue;
+			}
+		}
 
-		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
-			PROT_READ | PROT_WRITE,
-			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 		if (ptr != addr)
 			tst_brk(TBROK, "Failed to mmap at desired addr");
 
 		memset(addr, 0, tcases[n].tpages * hpsz);
 
 		if (tcases[n].offline) {
-			if (do_soft_offline(tcases[n].tpages) == EINVAL) {
+			ret = do_soft_offline(tcases[n].tpages);
+
+			if (ret == EINVAL) {
 				SAFE_KILL(cpid, SIGKILL);
 				SAFE_WAITPID(cpid, &status, 0);
 				SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
--

2.20.1

Hello,

Quoted 28 lines · Li Wang

The test#2 is going to simulate the race condition, where move_pages() and soft offline are called on a single hugetlb page concurrently. But, it return EBUSY and report FAIL in soft-offline a moving hugepage as a result sometimes.

The root cause seems a call to page_huge_active return false, then the soft offline action will failed to isolate hugepage with EBUSY return as below call trace:

In Parent:
madvise(..., MADV_SOFT_OFFLINE)
...
soft_offline_page
soft_offline_in_use_page
soft_offline_huge_page
isolate_huge_page
page_huge_active
# return false at here

In Child:
move_pages()
...
do_move_pages
do_move_pages_to_node
add_page_for_migration
isolate_huge_page
# it has already isolated the hugepage

Quoted 1 line

In this patch, I simply regard the returned EBUSY as a normal situation

and

Quoted 1 line

mask it in error handler. Because move_pages is calling

add_page_for_migration

Quoted 1 line

to isolate hugepage before do migration, so that's very possible to hit

the

Quoted 1 line

collision and return EBUSY on the same page.

We also get EIO (on aarch64) and ENOMEM (on x86). From looking at migrate_pages, this seems normal, although the behaviour on older kernels is different to newer ones.

On OpenSUSE with kernel 5.2 the test completes without any problem, but on SLES kernel 5.12 we get the other error codes.

TBH I'm not sure what we are testing when checking the return value of MADV_SOFT_OFFLINE? The bug is not reproduced if madvise always fails, so the test should pass right?

Can you help to check if these commits have been backported to SLEL kernel-5.12?

    commit e66f17ff71772b209eed39de35aaa99ba819c93d
    commit c9d398fa237882ea07167e23bcfc5e6847066518
    commit 4643d67e8cb0b3536ef0ab5cddd1cedc73fa14ad

The move_pages12 test actually found three regression BUG which has all been fixed in the mainline kernel so far.

Quoted 6 lines

TBH I'm not sure what we are testing when checking the return value of MADV_SOFT_OFFLINE? The bug is not reproduced if madvise always fails, so the test should pass right?

The return value checking of MADV_SOFT_OFFLINE are two:
EINVAL - To make sure system support MADV_SOFT_OFFLINE EBUSY - To make ignore the defect(EBUSY when soft-offlining hugepage)
designed by the kernel

The madvise(MADV_SOFT_OFFLINE) should not always fail, it might get failure occasionally on EBUSY(ignored already), but exit with TCONF if hit EINVAL.

@MIke & @Naoyo, If I was wrong please correct me.