Re: [PATCH v2 2/2] selftests/mm: test fork of a device DAX PUD

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

On 11/5/26 18:00, Konstantin Khorenko wrote:
> Fork a process that maps a device DAX instance with a PUD, in a VMA
> whose page tables fork copies because it has a guard region. The child
> must account the copied PUD in its RssFile, which a new rss_file() in
> vm_util reads as rss_anon() reads RssAnon.
> 
> Without a reference, an rmap and a counter for the copied PUD, the
> child's RssFile misses the PUD, and its exit leaves the child's file
> counter negative: "Bad rss-counter state".
> 
> It runs only on a device named in argv[1] or $FORK_DAX_PUD_DEVICE: on
> a kernel without the fix it leaves the device folio's mapcount below
> zero.
> 
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  MAINTAINERS                               |   1 +
>  tools/testing/selftests/mm/Makefile       |   1 +
>  tools/testing/selftests/mm/fork_dax_pud.c | 114 ++++++++++++++++++++++++++++++
>  tools/testing/selftests/mm/run_vmtests.sh |   1 +
>  tools/testing/selftests/mm/vm_util.c      |  21 ++++++
>  tools/testing/selftests/mm/vm_util.h      |   1 +
>  6 files changed, 139 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d63d8b2152773..247e16f3276f4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17931,6 +17931,7 @@ F:	mm/huge_memory.c
>  F:	mm/khugepaged.c
>  F:	mm/mm_slot.h
>  F:	tools/mm/thpmaps
> +F:	tools/testing/selftests/mm/fork_dax_pud.c
>  F:	tools/testing/selftests/mm/khugepaged.c
>  F:	tools/testing/selftests/mm/split_huge_page_test.c
>  F:	tools/testing/selftests/mm/transhuge-stress.c
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 0ea72e9979347..130500f5f6e0a 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -108,6 +108,7 @@ TEST_GEN_FILES += folio_split_race_test
>  TEST_GEN_FILES += folio_order_check
>  TEST_GEN_FILES += khugepaged_sync_check
>  TEST_GEN_FILES += khugepaged_race
> +TEST_GEN_FILES += fork_dax_pud
>  TEST_GEN_FILES += soft-dirty
>  
>  ifeq ($(ARCH),x86_64)
> diff --git a/tools/testing/selftests/mm/fork_dax_pud.c b/tools/testing/selftests/mm/fork_dax_pud.c
> new file mode 100644
> index 0000000000000..0d19c30b1dc6f
> --- /dev/null
> +++ b/tools/testing/selftests/mm/fork_dax_pud.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * fork() of a process that maps a device DAX instance with a PUD, in a VMA
> + * whose page tables fork copies because it has a guard region. The child
> + * must account the copied PUD in its RssFile.
> + *
> + * Runs only on a device named in argv[1] or $FORK_DAX_PUD_DEVICE that
> + * nothing else maps, with 1 GiB alignment and at least 2 GiB. To make one
> + * on pmem emulated in RAM, with CONFIG_X86_PMEM_LEGACY and
> + * CONFIG_DEV_DAX_PMEM:
> + *
> + *	boot with memmap=3G!4G
> + *	ndctl create-namespace -f -e namespace0.0 -m devdax -a 1G
> + *	FORK_DAX_PUD_DEVICE=/dev/dax0.0 ./fork_dax_pud
> + */
> +#define _GNU_SOURCE
> +#include <asm-generic/mman.h> /* Force the import of the tools version. */
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <setjmp.h>
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <sys/wait.h>
> +
> +#include "kselftest.h"
> +#include "vm_util.h"
> +
> +#define SZ_1G		(1UL << 30)
> +#define DAX_SYSFS	"/sys/bus/dax/devices"
> +
> +static sigjmp_buf sigbus_jmp;
> +
> +static void sigbus_handler(int sig)
> +{
> +	siglongjmp(sigbus_jmp, 1);
> +}
> +
> +static bool dax_attr(const char *dev, const char *attr, unsigned long *val)
> +{
> +	char path[PATH_MAX];
> +
> +	snprintf(path, sizeof(path), DAX_SYSFS "/%s/%s", dev, attr);
> +	return !read_sysfs(path, val);
> +}
> +
> +static bool dax_dev_ok(const char *path)
> +{
> +	const char *name = strrchr(path, '/');
> +	unsigned long align, size;
> +
> +	name = name ? name + 1 : path;
> +	return dax_attr(name, "align", &align) && align == SZ_1G &&
> +	       dax_attr(name, "size", &size) && size >= 2 * SZ_1G;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	const char *path = argc > 1 ? argv[1] : getenv("FORK_DAX_PUD_DEVICE");
> +	char *area, *mem;
> +	int fd, status;
> +	pid_t pid;
> +
> +	ksft_print_header();
> +	if (!path)
> +		ksft_exit_skip("no devdax device given\n");
> +	if (!dax_dev_ok(path))
> +		ksft_exit_skip("%s: needs 1 GiB alignment and 2 GiB\n", path);
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		ksft_exit_skip("%s: %s\n", path, strerror(errno));
> +	area = mmap(NULL, 3 * SZ_1G, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	if (area == MAP_FAILED)
> +		ksft_exit_fail_perror("mmap");
> +	mem = (char *)(((unsigned long)area + SZ_1G - 1) & ~(SZ_1G - 1));
> +	mem = mmap(mem, 2 * SZ_1G, PROT_READ, MAP_SHARED | MAP_FIXED, fd, 0);
> +	if (mem == MAP_FAILED)
> +		ksft_exit_skip("mmap %s: %s\n", path, strerror(errno));
> +
> +	/* A PUD in the first GiB, a guard page at the end of the second. */
> +	signal(SIGBUS, sigbus_handler);
> +	if (sigsetjmp(sigbus_jmp, 1))
> +		ksft_exit_skip("%s cannot be mapped with a PUD\n", path);
> +	FORCE_READ(mem[0]);
> +	signal(SIGBUS, SIG_DFL);
> +	if (madvise(mem + 2 * SZ_1G - getpagesize(), getpagesize(),
> +		    MADV_GUARD_INSTALL))
> +		ksft_exit_skip("MADV_GUARD_INSTALL: %s\n", strerror(errno));
> +
> +	ksft_set_plan(1);
> +	fflush(stdout);
> +	pid = fork();
> +	if (pid < 0)
> +		ksft_exit_fail_perror("fork");
> +	if (!pid)
> +		_exit(rss_file() < (SZ_1G >> 10) ? 10 : 0);
> +	if (waitpid(pid, &status, 0) != pid)
> +		ksft_exit_fail_perror("waitpid");
> +
> +	if (!WIFEXITED(status) || WEXITSTATUS(status))
> +		ksft_test_result_fail("child does not account the copied PUD\n");
> +	else
> +		ksft_test_result_pass("devdax PUD copied by fork\n");
> +
> +	munmap(area, 3 * SZ_1G);
> +	close(fd);
> +	ksft_finished();
> +}

I prefer to not merge this. It requires an actual dax device, so it's not very
applicable on CI systems, and we don't want selftests to take parameters in one
form or the other.

-- 
Cheers,

David