[PATCH 0/8] selftests/vm: gup_test, hmm-tests, assorted improvements

[PATCH 0/8] selftests/vm: gup_test, hmm-tests, assorted improvements

[John Hubbard]

This is based on the latest mmotm.

Summary: This series provides two main things, and a number of smaller
supporting goodies. The two main points are:

1) Add a new sub-test to gup_test, which in turn is a renamed version of
gup_benchmark. This sub-test allows nicer testing of dump_pages(), at
least on user-space pages.

For quite a while, I was doing a quick hack to gup_test.c whenever I
wanted to try out changes to dump_page(). Then Matthew Wilcox asked me
what I meant when I said "I used my dump_page() unit test", and I
realized that it might be nice to check in a polished up version of
that.

Details about how it works and how to use it are in the commit
description for patch #6.

2) Fixes a limitation of hmm-tests: these tests are incredibly useful,
but only if people actually build and run them. And it turns out that
libhugetlbfs is a little too effective at throwing a wrench in the
works, there. So I've added a little configuration check that removes
just two of the 21 hmm-tests, if libhugetlbfs is not available.

Further details in the commit description of patch #8.

Other smaller things that this series does:

a) Remove code duplication by creating gup_test.h.

b) Clear up the sub-test organization, and their invocation within
run_vmtests.sh.

c) Other minor assorted improvements.

John Hubbard (8):
  mm/gup_benchmark: rename to mm/gup_test
  selftests/vm: use a common gup_test.h
  selftests/vm: rename run_vmtests --> run_vmtests.sh
  selftests/vm: minor cleanup: Makefile and gup_test.c
  selftests/vm: only some gup_test items are really benchmarks
  selftests/vm: gup_test: introduce the dump_pages() sub-test
  selftests/vm: run_vmtest.sh: update and clean up gup_test invocation
  selftests/vm: hmm-tests: remove the libhugetlbfs dependency

 Documentation/core-api/pin_user_pages.rst     |   6 +-
 arch/s390/configs/debug_defconfig             |   2 +-
 arch/s390/configs/defconfig                   |   2 +-
 mm/Kconfig                                    |  21 +-
 mm/Makefile                                   |   2 +-
 mm/{gup_benchmark.c => gup_test.c}            | 109 ++++++----
 mm/gup_test.h                                 |  32 +++
 tools/testing/selftests/vm/.gitignore         |   3 +-
 tools/testing/selftests/vm/Makefile           |  38 +++-
 tools/testing/selftests/vm/check_config.sh    |  30 +++
 tools/testing/selftests/vm/config             |   2 +-
 tools/testing/selftests/vm/gup_benchmark.c    | 137 -------------
 tools/testing/selftests/vm/gup_test.c         | 188 ++++++++++++++++++
 tools/testing/selftests/vm/hmm-tests.c        |  10 +-
 .../vm/{run_vmtests => run_vmtest.sh}         |  24 ++-
 15 files changed, 403 insertions(+), 203 deletions(-)
 rename mm/{gup_benchmark.c => gup_test.c} (59%)
 create mode 100644 mm/gup_test.h
 create mode 100755 tools/testing/selftests/vm/check_config.sh
 delete mode 100644 tools/testing/selftests/vm/gup_benchmark.c
 create mode 100644 tools/testing/selftests/vm/gup_test.c
 rename tools/testing/selftests/vm/{run_vmtests => run_vmtest.sh} (91%)

-- 
2.28.0

[PATCH 1/8] mm/gup_benchmark: rename to mm/gup_test

[John Hubbard]

Rename nearly every "gup_benchmark" reference and file name to
"gup_test". The one exception is for the actual gup benchmark test
itself.

The current code already does a *little* bit more than benchmarking,
and definitely covers more than get_user_pages_fast(). More importantly,
however, subsequent patches are about to add some functionality that is
non-benchmark related.

Closely related changes:

* Kconfig: in addition to renaming the options from GUP_BENCHMARK to
  GUP_TEST, update the help text to reflect that it's no longer a
  benchmark-only test.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 Documentation/core-api/pin_user_pages.rst     |  6 ++--
 arch/s390/configs/debug_defconfig             |  2 +-
 arch/s390/configs/defconfig                   |  2 +-
 mm/Kconfig                                    | 15 +++++---
 mm/Makefile                                   |  2 +-
 mm/{gup_benchmark.c => gup_test.c}            | 36 +++++++++----------
 tools/testing/selftests/vm/.gitignore         |  2 +-
 tools/testing/selftests/vm/Makefile           |  2 +-
 tools/testing/selftests/vm/config             |  2 +-
 .../vm/{gup_benchmark.c => gup_test.c}        | 16 ++++-----
 tools/testing/selftests/vm/run_vmtests        |  8 ++---
 11 files changed, 49 insertions(+), 44 deletions(-)
 rename mm/{gup_benchmark.c => gup_test.c} (80%)
 rename tools/testing/selftests/vm/{gup_benchmark.c => gup_test.c} (85%)

diff --git a/Documentation/core-api/pin_user_pages.rst b/Documentation/core-api/pin_user_pages.rst
index 7ca8c7bac650..eae972b23224 100644
--- a/Documentation/core-api/pin_user_pages.rst
+++ b/Documentation/core-api/pin_user_pages.rst
@@ -221,12 +221,12 @@ Unit testing
 ============
 This file::
 
- tools/testing/selftests/vm/gup_benchmark.c
+ tools/testing/selftests/vm/gup_test.c
 
 has the following new calls to exercise the new pin*() wrapper functions:
 
-* PIN_FAST_BENCHMARK (./gup_benchmark -a)
-* PIN_BENCHMARK (./gup_benchmark -b)
+* PIN_FAST_BENCHMARK (./gup_test -a)
+* PIN_BENCHMARK (./gup_test -b)
 
 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/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index 0784bf3caf43..c624f4b1ad33 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -100,7 +100,7 @@ CONFIG_ZSMALLOC_STAT=y
 CONFIG_DEFERRED_STRUCT_PAGE_INIT=y
 CONFIG_IDLE_PAGE_TRACKING=y
 CONFIG_PERCPU_STATS=y
-CONFIG_GUP_BENCHMARK=y
+CONFIG_GUP_TEST=y
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_PACKET_DIAG=m
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 905bc8c4cfaf..878b89706998 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -94,7 +94,7 @@ CONFIG_ZSMALLOC_STAT=y
 CONFIG_DEFERRED_STRUCT_PAGE_INIT=y
 CONFIG_IDLE_PAGE_TRACKING=y
 CONFIG_PERCPU_STATS=y
-CONFIG_GUP_BENCHMARK=y
+CONFIG_GUP_TEST=y
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_PACKET_DIAG=m
diff --git a/mm/Kconfig b/mm/Kconfig
index 1896860cf23a..588984ee5fb4 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -834,13 +834,18 @@ config PERCPU_STATS
 	  information includes global and per chunk statistics, which can
 	  be used to help understand percpu memory usage.
 
-config GUP_BENCHMARK
-	bool "Enable infrastructure for get_user_pages() and related calls benchmarking"
+config GUP_TEST
+	bool "Enable infrastructure for get_user_pages()-related unit tests"
 	help
-	  Provides /sys/kernel/debug/gup_benchmark that helps with testing
-	  performance of get_user_pages() and related calls.
+	  Provides /sys/kernel/debug/gup_test, which in turn provides a way
+	  to make ioctl calls that can launch kernel-based unit tests for
+	  the get_user_pages*() and pin_user_pages*() family of API calls.
 
-	  See tools/testing/selftests/vm/gup_benchmark.c
+	  These tests include benchmark testing of the _fast variants of
+	  get_user_pages*() and pin_user_pages*(), as well as smoke tests of
+	  the non-_fast variants.
+
+	  See tools/testing/selftests/vm/gup_test.c
 
 config GUP_GET_PTE_LOW_HIGH
 	bool
diff --git a/mm/Makefile b/mm/Makefile
index cae063dc8298..21fc9af33f9b 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -90,7 +90,7 @@ obj-$(CONFIG_PAGE_COUNTER) += page_counter.o
 obj-$(CONFIG_MEMCG) += memcontrol.o vmpressure.o
 obj-$(CONFIG_MEMCG_SWAP) += swap_cgroup.o
 obj-$(CONFIG_CGROUP_HUGETLB) += hugetlb_cgroup.o
-obj-$(CONFIG_GUP_BENCHMARK) += gup_benchmark.o
+obj-$(CONFIG_GUP_TEST) += gup_test.o
 obj-$(CONFIG_MEMORY_FAILURE) += memory-failure.o
 obj-$(CONFIG_HWPOISON_INJECT) += hwpoison-inject.o
 obj-$(CONFIG_DEBUG_KMEMLEAK) += kmemleak.o
diff --git a/mm/gup_benchmark.c b/mm/gup_test.c
similarity index 80%
rename from mm/gup_benchmark.c
rename to mm/gup_test.c
index 464cae1fa3ea..10f41c0528de 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_test.c
@@ -5,13 +5,13 @@
 #include <linux/ktime.h>
 #include <linux/debugfs.h>
 
-#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_benchmark)
-#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_benchmark)
-#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_benchmark)
-#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_benchmark)
-#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_benchmark)
+#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
+#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
+#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
+#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
+#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
 
-struct gup_benchmark {
+struct gup_test {
 	__u64 get_delta_usec;
 	__u64 put_delta_usec;
 	__u64 addr;
@@ -56,7 +56,7 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
 			if (WARN(!page_maybe_dma_pinned(page),
 				 "pages[%lu] is NOT dma-pinned\n", i)) {
 
-				dump_page(page, "gup_benchmark failure");
+				dump_page(page, "gup_test failure");
 				break;
 			}
 		}
@@ -64,8 +64,8 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
 	}
 }
 
-static int __gup_benchmark_ioctl(unsigned int cmd,
-		struct gup_benchmark *gup)
+static int __gup_test_ioctl(unsigned int cmd,
+		struct gup_test *gup)
 {
 	ktime_t start_time, end_time;
 	unsigned long i, nr_pages, addr, next;
@@ -155,10 +155,10 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
 	return ret;
 }
 
