[PATCH v6 0/6] selftests/mm: separate GUP microbenchmarking from functional testing

[PATCH v6 0/6] selftests/mm: separate GUP microbenchmarking from functional testing

[Sarthak Sharma]

gup_test.c currently serves two separate purposes: benchmarking
(GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK and PIN_LONGTERM_BENCHMARK) and
functional testing (GUP_BASIC_TEST, PIN_BASIC_TEST and
DUMP_USER_PAGES_TEST). Keeping both in one program makes the functional
tests harder to run and report individually, while run_vmtests.sh has to
invoke the program repeatedly with different options.

Separate these roles into tools/mm/gup_bench for benchmarking and
tools/testing/selftests/mm/gup for functional testing. Move the shared
file and hugepage helpers to tools/lib/mm/ so both programs can use them
without duplicating the implementation.

Patch 1 makes read_file(), write_file(), read_num() and write_num()
return errors to their callers instead of exiting. It also makes
read_num() reject negative and malformed values and updates the existing
callers to handle failures.

Patch 2 moves these file helpers from vm_util.c to tools/lib/mm/. It keeps
them available to the mm selftests through vm_util.h and adjusts the
selftests build accordingly.

Patch 3 moves hugepage_settings.[ch] from selftests/mm to tools/lib/mm/.
It also removes its kselftest dependency while preserving TAP-compatible
diagnostics for selftest users.

Patch 4 moves the existing gup_test implementation from selftests/mm to
tools/mm as gup_bench. This keeps the code movement separate from the
subsequent changes and makes it easier to review.

Patch 5 removes the functional test modes and kselftest dependency from
gup_bench. When run without arguments, it performs one GUP_FAST benchmark
using the existing defaults instead of running the whole matrix. Other 
benchmark configurations can be selected through command-line options.

Patch 6 adds a new harness-based GUP selftest. It covers THP, non-THP and
HugeTLB mappings across private/shared and read/write variants. For each
variant, it tests get_user_pages(), get_user_pages_fast(),
pin_user_pages(), pin_user_pages_fast(), long-term pinning and both dump
modes using four batch sizes. It also preserves the existing sparse dump
coverage for pages 0, 19 and 0x1000.

---
Changes in v6:
- Rebase onto v7.2-rc5
- Address feedback from David, Mark, John, Dev and Sashiko
- Keep the shared file helpers silent and let callers report errors
- Move the existing gup_test into tools/mm before changing its behaviour
- Report ioctl failures from gup_bench worker threads
- Remove the default benchmark matrix and run one default benchmark instead
- Add the new harness based functional selftest in a separate final patch
- Drop the unrelated thp_swap_allocator_test .gitignore change

Changes in v5:
- Rebase onto v7.2-rc3
- Address feedback from Mike, John, Dev and Sashiko
- Make file helper diagnostics TAP-compatible
- Validate read_num() input and propagate errors while restoring shared
  memory limits
- Preserve build bisectability for the x86 protection_keys test
- Preserve HugeTLB diagnostics using TAP-compatible printf() output
- Validate the remaining numeric arguments accepted by gup_bench

Changes in v4:
- Address review feedback from Mike and Sashiko
- Add a preparatory patch so shared file helpers return errors instead of exiting
- Reduce include churn by keeping shared helpers exposed through vm_util.h
- Preserve HugeTLB diagnostics and restore HugeTLB state more carefully
- Fix selftests/mm build details after moving helpers to tools/lib/mm
- Tighten gup_bench argument handling and gup_test setup/sparse-dump coverage

Changes in v3:
- Address v2 feedback from Sashiko
- Add shared file_utils helpers under tools/lib/mm
- Move hugepage_settings out of selftests and into tools/lib/mm
- Convert gup_bench to use the shared tools/lib/mm helpers
- Guard against invalid thread counts in gup_bench
- Handle thread-array allocation failure cleanly in gup_bench
- Restore HugeTLB settings on setup failure in gup_test
- Add sparse DUMP_USER_PAGES_TEST coverage for pages 0, 19 and 0x1000

Changes in v2:
- Address v1 feedback from Sashiko
- Add fast and long-term GUP/PUP coverage
- Sweep nr_pages_per_call over 1, 512, 123 and all pages
- Call madvise(MADV_NOHUGEPAGE) in non-THP variants
- Use 256 MB for HugeTLB fixtures
- Restore HugeTLB settings from fixture teardown
- Report nr_pages_per_call for each iteration
- Update the pin_user_pages unit-testing documentation

Previous versions:
v5: lore.kernel.org/all/20260716123226.197736-1-sarthak.sharma@arm.com/
v4: lore.kernel.org/all/20260527142432.230127-1-sarthak.sharma@arm.com/
v3: lore.kernel.org/all/20260521111801.173019-1-sarthak.sharma@arm.com/
v2: lore.kernel.org/all/20260519120506.184512-1-sarthak.sharma@arm.com/
v1: lore.kernel.org/all/20260515084840.174652-1-sarthak.sharma@arm.com/

Sarthak Sharma (6):
  selftests/mm: make file helpers return errors
  tools/lib/mm: add shared file helpers
  tools/lib/mm: move hugepage_settings out of selftests
  tools/mm: move gup_test from selftests/mm to tools/mm
  tools/mm: make gup_bench a benchmark only tool
  selftests/mm: add a GUP selftest

 Documentation/core-api/pin_user_pages.rst     |  14 +-
 MAINTAINERS                                   |   4 +-
 tools/lib/mm/file_utils.c                     |  96 +++++
 tools/lib/mm/file_utils.h                     |  12 +
 .../selftests => lib}/mm/hugepage_settings.c  | 111 +++++-
 .../selftests => lib}/mm/hugepage_settings.h  |   0
 tools/mm/.gitignore                           |   1 +
 tools/mm/Makefile                             |  10 +-
 tools/mm/gup_bench.c                          | 347 ++++++++++++++++++
 tools/testing/selftests/mm/.gitignore         |   2 +-
 tools/testing/selftests/mm/Makefile           |  15 +-
 tools/testing/selftests/mm/compaction_test.c  |   2 +-
 tools/testing/selftests/mm/cow.c              |   1 -
 .../selftests/mm/folio_split_race_test.c      |   1 -
 tools/testing/selftests/mm/guard-regions.c    |   1 -
 tools/testing/selftests/mm/gup.c              | 341 +++++++++++++++++
 tools/testing/selftests/mm/gup_longterm.c     |   1 -
 tools/testing/selftests/mm/gup_test.c         | 275 --------------
 tools/testing/selftests/mm/hmm-tests.c        |   6 +-
 tools/testing/selftests/mm/hugetlb-madvise.c  |   1 -
 tools/testing/selftests/mm/hugetlb-mmap.c     |   1 -
 tools/testing/selftests/mm/hugetlb-mremap.c   |   1 -
 tools/testing/selftests/mm/hugetlb-shm.c      |   1 -
 .../selftests/mm/hugetlb-soft-offline.c       |   2 +-
 tools/testing/selftests/mm/hugetlb-vmemmap.c  |   1 -
 tools/testing/selftests/mm/hugetlb_dio.c      |   1 -
 .../selftests/mm/hugetlb_fault_after_madv.c   |   1 -
 .../selftests/mm/hugetlb_madv_vs_map.c        |   1 -
 tools/testing/selftests/mm/khugepaged.c       |  15 +-
 tools/testing/selftests/mm/ksm_tests.c        |   1 -
 tools/testing/selftests/mm/migration.c        |   5 +-
 tools/testing/selftests/mm/pagemap_ioctl.c    |   1 -
 .../testing/selftests/mm/prctl_thp_disable.c  |   1 -
 tools/testing/selftests/mm/protection_keys.c  |   2 +-
 tools/testing/selftests/mm/run_vmtests.sh     |  37 +-
 tools/testing/selftests/mm/soft-dirty.c       |   1 -
 .../selftests/mm/split_huge_page_test.c       |   6 +-
 tools/testing/selftests/mm/thuge-gen.c        |   1 -
 tools/testing/selftests/mm/transhuge-stress.c |   1 -
 tools/testing/selftests/mm/uffd-common.h      |   1 -
 tools/testing/selftests/mm/uffd-wp-mremap.c   |   2 +-
 .../selftests/mm/va_high_addr_switch.c        |   1 -
 tools/testing/selftests/mm/vm_util.c          | 103 ++----
 tools/testing/selftests/mm/vm_util.h          |   7 +-
 44 files changed, 977 insertions(+), 458 deletions(-)
 create mode 100644 tools/lib/mm/file_utils.c
 create mode 100644 tools/lib/mm/file_utils.h
 rename tools/{testing/selftests => lib}/mm/hugepage_settings.c (87%)
 rename tools/{testing/selftests => lib}/mm/hugepage_settings.h (100%)
 create mode 100644 tools/mm/gup_bench.c
 create mode 100644 tools/testing/selftests/mm/gup.c
 delete mode 100644 tools/testing/selftests/mm/gup_test.c


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.39.5

[PATCH v6 1/6] selftests/mm: make file helpers return errors

[Sarthak Sharma]

Change read_file(), write_file(), read_num() and write_num() in vm_util.c
to report failures to callers instead of exiting from the helper.

Make read_file() return a negative errno on failure instead of 0, so
callers can distinguish a successful read from an I/O error. Also make
read_num() reject negative and malformed values.

Update callers to print diagnostics and fail wherever required. This
patch prepares the helpers to be moved to tools/lib/mm without
kselftest dependency.

Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 .../testing/selftests/mm/hugepage_settings.c  |  97 ++++++++++++++---
 tools/testing/selftests/mm/khugepaged.c       |  14 ++-
 .../selftests/mm/split_huge_page_test.c       |   5 +-
 tools/testing/selftests/mm/vm_util.c          | 102 +++++++++++++-----
 tools/testing/selftests/mm/vm_util.h          |   6 +-
 5 files changed, 175 insertions(+), 49 deletions(-)

diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
index 2eab2110ac6a..db0db8a3df7c 100644
--- a/tools/testing/selftests/mm/hugepage_settings.c
+++ b/tools/testing/selftests/mm/hugepage_settings.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <errno.h>
 
 #include "vm_util.h"
 #include "hugepage_settings.h"
@@ -61,8 +62,10 @@ int thp_read_string(const char *name, const char * const strings[])
 		exit(EXIT_FAILURE);
 	}
 
-	if (!read_file(path, buf, sizeof(buf))) {
-		perror(path);
+	ret = read_file(path, buf, sizeof(buf));
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
 		exit(EXIT_FAILURE);
 	}
 
@@ -103,12 +106,18 @@ void thp_write_string(const char *name, const char *val)
 		printf("%s: Pathname is too long\n", __func__);
 		exit(EXIT_FAILURE);
 	}
-	write_file(path, val, strlen(val) + 1);
+	ret = write_file(path, val, strlen(val) + 1);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
 }
 
 unsigned long thp_read_num(const char *name)
 {
 	char path[PATH_MAX];
+	unsigned long num;
 	int ret;
 
 	ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
@@ -116,7 +125,14 @@ unsigned long thp_read_num(const char *name)
 		printf("%s: Pathname is too long\n", __func__);
 		exit(EXIT_FAILURE);
 	}
-	return read_num(path);
+	ret = read_num(path, &num);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
+
+	return num;
 }
 
 void thp_write_num(const char *name, unsigned long num)
@@ -129,7 +145,12 @@ void thp_write_num(const char *name, unsigned long num)
 		printf("%s: Pathname is too long\n", __func__);
 		exit(EXIT_FAILURE);
 	}
-	write_num(path, num);
+	ret = write_num(path, num);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
 }
 
 void thp_read_settings(struct thp_settings *settings)
@@ -157,8 +178,16 @@ void thp_read_settings(struct thp_settings *settings)
 		.max_ptes_shared = thp_read_num("khugepaged/max_ptes_shared"),
 		.pages_to_scan = thp_read_num("khugepaged/pages_to_scan"),
 	};
-	if (dev_queue_read_ahead_path[0])
-		settings->read_ahead_kb = read_num(dev_queue_read_ahead_path);
+	if (dev_queue_read_ahead_path[0]) {
+		int ret = read_num(dev_queue_read_ahead_path,
+				   &settings->read_ahead_kb);
+
+		if (ret < 0) {
+			errno = -ret;
+			ksft_perror(dev_queue_read_ahead_path);
+			exit(EXIT_FAILURE);
+		}
+	}
 
 	for (i = 0; i < NR_ORDERS; i++) {
 		if (!((1 << i) & orders)) {
@@ -208,8 +237,16 @@ void thp_write_settings(struct thp_settings *settings)
 	thp_write_num("khugepaged/max_ptes_shared", khugepaged->max_ptes_shared);
 	thp_write_num("khugepaged/pages_to_scan", khugepaged->pages_to_scan);
 
-	if (dev_queue_read_ahead_path[0])
-		write_num(dev_queue_read_ahead_path, settings->read_ahead_kb);
+	if (dev_queue_read_ahead_path[0]) {
+		int ret = write_num(dev_queue_read_ahead_path,
+				    settings->read_ahead_kb);
+
+		if (ret < 0) {
+			errno = -ret;
+			ksft_perror(dev_queue_read_ahead_path);
+			exit(EXIT_FAILURE);
+		}
+	}
 
 	for (i = 0; i < NR_ORDERS; i++) {
 		if (!((1 << i) & orders))
@@ -307,8 +344,16 @@ static unsigned long __thp_supported_orders(bool is_shmem)
 		}
 
 		ret = read_file(path, buf, sizeof(buf));
-		if (ret)
-			orders |= 1UL << i;
+		if (ret < 0) {
+			if (ret != -ENOENT) {
+				errno = -ret;
+				ksft_perror(path);
+				exit(EXIT_FAILURE);
+			}
+			continue;
+		}
+
+		orders |= 1UL << i;
 	}
 
 	return orders;
@@ -425,28 +470,52 @@ static void hugetlb_sysfs_path(char *buf, size_t buflen,
 unsigned long hugetlb_nr_pages(unsigned long size)
 {
 	char path[PATH_MAX];
+	unsigned long nr;
+	int ret;
 
 	hugetlb_sysfs_path(path, sizeof(path), size, "nr_hugepages");
 
-	return read_num(path);
+	ret = read_num(path, &nr);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
+
+	return nr;
 }
 
 void hugetlb_set_nr_pages(unsigned long size, unsigned long nr)
 {
 	char path[PATH_MAX];
+	int ret;
 
 	hugetlb_sysfs_path(path, sizeof(path), size, "nr_hugepages");
 
-	write_num(path, nr);
+	ret = write_num(path, nr);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
 }
 
 unsigned long hugetlb_free_pages(unsigned long size)
 {
 	char path[PATH_MAX];
+	unsigned long nr;
+	int ret;
 
 	hugetlb_sysfs_path(path, sizeof(path), size, "free_hugepages");
 
-	return read_num(path);
+	ret = read_num(path, &nr);
+	if (ret < 0) {
+		errno = -ret;
+		ksft_perror(path);
+		exit(EXIT_FAILURE);
+	}
+
+	return nr;
 }
 
 static bool __hugetlb_setup(unsigned long size, unsigned long nr)
diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 10e8dedcb087..6a656bc6b266 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -119,6 +119,7 @@ static void get_finfo(const char *dir)
 	char buf[1 << 10];
 	char path[PATH_MAX];
 	char *str, *end;
+	int ret;
 
 	finfo.dir = dir;
 	stat(finfo.dir, &path_stat);
@@ -138,8 +139,9 @@ static void get_finfo(const char *dir)
 		     major(path_stat.st_dev), minor(path_stat.st_dev))
 	    >= sizeof(path))
 		ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
-	if (read_file(path, buf, sizeof(buf)) < 0)
-		ksft_exit_fail_perror("read_file(read_num)");
+	ret = read_file(path, buf, sizeof(buf));
+	if (ret < 0)
+		ksft_exit_fail_msg("read_file(%s): %s\n", path, strerror(-ret));
 	if (strstr(buf, "DEVTYPE=disk")) {
 		/* Found it */
 		if (snprintf(finfo.dev_queue_read_ahead_path,
@@ -319,7 +321,7 @@ static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup)
 {
 	const int open_opt = setup == FILE_SETUP_READ_ONLY_FS ? O_RDONLY : O_RDWR;
 	const int mmap_prot = setup == FILE_SETUP_READ_ONLY_FS ? PROT_READ : (PROT_READ | PROT_WRITE);
-	int fd;
+	int fd, ret;
 	void *p;
 	unsigned long size;
 
@@ -363,7 +365,11 @@ static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup)
 		ksft_exit_fail_perror("mmap()");
 
 	/* Drop page cache */
-	write_file("/proc/sys/vm/drop_caches", "3", 2);
+	ret = write_file("/proc/sys/vm/drop_caches", "3", 2);
+	if (ret < 0)
+		ksft_exit_fail_msg("write_file(drop_caches): %s\n",
+				   strerror(-ret));
+
 	success("OK");
 	return p;
 }
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index 32b991472f74..ef5ab96adc7e 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -269,7 +269,10 @@ static void write_debugfs(const char *fmt, ...)
 	if (ret >= INPUT_MAX)
 		ksft_exit_fail_msg("%s: Debugfs input is too long\n", __func__);
 
-	write_file(SPLIT_DEBUGFS, input, ret + 1);
+	ret = write_file(SPLIT_DEBUGFS, input, ret + 1);
+	if (ret < 0)
+		ksft_exit_fail_msg("write_file(%s): %s\n", SPLIT_DEBUGFS,
+				   strerror(-ret));
 }
 
 static char *allocate_zero_filled_hugepage(size_t len)
diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index 311fc5b4513e..e3ec077cda67 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -700,91 +700,139 @@ int unpoison_memory(unsigned long pfn)
 
 int read_file(const char *path, char *buf, size_t buflen)
 {
-	int fd;
+	int fd, err;
 	ssize_t numread;
 
 	fd = open(path, O_RDONLY);
 	if (fd == -1)
-		return 0;
+		return -errno;
 
 	numread = read(fd, buf, buflen - 1);
 	if (numread < 1) {
+		err = numread ? errno : ENODATA;
 		close(fd);
-		return 0;
+		return -err;
 	}
 
 	buf[numread] = '\0';
 	close(fd);
 
-	return (unsigned int) numread;
+	return (int)numread;
 }
 
-void write_file(const char *path, const char *buf, size_t buflen)
+int write_file(const char *path, const char *buf, size_t buflen)
 {
 	int fd, saved_errno;
 	ssize_t numwritten;
 
 	if (buflen < 2)
-		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
+		return -EINVAL;
 
 	fd = open(path, O_WRONLY);
 	if (fd == -1)
-		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
+		return -errno;
 
 	numwritten = write(fd, buf, buflen - 1);
 	saved_errno = errno;
 	close(fd);
-	errno = saved_errno;
+
 	if (numwritten < 0)
-		ksft_exit_fail_msg("%s write(%.*s) failed: %s\n", path, (int)(buflen - 1),
-				buf, strerror(errno));
-	if (numwritten != buflen - 1)
-		ksft_exit_fail_msg("%s write(%.*s) is truncated, expected %zu bytes, got %zd bytes\n",
-				path, (int)(buflen - 1), buf, buflen - 1, numwritten);
+		return -saved_errno;
+
+	if (numwritten != (ssize_t)(buflen - 1))
+		return -EIO;
+
+	return 0;
 }
 
-unsigned long read_num(const char *path)
+int read_num(const char *path, unsigned long *num)
 {
+	unsigned long val;
+	int ret;
 	char buf[21];
+	char *end;
 
-	if (read_file(path, buf, sizeof(buf)) < 0)
-		ksft_exit_fail_perror("read_file()");
+	if (!num)
+		return -EINVAL;
 
-	return strtoul(buf, NULL, 10);
+	ret = read_file(path, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	errno = 0;
+	val = strtoul(buf, &end, 10);
+	if (errno)
+		return -errno;
+
+	if (end == buf || buf[0] == '-')
+		return -EINVAL;
+
+	if (*end == '\n')
+		end++;
+
+	if (*end != '\0')
+		return -EINVAL;
+
+	*num = val;
+	return 0;
 }
 
-void write_num(const char *path, unsigned long num)
+int write_num(const char *path, unsigned long num)
 {
 	char buf[21];
 
 	sprintf(buf, "%lu", num);
-	write_file(path, buf, strlen(buf) + 1);
+	return write_file(path, buf, strlen(buf) + 1);
 }
 
 static unsigned long shmall, shmmax;
 
 void __shm_limits_restore(void)
 {
-	if (shmmax)
-		write_num("/proc/sys/kernel/shmmax", shmmax);
-	if (shmall)
-		write_num("/proc/sys/kernel/shmall", shmall);
+	int ret;
+
+	if (shmmax) {
+		ret = write_num("/proc/sys/kernel/shmmax", shmmax);
+		if (ret < 0)
+			ksft_exit_fail_msg("Failed to restore shmmax: %s\n",
+					   strerror(-ret));
+	}
+	if (shmall) {
+		ret = write_num("/proc/sys/kernel/shmall", shmall);
+		if (ret < 0)
+			ksft_exit_fail_msg("Failed to restore shmall: %s\n",
+					   strerror(-ret));
+	}
 }
 
 void shm_limits_prepare(unsigned long length)
 {
 	unsigned long nr = length / psize();
 	unsigned long val;
+	int ret;
+
+	ret = read_num("/proc/sys/kernel/shmmax", &val);
+	if (ret < 0)
+		ksft_exit_fail_msg("Failed to read /proc/sys/kernel/shmmax: %s\n",
+				   strerror(-ret));
 
-	val = read_num("/proc/sys/kernel/shmmax");
 	if (val < length) {
-		write_num("/proc/sys/kernel/shmmax", length);
+		ret = write_num("/proc/sys/kernel/shmmax", length);
+		if (ret < 0)
+			ksft_exit_fail_msg("Failed to write %lu to /proc/sys/kernel/shmmax: %s\n",
+					   length, strerror(-ret));
 		shmmax = val;
 	}
 
-	val = read_num("/proc/sys/kernel/shmall");
+	ret = read_num("/proc/sys/kernel/shmall", &val);
+	if (ret < 0)
+		ksft_exit_fail_msg("Failed to read /proc/sys/kernel/shmall: %s\n",
+				   strerror(-ret));
 	if (val < nr) {
-		write_num("/proc/sys/kernel/shmall", nr);
+		ret = write_num("/proc/sys/kernel/shmall", nr);
+		if (ret < 0)
+			ksft_exit_fail_msg("Failed to write %lu to /proc/sys/kernel/shmall: %s\n",
+					   nr, strerror(-ret));
 		shmall = val;
 	}
 }
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index ea8fc8fdf0eb..28c3d7c1faed 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -164,10 +164,10 @@ int unpoison_memory(unsigned long pfn);
 #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
 #define PAGEMAP_PFN(ent)	((ent) & ((1ull << 55) - 1))
 
-void write_file(const char *path, const char *buf, size_t buflen);
 int read_file(const char *path, char *buf, size_t buflen);
-unsigned long read_num(const char *path);
-void write_num(const char *path, unsigned long num);
+int write_file(const char *path, const char *buf, size_t buflen);
+int read_num(const char *path, unsigned long *num);
+int write_num(const char *path, unsigned long num);
 
 void shm_limits_prepare(unsigned long length);
 void __shm_limits_restore(void);
-- 
2.39.5

[PATCH v6 2/6] tools/lib/mm: add shared file helpers

[Sarthak Sharma]

Move read_file(), write_file(), read_num(), and write_num() out of
tools/testing/selftests/mm/vm_util.c into a new shared helper under
tools/lib/mm/.

These helpers are used by mm selftests today and will also be needed by
shared hugepage helpers in subsequent patches. Move them to a generic
location so they can be reused outside selftests as well.

Keep the helpers exposed to mm selftests through vm_util.h by including
the new shared header there, and link the new helper into the
selftests/mm build.

Update the explicit x86 protection_keys 32-bit and 64-bit build rules
to preserve prerequisite paths, now that file_utils.c is built from
tools/lib/mm.

Add tools/lib/mm/ to the MEMORY MANAGEMENT - MISC entry in MAINTAINERS.

Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 MAINTAINERS                          |  1 +
 tools/lib/mm/file_utils.c            | 96 ++++++++++++++++++++++++++++
 tools/lib/mm/file_utils.h            | 12 ++++
 tools/testing/selftests/mm/Makefile  | 11 ++--
 tools/testing/selftests/mm/vm_util.c | 87 -------------------------
 tools/testing/selftests/mm/vm_util.h |  6 +-
 6 files changed, 116 insertions(+), 97 deletions(-)
 create mode 100644 tools/lib/mm/file_utils.c
 create mode 100644 tools/lib/mm/file_utils.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 716acfc3d7c1..040a8e6664f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17131,6 +17131,7 @@ F:	mm/memory-tiers.c
 F:	mm/page_idle.c
 F:	mm/pgalloc-track.h
 F:	mm/process_vm_access.c
+F:	tools/lib/mm/
 F:	tools/testing/selftests/mm/
 
 MEMORY MANAGEMENT - NUMA MEMBLOCKS AND NUMA EMULATION
diff --git a/tools/lib/mm/file_utils.c b/tools/lib/mm/file_utils.c
new file mode 100644
index 000000000000..3e023052a668
--- /dev/null
+++ b/tools/lib/mm/file_utils.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "file_utils.h"
+
+int read_file(const char *path, char *buf, size_t buflen)
+{
+	int fd, err;
+	ssize_t numread;
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1)
+		return -errno;
+
+	numread = read(fd, buf, buflen - 1);
+	if (numread < 1) {
+		err = numread ? errno : ENODATA;
+		close(fd);
+		return -err;
+	}
+
+	buf[numread] = '\0';
+	close(fd);
+
+	return (int)numread;
+}
+
+int write_file(const char *path, const char *buf, size_t buflen)
+{
+	int fd, saved_errno;
+	ssize_t numwritten;
+
+	if (buflen < 2)
+		return -EINVAL;
+
+	fd = open(path, O_WRONLY);
+	if (fd == -1)
+		return -errno;
+
+	numwritten = write(fd, buf, buflen - 1);
+	saved_errno = errno;
+	close(fd);
+
+	if (numwritten < 0)
+		return -saved_errno;
+
+	if (numwritten != (ssize_t)(buflen - 1))
+		return -EIO;
+
+	return 0;
+}
+
+int read_num(const char *path, unsigned long *num)
+{
+	unsigned long val;
+	int ret;
+	char buf[21];
+	char *end;
+
+	if (!num)
+		return -EINVAL;
+
+	ret = read_file(path, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	errno = 0;
+	val = strtoul(buf, &end, 10);
+	if (errno)
+		return -errno;
+
+	if (end == buf || buf[0] == '-')
+		return -EINVAL;
+
+	if (*end == '\n')
+		end++;
+
+	if (*end != '\0')
+		return -EINVAL;
+
+	*num = val;
+	return 0;
+}
+
+int write_num(const char *path, unsigned long num)
+{
+	char buf[21];
+
+	sprintf(buf, "%lu", num);
+	return write_file(path, buf, strlen(buf) + 1);
+}
diff --git a/tools/lib/mm/file_utils.h b/tools/lib/mm/file_utils.h
new file mode 100644
index 000000000000..38576790a342
--- /dev/null
+++ b/tools/lib/mm/file_utils.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __MM_FILE_UTILS_H__
+#define __MM_FILE_UTILS_H__
+
+#include <stddef.h>
+
+int read_file(const char *path, char *buf, size_t buflen);
+int write_file(const char *path, const char *buf, size_t buflen);
+int read_num(const char *path, unsigned long *num);
+int write_num(const char *path, unsigned long num);
+
+#endif
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index e6df968f0971..70d6a9ccf9c9 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -37,7 +37,8 @@ endif
 # LDLIBS.
 MAKEFLAGS += --no-builtin-rules
 
-CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
+CFLAGS = -Wall -O2 -I $(top_srcdir) -I $(top_srcdir)/tools/lib
+CFLAGS += $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
 CFLAGS += -Wunreachable-code
 LDLIBS = -lrt -lpthread -lm
 
@@ -187,8 +188,8 @@ TEST_FILES += write_hugetlb_memory.sh
 
 include ../lib.mk
 
-$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c
-$(TEST_GEN_FILES): vm_util.c hugepage_settings.c
+$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
+$(TEST_GEN_FILES): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
 
 $(OUTPUT)/uffd-stress: uffd-common.c
 $(OUTPUT)/uffd-unit-tests: uffd-common.c
@@ -217,7 +218,7 @@ $(BINARIES_32): CFLAGS += -m32 -mxsave
 $(BINARIES_32): LDLIBS += -lrt -ldl -lm
 $(BINARIES_32): $(OUTPUT)/%_32: %.c
 	$(call msg,CC,,$@)
-	$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(notdir $^) $(LDLIBS) -o $@
+	$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $^ $(LDLIBS) -o $@
 $(foreach t,$(VMTARGETS),$(eval $(call gen-target-rule-32,$(t))))
 endif
 
@@ -226,7 +227,7 @@ $(BINARIES_64): CFLAGS += -m64 -mxsave
 $(BINARIES_64): LDLIBS += -lrt -ldl
 $(BINARIES_64): $(OUTPUT)/%_64: %.c
 	$(call msg,CC,,$@)
-	$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(notdir $^) $(LDLIBS) -o $@
+	$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $^ $(LDLIBS) -o $@
 $(foreach t,$(VMTARGETS),$(eval $(call gen-target-rule-64,$(t))))
 endif
 
diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index e3ec077cda67..36515b098ad0 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -698,93 +698,6 @@ int unpoison_memory(unsigned long pfn)
 	return ret > 0 ? 0 : -errno;
 }
 
-int read_file(const char *path, char *buf, size_t buflen)
-{
-	int fd, err;
-	ssize_t numread;
-
-	fd = open(path, O_RDONLY);
-	if (fd == -1)
-		return -errno;
-
-	numread = read(fd, buf, buflen - 1);
-	if (numread < 1) {
-		err = numread ? errno : ENODATA;
-		close(fd);
-		return -err;
-	}
-
-	buf[numread] = '\0';
-	close(fd);
-
-	return (int)numread;
-}
-
-int write_file(const char *path, const char *buf, size_t buflen)
-{
-	int fd, saved_errno;
-	ssize_t numwritten;
-
-	if (buflen < 2)
-		return -EINVAL;
-
-	fd = open(path, O_WRONLY);
-	if (fd == -1)
-		return -errno;
-
-	numwritten = write(fd, buf, buflen - 1);
-	saved_errno = errno;
-	close(fd);
-
-	if (numwritten < 0)
-		return -saved_errno;
-
-	if (numwritten != (ssize_t)(buflen - 1))
-		return -EIO;
-
-	return 0;
-}
-
-int read_num(const char *path, unsigned long *num)
-{
-	unsigned long val;
-	int ret;
-	char buf[21];
-	char *end;
-
-	if (!num)
-		return -EINVAL;
-
-	ret = read_file(path, buf, sizeof(buf));
-	if (ret < 0)
-		return ret;
-
-	errno = 0;
-	val = strtoul(buf, &end, 10);
-	if (errno)
-		return -errno;
-
-	if (end == buf || buf[0] == '-')
-		return -EINVAL;
-
-	if (*end == '\n')
-		end++;
-
-	if (*end != '\0')
-		return -EINVAL;
-
-	*num = val;
-	return 0;
-}
-
-int write_num(const char *path, unsigned long num)
-{
-	char buf[21];
-
-	sprintf(buf, "%lu", num);
-	return write_file(path, buf, strlen(buf) + 1);
-}
-
 static unsigned long shmall, shmmax;
 
 void __shm_limits_restore(void)
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index 28c3d7c1faed..d45135283732 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -8,6 +8,7 @@
 #include <unistd.h> /* _SC_PAGESIZE */
 #include "kselftest.h"
 #include <linux/fs.h>
+#include <mm/file_utils.h>
 
 #define BIT_ULL(nr)                   (1ULL << (nr))
 #define PM_SOFT_DIRTY                 BIT_ULL(55)
@@ -164,11 +165,6 @@ int unpoison_memory(unsigned long pfn);
 #define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
 #define PAGEMAP_PFN(ent)	((ent) & ((1ull << 55) - 1))
 
-int read_file(const char *path, char *buf, size_t buflen);
-int write_file(const char *path, const char *buf, size_t buflen);
-int read_num(const char *path, unsigned long *num);
-int write_num(const char *path, unsigned long num);
-
 void shm_limits_prepare(unsigned long length);
 void __shm_limits_restore(void);
 
-- 
2.39.5

[PATCH v6 3/6] tools/lib/mm: move hugepage_settings out of selftests

[Sarthak Sharma]

Move hugepage_settings.[ch] from tools/testing/selftests/mm/ to
tools/lib/mm/ so the THP and HugeTLB helpers can be shared more easily
between selftests and other tools.

Keep the helpers exposed to mm selftests through vm_util.h where possible,
and use direct <mm/hugepage_settings.h> includes for files that do not
include vm_util.h. Adjust the selftests/mm build to compile the moved
implementation from its new location.

Remove the remaining kselftest dependency from hugepage_settings.c.
Replace ksft_perror() with a new print_file_error() helper and
ksft_print_msg() with printf(). Prefix both with a # to keep them
TAP compatible.

Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 .../selftests => lib}/mm/hugepage_settings.c  | 54 ++++++++++---------
 .../selftests => lib}/mm/hugepage_settings.h  |  0
 tools/testing/selftests/mm/Makefile           |  6 ++-
 tools/testing/selftests/mm/compaction_test.c  |  2 +-
 tools/testing/selftests/mm/cow.c              |  1 -
 .../selftests/mm/folio_split_race_test.c      |  1 -
 tools/testing/selftests/mm/guard-regions.c    |  1 -
 tools/testing/selftests/mm/gup_longterm.c     |  1 -
 tools/testing/selftests/mm/gup_test.c         |  1 -
 tools/testing/selftests/mm/hmm-tests.c        |  6 +--
 tools/testing/selftests/mm/hugetlb-madvise.c  |  1 -
 tools/testing/selftests/mm/hugetlb-mmap.c     |  1 -
 tools/testing/selftests/mm/hugetlb-mremap.c   |  1 -
 tools/testing/selftests/mm/hugetlb-shm.c      |  1 -
 .../selftests/mm/hugetlb-soft-offline.c       |  2 +-
 tools/testing/selftests/mm/hugetlb-vmemmap.c  |  1 -
 tools/testing/selftests/mm/hugetlb_dio.c      |  1 -
 .../selftests/mm/hugetlb_fault_after_madv.c   |  1 -
 .../selftests/mm/hugetlb_madv_vs_map.c        |  1 -
 tools/testing/selftests/mm/khugepaged.c       |  1 -
 tools/testing/selftests/mm/ksm_tests.c        |  1 -
 tools/testing/selftests/mm/migration.c        |  5 +-
 tools/testing/selftests/mm/pagemap_ioctl.c    |  1 -
 .../testing/selftests/mm/prctl_thp_disable.c  |  1 -
 tools/testing/selftests/mm/protection_keys.c  |  2 +-
 tools/testing/selftests/mm/soft-dirty.c       |  1 -
 .../selftests/mm/split_huge_page_test.c       |  1 -
 tools/testing/selftests/mm/thuge-gen.c        |  1 -
 tools/testing/selftests/mm/transhuge-stress.c |  1 -
 tools/testing/selftests/mm/uffd-common.h      |  1 -
 tools/testing/selftests/mm/uffd-wp-mremap.c   |  2 +-
 .../selftests/mm/va_high_addr_switch.c        |  1 -
 tools/testing/selftests/mm/vm_util.h          |  1 +
 33 files changed, 43 insertions(+), 60 deletions(-)
 rename tools/{testing/selftests => lib}/mm/hugepage_settings.c (94%)
 rename tools/{testing/selftests => lib}/mm/hugepage_settings.h (100%)

diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/lib/mm/hugepage_settings.c
similarity index 94%
rename from tools/testing/selftests/mm/hugepage_settings.c
rename to tools/lib/mm/hugepage_settings.c
index db0db8a3df7c..4471f4989dbb 100644
--- a/tools/testing/selftests/mm/hugepage_settings.c
+++ b/tools/lib/mm/hugepage_settings.c
@@ -10,11 +10,16 @@
 #include <unistd.h>
 #include <errno.h>
 
-#include "vm_util.h"
+#include "file_utils.h"
 #include "hugepage_settings.h"
 
 #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
 #define MAX_SETTINGS_DEPTH 4
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#endif
+
 static struct thp_settings settings_stack[MAX_SETTINGS_DEPTH];
 static int settings_index;
 static struct thp_settings saved_settings;
@@ -49,6 +54,13 @@ static const char * const shmem_enabled_strings[] = {
 	NULL
 };
 
+static void print_file_error(const char *path, int ret)
+{
+	int err = -ret;
+
+	printf("# %s: %s (%d)\n", path, strerror(err), err);
+}
+
 int thp_read_string(const char *name, const char * const strings[])
 {
 	char path[PATH_MAX];
@@ -64,8 +76,7 @@ int thp_read_string(const char *name, const char * const strings[])
 
 	ret = read_file(path, buf, sizeof(buf));
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 
@@ -108,8 +119,7 @@ void thp_write_string(const char *name, const char *val)
 	}
 	ret = write_file(path, val, strlen(val) + 1);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 }
@@ -127,8 +137,7 @@ unsigned long thp_read_num(const char *name)
 	}
 	ret = read_num(path, &num);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 
@@ -147,8 +156,7 @@ void thp_write_num(const char *name, unsigned long num)
 	}
 	ret = write_num(path, num);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 }
@@ -183,8 +191,7 @@ void thp_read_settings(struct thp_settings *settings)
 				   &settings->read_ahead_kb);
 
 		if (ret < 0) {
-			errno = -ret;
-			ksft_perror(dev_queue_read_ahead_path);
+			print_file_error(dev_queue_read_ahead_path, ret);
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -242,8 +249,7 @@ void thp_write_settings(struct thp_settings *settings)
 				    settings->read_ahead_kb);
 
 		if (ret < 0) {
-			errno = -ret;
-			ksft_perror(dev_queue_read_ahead_path);
+			print_file_error(dev_queue_read_ahead_path, ret);
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -346,8 +352,7 @@ static unsigned long __thp_supported_orders(bool is_shmem)
 		ret = read_file(path, buf, sizeof(buf));
 		if (ret < 0) {
 			if (ret != -ENOENT) {
-				errno = -ret;
-				ksft_perror(path);
+				print_file_error(path, ret);
 				exit(EXIT_FAILURE);
 			}
 			continue;
@@ -427,8 +432,7 @@ int detect_hugetlb_page_sizes(unsigned long sizes[], int max)
 		if (sscanf(entry->d_name, "hugepages-%zukB", &kb) != 1)
 			continue;
 		sizes[count++] = kb * 1024;
-		ksft_print_msg("[INFO] detected hugetlb page size: %zu KiB\n",
-			       kb);
+		printf("# [INFO] detected hugetlb page size: %zu KiB\n", kb);
 	}
 	closedir(dir);
 	return count;
@@ -477,8 +481,7 @@ unsigned long hugetlb_nr_pages(unsigned long size)
 
 	ret = read_num(path, &nr);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 
@@ -494,8 +497,7 @@ void hugetlb_set_nr_pages(unsigned long size, unsigned long nr)
 
 	ret = write_num(path, nr);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 }
@@ -510,8 +512,7 @@ unsigned long hugetlb_free_pages(unsigned long size)
 
 	ret = read_num(path, &nr);
 	if (ret < 0) {
-		errno = -ret;
-		ksft_perror(path);
+		print_file_error(path, ret);
 		exit(EXIT_FAILURE);
 	}
 
@@ -571,7 +572,8 @@ unsigned long hugetlb_setup(unsigned long nr, unsigned long sizes[],
 		return 0;
 
 	if (nr_enabled > max) {
-		ksft_print_msg("detected %d huge page sizes, will only test %d\n", nr_enabled, max);
+		printf("# detected %d huge page sizes, will only test %d\n",
+		       nr_enabled, max);
 		nr_enabled = max;
 	}
 
@@ -643,8 +645,10 @@ static void hugepage_restore_settings_atexit(void)
 
 static void hugepage_restore_settings_sighandler(int sig)
 {
+	(void)sig;
+
 	/* exit() will invoke the hugepage_restore_settings_atexit handler. */
-	exit(KSFT_FAIL);
+	exit(EXIT_FAILURE);
 }
 
 void hugepage_save_settings(bool thp, bool hugetlb)
diff --git a/tools/testing/selftests/mm/hugepage_settings.h b/tools/lib/mm/hugepage_settings.h
similarity index 100%
rename from tools/testing/selftests/mm/hugepage_settings.h
rename to tools/lib/mm/hugepage_settings.h
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 70d6a9ccf9c9..e77c4cc41b47 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -188,8 +188,10 @@ TEST_FILES += write_hugetlb_memory.sh
 
 include ../lib.mk
 
-$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
-$(TEST_GEN_FILES): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
+$(TEST_GEN_PROGS): vm_util.c $(top_srcdir)/tools/lib/mm/hugepage_settings.c \
+		   $(top_srcdir)/tools/lib/mm/file_utils.c
+$(TEST_GEN_FILES): vm_util.c $(top_srcdir)/tools/lib/mm/hugepage_settings.c \
+		   $(top_srcdir)/tools/lib/mm/file_utils.c
 
 $(OUTPUT)/uffd-stress: uffd-common.c
 $(OUTPUT)/uffd-unit-tests: uffd-common.c
diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 5b582588e015..b2eaa490e7c2 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -15,9 +15,9 @@
 #include <errno.h>
 #include <unistd.h>
 #include <string.h>
+#include <mm/hugepage_settings.h>
 
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #define MAP_SIZE_MB	100
 #define MAP_SIZE	(MAP_SIZE_MB * 1024 * 1024)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index 0c627ea89ff7..3c3d129c91b2 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -29,7 +29,6 @@
 #include "../../../../mm/gup_test.h"
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 static size_t pagesize;
 static int pagemap_fd;
diff --git a/tools/testing/selftests/mm/folio_split_race_test.c b/tools/testing/selftests/mm/folio_split_race_test.c
index 6329e37fff4c..496b74b37476 100644
--- a/tools/testing/selftests/mm/folio_split_race_test.c
+++ b/tools/testing/selftests/mm/folio_split_race_test.c
@@ -25,7 +25,6 @@
 #include <unistd.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 uint64_t page_size;
 uint64_t pmd_pagesize;
diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
index b21df3040b1c..ed967b3c7f37 100644
--- a/tools/testing/selftests/mm/guard-regions.c
+++ b/tools/testing/selftests/mm/guard-regions.c
@@ -21,7 +21,6 @@
 #include <sys/uio.h>
 #include <unistd.h>
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #include "../pidfd/pidfd.h"
 
diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
index eb8963e9d98f..68c840bae082 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -29,7 +29,6 @@
 #include "../../../../mm/gup_test.h"
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 static size_t pagesize;
 static int nr_hugetlbsizes;
diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
index 3f841a96f870..5f44761dbec0 100644
--- a/tools/testing/selftests/mm/gup_test.c
+++ b/tools/testing/selftests/mm/gup_test.c
@@ -14,7 +14,6 @@
 #include <mm/gup_test.h>
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define MB (1UL << 20)
 
diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c
index 2f2b9879d100..e1bb031019f2 100644
--- a/tools/testing/selftests/mm/hmm-tests.c
+++ b/tools/testing/selftests/mm/hmm-tests.c
@@ -10,9 +10,6 @@
  * bugs.
  */
 
-#include "kselftest_harness.h"
-#include "hugepage_settings.h"
-
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -29,6 +26,9 @@
 #include <sys/mman.h>
 #include <sys/ioctl.h>
 #include <sys/time.h>
+#include <mm/hugepage_settings.h>
+
+#include "kselftest_harness.h"
 
 /*
  * This is a private UAPI to the kernel test module so it isn't exported
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
index 555b4b3d1430..57cf790ca478 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -14,7 +14,6 @@
 #include <fcntl.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #define MIN_FREE_PAGES	20
 #define NR_HUGE_PAGES	10	/* common number of pages to map/allocate */
diff --git a/tools/testing/selftests/mm/hugetlb-mmap.c b/tools/testing/selftests/mm/hugetlb-mmap.c
index 0f2aad1b7dbd..a458becf5832 100644
--- a/tools/testing/selftests/mm/hugetlb-mmap.c
+++ b/tools/testing/selftests/mm/hugetlb-mmap.c
@@ -18,7 +18,6 @@
 #include <linux/memfd.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #define LENGTH (256UL*1024*1024)
 #define PROTECTION (PROT_READ | PROT_WRITE)
diff --git a/tools/testing/selftests/mm/hugetlb-mremap.c b/tools/testing/selftests/mm/hugetlb-mremap.c
index ed3d92e862d8..9b724af66e93 100644
--- a/tools/testing/selftests/mm/hugetlb-mremap.c
+++ b/tools/testing/selftests/mm/hugetlb-mremap.c
@@ -26,7 +26,6 @@
 #include <stdbool.h>
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define DEFAULT_LENGTH_MB 10UL
 #define MB_TO_BYTES(x) (x * 1024 * 1024)
diff --git a/tools/testing/selftests/mm/hugetlb-shm.c b/tools/testing/selftests/mm/hugetlb-shm.c
index 3ff7f062b7eb..f4514da49e1d 100644
--- a/tools/testing/selftests/mm/hugetlb-shm.c
+++ b/tools/testing/selftests/mm/hugetlb-shm.c
@@ -29,7 +29,6 @@
 #include <sys/mman.h>
 
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define LENGTH (256UL*1024*1024)
 
diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c b/tools/testing/selftests/mm/hugetlb-soft-offline.c
index bc202e4ed2bd..20864e7d4e0c 100644
--- a/tools/testing/selftests/mm/hugetlb-soft-offline.c
+++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c
@@ -21,9 +21,9 @@
 #include <sys/mman.h>
 #include <sys/statfs.h>
 #include <sys/types.h>
+#include <mm/hugepage_settings.h>
 
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #ifndef MADV_SOFT_OFFLINE
 #define MADV_SOFT_OFFLINE 101
diff --git a/tools/testing/selftests/mm/hugetlb-vmemmap.c b/tools/testing/selftests/mm/hugetlb-vmemmap.c
index 507df78a158d..c46a656c25a0 100644
--- a/tools/testing/selftests/mm/hugetlb-vmemmap.c
+++ b/tools/testing/selftests/mm/hugetlb-vmemmap.c
@@ -11,7 +11,6 @@
 #include <sys/mman.h>
 #include <fcntl.h>
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define PAGE_COMPOUND_HEAD	(1UL << 15)
 #define PAGE_COMPOUND_TAIL	(1UL << 16)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c b/tools/testing/selftests/mm/hugetlb_dio.c
index fb4600570e13..9495974eccbe 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -20,7 +20,6 @@
 #include <sys/syscall.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #ifndef STATX_DIOALIGN
 #define STATX_DIOALIGN		0x00002000U
diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
index 2dc158054f66..56c5a8533e9d 100644
--- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
+++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
@@ -10,7 +10,6 @@
 
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #define INLOOP_ITER 100
 
diff --git a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
index f94549efcc6f..2532a42b98df 100644
--- a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
+++ b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
@@ -25,7 +25,6 @@
 #include <unistd.h>
 
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define INLOOP_ITER 100
 
diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 6a656bc6b266..b16d1a3d960f 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -22,7 +22,6 @@
 #include "linux/magic.h"
 
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define BASE_ADDR ((void *)(1UL << 30))
 static unsigned long hpage_pmd_size;
diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c
index a050f4840cfa..cc37492cf32c 100644
--- a/tools/testing/selftests/mm/ksm_tests.c
+++ b/tools/testing/selftests/mm/ksm_tests.c
@@ -15,7 +15,6 @@
 #include "kselftest.h"
 #include <include/vdso/time64.h>
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define KSM_SYSFS_PATH "/sys/kernel/mm/ksm/"
 #define KSM_FP(s) (KSM_SYSFS_PATH s)
diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
index 29f7492453d4..4fe97c033261 100644
--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c
@@ -4,9 +4,6 @@
  * paths in the kernel.
  */
 
-#include "kselftest_harness.h"
-#include "hugepage_settings.h"
-
 #include <strings.h>
 #include <pthread.h>
 #include <numa.h>
@@ -16,6 +13,8 @@
 #include <sys/types.h>
 #include <signal.h>
 #include <time.h>
+
+#include "kselftest_harness.h"
 #include "vm_util.h"
 
 #define TWOMEG		(2<<20)
diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index 6f8971d5b3ce..804372803080 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -23,7 +23,6 @@
 
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #define PAGEMAP_BITS_ALL		(PAGE_IS_WPALLOWED | PAGE_IS_WRITTEN |	\
 					 PAGE_IS_FILE | PAGE_IS_PRESENT |	\
diff --git a/tools/testing/selftests/mm/prctl_thp_disable.c b/tools/testing/selftests/mm/prctl_thp_disable.c
index d8d9d1de57b8..a4f8451791cb 100644
--- a/tools/testing/selftests/mm/prctl_thp_disable.c
+++ b/tools/testing/selftests/mm/prctl_thp_disable.c
@@ -14,7 +14,6 @@
 #include <sys/wait.h>
 
 #include "kselftest_harness.h"
-#include "hugepage_settings.h"
 #include "vm_util.h"
 
 #ifndef PR_THP_DISABLE_EXCEPT_ADVISED
diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index 9a6d954ee371..5ba2033e8a09 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -45,8 +45,8 @@
 #include <unistd.h>
 #include <sys/ptrace.h>
 #include <setjmp.h>
+#include <mm/hugepage_settings.h>
 
-#include "hugepage_settings.h"
 #include "pkey-helpers.h"
 
 int iteration_nr = 1;
diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
index fb1864a68e1c..6c22ac9e93db 100644
--- a/tools/testing/selftests/mm/soft-dirty.c
+++ b/tools/testing/selftests/mm/soft-dirty.c
@@ -9,7 +9,6 @@
 
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define PAGEMAP_FILE_PATH "/proc/self/pagemap"
 #define TEST_ITERATIONS 10000
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index ef5ab96adc7e..8ff983e264f4 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -21,7 +21,6 @@
 #include <time.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 uint64_t pagesize;
 unsigned int pageshift;
diff --git a/tools/testing/selftests/mm/thuge-gen.c b/tools/testing/selftests/mm/thuge-gen.c
index 22b9c2f1c35d..0da15d530a1f 100644
--- a/tools/testing/selftests/mm/thuge-gen.c
+++ b/tools/testing/selftests/mm/thuge-gen.c
@@ -14,7 +14,6 @@
 #include <string.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 #if !defined(MAP_HUGETLB)
 #define MAP_HUGETLB	0x40000
diff --git a/tools/testing/selftests/mm/transhuge-stress.c b/tools/testing/selftests/mm/transhuge-stress.c
index 8eb0c5630e7e..96f72898ebe0 100644
--- a/tools/testing/selftests/mm/transhuge-stress.c
+++ b/tools/testing/selftests/mm/transhuge-stress.c
@@ -17,7 +17,6 @@
 #include <sys/mman.h>
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 int backing_fd = -1;
 int mmap_flags = MAP_ANONYMOUS | MAP_NORESERVE | MAP_PRIVATE;
diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
index 92a21b97f745..0723843a7626 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -37,7 +37,6 @@
 
 #include "kselftest.h"
 #include "vm_util.h"
-#include "hugepage_settings.h"
 
 #define UFFD_FLAGS	(O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY)
 
diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c
index c973d6722720..eb4b2433b00e 100644
--- a/tools/testing/selftests/mm/uffd-wp-mremap.c
+++ b/tools/testing/selftests/mm/uffd-wp-mremap.c
@@ -7,8 +7,8 @@
 #include <assert.h>
 #include <linux/mman.h>
 #include <sys/mman.h>
+#include <mm/hugepage_settings.h>
 #include "kselftest.h"
-#include "hugepage_settings.h"
 #include "uffd-common.h"
 
 static int pagemap_fd;
diff --git a/tools/testing/selftests/mm/va_high_addr_switch.c b/tools/testing/selftests/mm/va_high_addr_switch.c
index e24d7ba00b44..5a354a664d1f 100644
--- a/tools/testing/selftests/mm/va_high_addr_switch.c
+++ b/tools/testing/selftests/mm/va_high_addr_switch.c
@@ -11,7 +11,6 @@
 
 #include "vm_util.h"
 #include "kselftest.h"
-#include "hugepage_settings.h"
 
 /*
  * The hint addr value is used to allocate addresses
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index d45135283732..aa856f0d32d6 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -9,6 +9,7 @@
 #include "kselftest.h"
 #include <linux/fs.h>
 #include <mm/file_utils.h>
+#include <mm/hugepage_settings.h>
 
 #define BIT_ULL(nr)                   (1ULL << (nr))
 #define PM_SOFT_DIRTY                 BIT_ULL(55)
-- 
2.39.5

[PATCH v6 4/6] tools/mm: move gup_test from selftests/mm to tools/mm

[Sarthak Sharma]

Move tools/testing/selftests/mm/gup_test.c to tools/mm/gup_bench.c.
This is the first step in separating its benchmarking and
functional testing components. Later patches will make this a
purely benchmarking tool and a new functional testing selftest
will be introduced in selftests/mm.

Include hugepage_settings.h directly instead of vm_util.h and
use getpagesize() instead of psize().

Adjust the Makefiles and .gitignore files in both locations.
Remove the gup_test invocations from run_vmtests.sh and update
MAINTAINERS.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 MAINTAINERS                                   |  2 +-
 tools/mm/.gitignore                           |  1 +
 tools/mm/Makefile                             | 10 ++++--
 .../mm/gup_test.c => mm/gup_bench.c}          |  8 ++---
 tools/testing/selftests/mm/.gitignore         |  1 -
 tools/testing/selftests/mm/Makefile           |  1 -
 tools/testing/selftests/mm/run_vmtests.sh     | 36 -------------------
 7 files changed, 13 insertions(+), 46 deletions(-)
 rename tools/{testing/selftests/mm/gup_test.c => mm/gup_bench.c} (97%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 040a8e6664f4..5845fa74a5a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17027,8 +17027,8 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
 F:	mm/gup.c
 F:	mm/gup_test.c
 F:	mm/gup_test.h
+F:	tools/mm/gup_bench.c
 F:	tools/testing/selftests/mm/gup_longterm.c
-F:	tools/testing/selftests/mm/gup_test.c
 
 MEMORY MANAGEMENT - KSM (Kernel Samepage Merging)
 M:	Andrew Morton <akpm@linux-foundation.org>
diff --git a/tools/mm/.gitignore b/tools/mm/.gitignore
index 922879f93fc8..2bfb295af498 100644
--- a/tools/mm/.gitignore
+++ b/tools/mm/.gitignore
@@ -2,3 +2,4 @@
 slabinfo
 page-types
 page_owner_sort
+gup_bench
diff --git a/tools/mm/Makefile b/tools/mm/Makefile
index f5725b5c23aa..d82cc8c43ee0 100644
--- a/tools/mm/Makefile
+++ b/tools/mm/Makefile
@@ -3,13 +3,14 @@
 #
 include ../scripts/Makefile.include
 
-BUILD_TARGETS=page-types slabinfo page_owner_sort thp_swap_allocator_test
+BUILD_TARGETS=page-types slabinfo page_owner_sort thp_swap_allocator_test gup_bench
 INSTALL_TARGETS = $(BUILD_TARGETS) thpmaps
 
 LIB_DIR = ../lib/api
 LIBS = $(LIB_DIR)/libapi.a
+GUP_BENCH_OBJS = gup_bench.c ../lib/mm/hugepage_settings.c ../lib/mm/file_utils.c
 
-CFLAGS += -Wall -Wextra -I../lib/ -pthread
+CFLAGS += -Wall -Wextra -I../lib/ -I../.. -pthread
 LDFLAGS += $(LIBS) -pthread
 
 all: $(BUILD_TARGETS)
@@ -22,8 +23,11 @@ $(LIBS):
 %: %.c
 	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
 
+gup_bench: $(GUP_BENCH_OBJS) $(LIBS)
+	$(CC) $(CFLAGS) -o $@ $(GUP_BENCH_OBJS) $(LDFLAGS)
+
 clean:
-	$(RM) page-types slabinfo page_owner_sort thp_swap_allocator_test
+	$(RM) page-types slabinfo page_owner_sort thp_swap_allocator_test gup_bench
 	make -C $(LIB_DIR) clean
 
 sbindir ?= /usr/sbin
diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/mm/gup_bench.c
similarity index 97%
rename from tools/testing/selftests/mm/gup_test.c
rename to tools/mm/gup_bench.c
index 5f44761dbec0..da56aa5324d3 100644
--- a/tools/testing/selftests/mm/gup_test.c
+++ b/tools/mm/gup_bench.c
@@ -12,8 +12,8 @@
 #include <pthread.h>
 #include <assert.h>
 #include <mm/gup_test.h>
-#include "kselftest.h"
-#include "vm_util.h"
+#include <mm/hugepage_settings.h>
+#include "../testing/selftests/kselftest.h"
 
 #define MB (1UL << 20)
 
@@ -140,7 +140,7 @@ int main(int argc, char **argv)
 		case 'n':
 			nr_pages = atoi(optarg);
 			if (nr_pages < 0)
-				nr_pages = size / psize();
+				nr_pages = size / getpagesize();
 			break;
 		case 't':
 			thp = 1;
@@ -254,7 +254,7 @@ int main(int argc, char **argv)
 		madvise(p, size, MADV_NOHUGEPAGE);
 
 	/* Fault them in here, from user space. */
-	for (; (unsigned long)p < gup.addr + size; p += psize())
+	for (; (unsigned long)p < gup.addr + size; p += getpagesize())
 		p[0] = 0;
 
 	tid = malloc(sizeof(pthread_t) * nthreads);
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index 9ccd9e1447e6..c30cd7ce635e 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -38,7 +38,6 @@ uffd-wp-mremap
 mlock-intersect-test
 mlock-random-test
 virtual_address_range
-gup_test
 va_128TBswitch
 map_fixed_noreplace
 write_to_hugetlbfs
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index e77c4cc41b47..44b16cbaee5b 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -59,7 +59,6 @@ endif
 TEST_GEN_FILES = cow
 TEST_GEN_FILES += compaction_test
 TEST_GEN_FILES += gup_longterm
-TEST_GEN_FILES += gup_test
 TEST_GEN_FILES += hmm-tests
 TEST_GEN_FILES += hugetlb-madvise
 TEST_GEN_FILES += hugetlb-mmap
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 8c296dedf047..263e4940cc1d 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -130,30 +130,6 @@ test_selected() {
 	fi
 }
 
-run_gup_matrix() {
-    # -t: thp=on, -T: thp=off, -H: hugetlb=on
-    local hugetlb_mb=256
-
-    for huge in -t -T "-H -m $hugetlb_mb"; do
-        # -u: gup-fast, -U: gup-basic, -a: pin-fast, -b: pin-basic, -L: pin-longterm
-        for test_cmd in -u -U -a -b -L; do
-            # -w: write=1, -W: write=0
-            for write in -w -W; do
-                # -S: shared
-                for share in -S " "; do
-                    # -n: How many pages to fetch together?  512 is special
-                    # because it's default thp size (or 2M on x86), 123 to
-                    # just test partial gup when hit a huge in whatever form
-                    for num in "-n 1" "-n 512" "-n 123" "-n -1"; do
-                        CATEGORY="gup_test" run_test ./gup_test \
-                                $huge $test_cmd $write $share $num
-                    done
-                done
-            done
-        done
-    done
-}
-
 # filter 64bit architectures
 ARCH64STR="arm64 mips64 parisc64 ppc64 ppc64le riscv64 s390x sparc64 x86_64"
 if [ -z "$ARCH" ]; then
@@ -276,18 +252,6 @@ fi
 
 CATEGORY="mmap" run_test ./map_fixed_noreplace
 
-if $RUN_ALL; then
-    run_gup_matrix
-else
-    # get_user_pages_fast() benchmark
-    CATEGORY="gup_test" run_test ./gup_test -u -n 1
-    CATEGORY="gup_test" run_test ./gup_test -u -n -1
-    # pin_user_pages_fast() benchmark
-    CATEGORY="gup_test" run_test ./gup_test -a -n 1
-    CATEGORY="gup_test" run_test ./gup_test -a -n -1
-fi
-# Dump pages 0, 19, and 4096, using pin_user_pages:
-CATEGORY="gup_test" run_test ./gup_test -ct -F 0x1 0 19 0x1000
 CATEGORY="gup_test" run_test ./gup_longterm
 
 CATEGORY="userfaultfd" run_test ./uffd-unit-tests
-- 
2.39.5

[PATCH v6 5/6] tools/mm: make gup_bench a benchmark only tool

[Sarthak Sharma]

Remove the functional modes (GUP_BASIC_TEST, PIN_BASIC_TEST and
DUMP_USER_PAGES_TEST) from gup_bench. Drop kselftest dependency
and use normal diagnostics and exit statuses.

When no arguments are supplied, run a single GUP_FAST_BENCHMARK
with existing default values. Let users select other configs
through command line options. Also validate numeric arguments
and reject positional arguments.

Restore hugeTLB settings on failure and after every run. Also
handle failures without relying on assert() calls.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/mm/gup_bench.c | 329 ++++++++++++++++++++++++++-----------------
 1 file changed, 201 insertions(+), 128 deletions(-)

diff --git a/tools/mm/gup_bench.c b/tools/mm/gup_bench.c
index da56aa5324d3..966a5bf5aac0 100644
--- a/tools/mm/gup_bench.c
+++ b/tools/mm/gup_bench.c
@@ -10,10 +10,13 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <pthread.h>
-#include <assert.h>
+#include <stdbool.h>
+#include <stdatomic.h>
+#include <stdint.h>
+#include <limits.h>
+#include <string.h>
 #include <mm/gup_test.h>
 #include <mm/hugepage_settings.h>
-#include "../testing/selftests/kselftest.h"
 
 #define MB (1UL << 20)
 
@@ -25,6 +28,7 @@
 static unsigned long cmd = GUP_FAST_BENCHMARK;
 static int gup_fd, repeats = 1;
 static unsigned long size = 128 * MB;
+static atomic_int bench_error;
 /* Serialize prints */
 static pthread_mutex_t print_mutex = PTHREAD_MUTEX_INITIALIZER;
 
@@ -37,120 +41,155 @@ static char *cmd_to_str(unsigned long cmd)
 		return "PIN_FAST_BENCHMARK";
 	case PIN_LONGTERM_BENCHMARK:
 		return "PIN_LONGTERM_BENCHMARK";
-	case GUP_BASIC_TEST:
-		return "GUP_BASIC_TEST";
-	case PIN_BASIC_TEST:
-		return "PIN_BASIC_TEST";
-	case DUMP_USER_PAGES_TEST:
-		return "DUMP_USER_PAGES_TEST";
 	}
 	return "Unknown command";
 }
 
