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

[Konstantin Khorenko <khorenko@virtuozzo.com>]

On 11/5/26 20:30, David Hildenbrand (Arm) wrote:
> 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.

Sure, i will drop the test in v3 and just put a link to this test in the
cover letter and that's it.

-- 
Cheers,
Konstantin