-static long gup_benchmark_ioctl(struct file *filep, unsigned int cmd,
+static long gup_test_ioctl(struct file *filep, unsigned int cmd,
 		unsigned long arg)
 {
-	struct gup_benchmark gup;
+	struct gup_test gup;
 	int ret;
 
 	switch (cmd) {
@@ -175,7 +175,7 @@ static long gup_benchmark_ioctl(struct file *filep, unsigned int cmd,
 	if (copy_from_user(&gup, (void __user *)arg, sizeof(gup)))
 		return -EFAULT;
 
-	ret = __gup_benchmark_ioctl(cmd, &gup);
+	ret = __gup_test_ioctl(cmd, &gup);
 	if (ret)
 		return ret;
 
@@ -185,17 +185,17 @@ static long gup_benchmark_ioctl(struct file *filep, unsigned int cmd,
 	return 0;
 }
 
-static const struct file_operations gup_benchmark_fops = {
+static const struct file_operations gup_test_fops = {
 	.open = nonseekable_open,
-	.unlocked_ioctl = gup_benchmark_ioctl,
+	.unlocked_ioctl = gup_test_ioctl,
 };
 
-static int gup_benchmark_init(void)
+static int gup_test_init(void)
 {
-	debugfs_create_file_unsafe("gup_benchmark", 0600, NULL, NULL,
-				   &gup_benchmark_fops);
+	debugfs_create_file_unsafe("gup_test", 0600, NULL, NULL,
+				   &gup_test_fops);
 
 	return 0;
 }
 
-late_initcall(gup_benchmark_init);
+late_initcall(gup_test_init);
diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
index 849e8226395a..2c8ddcf41c0e 100644
--- a/tools/testing/selftests/vm/.gitignore
+++ b/tools/testing/selftests/vm/.gitignore
@@ -15,7 +15,7 @@ userfaultfd
 mlock-intersect-test
 mlock-random-test
 virtual_address_range
-gup_benchmark
+gup_test
 va_128TBswitch
 map_fixed_noreplace
 write_to_hugetlbfs
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 30873b19d04b..d1ae706d9927 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -23,7 +23,7 @@ MAKEFLAGS += --no-builtin-rules
 CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
 LDLIBS = -lrt
 TEST_GEN_FILES = compaction_test
-TEST_GEN_FILES += gup_benchmark
+TEST_GEN_FILES += gup_test
 TEST_GEN_FILES += hmm-tests
 TEST_GEN_FILES += hugepage-mmap
 TEST_GEN_FILES += hugepage-shm
diff --git a/tools/testing/selftests/vm/config b/tools/testing/selftests/vm/config
index 69dd0d1aa30b..60e82da0de85 100644
--- a/tools/testing/selftests/vm/config
+++ b/tools/testing/selftests/vm/config
@@ -3,4 +3,4 @@ CONFIG_USERFAULTFD=y
 CONFIG_TEST_VMALLOC=m
 CONFIG_DEVICE_PRIVATE=y
 CONFIG_TEST_HMM=m
-CONFIG_GUP_BENCHMARK=y
+CONFIG_GUP_TEST=y
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_test.c
similarity index 85%
rename from tools/testing/selftests/vm/gup_benchmark.c
rename to tools/testing/selftests/vm/gup_test.c
index 31f8bb086907..e930135727a2 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -14,18 +14,18 @@
 #define MB (1UL << 20)
 #define PAGE_SIZE sysconf(_SC_PAGESIZE)
 
-#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_benchmark)
-#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_benchmark)
+#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
+#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
 
 /* Similar to above, but use FOLL_PIN instead of FOLL_GET. */
-#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_benchmark)
-#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_benchmark)
-#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_benchmark)
+#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
+#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
+#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
 
 /* Just the flags we need, copied from mm.h: */
 #define FOLL_WRITE	0x01	/* check pte is writable */
 
-struct gup_benchmark {
+struct gup_test {
 	__u64 get_delta_usec;
 	__u64 put_delta_usec;
 	__u64 addr;
@@ -37,7 +37,7 @@ struct gup_benchmark {
 
 int main(int argc, char **argv)
 {
-	struct gup_benchmark gup;
+	struct gup_test gup;
 	unsigned long size = 128 * MB;
 	int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
 	int cmd = GUP_FAST_BENCHMARK, flags = MAP_PRIVATE;
@@ -104,7 +104,7 @@ int main(int argc, char **argv)
 	if (write)
 		gup.flags |= FOLL_WRITE;
 
-	fd = open("/sys/kernel/debug/gup_benchmark", O_RDWR);
+	fd = open("/sys/kernel/debug/gup_test", O_RDWR);
 	if (fd == -1)
 		perror("open"), exit(1);
 
diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtests
index a3f4f30f0a2e..d1843d5f3c30 100755
--- a/tools/testing/selftests/vm/run_vmtests
+++ b/tools/testing/selftests/vm/run_vmtests
@@ -124,9 +124,9 @@ else
 fi
 
 echo "--------------------------------------------"
-echo "running 'gup_benchmark -U' (normal/slow gup)"
+echo "running 'gup_test -U' (normal/slow gup)"
 echo "--------------------------------------------"
-./gup_benchmark -U
+./gup_test -U
 if [ $? -ne 0 ]; then
 	echo "[FAIL]"
 	exitcode=1
@@ -135,9 +135,9 @@ else
 fi
 
 echo "------------------------------------------"
-echo "running gup_benchmark -b (pin_user_pages)"
+echo "running gup_test -b (pin_user_pages)"
 echo "------------------------------------------"
-./gup_benchmark -b
+./gup_test -b
 if [ $? -ne 0 ]; then
 	echo "[FAIL]"
 	exitcode=1
-- 
2.28.0

[PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

Avoid the need to copy-paste the gup_test ioctl commands and the struct
gup_test definition, between the kernel and the user space application,
by providing a new header file for these. This allows easier and safer
adding of new ioctl calls, as well as reducing the overall line count.

Details: The header file has to be able to compile independently,
because of the arguably unfortunate way that the Makefile is written:
the Makefile tries to build all of its prerequisites, when really it
should be only building the .c files, and leaving the other
prerequisites (LOCAL_HDRS) as pure dependencies.

That Makefile limitation is probably not worth fixing, but it explains
why one of the includes had to be moved into the new header file.

Also: simplify the ioctl struct (struct gup_test), by deleting the
unused __expansion[10] field. This sort of thing is what you might see
in a stable ABI, but this low-level, kernel-developer-oriented
selftests/vm system is very much not subject to ABI stability. So
"expansion" and "reserved" fields are unnecessary here.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 mm/gup_test.c                         | 17 +----------------
 mm/gup_test.h                         | 22 ++++++++++++++++++++++
 tools/testing/selftests/vm/Makefile   |  2 ++
 tools/testing/selftests/vm/gup_test.c | 22 +---------------------
 4 files changed, 26 insertions(+), 37 deletions(-)
 create mode 100644 mm/gup_test.h

diff --git a/mm/gup_test.c b/mm/gup_test.c
index 10f41c0528de..a3c86d0fdff7 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -4,22 +4,7 @@
 #include <linux/uaccess.h>
 #include <linux/ktime.h>
 #include <linux/debugfs.h>
-
-#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
-#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
-#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
-#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
-#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
-
-struct gup_test {
-	__u64 get_delta_usec;
-	__u64 put_delta_usec;
-	__u64 addr;
-	__u64 size;
-	__u32 nr_pages_per_call;
-	__u32 flags;
-	__u64 expansion[10];	/* For future use */
-};
+#include "gup_test.h"
 
 static void put_back_pages(unsigned int cmd, struct page **pages,
 			   unsigned long nr_pages)
diff --git a/mm/gup_test.h b/mm/gup_test.h
new file mode 100644
index 000000000000..931c2f3f477a
--- /dev/null
+++ b/mm/gup_test.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __GUP_TEST_H
+#define __GUP_TEST_H
+
+#include <linux/types.h>
+
+#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
+#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
+#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
+#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
+#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
+
+struct gup_test {
+	__u64 get_delta_usec;
+	__u64 put_delta_usec;
+	__u64 addr;
+	__u64 size;
+	__u32 nr_pages_per_call;
+	__u32 flags;
+};
+
+#endif	/* __GUP_TEST_H */
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index d1ae706d9927..9cc6bc087461 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -130,3 +130,5 @@ endif
 $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
 
 $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
+
+$(OUTPUT)/gup_test: ../../../../mm/gup_test.h
diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
index e930135727a2..70db259582c3 100644
--- a/tools/testing/selftests/vm/gup_test.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -2,39 +2,19 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-
-#include <linux/types.h>
+#include "../../../../mm/gup_test.h"
 
 #define MB (1UL << 20)
 #define PAGE_SIZE sysconf(_SC_PAGESIZE)
 
-#define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
-#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
-
-/* Similar to above, but use FOLL_PIN instead of FOLL_GET. */
-#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
-#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
-#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
-
 /* Just the flags we need, copied from mm.h: */
 #define FOLL_WRITE	0x01	/* check pte is writable */
 
-struct gup_test {
-	__u64 get_delta_usec;
-	__u64 put_delta_usec;
-	__u64 addr;
-	__u64 size;
-	__u32 nr_pages_per_call;
-	__u32 flags;
-	__u64 expansion[10];	/* For future use */
-};
-
 int main(int argc, char **argv)
 {
 	struct gup_test gup;
-- 
2.28.0

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Jason Gunthorpe]

On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index d1ae706d9927..9cc6bc087461 100644
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -130,3 +130,5 @@ endif
>  $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
>  
>  $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
> +
> +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h

There is no reason to do this, the auto depends will pick up header
files, and gup_test.h isn't a generated file

Jason

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
>> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
>> index d1ae706d9927..9cc6bc087461 100644
>> +++ b/tools/testing/selftests/vm/Makefile
>> @@ -130,3 +130,5 @@ endif
>>   $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
>>   
>>   $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
>> +
>> +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h
> 
> There is no reason to do this, the auto depends will pick up header
> files, and gup_test.h isn't a generated file
> 

It is less capable than you might think. Without the admittedly ugly technique
above, it fails to build, and as you can see, the include paths that are fed to
gcc are just a single one: usr/include:

$ make
make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install
gcc -Wall -I ../../../../usr/include     gup_test.c 
/kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h 
/kernel_work/linux-next-github/tools/testing/selftests/kselftest.h ../../../../mm/gup_test.h -lrt -o 
/kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test
make[1]: Entering directory '/kernel_work/linux-next-github'
gup_test.c:10:10: fatal error: gup_test.h: No such file or directory
    10 | #include "gup_test.h"
       |          ^~~~~~~~~~~~


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Jason Gunthorpe]

On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
> On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
> > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
> > > diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> > > index d1ae706d9927..9cc6bc087461 100644
> > > +++ b/tools/testing/selftests/vm/Makefile
> > > @@ -130,3 +130,5 @@ endif
> > >   $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
> > >   $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
> > > +
> > > +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h
> > 
> > There is no reason to do this, the auto depends will pick up header
> > files, and gup_test.h isn't a generated file
> > 
> 
> It is less capable than you might think. Without the admittedly ugly technique
> above, it fails to build, and as you can see, the include paths that are fed to
> gcc are just a single one: usr/include:
> 
> $ make
> make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install
> gcc -Wall -I ../../../../usr/include     gup_test.c
> /kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h
> /kernel_work/linux-next-github/tools/testing/selftests/kselftest.h
> ../../../../mm/gup_test.h -lrt -o
> /kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test
> make[1]: Entering directory '/kernel_work/linux-next-github'
> gup_test.c:10:10: fatal error: gup_test.h: No such file or directory
>    10 | #include "gup_test.h"
>       |          ^~~~~~~~~~~~

You are supposed to use

  #include "../../../../mm/gup_test.h"

I have no idea what weird behavior the makefile is triggering that the
above include works

Jason

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
>>>> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
>>>> index d1ae706d9927..9cc6bc087461 100644
>>>> +++ b/tools/testing/selftests/vm/Makefile
>>>> @@ -130,3 +130,5 @@ endif
>>>>    $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
>>>>    $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
>>>> +
>>>> +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h
>>>
>>> There is no reason to do this, the auto depends will pick up header
>>> files, and gup_test.h isn't a generated file
>>>

Oh, I misread your comment! You were talking about this Makefile
dependency that I'm adding, rather than the ../'s in the path.

Well, for that though, it also has to stay as shown in this patch,
because of this:

I don't see any "gcc -m" type of dependency generation pass happening
in this relatively simple Make system. And so, without including an
explicit header file dependency (at least, that's the simplest way),
changes to gup_test.h are not detected. Both the Makefile code and the
observed behavior back this up. (I expect that this is because there is
less use of header files in this area, because most unit tests are
self-contained within a single .c file.)


>>
>> It is less capable than you might think. Without the admittedly ugly technique
>> above, it fails to build, and as you can see, the include paths that are fed to
>> gcc are just a single one: usr/include:
>>
>> $ make
>> make --no-builtin-rules ARCH=x86 -C ../../../.. headers_install
>> gcc -Wall -I ../../../../usr/include     gup_test.c
>> /kernel_work/linux-next-github/tools/testing/selftests/kselftest_harness.h
>> /kernel_work/linux-next-github/tools/testing/selftests/kselftest.h
>> ../../../../mm/gup_test.h -lrt -o
>> /kernel_work/linux-next-github/tools/testing/selftests/vm/gup_test
>> make[1]: Entering directory '/kernel_work/linux-next-github'
>> gup_test.c:10:10: fatal error: gup_test.h: No such file or directory
>>     10 | #include "gup_test.h"
>>        |          ^~~~~~~~~~~~
> 
> You are supposed to use
> 
>    #include "../../../../mm/gup_test.h"

Good, I'll leave it as I had it.


> I have no idea what weird behavior the makefile is triggering that the
> above include works
> 

See the commit description for yet another Makefile weird behavior point. :)

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Jason Gunthorpe]

On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
> On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
> > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
> > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
> > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
> > > > > diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> > > > > index d1ae706d9927..9cc6bc087461 100644
> > > > > +++ b/tools/testing/selftests/vm/Makefile
> > > > > @@ -130,3 +130,5 @@ endif
> > > > >    $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
> > > > >    $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
> > > > > +
> > > > > +$(OUTPUT)/gup_test: ../../../../mm/gup_test.h
> > > > 
> > > > There is no reason to do this, the auto depends will pick up header
> > > > files, and gup_test.h isn't a generated file
> > > > 
> 
> Oh, I misread your comment! You were talking about this Makefile
> dependency that I'm adding, rather than the ../'s in the path.
> 
> Well, for that though, it also has to stay as shown in this patch,
> because of this:
> 
> I don't see any "gcc -m" type of dependency generation pass happening
> in this relatively simple Make system. 

It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
and sucked into the build.

> And so, without including an explicit header file dependency (at
> least, that's the simplest way), changes to gup_test.h are not
> detected.

Shouldn't be

> Both the Makefile code and the observed behavior back this up. (I
> expect that this is because there is less use of header files in
> this area, because most unit tests are self-contained within a
> single .c file.)

Something else is very wrong then.

Jason

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/29/20 10:55 AM, Jason Gunthorpe wrote:
> On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
>> On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
>>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
>>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
>>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
...
>> I don't see any "gcc -m" type of dependency generation pass happening
>> in this relatively simple Make system.
> 
> It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
> and sucked into the build.

You are thinking of kbuild. This is not kbuild. There are no such artifacts
being generated.

>> And so, without including an explicit header file dependency (at
>> least, that's the simplest way), changes to gup_test.h are not
>> detected.
> 
> Shouldn't be
> 
>> Both the Makefile code and the observed behavior back this up. (I
>> expect that this is because there is less use of header files in
>> this area, because most unit tests are self-contained within a
>> single .c file.)
> 
> Something else is very wrong then.
> 

Not really, it's just a less-cabable system than kbuild.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Jason Gunthorpe]

On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote:
> On 9/29/20 10:55 AM, Jason Gunthorpe wrote:
> > On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
> > > On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
> > > > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
> > > > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
> > > > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
> ...
> > > I don't see any "gcc -m" type of dependency generation pass happening
> > > in this relatively simple Make system.
> > 
> > It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
> > and sucked into the build.
> 
> You are thinking of kbuild. This is not kbuild. There are no such artifacts
> being generated.

Oh. Really? That's horrible.

Jason

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/29/20 12:08 PM, Jason Gunthorpe wrote:
> On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote:
>> On 9/29/20 10:55 AM, Jason Gunthorpe wrote:
>>> On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
>>>> On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
>>>>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
>>>>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
>>>>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
>> ...
>>>> I don't see any "gcc -m" type of dependency generation pass happening
>>>> in this relatively simple Make system.
>>>
>>> It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
>>> and sucked into the build.
>>
>> You are thinking of kbuild. This is not kbuild. There are no such artifacts
>> being generated.
> 
> Oh. Really? That's horrible.
> 

Well, yes, it's not a perfect build system down here in selftests/. Are you saying
that it is worth upgrading? I'm open to suggestions and ideas for improvements,
and at the moment, I have the miniature build system here mostly loaded into my
head. So for a brief shining moment I can probably understand it well enough to
work on it. :)


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Jason Gunthorpe]

On Tue, Sep 29, 2020 at 12:48:43PM -0700, John Hubbard wrote:
> On 9/29/20 12:08 PM, Jason Gunthorpe wrote:
> > On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote:
> > > On 9/29/20 10:55 AM, Jason Gunthorpe wrote:
> > > > On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
> > > > > On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
> > > > > > On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
> > > > > > > On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
> > > > > > > > On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
> > > ...
> > > > > I don't see any "gcc -m" type of dependency generation pass happening
> > > > > in this relatively simple Make system.
> > > > 
> > > > It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
> > > > and sucked into the build.
> > > 
> > > You are thinking of kbuild. This is not kbuild. There are no such artifacts
> > > being generated.
> > 
> > Oh. Really? That's horrible.
> > 
> 
> Well, yes, it's not a perfect build system down here in selftests/. Are you saying
> that it is worth upgrading? I'm open to suggestions and ideas for improvements,
> and at the moment, I have the miniature build system here mostly loaded into my
> head. So for a brief shining moment I can probably understand it well enough to
> work on it. :)

I only remarked because I didn't know it wasn't using kbuild. I
thought it would have used the existing HOSTCC stuff, not sure why it
is special.

The only investment that seems worthwhile would be to switch it to use
the normal kbuild stuff??

Jason

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Shuah Khan]

On 9/29/20 1:53 PM, Jason Gunthorpe wrote:
> On Tue, Sep 29, 2020 at 12:48:43PM -0700, John Hubbard wrote:
>> On 9/29/20 12:08 PM, Jason Gunthorpe wrote:
>>> On Tue, Sep 29, 2020 at 11:59:55AM -0700, John Hubbard wrote:
>>>> On 9/29/20 10:55 AM, Jason Gunthorpe wrote:
>>>>> On Tue, Sep 29, 2020 at 10:44:31AM -0700, John Hubbard wrote:
>>>>>> On 9/29/20 9:35 AM, Jason Gunthorpe wrote:
>>>>>>> On Mon, Sep 28, 2020 at 01:10:24PM -0700, John Hubbard wrote:
>>>>>>>> On 9/28/20 5:57 AM, Jason Gunthorpe wrote:
>>>>>>>>> On Sun, Sep 27, 2020 at 11:21:53PM -0700, John Hubbard wrote:
>>>> ...
>>>>>> I don't see any "gcc -m" type of dependency generation pass happening
>>>>>> in this relatively simple Make system.
>>>>>
>>>>> It happens with -MD, all the deps are stored in files like mm/.init-mm.o.cmd
>>>>> and sucked into the build.
>>>>
>>>> You are thinking of kbuild. This is not kbuild. There are no such artifacts
>>>> being generated.
>>>
>>> Oh. Really? That's horrible.
>>>
>>
>> Well, yes, it's not a perfect build system down here in selftests/. Are you saying
>> that it is worth upgrading? I'm open to suggestions and ideas for improvements,
>> and at the moment, I have the miniature build system here mostly loaded into my
>> head. So for a brief shining moment I can probably understand it well enough to
>> work on it. :)
> 
> I only remarked because I didn't know it wasn't using kbuild. I
> thought it would have used the existing HOSTCC stuff, not sure why it
> is special.
> 
> The only investment that seems worthwhile would be to switch it to use
> the normal kbuild stuff??
> 

I explored switching to kbuild at the kernel summit last year during
my kselftest where are we talk.

There was push back from several developers. We can definitely explore
it as long as we can still support being able to build and run
individual subsystem tests and doesn't break workflow for developers.

If you are up for it, propose a patch and we can discuss it.

thanks,
-- Shuah

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/29/20 1:00 PM, Shuah Khan wrote:
> On 9/29/20 1:53 PM, Jason Gunthorpe wrote:
>> I only remarked because I didn't know it wasn't using kbuild. I
>> thought it would have used the existing HOSTCC stuff, not sure why it
>> is special.
>>
>> The only investment that seems worthwhile would be to switch it to use
>> the normal kbuild stuff??
>>
> 
> I explored switching to kbuild at the kernel summit last year during
> my kselftest where are we talk.
> 
> There was push back from several developers. We can definitely explore
> it as long as we can still support being able to build and run
> individual subsystem tests and doesn't break workflow for developers.
> 

Do you have a link or two for that? Especially about the pushback, and
conclusions reached, if any.



thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[Shuah Khan]

On 9/29/20 2:11 PM, John Hubbard wrote:
> On 9/29/20 1:00 PM, Shuah Khan wrote:
>> On 9/29/20 1:53 PM, Jason Gunthorpe wrote:
>>> I only remarked because I didn't know it wasn't using kbuild. I
>>> thought it would have used the existing HOSTCC stuff, not sure why it
>>> is special.
>>>
>>> The only investment that seems worthwhile would be to switch it to use
>>> the normal kbuild stuff??
>>>
>>
>> I explored switching to kbuild at the kernel summit last year during
>> my kselftest where are we talk.
>>
>> There was push back from several developers. We can definitely explore
>> it as long as we can still support being able to build and run
>> individual subsystem tests and doesn't break workflow for developers.
>>
> 
> Do you have a link or two for that? Especially about the pushback, and
> conclusions reached, if any.
> 

Unfortunately no. A I recall it was workflow related issues and ease of
running individual subsystem tests and backwards compatibility with
stables.

Let's start a new discussion and see where we land.

thanks,
-- Shuah

Re: [PATCH 2/8] selftests/vm: use a common gup_test.h

[John Hubbard]

On 9/29/20 1:20 PM, Shuah Khan wrote:
> On 9/29/20 2:11 PM, John Hubbard wrote:
...
>> Do you have a link or two for that? Especially about the pushback, and
>> conclusions reached, if any.
>>
> 
> Unfortunately no. A I recall it was workflow related issues and ease of
> running individual subsystem tests and backwards compatibility with
> stables.
> 
> Let's start a new discussion and see where we land.
> 
OK, sure. I can take a quick pass at converting just the selftests/vm
directory to kbuild, and post that as an RFC to start the discussion.


thanks,
-- 
John Hubbard
NVIDIA

[PATCH 3/8] selftests/vm: rename run_vmtests --> run_vmtests.sh

[John Hubbard]

Rename to *.sh, in order to match the conventions of all of the other
items in selftest/vm.

The only reason not to use a .sh suffix a shell script like this, might
be to make it look more like a normal program, but that's not an issue
here.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vm/Makefile                       | 2 +-
 tools/testing/selftests/vm/{run_vmtests => run_vmtest.sh} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/testing/selftests/vm/{run_vmtests => run_vmtest.sh} (100%)

diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 9cc6bc087461..5a3bd0c497b6 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -69,7 +69,7 @@ TEST_GEN_FILES += virtual_address_range
 TEST_GEN_FILES += write_to_hugetlbfs
 endif
 
-TEST_PROGS := run_vmtests
+TEST_PROGS := run_vmtests.sh
 
 TEST_FILES := test_vmalloc.sh
 
diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtest.sh
similarity index 100%
rename from tools/testing/selftests/vm/run_vmtests
rename to tools/testing/selftests/vm/run_vmtest.sh
-- 
2.28.0

[PATCH 4/8] selftests/vm: minor cleanup: Makefile and gup_test.c

[John Hubbard]

A few cleanups that don't deserve separate patches, but that
also should not clutter up other functional changes:

1. Remove an unnecessary #include <prctl.h>

2. Restore the sorted order of TEST_GEN_FILES.

3. Add -lpthread to the common LDLIBS, as it is harmless and several
   tests use it. Including, soon, gup_test.c. This gets rid of one
   special rule already.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vm/Makefile   | 10 ++++------
 tools/testing/selftests/vm/gup_test.c |  1 -
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 5a3bd0c497b6..2579242386ac 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -21,14 +21,15 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/')
 MAKEFLAGS += --no-builtin-rules
 
 CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
-LDLIBS = -lrt
+LDLIBS = -lrt -lpthread
 TEST_GEN_FILES = compaction_test
 TEST_GEN_FILES += gup_test
 TEST_GEN_FILES += hmm-tests
 TEST_GEN_FILES += hugepage-mmap
 TEST_GEN_FILES += hugepage-shm
-TEST_GEN_FILES += map_hugetlb
+TEST_GEN_FILES += khugepaged
 TEST_GEN_FILES += map_fixed_noreplace
+TEST_GEN_FILES += map_hugetlb
 TEST_GEN_FILES += map_populate
 TEST_GEN_FILES += mlock-random-test
 TEST_GEN_FILES += mlock2-tests
@@ -37,7 +38,6 @@ TEST_GEN_FILES += on-fault-limit
 TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += userfaultfd
-TEST_GEN_FILES += khugepaged
 
 ifeq ($(ARCH),x86_64)
 CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_32bit_program.c -m32)
@@ -76,7 +76,7 @@ TEST_FILES := test_vmalloc.sh
 KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
-$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs -lpthread
+$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs
 
 ifeq ($(ARCH),x86_64)
 BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
@@ -127,8 +127,6 @@ warn_32bit_failure:
 endif
 endif
 
-$(OUTPUT)/userfaultfd: LDLIBS += -lpthread
-
 $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
 
 $(OUTPUT)/gup_test: ../../../../mm/gup_test.h
diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
index 70db259582c3..4e9f5d0ed0fc 100644
--- a/tools/testing/selftests/vm/gup_test.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -4,7 +4,6 @@
 #include <unistd.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
-#include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include "../../../../mm/gup_test.h"
-- 
2.28.0

[PATCH 5/8] selftests/vm: only some gup_test items are really benchmarks

[John Hubbard]

Therefore, some minor cleanup and improvements are in order:

1. Rename the other items appropriately.

2. Stop reporting timing information on the non-benchmark items. It's
   still being recorded and is available, but there's no point in
   cluttering up the report with data that no one reasonably needs to
   check.

3. Don't do iterations, for non-benchmark items.

4. Print out a shorter, more appropriate report for the non-benchmark
   tests.

5. Add the command that was run, to the report. This really helps, as
   there are quite a lot of options now.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 Documentation/core-api/pin_user_pages.rst |  2 +-
 mm/gup_test.c                             | 14 +++----
 mm/gup_test.h                             |  8 ++--
 tools/testing/selftests/vm/gup_test.c     | 47 +++++++++++++++++++----
 4 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/Documentation/core-api/pin_user_pages.rst b/Documentation/core-api/pin_user_pages.rst
index eae972b23224..fcf605be43d0 100644
--- a/Documentation/core-api/pin_user_pages.rst
+++ b/Documentation/core-api/pin_user_pages.rst
@@ -226,7 +226,7 @@ This file::
 has the following new calls to exercise the new pin*() wrapper functions:
 
 * PIN_FAST_BENCHMARK (./gup_test -a)
-* PIN_BENCHMARK (./gup_test -b)
+* PIN_BASIC_TEST (./gup_test -b)
 
 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/mm/gup_test.c b/mm/gup_test.c
index a3c86d0fdff7..a980c4a194f0 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -13,13 +13,13 @@ static void put_back_pages(unsigned int cmd, struct page **pages,
 
 	switch (cmd) {
 	case GUP_FAST_BENCHMARK:
-	case GUP_BENCHMARK:
+	case GUP_BASIC_TEST:
 		for (i = 0; i < nr_pages; i++)
 			put_page(pages[i]);
 		break;
 
 	case PIN_FAST_BENCHMARK:
-	case PIN_BENCHMARK:
+	case PIN_BASIC_TEST:
 	case PIN_LONGTERM_BENCHMARK:
 		unpin_user_pages(pages, nr_pages);
 		break;
@@ -34,7 +34,7 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
 
 	switch (cmd) {
 	case PIN_FAST_BENCHMARK:
-	case PIN_BENCHMARK:
+	case PIN_BASIC_TEST:
 	case PIN_LONGTERM_BENCHMARK:
 		for (i = 0; i < nr_pages; i++) {
 			page = pages[i];
@@ -87,7 +87,7 @@ static int __gup_test_ioctl(unsigned int cmd,
 			nr = get_user_pages_fast(addr, nr, gup->flags,
 						 pages + i);
 			break;
-		case GUP_BENCHMARK:
+		case GUP_BASIC_TEST:
 			nr = get_user_pages(addr, nr, gup->flags, pages + i,
 					    NULL);
 			break;
@@ -95,7 +95,7 @@ static int __gup_test_ioctl(unsigned int cmd,
 			nr = pin_user_pages_fast(addr, nr, gup->flags,
 						 pages + i);
 			break;
-		case PIN_BENCHMARK:
+		case PIN_BASIC_TEST:
 			nr = pin_user_pages(addr, nr, gup->flags, pages + i,
 					    NULL);
 			break;
@@ -148,10 +148,10 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
 
 	switch (cmd) {
 	case GUP_FAST_BENCHMARK:
-	case GUP_BENCHMARK:
 	case PIN_FAST_BENCHMARK:
-	case PIN_BENCHMARK:
 	case PIN_LONGTERM_BENCHMARK:
+	case GUP_BASIC_TEST:
+	case PIN_BASIC_TEST:
 		break;
 	default:
 		return -EINVAL;
diff --git a/mm/gup_test.h b/mm/gup_test.h
index 931c2f3f477a..921b4caad8ef 100644
--- a/mm/gup_test.h
+++ b/mm/gup_test.h
@@ -5,10 +5,10 @@
 #include <linux/types.h>
 
 #define GUP_FAST_BENCHMARK	_IOWR('g', 1, struct gup_test)
-#define GUP_BENCHMARK		_IOWR('g', 2, struct gup_test)
-#define PIN_FAST_BENCHMARK	_IOWR('g', 3, struct gup_test)
-#define PIN_BENCHMARK		_IOWR('g', 4, struct gup_test)
-#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 5, struct gup_test)
+#define PIN_FAST_BENCHMARK	_IOWR('g', 2, struct gup_test)
+#define PIN_LONGTERM_BENCHMARK	_IOWR('g', 3, struct gup_test)
+#define GUP_BASIC_TEST		_IOWR('g', 4, struct gup_test)
+#define PIN_BASIC_TEST		_IOWR('g', 5, struct gup_test)
 
 struct gup_test {
 	__u64 get_delta_usec;
diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
index 4e9f5d0ed0fc..67d57a1cc8b6 100644
--- a/tools/testing/selftests/vm/gup_test.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -14,12 +14,30 @@
 /* Just the flags we need, copied from mm.h: */
 #define FOLL_WRITE	0x01	/* check pte is writable */
 
+static char *cmd_to_str(unsigned long cmd)
+{
+	switch (cmd) {
+	case GUP_FAST_BENCHMARK:
+		return "GUP_FAST_BENCHMARK";
+	case PIN_FAST_BENCHMARK:
+		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";
+	}
+	return "Unknown command";
+}
+
 int main(int argc, char **argv)
 {
 	struct gup_test gup;
 	unsigned long size = 128 * MB;
 	int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
-	int cmd = GUP_FAST_BENCHMARK, flags = MAP_PRIVATE;
+	int cmd = GUP_FAST_BENCHMARK;
+	int flags = MAP_PRIVATE;
 	char *file = "/dev/zero";
 	char *p;
 
@@ -29,7 +47,7 @@ int main(int argc, char **argv)
 			cmd = PIN_FAST_BENCHMARK;
 			break;
 		case 'b':
-			cmd = PIN_BENCHMARK;
+			cmd = PIN_BASIC_TEST;
 			break;
 		case 'L':
 			cmd = PIN_LONGTERM_BENCHMARK;
@@ -50,7 +68,7 @@ int main(int argc, char **argv)
 			thp = 0;
 			break;
 		case 'U':
-			cmd = GUP_BENCHMARK;
+			cmd = GUP_BASIC_TEST;
 			break;
 		case 'u':
 			cmd = GUP_FAST_BENCHMARK;
@@ -100,16 +118,29 @@ int main(int argc, char **argv)
 	for (; (unsigned long)p < gup.addr + size; p += PAGE_SIZE)
 		p[0] = 0;
 
-	for (i = 0; i < repeats; i++) {
+	/* 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;
+			if (ioctl(fd, cmd, &gup))
+				perror("ioctl"), exit(1);
+
+			printf("%s: Time: get:%lld put:%lld us",
+			       cmd_to_str(cmd), gup.get_delta_usec,
+			       gup.put_delta_usec);
+			if (gup.size != size)
+				printf(", truncated (size: %lld)", gup.size);
+			printf("\n");
+		}
+	} else {
 		gup.size = size;
 		if (ioctl(fd, cmd, &gup))
 			perror("ioctl"), exit(1);
 
-		printf("Time: get:%lld put:%lld us", gup.get_delta_usec,
-			gup.put_delta_usec);
+		printf("%s: done\n", cmd_to_str(cmd));
 		if (gup.size != size)
-			printf(", truncated (size: %lld)", gup.size);
-		printf("\n");
+			printf("Truncated (size: %lld)\n", gup.size);
 	}
 
 	return 0;
-- 
2.28.0

[PATCH 6/8] selftests/vm: gup_test: introduce the dump_pages() sub-test

[John Hubbard]

For quite a while, I was doing a quick hack to gup_test.c (previously,
gup_benchmark.c) whenever I wanted to try out my changes to dump_page().
This makes that hack unnecessary, and instead allows anyone to easily
get the same coverage from a user space program. That saves a lot of
time because you don't have to change the kernel, in order to test
different pages and options.

The new sub-test takes advantage of the existing gup_test
infrastructure, which already provides a simple user space program, some
allocated user space pages, an ioctl call, pinning of those pages (via
either get_user_pages or pin_user_pages) and a corresponding kernel-side
test invocation. There's not much more required, mainly just a couple of
inputs from the user.

In fact, the new test re-uses the existing command line options in order
to get various helpful combinations (THP or normal, _fast or slow gup,
gup vs. pup, and more).

New command line options are: which pages to dump, and what type of
"get/pin" to use.

In order to figure out which pages to dump, the logic is:

* If the user doesn't specify anything, the page 0 (the first page in
the address range that the program sets up for testing) is dumped.

* Or, the user can type up to 8 page indices anywhere on the command
line. If you type more than 8, then it uses the first 8 and ignores the
remaining items.

For example:

    ./gup_test -ct -F 1 0 19 0x1000

Meaning:
    -c:          dump pages sub-test
    -t:          use THP pages
    -F 1:        use pin_user_pages() instead of get_user_pages()
    0 19 0x1000: dump pages 0, 19, and 4096

Also, invoke the new test from run_vmtests.sh. This keeps it in use, and
also provides a good example of how to invoke it.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 mm/Kconfig                            |  6 +++
 mm/gup_test.c                         | 54 ++++++++++++++++++++++++++-
 mm/gup_test.h                         | 10 +++++
 tools/testing/selftests/vm/gup_test.c | 47 +++++++++++++++++++++--
 4 files changed, 112 insertions(+), 5 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 588984ee5fb4..f7c4c21e5cb1 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -845,6 +845,12 @@ config GUP_TEST
 	  get_user_pages*() and pin_user_pages*(), as well as smoke tests of
 	  the non-_fast variants.
 
+	  There is also a sub-test that allows running dump_page() on any
+	  of up to eight pages (selected by command line args) within the
+	  range of user-space addresses. These pages are either pinned via
+	  pin_user_pages*(), or pinned via get_user_pages*(), as specified
+	  by other command line arguments.
+
 	  See tools/testing/selftests/vm/gup_test.c
 
 config GUP_GET_PTE_LOW_HIGH
diff --git a/mm/gup_test.c b/mm/gup_test.c
index a980c4a194f0..e79dc364eafb 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -7,7 +7,7 @@
 #include "gup_test.h"
 
 static void put_back_pages(unsigned int cmd, struct page **pages,
-			   unsigned long nr_pages)
+			   unsigned long nr_pages, unsigned int gup_test_flags)
 {
 	unsigned long i;
 
@@ -23,6 +23,15 @@ static void put_back_pages(unsigned int cmd, struct page **pages,
 	case PIN_LONGTERM_BENCHMARK:
 		unpin_user_pages(pages, nr_pages);
 		break;
+	case DUMP_USER_PAGES_TEST:
+		if (gup_test_flags & GUP_TEST_FLAG_DUMP_PAGES_USE_PIN) {
+			unpin_user_pages(pages, nr_pages);
+		} else {
+			for (i = 0; i < nr_pages; i++)
+				put_page(pages[i]);
+
+		}
+		break;
 	}
 }
 
@@ -49,6 +58,37 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
 	}
 }
 
+static void dump_pages_test(struct gup_test *gup, struct page **pages,
+			    unsigned long nr_pages)
+{
+	unsigned int index_to_dump;
+	unsigned int i;
+
+	/*
+	 * Zero out any user-supplied page index that is out of range. Remember:
+	 * .which_pages[] contains a 1-based set of page indices.
+	 */
+	for (i = 0; i < GUP_TEST_MAX_PAGES_TO_DUMP; i++) {
+		if (gup->which_pages[i] > nr_pages) {
+			pr_warn("ZEROING due to out of range: .which_pages[%u]: %u\n",
+				i, gup->which_pages[i]);
+			gup->which_pages[i] = 0;
+		}
+	}
+
+	for (i = 0; i < GUP_TEST_MAX_PAGES_TO_DUMP; i++) {
+		index_to_dump = gup->which_pages[i];
+
+		if (index_to_dump) {
+			index_to_dump--; // Decode from 1-based, to 0-based
+			pr_info("---- page #%u, starting from user virt addr: 0x%llx\n",
+				index_to_dump, gup->addr);
+			dump_page(pages[index_to_dump],
+				  "gup_test: dump_pages() test");
+		}
+	}
+}
+
 static int __gup_test_ioctl(unsigned int cmd,
 		struct gup_test *gup)
 {
@@ -104,6 +144,14 @@ static int __gup_test_ioctl(unsigned int cmd,
 					    gup->flags | FOLL_LONGTERM,
 					    pages + i, NULL);
 			break;
+		case DUMP_USER_PAGES_TEST:
+			if (gup->flags & GUP_TEST_FLAG_DUMP_PAGES_USE_PIN)
+				nr = pin_user_pages(addr, nr, gup->flags,
+						    pages + i, NULL);
+			else
+				nr = get_user_pages(addr, nr, gup->flags,
+						    pages + i, NULL);
+			break;
 		default:
 			kvfree(pages);
 			ret = -EINVAL;
@@ -127,10 +175,11 @@ static int __gup_test_ioctl(unsigned int cmd,
 	 * state: print a warning if any non-dma-pinned pages are found:
 	 */
 	verify_dma_pinned(cmd, pages, nr_pages);
+	dump_pages_test(gup, pages, nr_pages);
 
 	start_time = ktime_get();
 
-	put_back_pages(cmd, pages, nr_pages);
+	put_back_pages(cmd, pages, nr_pages, gup->flags);
 
 	end_time = ktime_get();
 	gup->put_delta_usec = ktime_us_delta(end_time, start_time);
@@ -152,6 +201,7 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
 	case PIN_LONGTERM_BENCHMARK:
 	case GUP_BASIC_TEST:
 	case PIN_BASIC_TEST:
+	case DUMP_USER_PAGES_TEST:
 		break;
 	default:
 		return -EINVAL;
diff --git a/mm/gup_test.h b/mm/gup_test.h
index 921b4caad8ef..90a6713d50eb 100644
--- a/mm/gup_test.h
+++ b/mm/gup_test.h
@@ -9,6 +9,11 @@
 #define PIN_LONGTERM_BENCHMARK	_IOWR('g', 3, struct gup_test)
 #define GUP_BASIC_TEST		_IOWR('g', 4, struct gup_test)
 #define PIN_BASIC_TEST		_IOWR('g', 5, struct gup_test)
+#define DUMP_USER_PAGES_TEST	_IOWR('g', 6, struct gup_test)
+
+#define GUP_TEST_MAX_PAGES_TO_DUMP		8
+
+#define GUP_TEST_FLAG_DUMP_PAGES_USE_PIN	0x1
 
 struct gup_test {
 	__u64 get_delta_usec;
@@ -17,6 +22,11 @@ struct gup_test {
 	__u64 size;
 	__u32 nr_pages_per_call;
 	__u32 flags;
+	/*
+	 * Each non-zero entry is the number of the page (1-based: first page is
+	 * page 1, so that zero entries mean "do nothing") from the .addr base.
+	 */
+	__u32 which_pages[GUP_TEST_MAX_PAGES_TO_DUMP];
 };
 
 #endif	/* __GUP_TEST_H */
diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
index 67d57a1cc8b6..68137b337114 100644
--- a/tools/testing/selftests/vm/gup_test.c
+++ b/tools/testing/selftests/vm/gup_test.c
@@ -27,21 +27,23 @@ static char *cmd_to_str(unsigned long cmd)
 		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";
 }
 
 int main(int argc, char **argv)
 {
-	struct gup_test gup;
+	struct gup_test gup = { 0 };
 	unsigned long size = 128 * MB;
 	int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
-	int cmd = GUP_FAST_BENCHMARK;
+	unsigned long cmd = GUP_FAST_BENCHMARK;
 	int flags = MAP_PRIVATE;
 	char *file = "/dev/zero";
 	char *p;
 
-	while ((opt = getopt(argc, argv, "m:r:n:f:abtTLUuwSH")) != -1) {
+	while ((opt = getopt(argc, argv, "m:r:n:F:f:abctTLUuwSH")) != -1) {
 		switch (opt) {
 		case 'a':
 			cmd = PIN_FAST_BENCHMARK;
@@ -52,6 +54,21 @@ int main(int argc, char **argv)
 		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 'F':
+			/* strtol, so you can pass flags in hex form */
+			gup.flags = strtol(optarg, 0, 0);
+			break;
 		case 'm':
 			size = atoi(optarg) * MB;
 			break;
@@ -91,6 +108,30 @@ int main(int argc, char **argv)
 		}
 	}
 
+	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++;
+		}
+	}
+
 	filed = open(file, O_RDWR|O_CREAT);
 	if (filed < 0) {
 		perror("open");
-- 
2.28.0

Re: [PATCH 6/8] selftests/vm: gup_test: introduce the dump_pages() sub-test

[Ira Weiny]

On Sun, Sep 27, 2020 at 11:21:57PM -0700, John Hubbard wrote:
> For quite a while, I was doing a quick hack to gup_test.c (previously,
> gup_benchmark.c) whenever I wanted to try out my changes to dump_page().
> This makes that hack unnecessary, and instead allows anyone to easily
> get the same coverage from a user space program. That saves a lot of
> time because you don't have to change the kernel, in order to test
> different pages and options.
> 
> The new sub-test takes advantage of the existing gup_test
> infrastructure, which already provides a simple user space program, some
> allocated user space pages, an ioctl call, pinning of those pages (via
> either get_user_pages or pin_user_pages) and a corresponding kernel-side
> test invocation. There's not much more required, mainly just a couple of
> inputs from the user.
> 
> In fact, the new test re-uses the existing command line options in order
> to get various helpful combinations (THP or normal, _fast or slow gup,
> gup vs. pup, and more).
> 
> New command line options are: which pages to dump, and what type of
> "get/pin" to use.
> 
> In order to figure out which pages to dump, the logic is:
> 
> * If the user doesn't specify anything, the page 0 (the first page in
> the address range that the program sets up for testing) is dumped.
> 
> * Or, the user can type up to 8 page indices anywhere on the command
> line. If you type more than 8, then it uses the first 8 and ignores the
> remaining items.
> 
> For example:
> 
>     ./gup_test -ct -F 1 0 19 0x1000
> 
> Meaning:
>     -c:          dump pages sub-test
>     -t:          use THP pages
>     -F 1:        use pin_user_pages() instead of get_user_pages()
>     0 19 0x1000: dump pages 0, 19, and 4096
> 
> Also, invoke the new test from run_vmtests.sh. This keeps it in use, and

I don't see a change to run_vmtests.sh?

Ira

> also provides a good example of how to invoke it.
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  mm/Kconfig                            |  6 +++
>  mm/gup_test.c                         | 54 ++++++++++++++++++++++++++-
>  mm/gup_test.h                         | 10 +++++
>  tools/testing/selftests/vm/gup_test.c | 47 +++++++++++++++++++++--
>  4 files changed, 112 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 588984ee5fb4..f7c4c21e5cb1 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -845,6 +845,12 @@ config GUP_TEST
>  	  get_user_pages*() and pin_user_pages*(), as well as smoke tests of
>  	  the non-_fast variants.
>  
> +	  There is also a sub-test that allows running dump_page() on any
> +	  of up to eight pages (selected by command line args) within the
> +	  range of user-space addresses. These pages are either pinned via
> +	  pin_user_pages*(), or pinned via get_user_pages*(), as specified
> +	  by other command line arguments.
> +
>  	  See tools/testing/selftests/vm/gup_test.c
>  
>  config GUP_GET_PTE_LOW_HIGH
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index a980c4a194f0..e79dc364eafb 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -7,7 +7,7 @@
>  #include "gup_test.h"
>  
>  static void put_back_pages(unsigned int cmd, struct page **pages,
> -			   unsigned long nr_pages)
> +			   unsigned long nr_pages, unsigned int gup_test_flags)
>  {
>  	unsigned long i;
>  
> @@ -23,6 +23,15 @@ static void put_back_pages(unsigned int cmd, struct page **pages,
>  	case PIN_LONGTERM_BENCHMARK:
>  		unpin_user_pages(pages, nr_pages);
>  		break;
> +	case DUMP_USER_PAGES_TEST:
> +		if (gup_test_flags & GUP_TEST_FLAG_DUMP_PAGES_USE_PIN) {
> +			unpin_user_pages(pages, nr_pages);
> +		} else {
> +			for (i = 0; i < nr_pages; i++)
> +				put_page(pages[i]);
> +
> +		}
> +		break;
>  	}
>  }
>  
> @@ -49,6 +58,37 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
>  	}
>  }
>  
> +static void dump_pages_test(struct gup_test *gup, struct page **pages,
> +			    unsigned long nr_pages)
> +{
> +	unsigned int index_to_dump;
> +	unsigned int i;
> +
> +	/*
> +	 * Zero out any user-supplied page index that is out of range. Remember:
> +	 * .which_pages[] contains a 1-based set of page indices.
> +	 */
> +	for (i = 0; i < GUP_TEST_MAX_PAGES_TO_DUMP; i++) {
> +		if (gup->which_pages[i] > nr_pages) {
> +			pr_warn("ZEROING due to out of range: .which_pages[%u]: %u\n",
> +				i, gup->which_pages[i]);
> +			gup->which_pages[i] = 0;
> +		}
> +	}
> +
> +	for (i = 0; i < GUP_TEST_MAX_PAGES_TO_DUMP; i++) {
> +		index_to_dump = gup->which_pages[i];
> +
> +		if (index_to_dump) {
> +			index_to_dump--; // Decode from 1-based, to 0-based
> +			pr_info("---- page #%u, starting from user virt addr: 0x%llx\n",
> +				index_to_dump, gup->addr);
> +			dump_page(pages[index_to_dump],
> +				  "gup_test: dump_pages() test");
> +		}
> +	}
> +}
> +
>  static int __gup_test_ioctl(unsigned int cmd,
>  		struct gup_test *gup)
>  {
> @@ -104,6 +144,14 @@ static int __gup_test_ioctl(unsigned int cmd,
>  					    gup->flags | FOLL_LONGTERM,
>  					    pages + i, NULL);
>  			break;
> +		case DUMP_USER_PAGES_TEST:
> +			if (gup->flags & GUP_TEST_FLAG_DUMP_PAGES_USE_PIN)
> +				nr = pin_user_pages(addr, nr, gup->flags,
> +						    pages + i, NULL);
> +			else
> +				nr = get_user_pages(addr, nr, gup->flags,
> +						    pages + i, NULL);
> +			break;
>  		default:
>  			kvfree(pages);
>  			ret = -EINVAL;
> @@ -127,10 +175,11 @@ static int __gup_test_ioctl(unsigned int cmd,
>  	 * state: print a warning if any non-dma-pinned pages are found:
>  	 */
>  	verify_dma_pinned(cmd, pages, nr_pages);
> +	dump_pages_test(gup, pages, nr_pages);
>  
>  	start_time = ktime_get();
>  
> -	put_back_pages(cmd, pages, nr_pages);
> +	put_back_pages(cmd, pages, nr_pages, gup->flags);
>  
>  	end_time = ktime_get();
>  	gup->put_delta_usec = ktime_us_delta(end_time, start_time);
> @@ -152,6 +201,7 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
>  	case PIN_LONGTERM_BENCHMARK:
>  	case GUP_BASIC_TEST:
>  	case PIN_BASIC_TEST:
> +	case DUMP_USER_PAGES_TEST:
>  		break;
>  	default:
>  		return -EINVAL;
> diff --git a/mm/gup_test.h b/mm/gup_test.h
> index 921b4caad8ef..90a6713d50eb 100644
> --- a/mm/gup_test.h
> +++ b/mm/gup_test.h
> @@ -9,6 +9,11 @@
>  #define PIN_LONGTERM_BENCHMARK	_IOWR('g', 3, struct gup_test)
>  #define GUP_BASIC_TEST		_IOWR('g', 4, struct gup_test)
>  #define PIN_BASIC_TEST		_IOWR('g', 5, struct gup_test)
> +#define DUMP_USER_PAGES_TEST	_IOWR('g', 6, struct gup_test)
> +
> +#define GUP_TEST_MAX_PAGES_TO_DUMP		8
> +
> +#define GUP_TEST_FLAG_DUMP_PAGES_USE_PIN	0x1
>  
>  struct gup_test {
>  	__u64 get_delta_usec;
> @@ -17,6 +22,11 @@ struct gup_test {
>  	__u64 size;
>  	__u32 nr_pages_per_call;
>  	__u32 flags;
> +	/*
> +	 * Each non-zero entry is the number of the page (1-based: first page is
> +	 * page 1, so that zero entries mean "do nothing") from the .addr base.
> +	 */
> +	__u32 which_pages[GUP_TEST_MAX_PAGES_TO_DUMP];
>  };
>  
>  #endif	/* __GUP_TEST_H */
> diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
> index 67d57a1cc8b6..68137b337114 100644
> --- a/tools/testing/selftests/vm/gup_test.c
> +++ b/tools/testing/selftests/vm/gup_test.c
> @@ -27,21 +27,23 @@ static char *cmd_to_str(unsigned long cmd)
>  		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";
>  }
>  
>  int main(int argc, char **argv)
>  {
> -	struct gup_test gup;
> +	struct gup_test gup = { 0 };
>  	unsigned long size = 128 * MB;
>  	int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
> -	int cmd = GUP_FAST_BENCHMARK;
> +	unsigned long cmd = GUP_FAST_BENCHMARK;
>  	int flags = MAP_PRIVATE;
>  	char *file = "/dev/zero";
>  	char *p;
>  
> -	while ((opt = getopt(argc, argv, "m:r:n:f:abtTLUuwSH")) != -1) {
> +	while ((opt = getopt(argc, argv, "m:r:n:F:f:abctTLUuwSH")) != -1) {
>  		switch (opt) {
>  		case 'a':
>  			cmd = PIN_FAST_BENCHMARK;
> @@ -52,6 +54,21 @@ int main(int argc, char **argv)
>  		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 'F':
> +			/* strtol, so you can pass flags in hex form */
> +			gup.flags = strtol(optarg, 0, 0);
> +			break;
>  		case 'm':
>  			size = atoi(optarg) * MB;
>  			break;
> @@ -91,6 +108,30 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> +	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++;
> +		}
> +	}
> +
>  	filed = open(file, O_RDWR|O_CREAT);
>  	if (filed < 0) {
>  		perror("open");
> -- 
> 2.28.0
> 
> 

[PATCH 7/8] selftests/vm: run_vmtest.sh: update and clean up gup_test invocation

[John Hubbard]

Run benchmarks on the _fast variants of gup and pup, as originally
intended.

Run the new gup_test sub-test: dump pages. In addition to exercising the
dump_page() call, it also demonstrates the various options you can use
to specify which pages to dump, and how.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vm/run_vmtest.sh | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/vm/run_vmtest.sh b/tools/testing/selftests/vm/run_vmtest.sh
index d1843d5f3c30..e3a8b14d9df6 100755
--- a/tools/testing/selftests/vm/run_vmtest.sh
+++ b/tools/testing/selftests/vm/run_vmtest.sh
@@ -124,9 +124,9 @@ else
 fi
 
 echo "--------------------------------------------"
-echo "running 'gup_test -U' (normal/slow gup)"
+echo "running 'gup_test -u' (fast gup benchmark)"
 echo "--------------------------------------------"
-./gup_test -U
+./gup_test -u
 if [ $? -ne 0 ]; then
 	echo "[FAIL]"
 	exitcode=1
@@ -134,10 +134,22 @@ else
 	echo "[PASS]"
 fi
 
-echo "------------------------------------------"
-echo "running gup_test -b (pin_user_pages)"
-echo "------------------------------------------"
-./gup_test -b
+echo "---------------------------------------------------"
+echo "running gup_test -a (pin_user_pages_fast benchmark)"
+echo "---------------------------------------------------"
+./gup_test -a
+if [ $? -ne 0 ]; then
+	echo "[FAIL]"
+	exitcode=1
+else
+	echo "[PASS]"
+fi
+
+echo "--------------------------------------------------------------"
+echo "running gup_test -ct -F 0x1 0 19 0x1000"
+echo "   Dumps pages 0, 19, and 4096, using pin_user_pages (-F 0x1)"
+echo "--------------------------------------------------------------"
+./gup_test -ct -F 0x1 0 19 0x1000
 if [ $? -ne 0 ]; then
 	echo "[FAIL]"
 	exitcode=1
-- 
2.28.0

Re: [PATCH 7/8] selftests/vm: run_vmtest.sh: update and clean up gup_test invocation

[Ira Weiny]

On Sun, Sep 27, 2020 at 11:21:58PM -0700, John Hubbard wrote:
> Run benchmarks on the _fast variants of gup and pup, as originally
> intended.
> 
> Run the new gup_test sub-test: dump pages. In addition to exercising the
> dump_page() call, it also demonstrates the various options you can use
> to specify which pages to dump, and how.
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  tools/testing/selftests/vm/run_vmtest.sh | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/run_vmtest.sh b/tools/testing/selftests/vm/run_vmtest.sh
> index d1843d5f3c30..e3a8b14d9df6 100755
> --- a/tools/testing/selftests/vm/run_vmtest.sh
> +++ b/tools/testing/selftests/vm/run_vmtest.sh
> @@ -124,9 +124,9 @@ else
>  fi
>  
>  echo "--------------------------------------------"
> -echo "running 'gup_test -U' (normal/slow gup)"
> +echo "running 'gup_test -u' (fast gup benchmark)"
>  echo "--------------------------------------------"
> -./gup_test -U
> +./gup_test -u
>  if [ $? -ne 0 ]; then
>  	echo "[FAIL]"
>  	exitcode=1
> @@ -134,10 +134,22 @@ else
>  	echo "[PASS]"
>  fi
>  
> -echo "------------------------------------------"
> -echo "running gup_test -b (pin_user_pages)"
> -echo "------------------------------------------"
> -./gup_test -b
> +echo "---------------------------------------------------"
> +echo "running gup_test -a (pin_user_pages_fast benchmark)"
> +echo "---------------------------------------------------"
> +./gup_test -a
> +if [ $? -ne 0 ]; then
> +	echo "[FAIL]"
> +	exitcode=1
> +else
> +	echo "[PASS]"
> +fi
> +
> +echo "--------------------------------------------------------------"
> +echo "running gup_test -ct -F 0x1 0 19 0x1000"
> +echo "   Dumps pages 0, 19, and 4096, using pin_user_pages (-F 0x1)"
> +echo "--------------------------------------------------------------"
> +./gup_test -ct -F 0x1 0 19 0x1000

Ah here it is...  Maybe just remove that from the previous commit message.

Ira

>  if [ $? -ne 0 ]; then
>  	echo "[FAIL]"
>  	exitcode=1
> -- 
> 2.28.0
> 
> 

Re: [PATCH 7/8] selftests/vm: run_vmtest.sh: update and clean up gup_test invocation

[John Hubbard]

On 9/28/20 12:26 PM, Ira Weiny wrote:
> On Sun, Sep 27, 2020 at 11:21:58PM -0700, John Hubbard wrote:
...
>> +echo "--------------------------------------------------------------"
>> +echo "running gup_test -ct -F 0x1 0 19 0x1000"
>> +echo "   Dumps pages 0, 19, and 4096, using pin_user_pages (-F 0x1)"
>> +echo "--------------------------------------------------------------"
>> +./gup_test -ct -F 0x1 0 19 0x1000
> 
> Ah here it is...  Maybe just remove that from the previous commit message.
> 
> Ira

Yes, will do, thanks for spotting that.

thanks,
-- 
John Hubbard
NVIDIA

[PATCH 8/8] selftests/vm: hmm-tests: remove the libhugetlbfs dependency

[John Hubbard]

HMM selftests are incredibly useful, but they are only effective if
people actually build and run them. All the other tests in selftests/vm
can be built with very standard, always-available libraries: libpthread,
librt. The hmm-tests.c program, on the other hand, requires something
that is (much) less readily available: libhugetlbfs. And so the build
will typically fail for many developers.

A simple attempt to install libhugetlbfs will also run into
complications on some common distros these days: Fedora and Arch Linux
(yes, Arch AUR has it, but that's fragile, as always with AUR). The
library is not maintained actively enough at the moment, for distros to
deal with it. I had to build it from source, for Fedora, and that didn't
go too smoothly either.

It turns out that, out of 21 tests in hmm-tests.c, only 2 actually
require functionality from libhugetlbfs. Therefore, if libhugetlbfs is
missing, simply ifdef those two tests out and allow the developer to at
least have the other 19 tests, if they don't want to pause to work
through the above issues. Also issue a warning, so that it's clear that
there is an imperfection in the build.

In order to do that, a tiny shell script (check_config.sh) runs a quick
compile (not link, that's too prone to false failures with library
paths), and basically, if the compiler doesn't find hugetlbfs.h in its
standard locations, then the script concludes that libhugetlbfs is not
available. The output is in two files, one for inclusion in hmm-test.c
(local_config.h), and one for inclusion in the Makefile
(local_config.mk).

Cc: Ralph Campbell <rcampbell@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/vm/.gitignore      |  1 +
 tools/testing/selftests/vm/Makefile        | 24 +++++++++++++++--
 tools/testing/selftests/vm/check_config.sh | 30 ++++++++++++++++++++++
 tools/testing/selftests/vm/hmm-tests.c     | 10 +++++++-
 4 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100755 tools/testing/selftests/vm/check_config.sh

diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
index 2c8ddcf41c0e..e90d28bcd518 100644
--- a/tools/testing/selftests/vm/.gitignore
+++ b/tools/testing/selftests/vm/.gitignore
@@ -20,3 +20,4 @@ va_128TBswitch
 map_fixed_noreplace
 write_to_hugetlbfs
 hmm-tests
+local_config.*
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 2579242386ac..986a90fa9653 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for vm selftests
+
+include local_config.mk
+
 uname_M := $(shell uname -m 2>/dev/null || echo not)
 MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/')
 
@@ -76,8 +79,6 @@ TEST_FILES := test_vmalloc.sh
 KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
-$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs
-
 ifeq ($(ARCH),x86_64)
 BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
 BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
@@ -130,3 +131,22 @@ endif
 $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
 
 $(OUTPUT)/gup_test: ../../../../mm/gup_test.h
+
+$(OUTPUT)/hmm-tests: local_config.h
+
+# HMM_EXTRA_LIBS may get set in local_config.mk, or it may be left empty.
+$(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS)
+
+local_config.mk local_config.h: check_config.sh
+	./check_config.sh
+
+EXTRA_CLEAN += local_config.mk local_config.h
+
+ifeq ($(HMM_EXTRA_LIBS),)
+all: warn_missing_hugelibs
+
+warn_missing_hugelibs:
+	@echo ; \
+	echo "Warning: missing libhugetlbfs support. Some HMM tests will be skipped." ; \
+	echo
+endif
diff --git a/tools/testing/selftests/vm/check_config.sh b/tools/testing/selftests/vm/check_config.sh
new file mode 100755
index 000000000000..651a4b192479
--- /dev/null
+++ b/tools/testing/selftests/vm/check_config.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Probe for libraries and create header files to record the results. Both C
+# header files and Makefile include fragments are created.
+
+OUTPUT_H_FILE=local_config.h
+OUTPUT_MKFILE=local_config.mk
+
+# libhugetlbfs
+tmpname=$(mktemp)
+tmpfile_c=${tmpname}.c
+tmpfile_o=${tmpname}.o
+
+echo "#include <sys/types.h>"        > $tmpfile_c
+echo "#include <hugetlbfs.h>"       >> $tmpfile_c
+echo "int func(void) { return 0; }" >> $tmpfile_c
+
+gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1
+
+if [ -f $tmpfile_o ]; then
+    echo "#define LOCAL_CONFIG_HAVE_LIBHUGETLBFS 1" > $OUTPUT_H_FILE
+    echo "HMM_EXTRA_LIBS = -lhugetlbfs"             > $OUTPUT_MKFILE
+else
+    echo "// No libhugetlbfs support found"      > $OUTPUT_H_FILE
+    echo "# No libhugetlbfs support found, so:"  > $OUTPUT_MKFILE
+    echo "HMM_EXTRA_LIBS = "                    >> $OUTPUT_MKFILE
+fi
+
+rm ${tmpname}.*
diff --git a/tools/testing/selftests/vm/hmm-tests.c b/tools/testing/selftests/vm/hmm-tests.c
index 0a28a6a29581..6b79723d7dc6 100644
--- a/tools/testing/selftests/vm/hmm-tests.c
+++ b/tools/testing/selftests/vm/hmm-tests.c
@@ -21,12 +21,16 @@
 #include <strings.h>
 #include <time.h>
 #include <pthread.h>
-#include <hugetlbfs.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <sys/ioctl.h>
 
+#include "./local_config.h"
+#ifdef LOCAL_CONFIG_HAVE_LIBHUGETLBFS
+#include <hugetlbfs.h>
+#endif
+
 /*
  * This is a private UAPI to the kernel test module so it isn't exported
  * in the usual include/uapi/... directory.
@@ -662,6 +666,7 @@ TEST_F(hmm, anon_write_huge)
 	hmm_buffer_free(buffer);
 }
 
+#ifdef LOCAL_CONFIG_HAVE_LIBHUGETLBFS
 /*
  * Write huge TLBFS page.
  */
@@ -720,6 +725,7 @@ TEST_F(hmm, anon_write_hugetlbfs)
 	buffer->ptr = NULL;
 	hmm_buffer_free(buffer);
 }
+#endif /* LOCAL_CONFIG_HAVE_LIBHUGETLBFS */
 
 /*
  * Read mmap'ed file memory.
@@ -1336,6 +1342,7 @@ TEST_F(hmm2, snapshot)
 	hmm_buffer_free(buffer);
 }
 
+#ifdef LOCAL_CONFIG_HAVE_LIBHUGETLBFS
 /*
  * Test the hmm_range_fault() HMM_PFN_PMD flag for large pages that
  * should be mapped by a large page table entry.
@@ -1411,6 +1418,7 @@ TEST_F(hmm, compound)
 	buffer->ptr = NULL;
 	hmm_buffer_free(buffer);
 }
+#endif /* LOCAL_CONFIG_HAVE_LIBHUGETLBFS */
 
 /*
  * Test two devices reading the same memory (double mapped).
-- 
2.28.0

Re: [PATCH 8/8] selftests/vm: hmm-tests: remove the libhugetlbfs dependency

[Jason Gunthorpe]

On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote:

> @@ -76,8 +79,6 @@ TEST_FILES := test_vmalloc.sh
>  KSFT_KHDR_INSTALL := 1
>  include ../lib.mk
>  
> -$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs
> -
>  ifeq ($(ARCH),x86_64)
>  BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
>  BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
> @@ -130,3 +131,22 @@ endif
>  $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
>  
>  $(OUTPUT)/gup_test: ../../../../mm/gup_test.h
> +
> +$(OUTPUT)/hmm-tests: local_config.h
> +
> +# HMM_EXTRA_LIBS may get set in local_config.mk, or it may be left empty.
> +$(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS)
> +
> +local_config.mk local_config.h: check_config.sh
> +	./check_config.sh
> +
> +EXTRA_CLEAN += local_config.mk local_config.h
> +
> +ifeq ($(HMM_EXTRA_LIBS),)
> +all: warn_missing_hugelibs
> +
> +warn_missing_hugelibs:
> +	@echo ; \
> +	echo "Warning: missing libhugetlbfs support. Some HMM tests will be skipped." ; \
> +	echo
> +endif
> diff --git a/tools/testing/selftests/vm/check_config.sh b/tools/testing/selftests/vm/check_config.sh
> new file mode 100755
> index 000000000000..651a4b192479
> +++ b/tools/testing/selftests/vm/check_config.sh
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Probe for libraries and create header files to record the results. Both C
> +# header files and Makefile include fragments are created.
> +
> +OUTPUT_H_FILE=local_config.h
> +OUTPUT_MKFILE=local_config.mk
> +
> +# libhugetlbfs
> +tmpname=$(mktemp)
> +tmpfile_c=${tmpname}.c
> +tmpfile_o=${tmpname}.o
> +
> +echo "#include <sys/types.h>"        > $tmpfile_c
> +echo "#include <hugetlbfs.h>"       >> $tmpfile_c
> +echo "int func(void) { return 0; }" >> $tmpfile_c
> +
> +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1

This gcc has to come from some makefile variable

This is kind of janky :\

Could we just not use libhugetlbfs? Doesn't it all just boil down to
creating a file in /dev/huge? Eg look at tools/testing/selftests/vm/hugepage-mmap.c

Jason

Re: [PATCH 8/8] selftests/vm: hmm-tests: remove the libhugetlbfs dependency

[John Hubbard]

On 9/28/20 6:02 AM, Jason Gunthorpe wrote:
> On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote:
...
>> +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1
> 
> This gcc has to come from some makefile variable

ahem, yes, that really should have just been $(CC), will change to that.

> 
> This is kind of janky :\
> 
> Could we just not use libhugetlbfs? Doesn't it all just boil down to
> creating a file in /dev/huge? Eg look at tools/testing/selftests/vm/hugepage-mmap.c
> 

Well, the situation is a lot worse than that, because hmm-tests.c is using a few
helper functions that end up pulling in more and more.

My first attempt was actually in your direction: just grab a bit of code from the
library and drop it in. But that ended up turning into several pages of code from
quite a few functions and definitions, and it was looking maybe excessive. (I
can look at it again, though. Maybe it feels less excessive if there are no other
acceptible alternatives.)

So then I thought, why not just *delete* those two routines from hmm-tests.c? But
Ralph didn't like that because he notes that hmm_range_fault() loses some useful
test coverage by being exercised with hugetlbfs.

So finally I landed here, which is so far, the smallest change that would be
potentially acceptible: a couple dozen lines, in order to selectively disable the
problematic routines.

Anyway, thoughts? I like the current approach but am open to anything that makes
hmm-test Just Work for more people, on the *first* try.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 8/8] selftests/vm: hmm-tests: remove the libhugetlbfs dependency

[John Hubbard]

On 9/28/20 1:18 PM, John Hubbard wrote:
> On 9/28/20 6:02 AM, Jason Gunthorpe wrote:
>> On Sun, Sep 27, 2020 at 11:21:59PM -0700, John Hubbard wrote:
> ...
>>> +gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1
>>
>> This gcc has to come from some makefile variable
  I plan on posting a v2 with this additional change, to fix the above point:

diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 986a90fa9653..019cbb7f3cf8 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -138,7 +138,7 @@ $(OUTPUT)/hmm-tests: local_config.h
  $(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS)

  local_config.mk local_config.h: check_config.sh
-	./check_config.sh
+	./check_config.sh $(CC)

  EXTRA_CLEAN += local_config.mk local_config.h

diff --git a/tools/testing/selftests/vm/check_config.sh b/tools/testing/selftests/vm/check_config.sh
index 651a4b192479..079c8a40b85d 100755
--- a/tools/testing/selftests/vm/check_config.sh
+++ b/tools/testing/selftests/vm/check_config.sh
@@ -16,7 +16,8 @@ echo "#include <sys/types.h>"        > $tmpfile_c
  echo "#include <hugetlbfs.h>"       >> $tmpfile_c
  echo "int func(void) { return 0; }" >> $tmpfile_c

-gcc -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1
+CC=${1:?"Usage: $0 <compiler> # example compiler: gcc"}
+$CC -c $tmpfile_c -o $tmpfile_o >/dev/null 2>&1

  if [ -f $tmpfile_o ]; then
      echo "#define LOCAL_CONFIG_HAVE_LIBHUGETLBFS 1" > $OUTPUT_H_FILE



thanks,
-- 
John Hubbard
NVIDIA