Re: [PATCH v2 2/2] selftests/mm: add stale walk->action race test

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Aug 12, 2026 at 01:18:58AM +0900, Hyunwoo Kim wrote:
> The added pagewalk_race_test maps two PMDs and faults in 2MB of the first
> one.  A second thread then faults in the second PMD and drops it again with
> MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping
> from /proc/self/smaps.
>
> Clearing the second PMD while smaps_pte_range() runs leaves walk->action
> erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried,
> so the first PMD is counted twice and Rss comes out twice as large as what
> was faulted in.
>
> mincore() is the caller named in the fix, but the second walk writes past
> the length mincore() copies back, so it cannot be seen from userspace
> there.  smaps reports what the callbacks counted, so the duplicate shows up
> in Rss.
>
> A failure can only come from the kernel counting the same page twice, so
> missing the race is harmless.  On an unfixed kernel the test fails after a
> few hundred reads at most and takes about half a second.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  tools/testing/selftests/mm/.gitignore         |   1 +
>  tools/testing/selftests/mm/Makefile           |   2 +
>  tools/testing/selftests/mm/ksft_pagewalk.sh   |   4 +
>  .../testing/selftests/mm/pagewalk_race_test.c | 138 ++++++++++++++++++
>  tools/testing/selftests/mm/run_vmtests.sh     |   2 +
>  tools/testing/selftests/mm/vm_util.h          |   1 +
>  6 files changed, 148 insertions(+)
>  create mode 100755 tools/testing/selftests/mm/ksft_pagewalk.sh
>  create mode 100644 tools/testing/selftests/mm/pagewalk_race_test.c
>
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index 9ccd9e1447e66b..92f981f97740fd 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -66,3 +66,4 @@ merge
>  prctl_thp_disable
>  rmap
>  folio_split_race_test
> +pagewalk_race_test
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index e6df968f0971c8..cde9b22f121d4b 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -105,6 +105,7 @@ TEST_GEN_FILES += guard-regions
>  TEST_GEN_FILES += merge
>  TEST_GEN_FILES += rmap
>  TEST_GEN_FILES += folio_split_race_test
> +TEST_GEN_FILES += pagewalk_race_test
>
>  ifneq ($(ARCH),arm64)
>  TEST_GEN_FILES += soft-dirty
> @@ -163,6 +164,7 @@ TEST_PROGS += ksft_mlock.sh
>  TEST_PROGS += ksft_mmap.sh
>  TEST_PROGS += ksft_mremap.sh
>  TEST_PROGS += ksft_pagemap.sh
> +TEST_PROGS += ksft_pagewalk.sh
>  TEST_PROGS += ksft_pfnmap.sh
>  TEST_PROGS += ksft_pkey.sh
>  TEST_PROGS += ksft_process_madv.sh
> diff --git a/tools/testing/selftests/mm/ksft_pagewalk.sh b/tools/testing/selftests/mm/ksft_pagewalk.sh
> new file mode 100755
> index 00000000000000..6f6c3ee1c13ef4
> --- /dev/null
> +++ b/tools/testing/selftests/mm/ksft_pagewalk.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: GPL-2.0
> +
> +./run_vmtests.sh -t pagewalk
> diff --git a/tools/testing/selftests/mm/pagewalk_race_test.c b/tools/testing/selftests/mm/pagewalk_race_test.c
> new file mode 100644
> index 00000000000000..42fd6e75e821ed
> --- /dev/null
> +++ b/tools/testing/selftests/mm/pagewalk_race_test.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Regression test for a stale walk->action escaping walk_pmd_range() and
> + * making walk_pud_range() walk the same range twice.
> + *
> + * The mapping is two PMDs inside one PUD. PMD 0 is populated once and left
> + * alone, PMD 1 is faulted in and dropped again by a second thread. Clearing
> + * PMD 1 under smaps_pte_range() makes it raise ACTION_AGAIN, and since it is
> + * the last entry the stale value leaves walk_pmd_range(), so smaps accounts
> + * PMD 0 twice. A kernel that does not reclaim the emptied page table never
> + * clears PMD 1 and so never hits the race.
> + *
> + * A hit can only come from the kernel counting the same page twice, so the
> + * test cannot fail spuriously.
> + */
> +#define _GNU_SOURCE
> +
> +#include <pthread.h>
> +#include <stdatomic.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include "vm_util.h"
> +#include "kselftest.h"
> +
> +#define NR_PMDS		2
> +#define NR_ROUNDS	20000

I did say please don't let this be slow :) if the test takes longer than say
200ms then just let it be less reliably repro maybe?

> +/* Cap on how much of PMD 0 to fault in, so that a large PMD stays cheap. */
> +#define POP_MAX		(2 * 1024 * 1024)
> +
> +static char *area;
> +static size_t pmd_size;
> +static atomic_int stop;
> +
> +static void *racer(void *arg)
> +{
> +	char *pmd1 = area + pmd_size;
> +
> +	while (atomic_load_explicit(&stop, memory_order_acquire) == 0) {
> +		/* madvise() below keeps the compiler from lifting this out. */

Err what? :) this seems crazy? You are using an madvise to prevent a compiler
optinisation how? :) isn't atomic_load_explicit() a compiler barrier anyway?

I think you can probably do this better... see FORCE_READ() for use of volatile
to achieve the same thing for a read.

