Re: [PATCH v5 4/4] selftests/mm: regression test for panic_on_unrecoverable_memory_failure

Miaohe Lin
2026-04-28T02:22:17Z

On 2026/4/24 20:24, Breno Leitao wrote:
> Add a test that enables vm.panic_on_unrecoverable_memory_failure and
> injects MADV_HWPOISON on a userspace anonymous page. The page must
> still be recovered via SIGBUS — it must not trigger a kernel panic.
>
> This is the regression test for the panic_on_unrecoverable_mf()
> recheck: a buddy page being concurrently allocated to userspace can
> briefly land on the MF_MSG_KERNEL_HIGH_ORDER branch (refcount 0, not in
> buddy), and without the recheck the kernel would panic on what is
> actually a recoverable userspace page.
>
> Run in a forked child so the SIGBUS path is fully exercised; if the
> kernel ever regresses and panics, the host VM dies and the harness
> reports the binary as never returning, which is itself a clear
> failure signal.
>
> Skips when the sysctl is not present (feature not built in) or when
> the test cannot write to it (insufficient privilege). Saves and
> restores the original sysctl value.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>

Thanks for adding a selftest. Some comments below.

> ---
> tools/testing/selftests/mm/memory-failure.c | 84 +++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/memory-failure.c b/tools/testing/selftests/mm/memory-failure.c
> index 032ed952057c6..9cb8d694aee94 100644
> --- a/tools/testing/selftests/mm/memory-failure.c
> +++ b/tools/testing/selftests/mm/memory-failure.c
> @@ -17,9 +17,13 @@
> #include <sys/vfs.h>
> #include <linux/magic.h>
> #include <errno.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
>
> #include "vm_util.h"
>
> +#define PANIC_SYSCTL "/proc/sys/vm/panic_on_unrecoverable_memory_failure"
> +
> enum inject_type {
> MADV_HARD,
> MADV_SOFT,
> @@ -355,4 +359,84 @@ TEST_F(memory_failure, dirty_pagecache)
> ASSERT_EQ(close(fd), 0);
> }
>
> +static int read_sysctl_int(const char *path, int *out)
> +{
> + char buf[16];
> + int fd, n;
> +
> + fd = open(path, O_RDONLY);
> + if (fd < 0)
> + return -1;
> + n = read(fd, buf, sizeof(buf) - 1);
> + close(fd);
> + if (n <= 0)
> + return -1;
> + buf[n] = '\0';
> + *out = atoi(buf);
> + return 0;
> +}
> +
> +static int write_sysctl_int(const char *path, int val)
> +{
> + char buf[16];
> + int fd, len, ret = 0;
> +
> + fd = open(path, O_WRONLY);
> + if (fd < 0)
> + return -1;
> + len = snprintf(buf, sizeof(buf), "%d\n", val);
> + if (write(fd, buf, len) != len)
> + ret = -1;
> + close(fd);
> + return ret;
> +}

There are write_sysfs and read_sysfs in vm_util.c. Can we reuse those?

> +
> +/*
> + * Regression test for vm.panic_on_unrecoverable_memory_failure.
> + *
> + * With the sysctl on, hwpoison injection on a userspace anonymous page
> + * must still be recovered via SIGBUS — it must not trigger a kernel
> + * panic. This guards the panic_on_unrecoverable_mf() recheck that rules
> + * out concurrent buddy allocations being misclassified as unrecoverable
> + * kernel pages (MF_MSG_KERNEL_HIGH_ORDER).
> + *
> + * If the kernel regresses and panics, the host VM dies and the test
> + * harness will report the binary as never having returned — which is
> + * itself a clear failure signal.
> + */
> +TEST(panic_on_unrecoverable_user_page)
> +{
> + unsigned long page_size;
> + int saved, status;
> + void *addr;
> + pid_t pid;
> +
> + if (read_sysctl_int(PANIC_SYSCTL, &saved))
> + SKIP(return, "%s not available\n", PANIC_SYSCTL);
> + if (write_sysctl_int(PANIC_SYSCTL, 1))
> + SKIP(return, "cannot enable %s (need root?)\n", PANIC_SYSCTL);
> +
> + page_size = sysconf(_SC_PAGESIZE);
> +
> + pid = fork();
> + ASSERT_NE(pid, -1);
> + if (pid == 0) {
> + addr = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> + if (addr == MAP_FAILED)
> + _exit(1);
> + *(volatile char *)addr = 1;
> + if (madvise(addr, page_size, MADV_HWPOISON))
> + _exit(2);
> + FORCE_READ(*(volatile char *)addr);
> + _exit(0); /* unreachable: SIGBUS expected */
> + }
> +
> + ASSERT_EQ(waitpid(pid, &status, 0), pid);
> + write_sysctl_int(PANIC_SYSCTL, saved);
> +
> + ASSERT_TRUE(WIFSIGNALED(status));
> + ASSERT_EQ(WTERMSIG(status), SIGBUS);
> +}

Could you restructure this test using the similar format as other functions, e.g. TEST_F(memory_failure, anon), in
this file? It would be good to keep them in same style.

Thanks.
.