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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/11/26 13:09, Sarthak Sharma 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 5 test cases for every variant: get_user_pages,
> get_user_pages_fast, pin_user_pages, pin_user_pages_fast and
> pin_user_pages_longterm.
> 
> Use two default hugeTLB pages and derive the mapping size from
> their size. This exercises GUP both within a single HugeTLB page
> and across a HugeTLB boundary, without reserving an excessive
> number of pages.
> 
> 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. In total the selftest reports 60 TAP cases and issues
> 240 ioctls.
> 
> Do not carry DUMP_USER_PAGES_TEST into the new selftest because its
> output is written to the kernel log and the selftest does not verify
> that output.
> 
> Add the new gup binary to the selftests/mm build, run_vmtests.sh and
> MAINTAINERS. Update mm/Kconfig to describe the benchmark and
> selftest split.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Tested-by: Muhammad Usama Anjum <usama.anjum@arm.com>
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> ---
>  MAINTAINERS                               |   1 +
>  mm/Kconfig                                |  19 +-
>  tools/testing/selftests/mm/Makefile       |   1 +
>  tools/testing/selftests/mm/gup.c          | 263 ++++++++++++++++++++++
>  tools/testing/selftests/mm/run_vmtests.sh |   1 +
>  5 files changed, 273 insertions(+), 12 deletions(-)
>  create mode 100644 tools/testing/selftests/mm/gup.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cae3af861a7c..c47e655c4111 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17188,6 +17188,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/mm/Kconfig b/mm/Kconfig
> index c180d40cd671..61ab1b8a2ebd 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1291,24 +1291,19 @@ config PERCPU_STATS
>  	  be used to help understand percpu memory usage.
>  
>  config GUP_TEST
> -	bool "Enable infrastructure for get_user_pages()-related unit tests"
> +	bool "Enable infrastructure for get_user_pages()-related unit tests and benchmarks"
>  	depends on DEBUG_FS
>  	help
>  	  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.
> +	  to make ioctl calls that can launch kernel-based unit tests and
> +	  benchmarks for the get_user_pages*() and pin_user_pages*() families
> +	  of API calls.
>  
> -	  These tests include benchmark testing of the _fast variants of
> -	  get_user_pages*() and pin_user_pages*(), as well as smoke tests of
> +	  These include benchmark testing of the _fast variants of
> +	  get_user_pages*() and pin_user_pages*(), as well as 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/mm/gup_test.c
> +	  See tools/testing/selftests/mm/gup.c and tools/mm/gup_bench.c.
>  
>  comment "GUP_TEST needs to have DEBUG_FS enabled"
>  	depends on !GUP_TEST && !DEBUG_FS
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 11ca9b11fef1..9c03624fd293 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..31ae38e09136
> --- /dev/null
> +++ b/tools/testing/selftests/mm/gup.c
> @@ -0,0 +1,263 @@
> +// 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 */

BTW, it's odd that we support passing GUP-flags ... we should probably switch at
some point simpler attributes (bool write) or custom flags (but we don't seem to
need many ...).

> +
> +/* Page counts exercising single, THP-batch, partial, and full-mapping GUP. */
> +static const int nr_pages_list[] = { 1, 512, 123, -1 };


Would we want to calculate 512 dynamically at runtime using the PMD pagesize?
Could be something for a follow-up patch.

> +
> +#define GUP_TEST_FILE "/sys/kernel/debug/gup_test"
> +#define NR_HUGE_PAGES 2

I'd call this "NR_HUGETLB_PAGES".

[...]

> +
> +FIXTURE_SETUP(gup_test)
> +{
> +	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS;
> +	char *p;
> +
> +	self->size = 128 * MB;
> +
> +	if (variant->hugetlb) {
> +		if (!hp_size)
> +			SKIP(return, "HugeTLB not available\n");
> +
> +		if (hugetlb_free_default_pages() < NR_HUGE_PAGES)
> +			SKIP(return, "Not enough huge pages\n");
> +
> +		self->size = NR_HUGE_PAGES * hp_size;
> +		mmap_flags |= MAP_HUGETLB;
> +	}
> +
> +	if (variant->shared)
> +		mmap_flags = (mmap_flags & ~MAP_PRIVATE) | MAP_SHARED;
> +
> +	/* 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->addr = mmap(NULL, self->size, PROT_READ | PROT_WRITE,
> +			  mmap_flags, -1, 0);
> +
> +	ASSERT_NE(self->addr, MAP_FAILED) {
> +		int err = errno;
> +
> +		close(self->gup_fd);
> +		TH_LOG("mmap failed: %s", strerror(err));
> +	}
> +
> +	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);
> +}
> +
> +static void run_gup_cmd(struct __test_metadata *_metadata,
> +			 FIXTURE_DATA(gup_test) *self,
> +			 const FIXTURE_VARIANT(gup_test) *variant,
> +			 unsigned long command)

We prefer two tab indents.

> +{
> +	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],
> +			.gup_flags = variant->write ? FOLL_WRITE : 0,
> +		};
> +
> +		TH_LOG("nr_pages_per_call=%u", gup.nr_pages_per_call);
> +		ASSERT_EQ(ioctl(self->gup_fd, command, &gup), 0);
> +		ASSERT_EQ(gup.size, self->size);
> +	}
> +}
> +
> +TEST_F(gup_test, get_user_pages)
> +{
> +	run_gup_cmd(_metadata, self, variant, GUP_BASIC_TEST);
> +}
> +
> +TEST_F(gup_test, pin_user_pages)
> +{
> +	run_gup_cmd(_metadata, self, variant, PIN_BASIC_TEST);
> +}
> +
> +TEST_F(gup_test, get_user_pages_fast)
> +{
> +	run_gup_cmd(_metadata, self, variant, GUP_FAST_BENCHMARK);
> +}
> +
> +TEST_F(gup_test, pin_user_pages_fast)
> +{
> +	run_gup_cmd(_metadata, self, variant, PIN_FAST_BENCHMARK);
> +}
> +
> +TEST_F(gup_test, pin_user_pages_longterm)
> +{
> +	run_gup_cmd(_metadata, self, variant, PIN_LONGTERM_BENCHMARK);
> +}

Heh, is there actually a reason why these kernel things are called _BENCHMARK?

I think they are really just tests that can be used for benchmarking ... the
measurement logic is entirely in user space.

We could consider cleaning that up as a follow-up.

> +
> +int main(int argc, char **argv)
> +{
> +	int fd;
> +
> +	fd = open(GUP_TEST_FILE, O_RDWR);

Could do

const int fd = open(GUP_TEST_FILE, O_RDWR);


Thanks for doing that!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>


I think reasonable extensions will be to execute tests on all available mTHP
sizes and all available hugetlb sizes, similar to what cow.c already does.

Can you look into that as part of some follow-up work? mTHP support will be
interesting for testing some of the patches Rik has been working on.

-- 
Cheers,

David