But why would zapping a PMD achieve anything for the compiler? So confused

> +		*pmd1 = 1;
> +		madvise(pmd1, pmd_size, MADV_DONTNEED);
> +	}
> +	return NULL;
> +}
> +
> +static unsigned long smaps_rss_kb(void)
> +{
> +	char buf[1024];
> +	char *entry;
> +
> +	entry = __get_smap_entry(area, "Rss:", buf, sizeof(buf));
> +	if (!entry)
> +		ksft_exit_fail_msg("no Rss: entry for the test mapping\n");
> +
> +	return strtoul(entry, NULL, 10);
> +}
> +
> +int main(void)
> +{
> +	unsigned long max_rss_kb, rss_kb = 0;
> +	size_t size, pop_size, i;
> +	pthread_t thread;
> +	char *raw;
> +
> +	ksft_print_header();
> +
> +	pmd_size = read_pmd_pagesize();
> +	if (!pmd_size)
> +		ksft_exit_skip("Cannot determine PMD size\n");
> +
> +	if (sysconf(_SC_NPROCESSORS_ONLN) < 2)
> +		ksft_exit_skip("Need at least 2 CPUs to race\n");
> +
> +	size = NR_PMDS * pmd_size;
> +
> +	/*
> +	 * Align to the mapping size to stay inside one PUD, then trim the
> +	 * slack so that smaps has exactly one VMA to report.
> +	 */
> +	raw = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE,
> +		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);

Why MAP_NORESERVE?

> +	if (raw == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap failed\n");
> +
> +	area = (char *)(((unsigned long)raw + size - 1) & ~(size - 1));

This is horrible, break it out into a sensible helper. Magical bitwise stuff all
compressed into one is not something we want.

> +	if (area != raw)
> +		munmap(raw, area - raw);
> +	if (raw + 2 * size != area + size)
> +		munmap(area + size, raw + 2 * size - (area + size));

Again you're compressing things too much. Keep it simple.

> +
> +	/* A huge PMD never reaches pte_offset_map_lock(), so keep them out. */
> +	if (madvise(area, size, MADV_NOHUGEPAGE))
> +		ksft_exit_skip("MADV_NOHUGEPAGE failed\n");

No idea why you'd skip on this.

This is another reason why you should use the kselftest harness, ASSERT_EQ(...,
0) is much easier... and it has sensible skipping stuff too.

> +
> +	pop_size = pmd_size < POP_MAX ? pmd_size : POP_MAX;
> +	memset(area, 1, pop_size);
> +
> +	max_rss_kb = (pop_size >> 10) + 256;
> +
> +	/* Over the limit before racing means this is not our own mapping. */
> +	rss_kb = smaps_rss_kb();
> +	if (rss_kb > max_rss_kb)
> +		ksft_exit_fail_msg("Rss is %lu kB before racing, expected at most %lu kB\n",
> +				   rss_kb, max_rss_kb);
> +
> +	ksft_set_plan(1);
> +	ksft_print_msg("racing smaps against MADV_DONTNEED, %d rounds\n",
> +		       NR_ROUNDS);
> +
> +	if (pthread_create(&thread, NULL, racer, NULL))
> +		ksft_exit_fail_msg("pthread_create failed\n");
> +
> +	for (i = 0; i < NR_ROUNDS; i++) {
> +		rss_kb = smaps_rss_kb();
> +		if (rss_kb > max_rss_kb)
> +			break;
> +	}
> +
> +	atomic_store_explicit(&stop, 1, memory_order_release);
> +	pthread_join(thread, NULL);
> +
> +	if (i < NR_ROUNDS) {
> +		ksft_print_msg("walk ran twice over the same range\n");
> +		ksft_test_result_fail("Rss %lu kB exceeds %lu kB, round %zu\n",
> +				      rss_kb, max_rss_kb, i);
> +	} else {
> +		ksft_test_result_pass("Rss within %lu kB over %d rounds\n",
> +				      max_rss_kb, NR_ROUNDS);
> +	}
> +
> +	ksft_exit(i == NR_ROUNDS);
> +
> +	return 0;

Putting this all into one function is disgusting. It might be test code but
that's new excuse for being schloppy. Separate things out into functions please and....

> +}

...use the kselftest_harness, see guard_regions.c for an example of how it's
used.

I know it's just 1 test but it avoids all the stupid plan and manual
ksft_test_*() invocations and gets you ASSERT_*() etc.

> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 8c296dedf0474d..d90c6370814f7a 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -398,6 +398,8 @@ fi
>
>  CATEGORY="pagemap" run_test ./pagemap_ioctl
>
> +CATEGORY="pagewalk" run_test ./pagewalk_race_test
> +
>  CATEGORY="pfnmap" run_test ./pfnmap
>
>  # COW tests
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index ea8fc8fdf0eb0b..62292e2417d162 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -88,6 +88,7 @@ bool pagemap_is_populated(int fd, char *start);
>  unsigned long pagemap_get_pfn(int fd, char *start);
>  void clear_softdirty(void);
>  bool check_for_pattern(FILE *fp, const char *pattern, char *buf, size_t len);
> +char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len);
>  uint64_t read_pmd_pagesize(void);
>  unsigned long rss_anon(void);
>  bool check_huge_anon(void *addr, int nr_hpages, uint64_t hpage_size);
> --
> 2.43.0
>

--
Cheers, Lorenzo