+static long parse_long_arg_base(const char *arg, const char *name, int base)
+{
+	char *end;
+	long val;
+
+	errno = 0;
+	val = strtol(arg, &end, base);
+	if (errno || end == arg || *end != '\0') {
+		fprintf(stderr, "Invalid %s '%s'\n", name, arg);
+		exit(1);
+	}
+	return val;
+}
+
+static long parse_long_arg(const char *arg, const char *name)
+{
+	return parse_long_arg_base(arg, name, 10);
+}
+
+static long parse_positive_long_arg(const char *arg, const char *name)
+{
+	long val = parse_long_arg(arg, name);
+
+	if (val < 1) {
+		fprintf(stderr, "Invalid %s '%s'\n", name, arg);
+		exit(1);
+	}
+
+	return val;
+}
+
 void *gup_thread(void *data)
 {
 	struct gup_test gup = *(struct gup_test *)data;
 	int i, status;
 
-	/* Only report timing information on the *_BENCHMARK commands: */
-	if ((cmd == PIN_FAST_BENCHMARK) || (cmd == GUP_FAST_BENCHMARK) ||
-	     (cmd == PIN_LONGTERM_BENCHMARK)) {
-		for (i = 0; i < repeats; i++) {
-			gup.size = size;
-			status = ioctl(gup_fd, cmd, &gup);
-			if (status)
-				break;
+	for (i = 0; i < repeats; i++) {
+		gup.size = size;
+		status = ioctl(gup_fd, cmd, &gup);
+		if (status) {
+			int err = errno;
 
+			bench_error = 1;
 			pthread_mutex_lock(&print_mutex);
-			ksft_print_msg("%s: Time: get:%lld put:%lld us",
-				       cmd_to_str(cmd), gup.get_delta_usec,
-				       gup.put_delta_usec);
-			if (gup.size != size)
-				ksft_print_msg(", truncated (size: %lld)", gup.size);
-			ksft_print_msg("\n");
+			fprintf(stderr, "%s ioctl failed: %s\n", cmd_to_str(cmd),
+				strerror(err));
 			pthread_mutex_unlock(&print_mutex);
+			break;
 		}
-	} else {
-		gup.size = size;
-		status = ioctl(gup_fd, cmd, &gup);
-		if (status)
-			goto return_;
 
 		pthread_mutex_lock(&print_mutex);
-		ksft_print_msg("%s: done\n", cmd_to_str(cmd));
-		if (gup.size != size)
-			ksft_print_msg("Truncated (size: %lld)\n", gup.size);
+		if (gup.size == size)
+			printf("%s time: get:%lld us put:%lld us\n",
+			       cmd_to_str(cmd), gup.get_delta_usec,
+			       gup.put_delta_usec);
+		else
+			printf("%s time: get:%lld put:%lld us, truncated (size: %lld)\n",
+			       cmd_to_str(cmd), gup.get_delta_usec,
+			       gup.put_delta_usec, gup.size);
 		pthread_mutex_unlock(&print_mutex);
 	}
 
-return_:
-	ksft_test_result(!status, "ioctl status %d\n", status);
 	return NULL;
 }
 
 int main(int argc, char **argv)
 {
 	struct gup_test gup = { 0 };
-	int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
+	int filed, i, opt, nr_pages = 1, thp = -1, write = 1;
+	int nthreads = 1, ret, started_threads = 0;
 	int flags = MAP_PRIVATE;
-	char *file = "/dev/zero";
-	bool hugetlb = false;
+	const char *file = "/dev/zero";
+	bool hugetlb = false, restore_hugetlb = false;
+	unsigned long nr_pages_per_call;
 	pthread_t *tid;
 	char *p;
 
-	while ((opt = getopt(argc, argv, "m:r:n:F:f:abcj:tTLUuwWSHpz")) != -1) {
+	while ((opt = getopt(argc, argv, "m:r:n:F:f:aj:tTLuwWSH")) != -1) {
 		switch (opt) {
 		case 'a':
 			cmd = PIN_FAST_BENCHMARK;
 			break;
-		case 'b':
-			cmd = PIN_BASIC_TEST;
-			break;
 		case 'L':
 			cmd = PIN_LONGTERM_BENCHMARK;
 			break;
-		case 'c':
-			cmd = DUMP_USER_PAGES_TEST;
-			/*
-			 * Dump page 0 (index 1). May be overridden later, by
-			 * user's non-option arguments.
-			 *
-			 * .which_pages is zero-based, so that zero can mean "do
-			 * nothing".
-			 */
-			gup.which_pages[0] = 1;
-			break;
-		case 'p':
-			/* works only with DUMP_USER_PAGES_TEST */
-			gup.test_flags |= GUP_TEST_FLAG_DUMP_PAGES_USE_PIN;
-			break;
-		case 'F':
-			/* strtol, so you can pass flags in hex form */
-			gup.gup_flags = strtol(optarg, 0, 0);
+		case 'F': {
+			long val;
+
+			val = parse_long_arg_base(optarg, "GUP flags", 0);
+			if (val < 0 || val > UINT_MAX) {
+				fprintf(stderr, "Invalid GUP flags '%s'\n", optarg);
+				exit(1);
+			}
+
+			gup.gup_flags = val;
 			break;
-		case 'j':
-			nthreads = atoi(optarg);
+		}
+		case 'j': {
+			long val;
+
+			val = parse_positive_long_arg(optarg, "thread count");
+			if (val > INT_MAX ||
+			    (size_t)val > SIZE_MAX / sizeof(pthread_t)) {
+				fprintf(stderr, "Invalid thread count '%s'\n", optarg);
+				exit(1);
+			}
+			nthreads = val;
 			break;
+		}
 		case 'm':
-			size = atoi(optarg) * MB;
+			size = parse_positive_long_arg(optarg, "size");
+			if (size > ULONG_MAX / MB) {
+				fprintf(stderr, "Invalid size '%s'\n", optarg);
+				exit(1);
+			}
+			size *= MB;
 			break;
-		case 'r':
-			repeats = atoi(optarg);
+		case 'r': {
+			long val;
+
+			val = parse_positive_long_arg(optarg, "repeat count");
+			if (val > INT_MAX) {
+				fprintf(stderr, "Invalid repeat count '%s'\n", optarg);
+				exit(1);
+			}
+			repeats = val;
 			break;
-		case 'n':
-			nr_pages = atoi(optarg);
-			if (nr_pages < 0)
-				nr_pages = size / getpagesize();
+		}
+		case 'n': {
+			long val;
+
+			val = parse_long_arg(optarg, "page count");
+			if (val != -1 && (val < 1 || val > INT_MAX)) {
+				fprintf(stderr, "Invalid page count '%s'\n", optarg);
+				exit(1);
+			}
+			nr_pages = val;
 			break;
+		}
 		case 't':
 			thp = 1;
 			break;
 		case 'T':
 			thp = 0;
 			break;
-		case 'U':
-			cmd = GUP_BASIC_TEST;
-			break;
 		case 'u':
 			cmd = GUP_FAST_BENCHMARK;
 			break;
@@ -172,80 +211,93 @@ int main(int argc, char **argv)
 			hugetlb = true;
 			break;
 		default:
-			ksft_exit_fail_msg("Wrong argument\n");
+			fprintf(stderr, "Wrong argument\n");
+			exit(1);
 		}
 	}
 
-	if (optind < argc) {
-		int extra_arg_count = 0;
-		/*
-		 * For example:
-		 *
-		 *   ./gup_test -c 0 1 0x1001
-		 *
-		 * ...to dump pages 0, 1, and 4097
-		 */
-
-		while ((optind < argc) &&
-		       (extra_arg_count < GUP_TEST_MAX_PAGES_TO_DUMP)) {
-			/*
-			 * Do the 1-based indexing here, so that the user can
-			 * use normal 0-based indexing on the command line.
-			 */
-			long page_index = strtol(argv[optind], 0, 0) + 1;
-
-			gup.which_pages[extra_arg_count] = page_index;
-			extra_arg_count++;
-			optind++;
-		}
+	if (optind != argc) {
+		fprintf(stderr, "Unexpected argument '%s'\n", argv[optind]);
+		exit(1);
 	}
 
-	ksft_print_header();
-
 	if (hugetlb) {
 		unsigned long hp_size = default_huge_page_size();
 
-		if (!hp_size)
-			ksft_exit_skip("HugeTLB is unavailable\n");
+		if (!hp_size) {
+			fprintf(stderr, "Could not determine huge page size\n");
+			return 1;
+		}
+
+		if (size > ULONG_MAX - (hp_size - 1)) {
+			fprintf(stderr, "HugeTLB mapping size is too large\n");
+			return 1;
+		}
 
 		size = (size + hp_size - 1) & ~(hp_size - 1);
-		if (!hugetlb_setup_default(size / hp_size))
-			ksft_exit_skip("Not enough huge pages\n");
+		if (!hugetlb_setup_default(size / hp_size)) {
+			fprintf(stderr, "Not enough huge pages\n");
+			hugetlb_restore_settings();
+			return 1;
+		}
+		restore_hugetlb = true;
 	}
 
-	ksft_set_plan(nthreads);
-
-	filed = open(file, O_RDWR|O_CREAT, 0664);
-	if (filed < 0)
-		ksft_exit_fail_msg("Unable to open %s: %s\n", file, strerror(errno));
-
-	gup.nr_pages_per_call = nr_pages;
+	nr_pages_per_call = nr_pages < 0 ? size / getpagesize() :
+		(unsigned long)nr_pages;
+	if (nr_pages_per_call > UINT_MAX) {
+		fprintf(stderr, "Page count is too large\n");
+		if (restore_hugetlb)
+			hugetlb_restore_settings();
+		return 1;
+	}
+	gup.nr_pages_per_call = nr_pages_per_call;
 	if (write)
 		gup.gup_flags |= FOLL_WRITE;
 
+	filed = open(file, O_RDWR | O_CREAT, 0664);
+	if (filed < 0) {
+		fprintf(stderr, "Unable to open %s: %s\n", file, strerror(errno));
+		if (restore_hugetlb)
+			hugetlb_restore_settings();
+		return 1;
+	}
+
 	gup_fd = open(GUP_TEST_FILE, O_RDWR);
 	if (gup_fd == -1) {
-		switch (errno) {
-		case EACCES:
-			if (getuid())
-				ksft_print_msg("Please run this test as root\n");
-			break;
-		case ENOENT:
-			if (opendir("/sys/kernel/debug") == NULL)
-				ksft_print_msg("mount debugfs at /sys/kernel/debug\n");
-			ksft_print_msg("check if CONFIG_GUP_TEST is enabled in kernel config\n");
-			break;
-		default:
-			ksft_print_msg("failed to open %s: %s\n", GUP_TEST_FILE, strerror(errno));
-			break;
-		}
-		ksft_test_result_skip("Please run this test as root\n");
-		ksft_exit_pass();
+		int err = errno;
+
+		close(filed);
+		if (err == EACCES)
+			fprintf(stderr, "Please run as root\n");
+		else if (err == ENOENT) {
+			DIR *debugfs = opendir("/sys/kernel/debug");
+
+			if (!debugfs)
+				fprintf(stderr, "Mount debugfs at /sys/kernel/debug\n");
+			else {
+				closedir(debugfs);
+				fprintf(stderr, "Check CONFIG_GUP_TEST in kernel config\n");
+			}
+		} else
+			fprintf(stderr, "Failed to open %s: %s\n", GUP_TEST_FILE,
+				strerror(err));
+		if (restore_hugetlb)
+			hugetlb_restore_settings();
+		return 1;
 	}
 
 	p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, filed, 0);
-	if (p == MAP_FAILED)
-		ksft_exit_fail_msg("mmap: %s\n", strerror(errno));
+	if (p == MAP_FAILED) {
+		fprintf(stderr, "mmap: %s\n", strerror(errno));
+		close(filed);
+		close(gup_fd);
+
+		if (restore_hugetlb)
+			hugetlb_restore_settings();
+		return 1;
+	}
+	close(filed);
 	gup.addr = (unsigned long)p;
 
 	if (thp == 1)
@@ -258,17 +310,38 @@ int main(int argc, char **argv)
 		p[0] = 0;
 
 	tid = malloc(sizeof(pthread_t) * nthreads);
-	assert(tid);
+	if (!tid) {
+		fprintf(stderr, "Failed to allocate %d threads: %s\n",
+			nthreads, strerror(errno));
+		munmap((void *)gup.addr, size);
+		close(gup_fd);
+		if (restore_hugetlb)
+			hugetlb_restore_settings();
+		return 1;
+	}
+
 	for (i = 0; i < nthreads; i++) {
 		ret = pthread_create(&tid[i], NULL, gup_thread, &gup);
-		assert(ret == 0);
+		if (ret) {
+			fprintf(stderr, "pthread_create failed: %s\n", strerror(ret));
+			bench_error = 1;
+			break;
+		}
+		started_threads++;
 	}
-	for (i = 0; i < nthreads; i++) {
+	for (i = 0; i < started_threads; i++) {
 		ret = pthread_join(tid[i], NULL);
-		assert(ret == 0);
+		if (ret) {
+			fprintf(stderr, "pthread_join failed: %s\n", strerror(ret));
+			bench_error = 1;
+		}
 	}
 
 	free(tid);
+	munmap((void *)gup.addr, size);
+	close(gup_fd);
+	if (restore_hugetlb)
+		hugetlb_restore_settings();
 
-	ksft_exit_pass();
+	return bench_error ? 1 : 0;
 }
-- 
2.39.5

[PATCH v6 6/6] selftests/mm: add a GUP selftest

[Sarthak Sharma]

Add a new GUP selftest which uses kselftest_harness.h. Cover
12 mapping configurations: THP enabled, THP disabled and
HugeTLB, each across private/shared mappings and with/without
FOLL_WRITE. Run 7 testcases for every variant: get_user_pages,
get_user_pages_fast, pin_user_pages, pin_user_pages_fast,
pin_user_pages_longterm, and DUMP_USER_PAGES_TEST using both
get and pin.

Sweep four nr_pages_per_call values for each test: 1, 512, 123 and
all pages. This preserves the coverage previously provided by
run_gup_matrix(): 12 mapping combinations x 5 GUP/PUP operations x 4
batch sizes, for 240 ioctl calls. The two dump modes add another 96
calls.

Preserve the previous sparse dump coverage with a standalone test for
pages 0, 19 and 0x1000. In total the selftest reports 85 TAP
cases and issues 337 ioctls.

Add the new gup binary to the selftests/mm build, .gitignore,
run_vmtests.sh and MAINTAINERS. Update
Documentation/core-api/pin_user_pages.rst for the new test.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 Documentation/core-api/pin_user_pages.rst |  14 +-
 MAINTAINERS                               |   1 +
 tools/testing/selftests/mm/.gitignore     |   1 +
 tools/testing/selftests/mm/Makefile       |   1 +
 tools/testing/selftests/mm/gup.c          | 341 ++++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh |   1 +
 6 files changed, 355 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/mm/gup.c

diff --git a/Documentation/core-api/pin_user_pages.rst b/Documentation/core-api/pin_user_pages.rst
index c16ca163b55e..1564b16994ad 100644
--- a/Documentation/core-api/pin_user_pages.rst
+++ b/Documentation/core-api/pin_user_pages.rst
@@ -228,12 +228,18 @@ Unit testing
 ============
 This file::
 
- tools/testing/selftests/mm/gup_test.c
+ tools/testing/selftests/mm/gup.c
 
-has the following new calls to exercise the new pin*() wrapper functions:
+contains the following test cases to exercise pin_user_pages*():
 
-* PIN_FAST_BENCHMARK (./gup_test -a)
-* PIN_BASIC_TEST (./gup_test -b)
+* pin_user_pages via PIN_BASIC_TEST
+* pin_user_pages_fast via PIN_FAST_BENCHMARK
+* pin_user_pages_longterm via PIN_LONGTERM_BENCHMARK
+
+Run with::
+
+  make -C tools/testing/selftests/mm
+  ./tools/testing/selftests/mm/gup
 
 You can monitor how many total dma-pinned pages have been acquired and released
 since the system was booted, via two new /proc/vmstat entries: ::
diff --git a/MAINTAINERS b/MAINTAINERS
index 5845fa74a5a0..e59158c59a6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17028,6 +17028,7 @@ F:	mm/gup.c
 F:	mm/gup_test.c
 F:	mm/gup_test.h
 F:	tools/mm/gup_bench.c
+F:	tools/testing/selftests/mm/gup.c
 F:	tools/testing/selftests/mm/gup_longterm.c
 
 MEMORY MANAGEMENT - KSM (Kernel Samepage Merging)
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index c30cd7ce635e..bf3a409dc219 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -50,6 +50,7 @@ local_config.h
 local_config.mk
 ksm_functional_tests
 mdwe_test
+gup
 gup_longterm
 mkdirty
 va_high_addr_switch
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 44b16cbaee5b..0a15da7d3439 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -58,6 +58,7 @@ endif
 
 TEST_GEN_FILES = cow
 TEST_GEN_FILES += compaction_test
+TEST_GEN_FILES += gup
 TEST_GEN_FILES += gup_longterm
 TEST_GEN_FILES += hmm-tests
 TEST_GEN_FILES += hugetlb-madvise
diff --git a/tools/testing/selftests/mm/gup.c b/tools/testing/selftests/mm/gup.c
new file mode 100644
index 000000000000..a9b374b752df
--- /dev/null
+++ b/tools/testing/selftests/mm/gup.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0
+#define __SANE_USERSPACE_TYPES__ // Use ll64
+#include <fcntl.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
+#include <dirent.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <mm/gup_test.h>
+#include "vm_util.h"
+#include "kselftest_harness.h"
+
+#define MB (1UL << 20)
+
+/* Just the flags we need, copied from the kernel internals. */
+#define FOLL_WRITE	0x01	/* check pte is writable */
+
+/* Page counts exercising single, THP-batch, partial, and full-mapping GUP. */
+static const int nr_pages_list[] = { 1, 512, 123, -1 };
+
+#define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
+
+FIXTURE(gup_test)
+{
+	int gup_fd;
+	char *addr;
+	unsigned long size;
+};
+
+FIXTURE_VARIANT(gup_test)
+{
+	bool thp;
+	bool hugetlb;
+	bool write;
+	bool shared;
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_write)
+{
+	.thp = false,
+	.hugetlb = false,
+	.write = true,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_read)
+{
+	.thp = false,
+	.hugetlb = false,
+	.write = false,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_write_thp)
+{
+	.thp = true,
+	.hugetlb = false,
+	.write = true,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_read_thp)
+{
+	.thp = true,
+	.hugetlb = false,
+	.write = false,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_write_hugetlb)
+{
+	.thp = false,
+	.hugetlb = true,
+	.write = true,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, private_read_hugetlb)
+{
+	.thp = false,
+	.hugetlb = true,
+	.write = false,
+	.shared = false,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_write)
+{
+	.thp = false,
+	.hugetlb = false,
+	.write = true,
+	.shared = true,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_read)
+{
+	.thp = false,
+	.hugetlb = false,
+	.write = false,
+	.shared = true,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_write_thp)
+{
+	.thp = true,
+	.hugetlb = false,
+	.write = true,
+	.shared = true,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_read_thp)
+{
+	.thp = true,
+	.hugetlb = false,
+	.write = false,
+	.shared = true,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_write_hugetlb)
+{
+	.thp = false,
+	.hugetlb = true,
+	.write = true,
+	.shared = true,
+};
+
+FIXTURE_VARIANT_ADD(gup_test, shared_read_hugetlb)
+{
+	.thp = false,
+	.hugetlb = true,
+	.write = false,
+	.shared = true,
+};
+
+FIXTURE_SETUP(gup_test)
+{
+	int mmap_flags = MAP_PRIVATE;
+	int zero_fd;
+	char *p;
+
+	/* zero_fd has to be >= 0. Already checked in main() */
+	zero_fd = open("/dev/zero", O_RDWR);
+	ASSERT_GE(zero_fd, 0);
+
+	/* gup_fd has to be >= 0. Already checked in main() */
+	self->gup_fd = open(GUP_TEST_FILE, O_RDWR);
+	ASSERT_GE(self->gup_fd, 0);
+
+	self->size = variant->hugetlb ? 256 * MB : 128 * MB;
+
+	if (variant->hugetlb) {
+		unsigned long hp_size = default_huge_page_size();
+
+		if (!hp_size) {
+			close(zero_fd);
+			close(self->gup_fd);
+			SKIP(return, "HugeTLB not available\n");
+		}
+
+		self->size = (self->size + hp_size - 1) & ~(hp_size - 1);
+		if (!hugetlb_setup_default(self->size / hp_size)) {
+			hugetlb_restore_settings();
+			close(zero_fd);
+			close(self->gup_fd);
+			SKIP(return, "Not enough huge pages\n");
+		}
+
+		mmap_flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
+	}
+
+	if (variant->shared)
+		mmap_flags = (mmap_flags & ~MAP_PRIVATE) | MAP_SHARED;
+
+	self->addr = mmap(NULL, self->size, PROT_READ | PROT_WRITE,
+			  mmap_flags, zero_fd, 0);
+
+	ASSERT_NE(self->addr, MAP_FAILED) {
+		int err = errno;
+
+		close(zero_fd);
+		close(self->gup_fd);
+		if (variant->hugetlb)
+			hugetlb_restore_settings();
+		TH_LOG("mmap failed: %s", strerror(err));
+	}
+	close(zero_fd);
+
+	if (variant->thp)
+		madvise(self->addr, self->size, MADV_HUGEPAGE);
+	else if (!variant->hugetlb)
+		madvise(self->addr, self->size, MADV_NOHUGEPAGE);
+
+	for (p = self->addr; (unsigned long)p < (unsigned long)self->addr
+			+ self->size; p += psize())
+		p[0] = 0;
+}
+
+FIXTURE_TEARDOWN(gup_test)
+{
+	munmap(self->addr, self->size);
+	close(self->gup_fd);
+
+	if (variant->hugetlb)
+		hugetlb_restore_settings();
+}
+
+static void run_gup_cmd(struct __test_metadata *_metadata,
+			 FIXTURE_DATA(gup_test) *self,
+			 const FIXTURE_VARIANT(gup_test) *variant,
+			 unsigned long command,
+			 unsigned int test_flags,
+			 unsigned int which_page)
+{
+	int i;
+
+	for (i = 0; i < (int)ARRAY_SIZE(nr_pages_list); i++) {
+		struct gup_test gup = {
+			.addr = (unsigned long)self->addr,
+			.size = self->size,
+			.nr_pages_per_call = nr_pages_list[i] < 0 ?
+				self->size / psize() : nr_pages_list[i],
+			.test_flags = test_flags,
+		};
+
+		if (variant->write)
+			gup.gup_flags |= FOLL_WRITE;
+
+		gup.which_pages[0] = which_page;
+
+		TH_LOG("nr_pages_per_call=%u", gup.nr_pages_per_call);
+		ASSERT_EQ(ioctl(self->gup_fd, command, &gup), 0);
+	}
+}
+
+TEST_F(gup_test, get_user_pages)
+{
+	run_gup_cmd(_metadata, self, variant, GUP_BASIC_TEST, 0, 0);
+}
+
+TEST_F(gup_test, pin_user_pages)
+{
+	run_gup_cmd(_metadata, self, variant, PIN_BASIC_TEST, 0, 0);
+}
+
+TEST_F(gup_test, dump_user_pages_with_get)
+{
+	run_gup_cmd(_metadata, self, variant, DUMP_USER_PAGES_TEST, 0, 1);
+}
+
+TEST_F(gup_test, dump_user_pages_with_pin)
+{
+	run_gup_cmd(_metadata, self, variant, DUMP_USER_PAGES_TEST,
+		    GUP_TEST_FLAG_DUMP_PAGES_USE_PIN, 1);
+}
+
+TEST_F(gup_test, get_user_pages_fast)
+{
+	run_gup_cmd(_metadata, self, variant, GUP_FAST_BENCHMARK, 0, 0);
+}
+
+TEST_F(gup_test, pin_user_pages_fast)
+{
+	run_gup_cmd(_metadata, self, variant, PIN_FAST_BENCHMARK, 0, 0);
+}
+
+TEST_F(gup_test, pin_user_pages_longterm)
+{
+	run_gup_cmd(_metadata, self, variant, PIN_LONGTERM_BENCHMARK, 0, 0);
+}
+
+TEST(dump_user_pages_sparse_indices)
+{
+	struct gup_test gup = { 0 };
+	unsigned long size = 128 * MB;
+	int zero_fd, gup_fd;
+	char *addr, *p;
+
+	zero_fd = open("/dev/zero", O_RDWR);
+	ASSERT_GE(zero_fd, 0);
+
+	gup_fd = open(GUP_TEST_FILE, O_RDWR);
+	ASSERT_GE(gup_fd, 0);
+
+	addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, zero_fd, 0);
+	close(zero_fd);
+	ASSERT_NE(addr, MAP_FAILED);
+
+	madvise(addr, size, MADV_HUGEPAGE);
+
+	for (p = addr; (unsigned long)p < (unsigned long)addr + size;
+	     p += psize())
+		p[0] = 0;
+
+	gup.addr = (unsigned long)addr;
+	gup.size = size;
+	gup.nr_pages_per_call = size / psize();
+	gup.gup_flags = FOLL_WRITE;
+	gup.which_pages[0] = 1;
+	gup.which_pages[1] = 20;
+	gup.which_pages[2] = 0x1001;
+
+	ASSERT_EQ(ioctl(gup_fd, DUMP_USER_PAGES_TEST, &gup), 0);
+
+	munmap(addr, size);
+	close(gup_fd);
+}
+
+int main(int argc, char **argv)
+{
+	int fd;
+	char *file = "/dev/zero";
+
+	fd = open(file, O_RDWR);
+	if (fd < 0) {
+		ksft_print_header();
+		ksft_exit_fail_msg("Unable to open %s: %s\n", file, strerror(errno));
+	}
+	close(fd);
+
+	fd = open(GUP_TEST_FILE, O_RDWR);
+	if (fd == -1) {
+		ksft_print_header();
+		if (errno == EACCES)
+			ksft_exit_skip("Please run this test as root\n");
+		if (errno == ENOENT) {
+			DIR *debugfs = opendir("/sys/kernel/debug");
+
+			if (!debugfs) {
+				ksft_exit_skip("Mount debugfs at /sys/kernel/debug\n");
+			} else {
+				closedir(debugfs);
+				ksft_exit_skip("Check CONFIG_GUP_TEST in kernel config\n");
+			}
+		}
+		ksft_exit_fail_msg("Failed to open %s: %s\n", GUP_TEST_FILE, strerror(errno));
+	}
+	close(fd);
+
+	return test_harness_run(argc, argv);
+}
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 263e4940cc1d..8cc4c8f403da 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -252,6 +252,7 @@ fi
 
 CATEGORY="mmap" run_test ./map_fixed_noreplace
 
+CATEGORY="gup_test" run_test ./gup
 CATEGORY="gup_test" run_test ./gup_longterm
 
 CATEGORY="userfaultfd" run_test ./uffd-unit-tests
-- 
2.39.5

Re: [PATCH v6 0/6] selftests/mm: separate GUP microbenchmarking from functional testing

[Mike Rapoport]

> gup_test.c currently serves two separate purposes: benchmarking
> (GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK and PIN_LONGTERM_BENCHMARK) and
> functional testing (GUP_BASIC_TEST, PIN_BASIC_TEST and
> DUMP_USER_PAGES_TEST). Keeping both in one program makes the functional
> tests harder to run and report individually, while run_vmtests.sh has to
> invoke the program repeatedly with different options.
> 
> Separate these roles into tools/mm/gup_bench for benchmarking and
> tools/testing/selftests/mm/gup for functional testing. Move the shared
> file and hugepage helpers to tools/lib/mm/ so both programs can use them
> without duplicating the implementation.

This sets seems to regress hugetlb tests: 

https://github.com/linux-mm/linux-mm/actions/runs/30553882784/job/90909219578

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 1/6] selftests/mm: make file helpers return errors

[Mike Rapoport]

> Change read_file(), write_file(), read_num() and write_num() in vm_util.c
> to report failures to callers instead of exiting from the helper.
> 
> Make read_file() return a negative errno on failure instead of 0, so
> callers can distinguish a successful read from an I/O error. Also make
> read_num() reject negative and malformed values.
> 
> Update callers to print diagnostics and fail wherever required. This
> patch prepares the helpers to be moved to tools/lib/mm without
> kselftest dependency.
> 
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>
> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
> index 2eab2110ac6a..db0db8a3df7c 100644
> --- a/tools/testing/selftests/mm/hugepage_settings.c
> +++ b/tools/testing/selftests/mm/hugepage_settings.c
> @@ -8,6 +8,7 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <errno.h>
>  
>  #include "vm_util.h"
>  #include "hugepage_settings.h"
> @@ -61,8 +62,10 @@ int thp_read_string(const char *name, const char * const strings[])
>  		exit(EXIT_FAILURE);
>  	}
>  
> -	if (!read_file(path, buf, sizeof(buf))) {
> -		perror(path);
> +	ret = read_file(path, buf, sizeof(buf));
> +	if (ret < 0) {
> +		errno = -ret;
> +		ksft_perror(path);

I'm not a fan of changing errno, why can't we use

	ksft_print_msg("%s: %s\n", path, strerror(ret));

>  		exit(EXIT_FAILURE);
>  	}
>  
> @@ -700,91 +700,139 @@ int unpoison_memory(unsigned long pfn)
>  
>  int read_file(const char *path, char *buf, size_t buflen)
>  {
> -	int fd;
> +	int fd, err;
>  	ssize_t numread;
>  
>  	fd = open(path, O_RDONLY);
>  	if (fd == -1)
> -		return 0;
> +		return -errno;
>  
>  	numread = read(fd, buf, buflen - 1);
>  	if (numread < 1) {
> +		err = numread ? errno : ENODATA;
>  		close(fd);
> -		return 0;
> +		return -err;
>  	}
>  
>  	buf[numread] = '\0';
>  	close(fd);
>  
> -	return (unsigned int) numread;
> +	return (int)numread;

Do we really care about how many bytes we read?
Can't we return 0 for success and -error code for failure?

Will also make checks for read_file() return value neater.

>  }
>  
> -unsigned long read_num(const char *path)
> +int read_num(const char *path, unsigned long *num)
>  {
> +	unsigned long val;
> +	int ret;
>  	char buf[21];
> +	char *end;
>  
> -	if (read_file(path, buf, sizeof(buf)) < 0)
> -		ksft_exit_fail_perror("read_file()");
> +	if (!num)
> +		return -EINVAL;
>  
> -	return strtoul(buf, NULL, 10);
> +	ret = read_file(path, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	errno = 0;
> +	val = strtoul(buf, &end, 10);
> +	if (errno)
> +		return -errno;
> +
> +	if (end == buf || buf[0] == '-')
> +		return -EINVAL;

We can check the sign right after read_file() and skip errno dance
around strtoul().

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 2/6] tools/lib/mm: add shared file helpers

[Mike Rapoport]

> Move read_file(), write_file(), read_num(), and write_num() out of
> tools/testing/selftests/mm/vm_util.c into a new shared helper under
> tools/lib/mm/.
> 
> @@ -187,8 +188,8 @@ TEST_FILES += write_hugetlb_memory.sh
>  
>  include ../lib.mk
>  
> -$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c
> -$(TEST_GEN_FILES): vm_util.c hugepage_settings.c
> +$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
> +$(TEST_GEN_FILES): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c

I was too lazy to change the Makefile, but we should do something to
compile those once and link to every TEST_PROG that uses them.

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 3/6] tools/lib/mm: move hugepage_settings out of selftests

[Mike Rapoport]

> Move hugepage_settings.[ch] from tools/testing/selftests/mm/ to
> tools/lib/mm/ so the THP and HugeTLB helpers can be shared more easily
> between selftests and other tools.
> 
> Keep the helpers exposed to mm selftests through vm_util.h where possible,
> and use direct <mm/hugepage_settings.h> includes for files that do not
> include vm_util.h. Adjust the selftests/mm build to compile the moved
> implementation from its new location.
> 
> Remove the remaining kselftest dependency from hugepage_settings.c.
> Replace ksft_perror() with a new print_file_error() helper and
> ksft_print_msg() with printf(). Prefix both with a # to keep them
> TAP compatible.
> 
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>
> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/lib/mm/hugepage_settings.c
> similarity index 94%
> rename from tools/testing/selftests/mm/hugepage_settings.c
> rename to tools/lib/mm/hugepage_settings.c
> index db0db8a3df7c..4471f4989dbb 100644
> --- a/tools/testing/selftests/mm/hugepage_settings.c
> +++ b/tools/lib/mm/hugepage_settings.c
> @@ -10,11 +10,16 @@
>  #include <unistd.h>
>  #include <errno.h>
>  
> -#include "vm_util.h"
> +#include "file_utils.h"
>  #include "hugepage_settings.h"
>  
>  #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
>  #define MAX_SETTINGS_DEPTH 4
> +
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> +#endif
> +

We already have ~20 of those in tools, so one more won't hurt :)

But really definition of ARRAY_SIZE() belogs to a header, maybe
something to cleanup later.

>  static struct thp_settings settings_stack[MAX_SETTINGS_DEPTH];
>  static int settings_index;
>  static struct thp_settings saved_settings;
> @@ -49,6 +54,13 @@ static const char * const shmem_enabled_strings[] = {
>  	NULL
>  };
>  
> +static void print_file_error(const char *path, int ret)

Maybe print_file_access_error()?

> +{
> +	int err = -ret;
> +
> +	printf("# %s: %s (%d)\n", path, strerror(err), err);
> +}
> +

You cooud add this at the first patch and avoid errno dance there and
all the changes for printing below.

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 5/6] tools/mm: make gup_bench a benchmark only tool

[Mike Rapoport]

> Remove the functional modes (GUP_BASIC_TEST, PIN_BASIC_TEST and
> DUMP_USER_PAGES_TEST) from gup_bench. Drop kselftest dependency
> and use normal diagnostics and exit statuses.
> 
> When no arguments are supplied, run a single GUP_FAST_BENCHMARK
> with existing default values. Let users select other configs
> through command line options. Also validate numeric arguments
> and reject positional arguments.
> 
> Restore hugeTLB settings on failure and after every run. Also
> handle failures without relying on assert() calls.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>

...

>  int main(int argc, char **argv)
>  {
>  	struct gup_test gup = { 0 };
> -	int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
> +	int filed, i, opt, nr_pages = 1, thp = -1, write = 1;
> +	int nthreads = 1, ret, started_threads = 0;
>  	int flags = MAP_PRIVATE;
> -	char *file = "/dev/zero";
> -	bool hugetlb = false;
> +	const char *file = "/dev/zero";
> +	bool hugetlb = false, restore_hugetlb = false;
> +	unsigned long nr_pages_per_call;
>  	pthread_t *tid;
>  	char *p;
>  
> -	while ((opt = getopt(argc, argv, "m:r:n:F:f:abcj:tTLUuwWSHpz")) != -1) {
> +	while ((opt = getopt(argc, argv, "m:r:n:F:f:aj:tTLuwWSH")) != -1) {
>  		switch (opt) {
>  		case 'a':
>  			cmd = PIN_FAST_BENCHMARK;
>  			break;
> -		case 'b':
> -			cmd = PIN_BASIC_TEST;
> -			break;
>  		case 'L':
>  			cmd = PIN_LONGTERM_BENCHMARK;
>  			break;
> -		case 'c':
> -			cmd = DUMP_USER_PAGES_TEST;
> -			/*
> -			 * Dump page 0 (index 1). May be overridden later, by
> -			 * user's non-option arguments.
> -			 *
> -			 * .which_pages is zero-based, so that zero can mean "do
> -			 * nothing".
> -			 */
> -			gup.which_pages[0] = 1;
> -			break;
> -		case 'p':
> -			/* works only with DUMP_USER_PAGES_TEST */
> -			gup.test_flags |= GUP_TEST_FLAG_DUMP_PAGES_USE_PIN;
> -			break;
> -		case 'F':
> -			/* strtol, so you can pass flags in hex form */
> -			gup.gup_flags = strtol(optarg, 0, 0);
> +		case 'F': {
> +			long val;
> +
> +			val = parse_long_arg_base(optarg, "GUP flags", 0);
> +			if (val < 0 || val > UINT_MAX) {
> +				fprintf(stderr, "Invalid GUP flags '%s'\n", optarg);
> +				exit(1);
> +			}
> +
> +			gup.gup_flags = val;
>  			break;
> -		case 'j':
> -			nthreads = atoi(optarg);
> +		}
> +		case 'j': {
> +			long val;
> +
> +			val = parse_positive_long_arg(optarg, "thread count");
> +			if (val > INT_MAX ||
> +			    (size_t)val > SIZE_MAX / sizeof(pthread_t)) {
> +				fprintf(stderr, "Invalid thread count '%s'\n", optarg);
> +				exit(1);
> +			}
> +			nthreads = val;
>  			break;
> +		}
>  		case 'm':
> -			size = atoi(optarg) * MB;
> +			size = parse_positive_long_arg(optarg, "size");
> +			if (size > ULONG_MAX / MB) {
> +				fprintf(stderr, "Invalid size '%s'\n", optarg);
> +				exit(1);
> +			}
> +			size *= MB;
>  			break;
> -		case 'r':
> -			repeats = atoi(optarg);
> +		case 'r': {
> +			long val;
> +
> +			val = parse_positive_long_arg(optarg, "repeat count");
> +			if (val > INT_MAX) {
> +				fprintf(stderr, "Invalid repeat count '%s'\n", optarg);
> +				exit(1);
> +			}
> +			repeats = val;
>  			break;
> -		case 'n':
> -			nr_pages = atoi(optarg);
> -			if (nr_pages < 0)
> -				nr_pages = size / getpagesize();
> +		}
> +		case 'n': {
> +			long val;
> +
> +			val = parse_long_arg(optarg, "page count");

It's better to name the numbers parsing after what they do:
parse_flags() and parse_num().

> +			if (val != -1 && (val < 1 || val > INT_MAX)) {
 
And the limit checks seem wierd all over the place, like if we can loop
infinitely, why do we care about INT_MAX?

And what exact limit ULONG_MAX / MB or SIZE_MAX / sizeof(ptread_t) are
supposed to express?

> +				fprintf(stderr, "Invalid page count '%s'\n", optarg);
> +				exit(1);
> +			}
> +			nr_pages = val;
>  			break;

...

>  	if (hugetlb) {
>  		unsigned long hp_size = default_huge_page_size();
>  
> -		if (!hp_size)
> -			ksft_exit_skip("HugeTLB is unavailable\n");
> +		if (!hp_size) {
> +			fprintf(stderr, "Could not determine huge page size\n");
> +			return 1;
> +		}
> +
> +		if (size > ULONG_MAX - (hp_size - 1)) {
> +			fprintf(stderr, "HugeTLB mapping size is too large\n");
> +			return 1;
> +		}
>  
>  		size = (size + hp_size - 1) & ~(hp_size - 1);
> -		if (!hugetlb_setup_default(size / hp_size))
> -			ksft_exit_skip("Not enough huge pages\n");
> +		if (!hugetlb_setup_default(size / hp_size)) {
> +			fprintf(stderr, "Not enough huge pages\n");
> +			hugetlb_restore_settings();

you don't need to explicitly call hugetlb_restore_settings(),
_setup_defaults() sets up automatic restore on exit.

> +			return 1;
> +		}
> +		restore_hugetlb = true;
>  	}

...

>  	gup_fd = open(GUP_TEST_FILE, O_RDWR);
>  	if (gup_fd == -1) {
> -		switch (errno) {
> -		case EACCES:
> -			if (getuid())
> -				ksft_print_msg("Please run this test as root\n");
> -			break;
> -		case ENOENT:
> -			if (opendir("/sys/kernel/debug") == NULL)
> -				ksft_print_msg("mount debugfs at /sys/kernel/debug\n");
> -			ksft_print_msg("check if CONFIG_GUP_TEST is enabled in kernel config\n");
> -			break;
> -		default:
> -			ksft_print_msg("failed to open %s: %s\n", GUP_TEST_FILE, strerror(errno));
> -			break;
> -		}
> -		ksft_test_result_skip("Please run this test as root\n");
> -		ksft_exit_pass();
> +		int err = errno;
> +
> +		close(filed);
> +		if (err == EACCES)

What was wrong with switch (errno) ?

> +			fprintf(stderr, "Please run as root\n");

Please add root check upfront and skip EACCES here

> +		else if (err == ENOENT) {
> +			DIR *debugfs = opendir("/sys/kernel/debug");
> +
> +			if (!debugfs)
> +				fprintf(stderr, "Mount debugfs at /sys/kernel/debug\n");

Just replace the prints, no need to refactor the logic there.

> +			else {
> +				closedir(debugfs);
> +				fprintf(stderr, "Check CONFIG_GUP_TEST in kernel config\n");
> +			}
> +		} else
> +			fprintf(stderr, "Failed to open %s: %s\n", GUP_TEST_FILE,
> +				strerror(err));
> +		if (restore_hugetlb)
> +			hugetlb_restore_settings();
> +		return 1;
>  	}
>  
>  	p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, filed, 0);
> -	if (p == MAP_FAILED)
> -		ksft_exit_fail_msg("mmap: %s\n", strerror(errno));
> +	if (p == MAP_FAILED) {
> +		fprintf(stderr, "mmap: %s\n", strerror(errno));
> +		close(filed);
> +		close(gup_fd);
> +
> +		if (restore_hugetlb)
> +			hugetlb_restore_settings();

Use goto err_do_cleanup here and everywhere else. Piling cleanups in 
if (something_failed) is error prone and unmaintainable.

> +		return 1;
> +	}
> +	close(filed);
>  	gup.addr = (unsigned long)p;
>  
>  	if (thp == 1)
  
...

>  	free(tid);
> +	munmap((void *)gup.addr, size);
> +	close(gup_fd);
> +	if (restore_hugetlb)
> +		hugetlb_restore_settings();
>  
> -	ksft_exit_pass();
> +	return bench_error ? 1 : 0;

Using goto for cleanup gives you clean return 1 on error and return 0 on
success.

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 6/6] selftests/mm: add a GUP selftest

[Mike Rapoport]

> Add a new GUP selftest which uses kselftest_harness.h. Cover
> 12 mapping configurations: THP enabled, THP disabled and
> HugeTLB, each across private/shared mappings and with/without
> FOLL_WRITE. Run 7 testcases for every variant: get_user_pages,
> get_user_pages_fast, pin_user_pages, pin_user_pages_fast,
> pin_user_pages_longterm, and DUMP_USER_PAGES_TEST using both
> get and pin.
> 
> Sweep four nr_pages_per_call values for each test: 1, 512, 123 and
> all pages. This preserves the coverage previously provided by
> run_gup_matrix(): 12 mapping combinations x 5 GUP/PUP operations x 4
> batch sizes, for 240 ioctl calls. The two dump modes add another 96
> calls.
> 
> Preserve the previous sparse dump coverage with a standalone test for
> pages 0, 19 and 0x1000. In total the selftest reports 85 TAP
> cases and issues 337 ioctls.
> 
> Add the new gup binary to the selftests/mm build, .gitignore,
> run_vmtests.sh and MAINTAINERS. Update
> Documentation/core-api/pin_user_pages.rst for the new test.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>
> +
> +FIXTURE_SETUP(gup_test)
> +{
> +	int mmap_flags = MAP_PRIVATE;
> +	int zero_fd;
> +	char *p;
> +
> +	/* zero_fd has to be >= 0. Already checked in main() */
> +	zero_fd = open("/dev/zero", O_RDWR);
> +	ASSERT_GE(zero_fd, 0);
> +
> +	/* gup_fd has to be >= 0. Already checked in main() */
> +	self->gup_fd = open(GUP_TEST_FILE, O_RDWR);
> +	ASSERT_GE(self->gup_fd, 0);
> +
> +	self->size = variant->hugetlb ? 256 * MB : 128 * MB;

I'd derive the hugetbl variant size from the size of a huge page and
predefined number of huge pages.

> +
> +	if (variant->hugetlb) {
> +		unsigned long hp_size = default_huge_page_size();
> +
> +		if (!hp_size) {
> +			close(zero_fd);
> +			close(self->gup_fd);

You can move hugetlb setup after opening those and save the headache of
closing them.

And in any rate prefer 

	goto err_do_cleanup

to
	if (something_failed) {
		cleanup1();
		cleanup2();
	}

	if (something_else_failed) {
		cleanup1();
		cleanup2();
		cleanup3();
	}

> +			SKIP(return, "HugeTLB not available\n");
> +		}
> +
> +		self->size = (self->size + hp_size - 1) & ~(hp_size - 1);
> +		if (!hugetlb_setup_default(self->size / hp_size)) {
> +			hugetlb_restore_settings();

No need to call restore() here.

Also, if you use a constant number of huge pages you can just add
HUGETLB_SETUP_DEFAULT_PAGES(NR_HUGE_PAGES) somewhere in the begining and
you won't need to setup and teardown hugetlb explicitly for every
hugetlb test.


> +			close(zero_fd);
> +			close(self->gup_fd);
> +			SKIP(return, "Not enough huge pages\n");
> +		}
> +
> +		mmap_flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
> +	}
> +
> +	if (variant->shared)
> +		mmap_flags = (mmap_flags & ~MAP_PRIVATE) | MAP_SHARED;
> +
> +	self->addr = mmap(NULL, self->size, PROT_READ | PROT_WRITE,
> +			  mmap_flags, zero_fd, 0);
> +
> +	ASSERT_NE(self->addr, MAP_FAILED) {
> +		int err = errno;
> +
> +		close(zero_fd);
> +		close(self->gup_fd);
> +		if (variant->hugetlb)
> +			hugetlb_restore_settings();
> +		TH_LOG("mmap failed: %s", strerror(err));
> +	}
> +	close(zero_fd);
> +
> +	if (variant->thp)
> +		madvise(self->addr, self->size, MADV_HUGEPAGE);
> +	else if (!variant->hugetlb)
> +		madvise(self->addr, self->size, MADV_NOHUGEPAGE);
> +
> +	for (p = self->addr; (unsigned long)p < (unsigned long)self->addr
> +			+ self->size; p += psize())
> +		p[0] = 0;
> +}
> +
> +FIXTURE_TEARDOWN(gup_test)
> +{
> +	munmap(self->addr, self->size);
> +	close(self->gup_fd);
> +
> +	if (variant->hugetlb)
> +		hugetlb_restore_settings();
> +}
> +
> +static void run_gup_cmd(struct __test_metadata *_metadata,
> +			 FIXTURE_DATA(gup_test) *self,
> +			 const FIXTURE_VARIANT(gup_test) *variant,
> +			 unsigned long command,
> +			 unsigned int test_flags,
> +			 unsigned int which_page)
> +{
> +	int i;
> +
> +	for (i = 0; i < (int)ARRAY_SIZE(nr_pages_list); i++) {
> +		struct gup_test gup = {
> +			.addr = (unsigned long)self->addr,
> +			.size = self->size,
> +			.nr_pages_per_call = nr_pages_list[i] < 0 ?
> +				self->size / psize() : nr_pages_list[i],
> +			.test_flags = test_flags,
> +		};
> +
> +		if (variant->write)
> +			gup.gup_flags |= FOLL_WRITE;
> +
> +		gup.which_pages[0] = which_page;

Can't these to go to the static initialization?

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 3/6] tools/lib/mm: move hugepage_settings out of selftests

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On Mon, Aug 03, 2026 at 12:00:01PM +0300, Mike Rapoport wrote:

> > +#ifndef ARRAY_SIZE
> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> > +#endif

> We already have ~20 of those in tools, so one more won't hurt :)

> But really definition of ARRAY_SIZE() belogs to a header, maybe
> something to cleanup later.

tools/include/linux/kernel.h.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 0/6] selftests/mm: separate GUP microbenchmarking from functional testing

[Sarthak Sharma]

Hi Mike!

On 8/3/26 2:30 PM, Mike Rapoport wrote:
>> gup_test.c currently serves two separate purposes: benchmarking
>> (GUP_FAST_BENCHMARK, PIN_FAST_BENCHMARK and PIN_LONGTERM_BENCHMARK) and
>> functional testing (GUP_BASIC_TEST, PIN_BASIC_TEST and
>> DUMP_USER_PAGES_TEST). Keeping both in one program makes the functional
>> tests harder to run and report individually, while run_vmtests.sh has to
>> invoke the program repeatedly with different options.
>>
>> Separate these roles into tools/mm/gup_bench for benchmarking and
>> tools/testing/selftests/mm/gup for functional testing. Move the shared
>> file and hugepage helpers to tools/lib/mm/ so both programs can use them
>> without duplicating the implementation.
> 
> This sets seems to regress hugetlb tests: 
> 
> https://github.com/linux-mm/linux-mm/actions/runs/30553882784/job/90909219578
> 

Oh, thanks for pointing this out. I found out that the issue is in
thuge-gen.c, where we call test_shmget(getpagesize(), 0), which
internally calls hugetlb_free_pages(size). This attempts to read
/sys/kernel/mm/hugepages/hugepages-4kB, which does not exist.

Earlier, read_file() returned 0 for this case, so there was no issue.
Now that it returns an error for this, the test failed.

Will fix this.

Re: [PATCH v6 1/6] selftests/mm: make file helpers return errors

[Sarthak Sharma]

Hi Mike!

On 8/3/26 2:30 PM, Mike Rapoport wrote:
>> Change read_file(), write_file(), read_num() and write_num() in vm_util.c
>> to report failures to callers instead of exiting from the helper.
>>
>> Make read_file() return a negative errno on failure instead of 0, so
>> callers can distinguish a successful read from an I/O error. Also make
>> read_num() reject negative and malformed values.
>>
>> Update callers to print diagnostics and fail wherever required. This
>> patch prepares the helpers to be moved to tools/lib/mm without
>> kselftest dependency.
>>
>> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>>
>> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
>> index 2eab2110ac6a..db0db8a3df7c 100644
>> --- a/tools/testing/selftests/mm/hugepage_settings.c
>> +++ b/tools/testing/selftests/mm/hugepage_settings.c
>> @@ -8,6 +8,7 @@
>>  #include <stdlib.h>
>>  #include <string.h>
>>  #include <unistd.h>
>> +#include <errno.h>
>>  
>>  #include "vm_util.h"
>>  #include "hugepage_settings.h"
>> @@ -61,8 +62,10 @@ int thp_read_string(const char *name, const char * const strings[])
>>  		exit(EXIT_FAILURE);
>>  	}
>>  
>> -	if (!read_file(path, buf, sizeof(buf))) {
>> -		perror(path);
>> +	ret = read_file(path, buf, sizeof(buf));
>> +	if (ret < 0) {
>> +		errno = -ret;
>> +		ksft_perror(path);
> 
> I'm not a fan of changing errno, why can't we use
> 
> 	ksft_print_msg("%s: %s\n", path, strerror(ret));

Ack

> 
>>  		exit(EXIT_FAILURE);
>>  	}
>>  
>> @@ -700,91 +700,139 @@ int unpoison_memory(unsigned long pfn)
>>  
>>  int read_file(const char *path, char *buf, size_t buflen)
>>  {
>> -	int fd;
>> +	int fd, err;
>>  	ssize_t numread;
>>  
>>  	fd = open(path, O_RDONLY);
>>  	if (fd == -1)
>> -		return 0;
>> +		return -errno;
>>  
>>  	numread = read(fd, buf, buflen - 1);
>>  	if (numread < 1) {
>> +		err = numread ? errno : ENODATA;
>>  		close(fd);
>> -		return 0;
>> +		return -err;
>>  	}
>>  
>>  	buf[numread] = '\0';
>>  	close(fd);
>>  
>> -	return (unsigned int) numread;
>> +	return (int)numread;
> 
> Do we really care about how many bytes we read?
> Can't we return 0 for success and -error code for failure?
> 
> Will also make checks for read_file() return value neater.

Yeah, no caller actually uses the number of bytes read. Will make this
change.

> 
>>  }
>>  
>> -unsigned long read_num(const char *path)
>> +int read_num(const char *path, unsigned long *num)
>>  {
>> +	unsigned long val;
>> +	int ret;
>>  	char buf[21];
>> +	char *end;
>>  
>> -	if (read_file(path, buf, sizeof(buf)) < 0)
>> -		ksft_exit_fail_perror("read_file()");
>> +	if (!num)
>> +		return -EINVAL;
>>  
>> -	return strtoul(buf, NULL, 10);
>> +	ret = read_file(path, buf, sizeof(buf));
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	errno = 0;
>> +	val = strtoul(buf, &end, 10);
>> +	if (errno)
>> +		return -errno;
>> +
>> +	if (end == buf || buf[0] == '-')
>> +		return -EINVAL;
> 
> We can check the sign right after read_file() and skip errno dance
> around strtoul().

Yes, we can check the sign after read_file(), but still we would have to
check errno after strtoul() to see if an unsigned long overflow happened.

Re: [PATCH v6 2/6] tools/lib/mm: add shared file helpers

[Sarthak Sharma]

Hi Mike!

On 8/3/26 2:30 PM, Mike Rapoport wrote:
>> Move read_file(), write_file(), read_num(), and write_num() out of
>> tools/testing/selftests/mm/vm_util.c into a new shared helper under
>> tools/lib/mm/.
>>
>> @@ -187,8 +188,8 @@ TEST_FILES += write_hugetlb_memory.sh
>>  
>>  include ../lib.mk
>>  
>> -$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c
>> -$(TEST_GEN_FILES): vm_util.c hugepage_settings.c
>> +$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
>> +$(TEST_GEN_FILES): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
> 
> I was too lazy to change the Makefile, but we should do something to
> compile those once and link to every TEST_PROG that uses them.

Yes, we can do that. But since this is preexisting, let's keep it
separate from this series. I can send a patch for this separately. Would
that be okay?

Re: [PATCH v6 5/6] tools/mm: make gup_bench a benchmark only tool

[Sarthak Sharma]

Hi Mike!

On 8/3/26 2:30 PM, Mike Rapoport wrote:
>> Remove the functional modes (GUP_BASIC_TEST, PIN_BASIC_TEST and
>> DUMP_USER_PAGES_TEST) from gup_bench. Drop kselftest dependency
>> and use normal diagnostics and exit statuses.
>>
>> When no arguments are supplied, run a single GUP_FAST_BENCHMARK
>> with existing default values. Let users select other configs
>> through command line options. Also validate numeric arguments
>> and reject positional arguments.
>>
>> Restore hugeTLB settings on failure and after every run. Also
>> handle failures without relying on assert() calls.
>>
>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
>> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> 
> ...
> 
>>  int main(int argc, char **argv)
>>  {
>>  	struct gup_test gup = { 0 };
>> -	int filed, i, opt, nr_pages = 1, thp = -1, write = 1, nthreads = 1, ret;
>> +	int filed, i, opt, nr_pages = 1, thp = -1, write = 1;
>> +	int nthreads = 1, ret, started_threads = 0;
>>  	int flags = MAP_PRIVATE;
>> -	char *file = "/dev/zero";
>> -	bool hugetlb = false;
>> +	const char *file = "/dev/zero";
>> +	bool hugetlb = false, restore_hugetlb = false;
>> +	unsigned long nr_pages_per_call;
>>  	pthread_t *tid;
>>  	char *p;
>>  
>> -	while ((opt = getopt(argc, argv, "m:r:n:F:f:abcj:tTLUuwWSHpz")) != -1) {
>> +	while ((opt = getopt(argc, argv, "m:r:n:F:f:aj:tTLuwWSH")) != -1) {
>>  		switch (opt) {
>>  		case 'a':
>>  			cmd = PIN_FAST_BENCHMARK;
>>  			break;
>> -		case 'b':
>> -			cmd = PIN_BASIC_TEST;
>> -			break;
>>  		case 'L':
>>  			cmd = PIN_LONGTERM_BENCHMARK;
>>  			break;
>> -		case 'c':
>> -			cmd = DUMP_USER_PAGES_TEST;
>> -			/*
>> -			 * Dump page 0 (index 1). May be overridden later, by
>> -			 * user's non-option arguments.
>> -			 *
>> -			 * .which_pages is zero-based, so that zero can mean "do
>> -			 * nothing".
>> -			 */
>> -			gup.which_pages[0] = 1;
>> -			break;
>> -		case 'p':
>> -			/* works only with DUMP_USER_PAGES_TEST */
>> -			gup.test_flags |= GUP_TEST_FLAG_DUMP_PAGES_USE_PIN;
>> -			break;
>> -		case 'F':
>> -			/* strtol, so you can pass flags in hex form */
>> -			gup.gup_flags = strtol(optarg, 0, 0);
>> +		case 'F': {
>> +			long val;
>> +
>> +			val = parse_long_arg_base(optarg, "GUP flags", 0);
>> +			if (val < 0 || val > UINT_MAX) {
>> +				fprintf(stderr, "Invalid GUP flags '%s'\n", optarg);
>> +				exit(1);
>> +			}
>> +
>> +			gup.gup_flags = val;
>>  			break;
>> -		case 'j':
>> -			nthreads = atoi(optarg);
>> +		}
>> +		case 'j': {
>> +			long val;
>> +
>> +			val = parse_positive_long_arg(optarg, "thread count");
>> +			if (val > INT_MAX ||
>> +			    (size_t)val > SIZE_MAX / sizeof(pthread_t)) {
>> +				fprintf(stderr, "Invalid thread count '%s'\n", optarg);
>> +				exit(1);
>> +			}
>> +			nthreads = val;
>>  			break;
>> +		}
>>  		case 'm':
>> -			size = atoi(optarg) * MB;
>> +			size = parse_positive_long_arg(optarg, "size");
>> +			if (size > ULONG_MAX / MB) {
>> +				fprintf(stderr, "Invalid size '%s'\n", optarg);
>> +				exit(1);
>> +			}
>> +			size *= MB;
>>  			break;
>> -		case 'r':
>> -			repeats = atoi(optarg);
>> +		case 'r': {
>> +			long val;
>> +
>> +			val = parse_positive_long_arg(optarg, "repeat count");
>> +			if (val > INT_MAX) {
>> +				fprintf(stderr, "Invalid repeat count '%s'\n", optarg);
>> +				exit(1);
>> +			}
>> +			repeats = val;
>>  			break;
>> -		case 'n':
>> -			nr_pages = atoi(optarg);
>> -			if (nr_pages < 0)
>> -				nr_pages = size / getpagesize();
>> +		}
>> +		case 'n': {
>> +			long val;
>> +
>> +			val = parse_long_arg(optarg, "page count");
> 
> It's better to name the numbers parsing after what they do:
> parse_flags() and parse_num().

Ack

> 
>> +			if (val != -1 && (val < 1 || val > INT_MAX)) {
>  
> And the limit checks seem wierd all over the place, like if we can loop
> infinitely, why do we care about INT_MAX?

INT_MAX checks are there since nr_pages, nthreads and repeats are stored
as int.

But yes I can keep the parameters which are not there in the ioctl ABI
to be unsigned long, so these checks won't be required there.

> 
> And what exact limit ULONG_MAX / MB or SIZE_MAX / sizeof(ptread_t) are
> supposed to express?

ULONG_MAX / MB prevents size *= MB from overflowing. SIZE_MAX /
sizeof(ptread_t) prevents thread array allocation size from overflowing.
> 
>> +				fprintf(stderr, "Invalid page count '%s'\n", optarg);
>> +				exit(1);
>> +			}
>> +			nr_pages = val;
>>  			break;
> 
> ...
> 
>>  	if (hugetlb) {
>>  		unsigned long hp_size = default_huge_page_size();
>>  
>> -		if (!hp_size)
>> -			ksft_exit_skip("HugeTLB is unavailable\n");
>> +		if (!hp_size) {
>> +			fprintf(stderr, "Could not determine huge page size\n");
>> +			return 1;
>> +		}
>> +
>> +		if (size > ULONG_MAX - (hp_size - 1)) {
>> +			fprintf(stderr, "HugeTLB mapping size is too large\n");
>> +			return 1;
>> +		}
>>  
>>  		size = (size + hp_size - 1) & ~(hp_size - 1);
>> -		if (!hugetlb_setup_default(size / hp_size))
>> -			ksft_exit_skip("Not enough huge pages\n");
>> +		if (!hugetlb_setup_default(size / hp_size)) {
>> +			fprintf(stderr, "Not enough huge pages\n");
>> +			hugetlb_restore_settings();
> 
> you don't need to explicitly call hugetlb_restore_settings(),
> _setup_defaults() sets up automatic restore on exit.

Ack

> 
>> +			return 1;
>> +		}
>> +		restore_hugetlb = true;
>>  	}
> 
> ...
> 
>>  	gup_fd = open(GUP_TEST_FILE, O_RDWR);
>>  	if (gup_fd == -1) {
>> -		switch (errno) {
>> -		case EACCES:
>> -			if (getuid())
>> -				ksft_print_msg("Please run this test as root\n");
>> -			break;
>> -		case ENOENT:
>> -			if (opendir("/sys/kernel/debug") == NULL)
>> -				ksft_print_msg("mount debugfs at /sys/kernel/debug\n");
>> -			ksft_print_msg("check if CONFIG_GUP_TEST is enabled in kernel config\n");
>> -			break;
>> -		default:
>> -			ksft_print_msg("failed to open %s: %s\n", GUP_TEST_FILE, strerror(errno));
>> -			break;
>> -		}
>> -		ksft_test_result_skip("Please run this test as root\n");
>> -		ksft_exit_pass();
>> +		int err = errno;
>> +
>> +		close(filed);
>> +		if (err == EACCES)
> 
> What was wrong with switch (errno) ?
> 
>> +			fprintf(stderr, "Please run as root\n");>
> Please add root check upfront and skip EACCES here

Ack

> 
>> +		else if (err == ENOENT) {
>> +			DIR *debugfs = opendir("/sys/kernel/debug");
>> +
>> +			if (!debugfs)
>> +				fprintf(stderr, "Mount debugfs at /sys/kernel/debug\n");
> 
> Just replace the prints, no need to refactor the logic there.

Okay

> 
>> +			else {
>> +				closedir(debugfs);
>> +				fprintf(stderr, "Check CONFIG_GUP_TEST in kernel config\n");
>> +			}
>> +		} else
>> +			fprintf(stderr, "Failed to open %s: %s\n", GUP_TEST_FILE,
>> +				strerror(err));
>> +		if (restore_hugetlb)
>> +			hugetlb_restore_settings();
>> +		return 1;
>>  	}
>>  
>>  	p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, filed, 0);
>> -	if (p == MAP_FAILED)
>> -		ksft_exit_fail_msg("mmap: %s\n", strerror(errno));
>> +	if (p == MAP_FAILED) {
>> +		fprintf(stderr, "mmap: %s\n", strerror(errno));
>> +		close(filed);
>> +		close(gup_fd);
>> +
>> +		if (restore_hugetlb)
>> +			hugetlb_restore_settings();
> 
> Use goto err_do_cleanup here and everywhere else. Piling cleanups in 
> if (something_failed) is error prone and unmaintainable.

Ack

> 
>> +		return 1;
>> +	}
>> +	close(filed);
>>  	gup.addr = (unsigned long)p;
>>  
>>  	if (thp == 1)
>   
> ...
> 
>>  	free(tid);
>> +	munmap((void *)gup.addr, size);
>> +	close(gup_fd);
>> +	if (restore_hugetlb)
>> +		hugetlb_restore_settings();
>>  
>> -	ksft_exit_pass();
>> +	return bench_error ? 1 : 0;
> 
> Using goto for cleanup gives you clean return 1 on error and return 0 on
> success.

Re: [PATCH v6 6/6] selftests/mm: add a GUP selftest

[Sarthak Sharma]

Hi Mike!

On 8/3/26 2:30 PM, Mike Rapoport wrote:
>> Add a new GUP selftest which uses kselftest_harness.h. Cover
>> 12 mapping configurations: THP enabled, THP disabled and
>> HugeTLB, each across private/shared mappings and with/without
>> FOLL_WRITE. Run 7 testcases for every variant: get_user_pages,
>> get_user_pages_fast, pin_user_pages, pin_user_pages_fast,
>> pin_user_pages_longterm, and DUMP_USER_PAGES_TEST using both
>> get and pin.
>>
>> Sweep four nr_pages_per_call values for each test: 1, 512, 123 and
>> all pages. This preserves the coverage previously provided by
>> run_gup_matrix(): 12 mapping combinations x 5 GUP/PUP operations x 4
>> batch sizes, for 240 ioctl calls. The two dump modes add another 96
>> calls.
>>
>> Preserve the previous sparse dump coverage with a standalone test for
>> pages 0, 19 and 0x1000. In total the selftest reports 85 TAP
>> cases and issues 337 ioctls.
>>
>> Add the new gup binary to the selftests/mm build, .gitignore,
>> run_vmtests.sh and MAINTAINERS. Update
>> Documentation/core-api/pin_user_pages.rst for the new test.
>>
>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
>> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>>
>> +
>> +FIXTURE_SETUP(gup_test)
>> +{
>> +	int mmap_flags = MAP_PRIVATE;
>> +	int zero_fd;
>> +	char *p;
>> +
>> +	/* zero_fd has to be >= 0. Already checked in main() */
>> +	zero_fd = open("/dev/zero", O_RDWR);
>> +	ASSERT_GE(zero_fd, 0);
>> +
>> +	/* gup_fd has to be >= 0. Already checked in main() */
>> +	self->gup_fd = open(GUP_TEST_FILE, O_RDWR);
>> +	ASSERT_GE(self->gup_fd, 0);
>> +
>> +	self->size = variant->hugetlb ? 256 * MB : 128 * MB;
> 
> I'd derive the hugetbl variant size from the size of a huge page and
> predefined number of huge pages.
> 

I was following the existing logic that run_gup_matrix() had. I can
implement this. Any suggestions what number of huge pages we can fix?

>> +
>> +	if (variant->hugetlb) {
>> +		unsigned long hp_size = default_huge_page_size();
>> +
>> +		if (!hp_size) {
>> +			close(zero_fd);
>> +			close(self->gup_fd);
> 
> You can move hugetlb setup after opening those and save the headache of
> closing them.
> 
> And in any rate prefer 
> 
> 	goto err_do_cleanup
> 
> to
> 	if (something_failed) {
> 		cleanup1();
> 		cleanup2();
> 	}
> 
> 	if (something_else_failed) {
> 		cleanup1();
> 		cleanup2();
> 		cleanup3();
> 	}

Got it, will change.

> 
>> +			SKIP(return, "HugeTLB not available\n");
>> +		}
>> +
>> +		self->size = (self->size + hp_size - 1) & ~(hp_size - 1);
>> +		if (!hugetlb_setup_default(self->size / hp_size)) {
>> +			hugetlb_restore_settings();
> 
> No need to call restore() here.
> 
> Also, if you use a constant number of huge pages you can just add
> HUGETLB_SETUP_DEFAULT_PAGES(NR_HUGE_PAGES) somewhere in the begining and
> you won't need to setup and teardown hugetlb explicitly for every
> hugetlb test.

Ack

> 
> 
>> +			close(zero_fd);
>> +			close(self->gup_fd);
>> +			SKIP(return, "Not enough huge pages\n");
>> +		}
>> +
>> +		mmap_flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
>> +	}
>> +
>> +	if (variant->shared)
>> +		mmap_flags = (mmap_flags & ~MAP_PRIVATE) | MAP_SHARED;
>> +
>> +	self->addr = mmap(NULL, self->size, PROT_READ | PROT_WRITE,
>> +			  mmap_flags, zero_fd, 0);
>> +
>> +	ASSERT_NE(self->addr, MAP_FAILED) {
>> +		int err = errno;
>> +
>> +		close(zero_fd);
>> +		close(self->gup_fd);
>> +		if (variant->hugetlb)
>> +			hugetlb_restore_settings();
>> +		TH_LOG("mmap failed: %s", strerror(err));
>> +	}
>> +	close(zero_fd);
>> +
>> +	if (variant->thp)
>> +		madvise(self->addr, self->size, MADV_HUGEPAGE);
>> +	else if (!variant->hugetlb)
>> +		madvise(self->addr, self->size, MADV_NOHUGEPAGE);
>> +
>> +	for (p = self->addr; (unsigned long)p < (unsigned long)self->addr
>> +			+ self->size; p += psize())
>> +		p[0] = 0;
>> +}
>> +
>> +FIXTURE_TEARDOWN(gup_test)
>> +{
>> +	munmap(self->addr, self->size);
>> +	close(self->gup_fd);
>> +
>> +	if (variant->hugetlb)
>> +		hugetlb_restore_settings();
>> +}
>> +
>> +static void run_gup_cmd(struct __test_metadata *_metadata,
>> +			 FIXTURE_DATA(gup_test) *self,
>> +			 const FIXTURE_VARIANT(gup_test) *variant,
>> +			 unsigned long command,
>> +			 unsigned int test_flags,
>> +			 unsigned int which_page)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < (int)ARRAY_SIZE(nr_pages_list); i++) {
>> +		struct gup_test gup = {
>> +			.addr = (unsigned long)self->addr,
>> +			.size = self->size,
>> +			.nr_pages_per_call = nr_pages_list[i] < 0 ?
>> +				self->size / psize() : nr_pages_list[i],
>> +			.test_flags = test_flags,
>> +		};
>> +
>> +		if (variant->write)
>> +			gup.gup_flags |= FOLL_WRITE;
>> +
>> +		gup.which_pages[0] = which_page;
> 
> Can't these to go to the static initialization?

Will move both which_pages and gup_flags to the initialization.

Re: [PATCH v6 2/6] tools/lib/mm: add shared file helpers

[Mike Rapoport]

Hi Sarthak,

On Tue, Aug 04, 2026 at 12:13:06PM +0530, Sarthak Sharma wrote:
> Hi Mike!
> 
> On 8/3/26 2:30 PM, Mike Rapoport wrote:
> >> Move read_file(), write_file(), read_num(), and write_num() out of
> >> tools/testing/selftests/mm/vm_util.c into a new shared helper under
> >> tools/lib/mm/.
> >>
> >> @@ -187,8 +188,8 @@ TEST_FILES += write_hugetlb_memory.sh
> >>  
> >>  include ../lib.mk
> >>  
> >> -$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c
> >> -$(TEST_GEN_FILES): vm_util.c hugepage_settings.c
> >> +$(TEST_GEN_PROGS): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
> >> +$(TEST_GEN_FILES): vm_util.c hugepage_settings.c $(top_srcdir)/tools/lib/mm/file_utils.c
> > 
> > I was too lazy to change the Makefile, but we should do something to
> > compile those once and link to every TEST_PROG that uses them.
> 
> Yes, we can do that. But since this is preexisting, let's keep it
> separate from this series. I can send a patch for this separately. Would
> that be okay?

Works for me :) 

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 5/6] tools/mm: make gup_bench a benchmark only tool

[Mike Rapoport]

Hi Sarthak,

On Tue, Aug 04, 2026 at 01:04:13PM +0530, Sarthak Sharma wrote:
> Hi Mike!
> 
> On 8/3/26 2:30 PM, Mike Rapoport wrote:
> >  
> > And the limit checks seem wierd all over the place, like if we can loop
> > infinitely, why do we care about INT_MAX?
> 
> INT_MAX checks are there since nr_pages, nthreads and repeats are stored
> as int.
> 
> But yes I can keep the parameters which are not there in the ioctl ABI
> to be unsigned long, so these checks won't be required there.
> > 
> > And what exact limit ULONG_MAX / MB or SIZE_MAX / sizeof(ptread_t) are
> > supposed to express?
> 
> ULONG_MAX / MB prevents size *= MB from overflowing. SIZE_MAX /
> sizeof(ptread_t) prevents thread array allocation size from overflowing.

To me these checks seem overprotective from one side and not actually
serving a real purpose from the other.

Like e.g. requesting INT_MAX - 1 pages passes the check but it means almost 8TB
of memory and that's really wild.

I'd say let's keep things simple and if a user is stupid enough to run this
with peculiar command line just let it crash.

-- 
Sincerely yours,
Mike.

Re: [PATCH v6 6/6] selftests/mm: add a GUP selftest

[Mike Rapoport]

Hi Sarthak,

On Tue, Aug 04, 2026 at 01:08:05PM +0530, Sarthak Sharma wrote:
> Hi Mike!
> 
> On 8/3/26 2:30 PM, Mike Rapoport wrote:
> >> Add a new GUP selftest which uses kselftest_harness.h. Cover
> >> 12 mapping configurations: THP enabled, THP disabled and
> >> HugeTLB, each across private/shared mappings and with/without
> >> FOLL_WRITE. Run 7 testcases for every variant: get_user_pages,
> >> get_user_pages_fast, pin_user_pages, pin_user_pages_fast,
> >> pin_user_pages_longterm, and DUMP_USER_PAGES_TEST using both
> >> get and pin.
> >>
> >> Sweep four nr_pages_per_call values for each test: 1, 512, 123 and
> >> all pages. This preserves the coverage previously provided by
> >> run_gup_matrix(): 12 mapping combinations x 5 GUP/PUP operations x 4
> >> batch sizes, for 240 ioctl calls. The two dump modes add another 96
> >> calls.
> >>
> >> Preserve the previous sparse dump coverage with a standalone test for
> >> pages 0, 19 and 0x1000. In total the selftest reports 85 TAP
> >> cases and issues 337 ioctls.
> >>
> >> Add the new gup binary to the selftests/mm build, .gitignore,
> >> run_vmtests.sh and MAINTAINERS. Update
> >> Documentation/core-api/pin_user_pages.rst for the new test.
> >>
> >> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> >> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> >>
> >> +
> >> +FIXTURE_SETUP(gup_test)
> >> +{
> >> +	int mmap_flags = MAP_PRIVATE;
> >> +	int zero_fd;
> >> +	char *p;
> >> +
> >> +	/* zero_fd has to be >= 0. Already checked in main() */
> >> +	zero_fd = open("/dev/zero", O_RDWR);
> >> +	ASSERT_GE(zero_fd, 0);
> >> +
> >> +	/* gup_fd has to be >= 0. Already checked in main() */
> >> +	self->gup_fd = open(GUP_TEST_FILE, O_RDWR);
> >> +	ASSERT_GE(self->gup_fd, 0);
> >> +
> >> +	self->size = variant->hugetlb ? 256 * MB : 128 * MB;
> > 
> > I'd derive the hugetbl variant size from the size of a huge page and
> > predefined number of huge pages.
> > 
> 
> I was following the existing logic that run_gup_matrix() had. I can
> implement this. Any suggestions what number of huge pages we can fix?

You currently use 128 hugepages for the most common case of 2M default huge
page, so setting, say, NR_HUGEPAGES to 128 seems reasonable.

Later we could extend this to better deal with other huge page sizes.

-- 
Sincerely yours,
Mike.