[PATCH 1/2] mm/madvise: MADV_COLLAPSE return EAGAIN when page cannot be isolated

[PATCH 1/2] mm/madvise: MADV_COLLAPSE return EAGAIN when page cannot be isolated

[Zach O'Keefe]

MADV_COLLAPSE is a best-effort request that attempts to set an
actionable errno value if the request cannot be fulfilled at the
time.  EAGAIN should be used to communicate that a resource was
temporarily unavailable, but that the user may try again immediately.

SCAN_DEL_PAGE_LRU is an internal result code used when a page
cannot be isolated from it's LRU list.  Since this, like SCAN_PAGE_LRU,
is likely a transitory state, make MADV_COLLAPSE return EAGAIN so that
users know they may reattempt the operation.

Another important scenario to consider is race with khugepaged.
khugepaged might isolate a page while MADV_COLLAPSE is interested in it.
Even though racing with khugepaged might mean that the memory has
already been collapsed, signalling an errno that is non-intrinsic to
that memory or arguments provided to madvise(2) lets the user know that
future attempts might (and in this case likely would) succeed, and
avoids false-negative assumptions by the user.

Fixes: 7d8faaf15545 ("mm/madvise: introduce MADV_COLLAPSE sync hugepage collapse")
Signed-off-by: Zach O'Keefe <zokeefe@google.com>
---
 mm/khugepaged.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index beaba3952dd1..5dcb2c34651e 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2374,6 +2374,7 @@ static int madvise_collapse_errno(enum scan_result r)
 	/* Resource temporary unavailable - trying again might succeed */
 	case SCAN_PAGE_LOCK:
 	case SCAN_PAGE_LRU:
+	case SCAN_DEL_PAGE_LRU:
 		return -EAGAIN;
 	/*
 	 * Other: Trying again likely not to succeed / error intrinsic to
@@ -2456,6 +2457,7 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
 		case SCAN_PAGE_LOCK:
 		case SCAN_PAGE_COMPOUND:
 		case SCAN_PAGE_LRU:
+		case SCAN_DEL_PAGE_LRU:
 			last_fail = result;
 			break;
 		default:
-- 
2.37.3.998.g577e59143f-goog

[PATCH 2/2] selftests/vm: retry on EAGAIN for MADV_COLLAPSE selftest

[Zach O'Keefe]

MADV_COLLAPSE is a best-effort request that will set errno to an
actionable value if the request cannot be performed.

For example, if pages are not found on the LRU, or if they are currently
locked by something else, MADV_COLLAPSE will fail and set errno to
EAGAIN to inform callers that they may try again.

Since the khugepaged selftest is the first public use of MADV_COLLAPSE,
set a best practice of checking errno and retrying on EAGAIN.

Fixes: 9330694de59f ("selftests/vm: add MADV_COLLAPSE collapse context to selftests")
Signed-off-by: Zach O'Keefe <zokeefe@google.com>
---
 tools/testing/selftests/vm/khugepaged.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/vm/khugepaged.c b/tools/testing/selftests/vm/khugepaged.c
index b77b1e28cdb3..b55dc331af13 100644
--- a/tools/testing/selftests/vm/khugepaged.c
+++ b/tools/testing/selftests/vm/khugepaged.c
@@ -1,4 +1,5 @@
 #define _GNU_SOURCE
+#include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <signal.h>
@@ -477,6 +478,26 @@ static void fill_memory(int *p, unsigned long start, unsigned long end)
 		p[i * page_size / sizeof(*p)] = i + 0xdead0000;
 }
 
+/*
+ * MADV_COLLAPSE is a best-effort request and may fail if an internal
+ * resource is temporarily unavailable, in which case it will set errno to
+ * EAGAIN.  In such a case, immediately reattempt the operation one more
+ * time.
+ */
+static int madvise_collapse_retry(void *p, unsigned long size)
+{
+	bool retry = true;
+	int ret;
+
+retry:
+	ret = madvise(p, size, MADV_COLLAPSE);
+	if (ret && errno == EAGAIN && retry) {
+		retry = false;
+		goto retry;
+	}
+	return ret;
+}
+
 /*
  * Returns pmd-mapped hugepage in VMA marked VM_HUGEPAGE, filled with
  * validate_memory()'able contents.
@@ -531,7 +552,7 @@ static void madvise_collapse(const char *msg, char *p, int nr_hpages,
 
 	/* Clear VM_NOHUGEPAGE */
 	madvise(p, nr_hpages * hpage_pmd_size, MADV_HUGEPAGE);
-	ret = madvise(p, nr_hpages * hpage_pmd_size, MADV_COLLAPSE);
+	ret = madvise_collapse_retry(p, nr_hpages * hpage_pmd_size);
 	if (((bool)ret) == expect)
 		fail("Fail: Bad return value");
 	else if (check_huge(p, nr_hpages) != expect)
-- 
2.37.3.998.g577e59143f-goog