* [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
@ 2026-08-21 12:12 Samir Mulani
2026-08-21 18:39 ` [LTP] " linuxtestproject.agent
2026-08-31 13:34 ` [LTP] [PATCH v7] " Cyril Hrubis
0 siblings, 2 replies; 6+ messages in thread
From: Samir Mulani @ 2026-08-21 12:12 UTC (permalink / raw)
To: ltp; +Cc: Samir Mulani
Migrate the alloc-instantiate-race.c test from libhugetlbfs [1] to LTP
as hugemmap42.
This test is designed to detect a kernel allocation race introduced
with hugepage demand-faulting. The problem is that no lock is held
between allocating a hugepage and instantiating it in the
pagetables or page cache index. In between the two, the (huge)
page is cleared, so there's substantial time. Thus two processes
can race instantiating the (same) last available hugepage - one
will fail on the allocation, and thus cause an OOM fault even
though the page it actually wants is being instantiated by the
other racing process.
[1] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/alloc-instantiate-race.c
Signed-off-by: Samir Mulani <samir@linux.ibm.com>
---
v7:
- Fix commit message: s/hugemmap36/hugemmap42/
- Add hugemmap42_private runtest entry to exercise the MAP_PRIVATE
pthread path (-m private)
- Pass 0 instead of getpid() to sched_setaffinity() so each racer
thread pins itself, not the thread-group leader
- Fault in all p_sync pages before run_race() so hugepages are
actually consumed from the pool before the race starts
- Save sync_mapping_size at mmap() time and reuse it in cleanup()
instead of re-reading MEMINFO_HPAGE_FREE
- Use {} instead of {NULL, NULL, NULL} as options array sentinel
Link: https://lore.kernel.org/all/20250928030721.3537869-1-samir@linux.ibm.com/ #v3
Link: https://lore.kernel.org/ltp/20260317095559.5766-1-samir@linux.ibm.com/ #v4
Link: https://lore.kernel.org/ltp/20260504132405.333588-1-samir@linux.ibm.com/ #v5
Link: https://lore.kernel.org/ltp/20260818143106.43797-1-samir@linux.ibm.com/ #v6
runtest/hugetlb | 2 +
testcases/kernel/mem/.gitignore | 1 +
.../kernel/mem/hugetlb/hugemmap/hugemmap42.c | 317 ++++++++++++++++++
3 files changed, 320 insertions(+)
create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
diff --git a/runtest/hugetlb b/runtest/hugetlb
index 6b35c1f42..b7872368d 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -40,6 +40,8 @@ hugemmap35 hugemmap35
hugemmap36 hugemmap36
hugemmap37 hugemmap37
hugemmap38 hugemmap38
+hugemmap42 hugemmap42
+hugemmap42_private hugemmap42 -m private
hugemmap05_1 hugemmap05 -m
hugemmap05_2 hugemmap05 -s
hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index e63a6dde7..a0a877a58 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -40,6 +40,7 @@
/hugetlb/hugemmap/hugemmap36
/hugetlb/hugemmap/hugemmap37
/hugetlb/hugemmap/hugemmap38
+/hugetlb/hugemmap/hugemmap42
/hugetlb/hugeshmat/hugeshmat01
/hugetlb/hugeshmat/hugeshmat02
/hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
new file mode 100644
index 000000000..901ea07ab
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2005-2006, 2026 IBM Corporation
+ * Author: David Gibson & Adam Litke
+ */
+
+/*\
+ * This test is designed to detect a kernel allocation race introduced
+ * with hugepage demand-faulting. The problem is that no lock is held
+ * between allocating a hugepage and instantiating it in the
+ * pagetables or page cache index. In between the two, the (huge)
+ * page is cleared, so there's substantial time. Thus two processes
+ * can race instantiating the (same) last available hugepage - one
+ * will fail on the allocation, and thus cause an OOM fault even
+ * though the page it actually wants is being instantiated by the
+ * other racing process.
+ */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include "tst_test.h"
+#include "tst_safe_pthread.h"
+#include "hugetlb.h"
+
+#define MNTPOINT "hugetlbfs/"
+
+static char *str_op;
+static int child1, child2, race_type;
+static int fd_sync = -1;
+static int fd_race = -1;
+static void *p_race = MAP_FAILED;
+static void *p_sync = MAP_FAILED;
+static size_t sync_mapping_size;
+
+struct racer_info {
+ void *p;
+ int cpu;
+ int status;
+};
+
+static int one_racer(void *p, int cpu)
+{
+ volatile int *pi = p;
+ cpu_set_t *cpuset;
+ size_t mask_size;
+ int err;
+
+ cpuset = CPU_ALLOC(cpu + 1);
+ if (!cpuset)
+ tst_brk(TBROK | TERRNO, "CPU_ALLOC() failed");
+
+ mask_size = CPU_ALLOC_SIZE(cpu + 1);
+
+ /* Split onto different CPUs to encourage the race */
+ CPU_ZERO_S(mask_size, cpuset);
+ CPU_SET_S(cpu, mask_size, cpuset);
+
+ /*
+ * Use pid 0 so that sched_setaffinity() applies to the calling
+ * thread rather than the thread-group leader. getpid() returns
+ * the same TGID for every pthread in the process, so passing it
+ * would pin the main thread instead of the racer.
+ */
+ err = sched_setaffinity(0, mask_size, cpuset);
+ CPU_FREE(cpuset);
+ if (err == -1)
+ tst_brk(TBROK | TERRNO, "sched_setaffinity() failed");
+
+ /* Wait for parent to signal both racers to start */
+ TST_CHECKPOINT_WAIT(0);
+
+ /* Fault in the hugepage - triggers the race */
+ *pi = 1;
+
+ return 0;
+}
+
+static void proc_racer(void *p, int cpu)
+{
+ exit(one_racer(p, cpu));
+}
+
+static void *thread_racer(void *info)
+{
+ struct racer_info *ri = info;
+
+ ri->status = one_racer(ri->p, ri->cpu);
+ return ri;
+}
+
+static void check_online_cpus(int online_cpus[], int nr_cpus_needed)
+{
+ cpu_set_t cpuset;
+ int total_cpus, cpu_idx;
+
+ CPU_ZERO(&cpuset);
+
+ total_cpus = get_nprocs_conf();
+
+ if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuset) == -1)
+ tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");
+
+ tst_res(TINFO, "Online CPUs needed: %d, available: %d",
+ nr_cpus_needed, CPU_COUNT(&cpuset));
+
+ if (CPU_COUNT(&cpuset) < nr_cpus_needed)
+ tst_brk(TCONF, "At least %d online CPUs are required",
+ nr_cpus_needed);
+
+ cpu_idx = 0;
+ for (int i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
+ if (CPU_ISSET(i, &cpuset))
+ online_cpus[cpu_idx++] = i;
+ }
+
+ if (cpu_idx < nr_cpus_needed)
+ tst_brk(TBROK, "Unable to find enough online CPUs");
+}
+
+static void run_race(int rtype)
+{
+ void *tret1, *tret2;
+ int status1 = 0, status2 = 0;
+ int online_cpus[2];
+ long hpage_size;
+ pthread_t thread1, thread2;
+
+ check_online_cpus(online_cpus, 2);
+
+ hpage_size = tst_get_hugepage_size();
+
+ /* Get a new file for the final page */
+ fd_race = tst_creat_unlinked(MNTPOINT, 0, 0600);
+ tst_res(TINFO, "Mapping final page..");
+
+ p_race = SAFE_MMAP(NULL, hpage_size, PROT_READ | PROT_WRITE,
+ rtype, fd_race, 0);
+
+ if (rtype == MAP_SHARED) {
+ child1 = SAFE_FORK();
+ if (child1 == 0)
+ proc_racer(p_race, online_cpus[0]);
+
+ child2 = SAFE_FORK();
+ if (child2 == 0)
+ proc_racer(p_race, online_cpus[1]);
+
+ /* Wake both children to start the race simultaneously */
+ TST_CHECKPOINT_WAKE2(0, 2);
+
+ SAFE_WAITPID(child1, &status1, 0);
+ child1 = 0;
+ tst_res(TINFO, "Child 1 status: %x", status1);
+
+ SAFE_WAITPID(child2, &status2, 0);
+ child2 = 0;
+ tst_res(TINFO, "Child 2 status: %x", status2);
+
+ if (WIFSIGNALED(status1))
+ tst_res(TFAIL, "Child 1 killed by signal %s",
+ strsignal(WTERMSIG(status1)));
+ if (WIFSIGNALED(status2))
+ tst_res(TFAIL, "Child 2 killed by signal %s",
+ strsignal(WTERMSIG(status2)));
+ } else {
+ struct racer_info ri1 = {
+ .p = p_race,
+ .cpu = online_cpus[0],
+ .status = -1,
+ };
+ struct racer_info ri2 = {
+ .p = p_race,
+ .cpu = online_cpus[1],
+ .status = -1,
+ };
+
+ SAFE_PTHREAD_CREATE(&thread1, NULL, thread_racer, &ri1);
+ SAFE_PTHREAD_CREATE(&thread2, NULL, thread_racer, &ri2);
+
+ /* Wake both threads to start the race simultaneously */
+ TST_CHECKPOINT_WAKE2(0, 2);
+
+ SAFE_PTHREAD_JOIN(thread1, &tret1);
+ if (tret1 != &ri1)
+ tst_res(TFAIL, "Thread 1 returned %p not %p, killed?",
+ tret1, &ri1);
+
+ SAFE_PTHREAD_JOIN(thread2, &tret2);
+ if (tret2 != &ri2)
+ tst_res(TFAIL, "Thread 2 returned %p not %p, killed?",
+ tret2, &ri2);
+
+ status1 = ri1.status;
+ status2 = ri2.status;
+ }
+
+ if (status1 != 0)
+ tst_res(TFAIL, "Racer 1 terminated with code %d", status1);
+
+ if (status2 != 0)
+ tst_res(TFAIL, "Racer 2 terminated with code %d", status2);
+
+ if (status1 == 0 && status2 == 0)
+ tst_res(TPASS, "Test completed successfully");
+
+ SAFE_MUNMAP(p_race, hpage_size);
+ p_race = MAP_FAILED;
+
+ SAFE_CLOSE(fd_race);
+ fd_race = -1;
+}
+
+static void run_test(void)
+{
+ unsigned long totpages;
+ long hpage_size;
+
+ totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+ if (totpages < 2)
+ tst_brk(TCONF, "Not enough free hugepages, need at least 2");
+
+ hpage_size = tst_get_hugepage_size();
+
+ tst_res(TINFO, "Instantiating..");
+
+ fd_sync = tst_creat_unlinked(MNTPOINT, 0, 0600);
+
+ tst_res(TINFO, "Mapping %lu/%lu pages..", totpages - 1, totpages);
+ sync_mapping_size = (totpages - 1) * hpage_size;
+ p_sync = SAFE_MMAP(NULL, sync_mapping_size,
+ PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
+
+ /*
+ * Fault in every sync page to consume them from the hugepage pool.
+ * Without touching each page, mmap() does not allocate hugepages
+ * (no MAP_POPULATE), so all free hugepages remain available to the
+ * racers and the allocation race is never exercised.
+ */
+ for (unsigned long i = 0; i < totpages - 1; i++) {
+ volatile char *cp = (volatile char *)p_sync + i * hpage_size;
+
+ *cp = 0;
+ }
+
+ run_race(race_type);
+
+ SAFE_MUNMAP(p_sync, sync_mapping_size);
+ p_sync = MAP_FAILED;
+
+ SAFE_CLOSE(fd_sync);
+ fd_sync = -1;
+}
+
+static void setup(void)
+{
+ if (str_op) {
+ if (strcmp(str_op, "shared") == 0)
+ race_type = MAP_SHARED;
+ else if (strcmp(str_op, "private") == 0)
+ race_type = MAP_PRIVATE;
+ else
+ tst_brk(TBROK,
+ "Invalid parameter: use -m <private|shared>");
+ } else {
+ /* Default to shared if no option is passed */
+ race_type = MAP_SHARED;
+ }
+}
+
+static void cleanup(void)
+{
+ if (p_race != MAP_FAILED)
+ SAFE_MUNMAP(p_race, tst_get_hugepage_size());
+
+ if (fd_race >= 0)
+ SAFE_CLOSE(fd_race);
+
+ /*
+ * Use the saved sync_mapping_size rather than re-reading
+ * MEMINFO_HPAGE_FREE: the current free count differs from the
+ * mapping length and could cause munmap() to remove an adjacent
+ * mapping or leave part of p_sync mapped.
+ */
+ if (p_sync != MAP_FAILED)
+ SAFE_MUNMAP(p_sync, sync_mapping_size);
+
+ if (fd_sync >= 0)
+ SAFE_CLOSE(fd_sync);
+
+ if (child1 > 0) {
+ if (kill(child1, 0) == 0)
+ SAFE_KILL(child1, SIGKILL);
+ }
+
+ if (child2 > 0) {
+ if (kill(child2, 0) == 0)
+ SAFE_KILL(child2, SIGKILL);
+ }
+}
+
+static struct tst_test test = {
+ .options = (struct tst_option[]) {
+ {"m:", &str_op, "Type of mmap() mapping <private|shared>"},
+ {}
+ },
+ .needs_root = 1,
+ .mntpoint = MNTPOINT,
+ .needs_hugetlbfs = 1,
+ .needs_tmpdir = 1,
+ .setup = setup,
+ .cleanup = cleanup,
+ .test_all = run_test,
+ .hugepages = {2, TST_NEEDS},
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .min_cpus = 2,
+};
--
2.52.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
2026-08-21 12:12 [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs Samir Mulani
@ 2026-08-21 18:39 ` linuxtestproject.agent
2026-08-31 13:34 ` [LTP] [PATCH v7] " Cyril Hrubis
1 sibling, 0 replies; 6+ messages in thread
From: linuxtestproject.agent @ 2026-08-21 18:39 UTC (permalink / raw)
To: Samir Mulani; +Cc: ltp
Hi Samir,
On Aug 21, 2026, Samir Mulani wrote:
> hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
> + SAFE_PTHREAD_CREATE(&thread1, NULL, thread_racer, &ri1);
> + SAFE_PTHREAD_CREATE(&thread2, NULL, thread_racer, &ri2);
> +
> + /* Wake both threads to start the race simultaneously */
> + TST_CHECKPOINT_WAKE2(0, 2);
Medium: Could the MAP_PRIVATE pthread race run in a child process so that
the parent can translate SIGBUS into TFAIL?
The allocation failure under test makes hugetlb_no_page() return
VM_FAULT_SIGBUS. An unhandled SIGBUS in either thread terminates the whole
thread group, after which LTP's check_child_status() reports TBROK instead
of identifying the kernel regression as TFAIL. The MAP_SHARED variant
already translates signal deaths in its child racers.
> + cpu_set_t cpuset;
> + int total_cpus, cpu_idx;
> +
> + CPU_ZERO(&cpuset);
> +
> + total_cpus = get_nprocs_conf();
> +
> + if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuset) == -1)
> + tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");
Medium: Could this use a dynamic mask sized with tst_ncpus_max(),
CPU_ALLOC(), and CPU_ALLOC_SIZE()?
sched_getaffinity() returns EINVAL when the supplied buffer is smaller
than the kernel affinity mask. A kernel supporting more than CPU_SETSIZE
CPUs therefore produces TBROK even when the test has two usable CPUs.
> +#include <pthread.h>
> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
Medium: Could hugemmap42 be added to the Makefile's existing
CFLAGS += -pthread rule?
The test links on modern glibc because libpthread was merged into libc,
but older supported libc versions need -pthread to resolve the symbols
used by the safe pthread wrappers.
> +/hugetlb/hugemmap/hugemmap42
Low: Could this entry be moved to a leaf
testcases/kernel/mem/hugetlb/hugemmap/.gitignore?
New test binaries should be ignored in their own test directory rather
than in testcases/kernel/mem/.gitignore.
> + .hugepages = {2, TST_NEEDS},
> + .forks_child = 1,
> + .needs_checkpoints = 1,
> + .min_cpus = 2,
> +};
Low: Could the kernel fix for this regression be added as a linux-git
tag so that failures provide the missing-fix hint?
> + * other racing process.
> + */
> [...]
> + .needs_root = 1,
Low: Could the exported description explain that root is needed to
reserve hugepages and mount hugetlbfs?
> + SAFE_CLOSE(fd_race);
> + fd_race = -1;
> [...]
> + if (fd_race >= 0)
> + SAFE_CLOSE(fd_race);
Low: Could the redundant assignment be removed and the cleanup guard use
fd_race != -1, with the same changes for fd_sync?
SAFE_CLOSE() already sets the descriptor to -1, and != -1 is the LTP
descriptor-validity convention.
Verdict - Needs revision
---
Note:
The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.
Regards,
LTP AI Reviewer
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
2026-08-21 12:12 [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs Samir Mulani
2026-08-21 18:39 ` [LTP] " linuxtestproject.agent
@ 2026-08-31 13:34 ` Cyril Hrubis
2026-09-10 19:13 ` samir
1 sibling, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2026-08-31 13:34 UTC (permalink / raw)
To: Samir Mulani; +Cc: ltp
Hi!
> This test is designed to detect a kernel allocation race introduced
> with hugepage demand-faulting. The problem is that no lock is held
> between allocating a hugepage and instantiating it in the
> pagetables or page cache index. In between the two, the (huge)
> page is cleared, so there's substantial time. Thus two processes
> can race instantiating the (same) last available hugepage - one
> will fail on the allocation, and thus cause an OOM fault even
> though the page it actually wants is being instantiated by the
> other racing process.
>
> [1] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/alloc-instantiate-race.c
>
> Signed-off-by: Samir Mulani <samir@linux.ibm.com>
> ---
> v7:
> - Fix commit message: s/hugemmap36/hugemmap42/
> - Add hugemmap42_private runtest entry to exercise the MAP_PRIVATE
> pthread path (-m private)
> - Pass 0 instead of getpid() to sched_setaffinity() so each racer
> thread pins itself, not the thread-group leader
> - Fault in all p_sync pages before run_race() so hugepages are
> actually consumed from the pool before the race starts
> - Save sync_mapping_size at mmap() time and reuse it in cleanup()
> instead of re-reading MEMINFO_HPAGE_FREE
> - Use {} instead of {NULL, NULL, NULL} as options array sentinel
>
> Link: https://lore.kernel.org/all/20250928030721.3537869-1-samir@linux.ibm.com/ #v3
> Link: https://lore.kernel.org/ltp/20260317095559.5766-1-samir@linux.ibm.com/ #v4
> Link: https://lore.kernel.org/ltp/20260504132405.333588-1-samir@linux.ibm.com/ #v5
> Link: https://lore.kernel.org/ltp/20260818143106.43797-1-samir@linux.ibm.com/ #v6
>
> runtest/hugetlb | 2 +
> testcases/kernel/mem/.gitignore | 1 +
> .../kernel/mem/hugetlb/hugemmap/hugemmap42.c | 317 ++++++++++++++++++
> 3 files changed, 320 insertions(+)
> create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
>
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 6b35c1f42..b7872368d 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -40,6 +40,8 @@ hugemmap35 hugemmap35
> hugemmap36 hugemmap36
> hugemmap37 hugemmap37
> hugemmap38 hugemmap38
> +hugemmap42 hugemmap42
> +hugemmap42_private hugemmap42 -m private
This is ugly. The modern way how to run two subtests in LTP is to set
.tcnt = 2 and switch on the index in the run() function.
> hugemmap05_1 hugemmap05 -m
> hugemmap05_2 hugemmap05 -s
> hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index e63a6dde7..a0a877a58 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -40,6 +40,7 @@
> /hugetlb/hugemmap/hugemmap36
> /hugetlb/hugemmap/hugemmap37
> /hugetlb/hugemmap/hugemmap38
> +/hugetlb/hugemmap/hugemmap42
> /hugetlb/hugeshmat/hugeshmat01
> /hugetlb/hugeshmat/hugeshmat02
> /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
> new file mode 100644
> index 000000000..901ea07ab
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2005-2006, 2026 IBM Corporation
> + * Author: David Gibson & Adam Litke
> + */
> +
> +/*\
> + * This test is designed to detect a kernel allocation race introduced
> + * with hugepage demand-faulting. The problem is that no lock is held
> + * between allocating a hugepage and instantiating it in the
> + * pagetables or page cache index. In between the two, the (huge)
> + * page is cleared, so there's substantial time. Thus two processes
> + * can race instantiating the (same) last available hugepage - one
> + * will fail on the allocation, and thus cause an OOM fault even
> + * though the page it actually wants is being instantiated by the
> + * other racing process.
> + */
That sounds like a regression test, do we have a kernel commit for the
fix for the race?
> +#define _GNU_SOURCE
> +#include <pthread.h>
> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
> +#include "hugetlb.h"
> +
> +#define MNTPOINT "hugetlbfs/"
> +
> +static char *str_op;
> +static int child1, child2, race_type;
> +static int fd_sync = -1;
> +static int fd_race = -1;
> +static void *p_race = MAP_FAILED;
> +static void *p_sync = MAP_FAILED;
> +static size_t sync_mapping_size;
> +
> +struct racer_info {
> + void *p;
> + int cpu;
> + int status;
> +};
> +
> +static int one_racer(void *p, int cpu)
> +{
> + volatile int *pi = p;
> + cpu_set_t *cpuset;
> + size_t mask_size;
> + int err;
> +
> + cpuset = CPU_ALLOC(cpu + 1);
> + if (!cpuset)
> + tst_brk(TBROK | TERRNO, "CPU_ALLOC() failed");
> +
> + mask_size = CPU_ALLOC_SIZE(cpu + 1);
> +
> + /* Split onto different CPUs to encourage the race */
> + CPU_ZERO_S(mask_size, cpuset);
> + CPU_SET_S(cpu, mask_size, cpuset);
> +
> + /*
> + * Use pid 0 so that sched_setaffinity() applies to the calling
> + * thread rather than the thread-group leader. getpid() returns
> + * the same TGID for every pthread in the process, so passing it
> + * would pin the main thread instead of the racer.
> + */
> + err = sched_setaffinity(0, mask_size, cpuset);
> + CPU_FREE(cpuset);
> + if (err == -1)
> + tst_brk(TBROK | TERRNO, "sched_setaffinity() failed");
> +
> + /* Wait for parent to signal both racers to start */
> + TST_CHECKPOINT_WAIT(0);
> +
> + /* Fault in the hugepage - triggers the race */
> + *pi = 1;
If this is the actual race, it would probably make sense to actually
unmap/map and fault the page in a loop. We do have a tst_fuzzy_sync.h
that has a machinery for synchronizing two processes to hit a race.
Maybe it would make sense to use it.
> + return 0;
> +}
> +
> +static void proc_racer(void *p, int cpu)
> +{
> + exit(one_racer(p, cpu));
> +}
> +
> +static void *thread_racer(void *info)
> +{
> + struct racer_info *ri = info;
> +
> + ri->status = one_racer(ri->p, ri->cpu);
> + return ri;
> +}
> +
> +static void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> +{
> + cpu_set_t cpuset;
> + int total_cpus, cpu_idx;
> +
> + CPU_ZERO(&cpuset);
> +
> + total_cpus = get_nprocs_conf();
> +
> + if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuset) == -1)
> + tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");
> +
> + tst_res(TINFO, "Online CPUs needed: %d, available: %d",
> + nr_cpus_needed, CPU_COUNT(&cpuset));
> +
> + if (CPU_COUNT(&cpuset) < nr_cpus_needed)
> + tst_brk(TCONF, "At least %d online CPUs are required",
> + nr_cpus_needed);
> +
> + cpu_idx = 0;
> + for (int i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
> + if (CPU_ISSET(i, &cpuset))
> + online_cpus[cpu_idx++] = i;
> + }
> +
> + if (cpu_idx < nr_cpus_needed)
> + tst_brk(TBROK, "Unable to find enough online CPUs");
> +}
> +
> +static void run_race(int rtype)
> +{
> + void *tret1, *tret2;
> + int status1 = 0, status2 = 0;
> + int online_cpus[2];
> + long hpage_size;
> + pthread_t thread1, thread2;
> +
> + check_online_cpus(online_cpus, 2);
> +
> + hpage_size = tst_get_hugepage_size();
> +
> + /* Get a new file for the final page */
> + fd_race = tst_creat_unlinked(MNTPOINT, 0, 0600);
> + tst_res(TINFO, "Mapping final page..");
> +
> + p_race = SAFE_MMAP(NULL, hpage_size, PROT_READ | PROT_WRITE,
> + rtype, fd_race, 0);
> +
> + if (rtype == MAP_SHARED) {
> + child1 = SAFE_FORK();
> + if (child1 == 0)
> + proc_racer(p_race, online_cpus[0]);
> +
> + child2 = SAFE_FORK();
> + if (child2 == 0)
> + proc_racer(p_race, online_cpus[1]);
> +
> + /* Wake both children to start the race simultaneously */
> + TST_CHECKPOINT_WAKE2(0, 2);
> +
> + SAFE_WAITPID(child1, &status1, 0);
> + child1 = 0;
> + tst_res(TINFO, "Child 1 status: %x", status1);
> +
> + SAFE_WAITPID(child2, &status2, 0);
> + child2 = 0;
> + tst_res(TINFO, "Child 2 status: %x", status2);
> +
> + if (WIFSIGNALED(status1))
> + tst_res(TFAIL, "Child 1 killed by signal %s",
> + strsignal(WTERMSIG(status1)));
> + if (WIFSIGNALED(status2))
> + tst_res(TFAIL, "Child 2 killed by signal %s",
> + strsignal(WTERMSIG(status2)));
> + } else {
> + struct racer_info ri1 = {
> + .p = p_race,
> + .cpu = online_cpus[0],
> + .status = -1,
> + };
> + struct racer_info ri2 = {
> + .p = p_race,
> + .cpu = online_cpus[1],
> + .status = -1,
> + };
> +
> + SAFE_PTHREAD_CREATE(&thread1, NULL, thread_racer, &ri1);
> + SAFE_PTHREAD_CREATE(&thread2, NULL, thread_racer, &ri2);
> +
> + /* Wake both threads to start the race simultaneously */
> + TST_CHECKPOINT_WAKE2(0, 2);
> +
> + SAFE_PTHREAD_JOIN(thread1, &tret1);
> + if (tret1 != &ri1)
> + tst_res(TFAIL, "Thread 1 returned %p not %p, killed?",
> + tret1, &ri1);
> +
> + SAFE_PTHREAD_JOIN(thread2, &tret2);
> + if (tret2 != &ri2)
> + tst_res(TFAIL, "Thread 2 returned %p not %p, killed?",
> + tret2, &ri2);
> +
> + status1 = ri1.status;
> + status2 = ri2.status;
Here the ri1.status and ri2.status is completely bogus, since that
either stays initialize to -1 or is set to 0 by clean exit of the
functions. Since the description says that the bug triggers OOM all
threads of the process would be killed if we reproduce the bug and we
will never get to this place anyways.
I suppose that you need to actually fork and then run the two threads in
the forked process in order to isolate it from the main test process.
Also the two different test modes does not have nearly any code in
common, so these two should be in two different functions.
> + }
> +
> + if (status1 != 0)
> + tst_res(TFAIL, "Racer 1 terminated with code %d", status1);
> +
> + if (status2 != 0)
> + tst_res(TFAIL, "Racer 2 terminated with code %d", status2);
> +
> + if (status1 == 0 && status2 == 0)
> + tst_res(TPASS, "Test completed successfully");
> +
> + SAFE_MUNMAP(p_race, hpage_size);
> + p_race = MAP_FAILED;
> +
> + SAFE_CLOSE(fd_race);
> + fd_race = -1;
> +}
> +
> +static void run_test(void)
> +{
> + unsigned long totpages;
> + long hpage_size;
> +
> + totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
> + if (totpages < 2)
> + tst_brk(TCONF, "Not enough free hugepages, need at least 2");
This is done by the .hugepages = {2, TST_NEEDS} in the tst_test
structure.
> + hpage_size = tst_get_hugepage_size();
> +
> + tst_res(TINFO, "Instantiating..");
> +
> + fd_sync = tst_creat_unlinked(MNTPOINT, 0, 0600);
> +
> + tst_res(TINFO, "Mapping %lu/%lu pages..", totpages - 1, totpages);
> + sync_mapping_size = (totpages - 1) * hpage_size;
> + p_sync = SAFE_MMAP(NULL, sync_mapping_size,
> + PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
> +
> + /*
> + * Fault in every sync page to consume them from the hugepage pool.
> + * Without touching each page, mmap() does not allocate hugepages
> + * (no MAP_POPULATE), so all free hugepages remain available to the
> + * racers and the allocation race is never exercised.
> + */
> + for (unsigned long i = 0; i < totpages - 1; i++) {
> + volatile char *cp = (volatile char *)p_sync + i * hpage_size;
> +
> + *cp = 0;
> + }
> +
> + run_race(race_type);
> +
> + SAFE_MUNMAP(p_sync, sync_mapping_size);
> + p_sync = MAP_FAILED;
> +
> + SAFE_CLOSE(fd_sync);
> + fd_sync = -1;
The fd is set to -1 by the SAFE_CLOSE()
> +}
> +
> +static void setup(void)
> +{
> + if (str_op) {
> + if (strcmp(str_op, "shared") == 0)
> + race_type = MAP_SHARED;
> + else if (strcmp(str_op, "private") == 0)
> + race_type = MAP_PRIVATE;
> + else
> + tst_brk(TBROK,
> + "Invalid parameter: use -m <private|shared>");
> + } else {
> + /* Default to shared if no option is passed */
> + race_type = MAP_SHARED;
> + }
> +}
> +
> +static void cleanup(void)
> +{
> + if (p_race != MAP_FAILED)
> + SAFE_MUNMAP(p_race, tst_get_hugepage_size());
> +
> + if (fd_race >= 0)
fd_race != -1
> + SAFE_CLOSE(fd_race);
> +
> + /*
> + * Use the saved sync_mapping_size rather than re-reading
> + * MEMINFO_HPAGE_FREE: the current free count differs from the
> + * mapping length and could cause munmap() to remove an adjacent
> + * mapping or leave part of p_sync mapped.
> + */
> + if (p_sync != MAP_FAILED)
> + SAFE_MUNMAP(p_sync, sync_mapping_size);
> +
> + if (fd_sync >= 0)
> + SAFE_CLOSE(fd_sync);
> +
> + if (child1 > 0) {
> + if (kill(child1, 0) == 0)
> + SAFE_KILL(child1, SIGKILL);
> + }
> +
> + if (child2 > 0) {
> + if (kill(child2, 0) == 0)
> + SAFE_KILL(child2, SIGKILL);
> + }
> +}
> +
> +static struct tst_test test = {
> + .options = (struct tst_option[]) {
> + {"m:", &str_op, "Type of mmap() mapping <private|shared>"},
> + {}
> + },
> + .needs_root = 1,
> + .mntpoint = MNTPOINT,
> + .needs_hugetlbfs = 1,
> + .needs_tmpdir = 1,
> + .setup = setup,
> + .cleanup = cleanup,
> + .test_all = run_test,
> + .hugepages = {2, TST_NEEDS},
> + .forks_child = 1,
> + .needs_checkpoints = 1,
> + .min_cpus = 2,
> +};
> --
> 2.52.0
>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
2026-08-31 13:34 ` [LTP] [PATCH v7] " Cyril Hrubis
@ 2026-09-10 19:13 ` samir
0 siblings, 0 replies; 6+ messages in thread
From: samir @ 2026-09-10 19:13 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hello Cyril
On 31/08/26 7:04 pm, Cyril Hrubis wrote:
> Hi!
>> This test is designed to detect a kernel allocation race introduced
>> with hugepage demand-faulting. The problem is that no lock is held
>> between allocating a hugepage and instantiating it in the
>> pagetables or page cache index. In between the two, the (huge)
>> page is cleared, so there's substantial time. Thus two processes
>> can race instantiating the (same) last available hugepage - one
>> will fail on the allocation, and thus cause an OOM fault even
>> though the page it actually wants is being instantiated by the
>> other racing process.
>>
>> [1] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/alloc-instantiate-race.c
>>
>> Signed-off-by: Samir Mulani <samir@linux.ibm.com>
>> ---
>> v7:
>> - Fix commit message: s/hugemmap36/hugemmap42/
>> - Add hugemmap42_private runtest entry to exercise the MAP_PRIVATE
>> pthread path (-m private)
>> - Pass 0 instead of getpid() to sched_setaffinity() so each racer
>> thread pins itself, not the thread-group leader
>> - Fault in all p_sync pages before run_race() so hugepages are
>> actually consumed from the pool before the race starts
>> - Save sync_mapping_size at mmap() time and reuse it in cleanup()
>> instead of re-reading MEMINFO_HPAGE_FREE
>> - Use {} instead of {NULL, NULL, NULL} as options array sentinel
>>
>> Link: https://lore.kernel.org/all/20250928030721.3537869-1-samir@linux.ibm.com/ #v3
>> Link: https://lore.kernel.org/ltp/20260317095559.5766-1-samir@linux.ibm.com/ #v4
>> Link: https://lore.kernel.org/ltp/20260504132405.333588-1-samir@linux.ibm.com/ #v5
>> Link: https://lore.kernel.org/ltp/20260818143106.43797-1-samir@linux.ibm.com/ #v6
>>
>> runtest/hugetlb | 2 +
>> testcases/kernel/mem/.gitignore | 1 +
>> .../kernel/mem/hugetlb/hugemmap/hugemmap42.c | 317 ++++++++++++++++++
>> 3 files changed, 320 insertions(+)
>> create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
>>
>> diff --git a/runtest/hugetlb b/runtest/hugetlb
>> index 6b35c1f42..b7872368d 100644
>> --- a/runtest/hugetlb
>> +++ b/runtest/hugetlb
>> @@ -40,6 +40,8 @@ hugemmap35 hugemmap35
>> hugemmap36 hugemmap36
>> hugemmap37 hugemmap37
>> hugemmap38 hugemmap38
>> +hugemmap42 hugemmap42
>> +hugemmap42_private hugemmap42 -m private
> This is ugly. The modern way how to run two subtests in LTP is to set
> .tcnt = 2 and switch on the index in the run() function.
Will fix in the next version. Will remove the hugemmap42_private runtest
entry and the -m option, and use .tcnt = 2 with .test =
run_test(unsigned int n) switching on n instead.
>> hugemmap05_1 hugemmap05 -m
>> hugemmap05_2 hugemmap05 -s
>> hugemmap05_3 hugemmap05 -s -m
>> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
>> index e63a6dde7..a0a877a58 100644
>> --- a/testcases/kernel/mem/.gitignore
>> +++ b/testcases/kernel/mem/.gitignore
>> @@ -40,6 +40,7 @@
>> /hugetlb/hugemmap/hugemmap36
>> /hugetlb/hugemmap/hugemmap37
>> /hugetlb/hugemmap/hugemmap38
>> +/hugetlb/hugemmap/hugemmap42
>> /hugetlb/hugeshmat/hugeshmat01
>> /hugetlb/hugeshmat/hugeshmat02
>> /hugetlb/hugeshmat/hugeshmat03
>> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
>> new file mode 100644
>> index 000000000..901ea07ab
>> --- /dev/null
>> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
>> @@ -0,0 +1,317 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2005-2006, 2026 IBM Corporation
>> + * Author: David Gibson & Adam Litke
>> + */
>> +
>> +/*\
>> + * This test is designed to detect a kernel allocation race introduced
>> + * with hugepage demand-faulting. The problem is that no lock is held
>> + * between allocating a hugepage and instantiating it in the
>> + * pagetables or page cache index. In between the two, the (huge)
>> + * page is cleared, so there's substantial time. Thus two processes
>> + * can race instantiating the (same) last available hugepage - one
>> + * will fail on the allocation, and thus cause an OOM fault even
>> + * though the page it actually wants is being instantiated by the
>> + * other racing process.
>> + */
> That sounds like a regression test, do we have a kernel commit for the
> fix for the race?
Yes, the relevant commit is 3935baa9c9e6 ("hugepage: serialize hugepage
allocation and instantiation"). This commit introduced the
hugetlb_fault_mutex_table in mm/hugetlb.c which serializes hugepage
allocation and instantiation to prevent spurious OOMs when two CPUs race
to instantiate the same last available hugepage. The kernel comment at
the lock declaration confirms it: "prevent spurious OOMs when the
hugepage pool is fully utilized". Will add the linux-git tag in the next
version
>
>> +#define _GNU_SOURCE
>> +#include <pthread.h>
>> +#include "tst_test.h"
>> +#include "tst_safe_pthread.h"
>> +#include "hugetlb.h"
>> +
>> +#define MNTPOINT "hugetlbfs/"
>> +
>> +static char *str_op;
>> +static int child1, child2, race_type;
>> +static int fd_sync = -1;
>> +static int fd_race = -1;
>> +static void *p_race = MAP_FAILED;
>> +static void *p_sync = MAP_FAILED;
>> +static size_t sync_mapping_size;
>> +
>> +struct racer_info {
>> + void *p;
>> + int cpu;
>> + int status;
>> +};
>> +
>> +static int one_racer(void *p, int cpu)
>> +{
>> + volatile int *pi = p;
>> + cpu_set_t *cpuset;
>> + size_t mask_size;
>> + int err;
>> +
>> + cpuset = CPU_ALLOC(cpu + 1);
>> + if (!cpuset)
>> + tst_brk(TBROK | TERRNO, "CPU_ALLOC() failed");
>> +
>> + mask_size = CPU_ALLOC_SIZE(cpu + 1);
>> +
>> + /* Split onto different CPUs to encourage the race */
>> + CPU_ZERO_S(mask_size, cpuset);
>> + CPU_SET_S(cpu, mask_size, cpuset);
>> +
>> + /*
>> + * Use pid 0 so that sched_setaffinity() applies to the calling
>> + * thread rather than the thread-group leader. getpid() returns
>> + * the same TGID for every pthread in the process, so passing it
>> + * would pin the main thread instead of the racer.
>> + */
>> + err = sched_setaffinity(0, mask_size, cpuset);
>> + CPU_FREE(cpuset);
>> + if (err == -1)
>> + tst_brk(TBROK | TERRNO, "sched_setaffinity() failed");
>> +
>> + /* Wait for parent to signal both racers to start */
>> + TST_CHECKPOINT_WAIT(0);
>> +
>> + /* Fault in the hugepage - triggers the race */
>> + *pi = 1;
> If this is the actual race, it would probably make sense to actually
> unmap/map and fault the page in a loop. We do have a tst_fuzzy_sync.h
> that has a machinery for synchronizing two processes to hit a race.
> Maybe it would make sense to use it.
Will address in the next version. Will add tst_fuzzy_sync.h and wrap the
fault in a tst_fzsync_run_a/b() loop. After each iteration will unmap
p_race and truncate fd_race to zero to return the hugepage to the pool,
then remap for the next attempt.
>> + return 0;
>> +}
>> +
>> +static void proc_racer(void *p, int cpu)
>> +{
>> + exit(one_racer(p, cpu));
>> +}
>> +
>> +static void *thread_racer(void *info)
>> +{
>> + struct racer_info *ri = info;
>> +
>> + ri->status = one_racer(ri->p, ri->cpu);
>> + return ri;
>> +}
>> +
>> +static void check_online_cpus(int online_cpus[], int nr_cpus_needed)
>> +{
>> + cpu_set_t cpuset;
>> + int total_cpus, cpu_idx;
>> +
>> + CPU_ZERO(&cpuset);
>> +
>> + total_cpus = get_nprocs_conf();
>> +
>> + if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuset) == -1)
>> + tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");
>> +
>> + tst_res(TINFO, "Online CPUs needed: %d, available: %d",
>> + nr_cpus_needed, CPU_COUNT(&cpuset));
>> +
>> + if (CPU_COUNT(&cpuset) < nr_cpus_needed)
>> + tst_brk(TCONF, "At least %d online CPUs are required",
>> + nr_cpus_needed);
>> +
>> + cpu_idx = 0;
>> + for (int i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
>> + if (CPU_ISSET(i, &cpuset))
>> + online_cpus[cpu_idx++] = i;
>> + }
>> +
>> + if (cpu_idx < nr_cpus_needed)
>> + tst_brk(TBROK, "Unable to find enough online CPUs");
>> +}
>> +
>> +static void run_race(int rtype)
>> +{
>> + void *tret1, *tret2;
>> + int status1 = 0, status2 = 0;
>> + int online_cpus[2];
>> + long hpage_size;
>> + pthread_t thread1, thread2;
>> +
>> + check_online_cpus(online_cpus, 2);
>> +
>> + hpage_size = tst_get_hugepage_size();
>> +
>> + /* Get a new file for the final page */
>> + fd_race = tst_creat_unlinked(MNTPOINT, 0, 0600);
>> + tst_res(TINFO, "Mapping final page..");
>> +
>> + p_race = SAFE_MMAP(NULL, hpage_size, PROT_READ | PROT_WRITE,
>> + rtype, fd_race, 0);
>> +
>> + if (rtype == MAP_SHARED) {
>> + child1 = SAFE_FORK();
>> + if (child1 == 0)
>> + proc_racer(p_race, online_cpus[0]);
>> +
>> + child2 = SAFE_FORK();
>> + if (child2 == 0)
>> + proc_racer(p_race, online_cpus[1]);
>> +
>> + /* Wake both children to start the race simultaneously */
>> + TST_CHECKPOINT_WAKE2(0, 2);
>> +
>> + SAFE_WAITPID(child1, &status1, 0);
>> + child1 = 0;
>> + tst_res(TINFO, "Child 1 status: %x", status1);
>> +
>> + SAFE_WAITPID(child2, &status2, 0);
>> + child2 = 0;
>> + tst_res(TINFO, "Child 2 status: %x", status2);
>> +
>> + if (WIFSIGNALED(status1))
>> + tst_res(TFAIL, "Child 1 killed by signal %s",
>> + strsignal(WTERMSIG(status1)));
>> + if (WIFSIGNALED(status2))
>> + tst_res(TFAIL, "Child 2 killed by signal %s",
>> + strsignal(WTERMSIG(status2)));
>> + } else {
>> + struct racer_info ri1 = {
>> + .p = p_race,
>> + .cpu = online_cpus[0],
>> + .status = -1,
>> + };
>> + struct racer_info ri2 = {
>> + .p = p_race,
>> + .cpu = online_cpus[1],
>> + .status = -1,
>> + };
>> +
>> + SAFE_PTHREAD_CREATE(&thread1, NULL, thread_racer, &ri1);
>> + SAFE_PTHREAD_CREATE(&thread2, NULL, thread_racer, &ri2);
>> +
>> + /* Wake both threads to start the race simultaneously */
>> + TST_CHECKPOINT_WAKE2(0, 2);
>> +
>> + SAFE_PTHREAD_JOIN(thread1, &tret1);
>> + if (tret1 != &ri1)
>> + tst_res(TFAIL, "Thread 1 returned %p not %p, killed?",
>> + tret1, &ri1);
>> +
>> + SAFE_PTHREAD_JOIN(thread2, &tret2);
>> + if (tret2 != &ri2)
>> + tst_res(TFAIL, "Thread 2 returned %p not %p, killed?",
>> + tret2, &ri2);
>> +
>> + status1 = ri1.status;
>> + status2 = ri2.status;
> Here the ri1.status and ri2.status is completely bogus, since that
> either stays initialize to -1 or is set to 0 by clean exit of the
> functions. Since the description says that the bug triggers OOM all
> threads of the process would be killed if we reproduce the bug and we
> will never get to this place anyways.
>
> I suppose that you need to actually fork and then run the two threads in
> the forked process in order to isolate it from the main test process.
Will fix in the next version. Will remove struct racer_info and the
bogus status checks. For the MAP_PRIVATE path will fork a child first
and run both racing threads inside it, so an OOM kill only affects the
child. The parent will detect failure via WIFSIGNALED().
>
>
>
> Also the two different test modes does not have nearly any code in
> common, so these two should be in two different functions.
Will fix in the next version. Will split into two separate functions
run_shared_race() and run_private_race() and remove the combined
run_race(int rtype) with the if/else block
>> + }
>> +
>> + if (status1 != 0)
>> + tst_res(TFAIL, "Racer 1 terminated with code %d", status1);
>> +
>> + if (status2 != 0)
>> + tst_res(TFAIL, "Racer 2 terminated with code %d", status2);
>> +
>> + if (status1 == 0 && status2 == 0)
>> + tst_res(TPASS, "Test completed successfully");
>> +
>> + SAFE_MUNMAP(p_race, hpage_size);
>> + p_race = MAP_FAILED;
>> +
>> + SAFE_CLOSE(fd_race);
>> + fd_race = -1;
>> +}
>> +
>> +static void run_test(void)
>> +{
>> + unsigned long totpages;
>> + long hpage_size;
>> +
>> + totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
>> + if (totpages < 2)
>> + tst_brk(TCONF, "Not enough free hugepages, need at least 2");
> This is done by the .hugepages = {2, TST_NEEDS} in the tst_test
> structure.
Will keep the .hugepages = {2, TST_NEEDS} for the initial reservation
and remove the manual check from run_test() in the next version
>> + hpage_size = tst_get_hugepage_size();
>> +
>> + tst_res(TINFO, "Instantiating..");
>> +
>> + fd_sync = tst_creat_unlinked(MNTPOINT, 0, 0600);
>> +
>> + tst_res(TINFO, "Mapping %lu/%lu pages..", totpages - 1, totpages);
>> + sync_mapping_size = (totpages - 1) * hpage_size;
>> + p_sync = SAFE_MMAP(NULL, sync_mapping_size,
>> + PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
>> +
>> + /*
>> + * Fault in every sync page to consume them from the hugepage pool.
>> + * Without touching each page, mmap() does not allocate hugepages
>> + * (no MAP_POPULATE), so all free hugepages remain available to the
>> + * racers and the allocation race is never exercised.
>> + */
>> + for (unsigned long i = 0; i < totpages - 1; i++) {
>> + volatile char *cp = (volatile char *)p_sync + i * hpage_size;
>> +
>> + *cp = 0;
>> + }
>> +
>> + run_race(race_type);
>> +
>> + SAFE_MUNMAP(p_sync, sync_mapping_size);
>> + p_sync = MAP_FAILED;
>> +
>> + SAFE_CLOSE(fd_sync);
>> + fd_sync = -1;
> The fd is set to -1 by the SAFE_CLOSE()
>> +}
>> +
>> +static void setup(void)
>> +{
>> + if (str_op) {
>> + if (strcmp(str_op, "shared") == 0)
>> + race_type = MAP_SHARED;
>> + else if (strcmp(str_op, "private") == 0)
>> + race_type = MAP_PRIVATE;
>> + else
>> + tst_brk(TBROK,
>> + "Invalid parameter: use -m <private|shared>");
>> + } else {
>> + /* Default to shared if no option is passed */
>> + race_type = MAP_SHARED;
>> + }
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> + if (p_race != MAP_FAILED)
>> + SAFE_MUNMAP(p_race, tst_get_hugepage_size());
>> +
>> + if (fd_race >= 0)
> fd_race != -1
Will fix in the next version. Will remove the redundant fd_sync = -1 and
fd_race = -1 assignments after SAFE_CLOSE().
>> + SAFE_CLOSE(fd_race);
>> +
>> + /*
>> + * Use the saved sync_mapping_size rather than re-reading
>> + * MEMINFO_HPAGE_FREE: the current free count differs from the
>> + * mapping length and could cause munmap() to remove an adjacent
>> + * mapping or leave part of p_sync mapped.
>> + */
>> + if (p_sync != MAP_FAILED)
>> + SAFE_MUNMAP(p_sync, sync_mapping_size);
>> +
>> + if (fd_sync >= 0)
>> + SAFE_CLOSE(fd_sync);
>> +
>> + if (child1 > 0) {
>> + if (kill(child1, 0) == 0)
>> + SAFE_KILL(child1, SIGKILL);
>> + }
>> +
>> + if (child2 > 0) {
>> + if (kill(child2, 0) == 0)
>> + SAFE_KILL(child2, SIGKILL);
>> + }
>> +}
>> +
>> +static struct tst_test test = {
>> + .options = (struct tst_option[]) {
>> + {"m:", &str_op, "Type of mmap() mapping <private|shared>"},
>> + {}
>> + },
>> + .needs_root = 1,
>> + .mntpoint = MNTPOINT,
>> + .needs_hugetlbfs = 1,
>> + .needs_tmpdir = 1,
>> + .setup = setup,
>> + .cleanup = cleanup,
>> + .test_all = run_test,
>> + .hugepages = {2, TST_NEEDS},
>> + .forks_child = 1,
>> + .needs_checkpoints = 1,
>> + .min_cpus = 2,
>> +};
>> --
>> 2.52.0
>>
Thank you Cyril for the review comments.
Regards,
Samir
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v6] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
@ 2026-08-18 14:31 Samir Mulani
2026-08-18 20:27 ` [LTP] " linuxtestproject.agent
0 siblings, 1 reply; 6+ messages in thread
From: Samir Mulani @ 2026-08-18 14:31 UTC (permalink / raw)
To: ltp; +Cc: Samir Mulani
Migrate the alloc-instantiate-race.c test from libhugetlbfs [1] to LTP
as hugemmap36.
This test is designed to detect a kernel allocation race introduced
with hugepage demand-faulting. The problem is that no lock is held
between allocating a hugepage and instantiating it in the
pagetables or page cache index. In between the two, the (huge)
page is cleared, so there's substantial time. Thus two processes
can race instantiating the (same) last available hugepage - one
will fail on the allocation, and thus cause an OOM fault even
though the page it actually wants is being instantiated by the
other racing process.
[1] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/alloc-instantiate-race.c
Signed-off-by: Samir Mulani <samir@linux.ibm.com>
---
Link: https://lore.kernel.org/all/20250928030721.3537869-1-samir@linux.ibm.com/ #v3
Link: https://lore.kernel.org/ltp/20260317095559.5766-1-samir@linux.ibm.com/ #v4
Link:https://lore.kernel.org/ltp/20260504132405.333588-1-samir@linux.ibm.com/ #v5
v6:
- Fix copyright year, /*\ doc block, add #include "tst_test.h"
- Promote p_sync, fd_race, p_race to globals for cleanup() reachability
- Initialize fd_sync/fd_race = -1; reset to -1 after SAFE_CLOSE()
- Add p_race, fd_race, p_sync guards to cleanup()
runtest/hugetlb | 1 +
testcases/kernel/mem/.gitignore | 1 +
.../kernel/mem/hugetlb/hugemmap/Makefile | 2 +
.../kernel/mem/hugetlb/hugemmap/hugemmap42.c | 294 ++++++++++++++++++
4 files changed, 298 insertions(+)
create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
diff --git a/runtest/hugetlb b/runtest/hugetlb
index 6b35c1f42..b4c9dc6cd 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -40,6 +40,7 @@ hugemmap35 hugemmap35
hugemmap36 hugemmap36
hugemmap37 hugemmap37
hugemmap38 hugemmap38
+hugemmap42 hugemmap42
hugemmap05_1 hugemmap05 -m
hugemmap05_2 hugemmap05 -s
hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index e63a6dde7..a0a877a58 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -40,6 +40,7 @@
/hugetlb/hugemmap/hugemmap36
/hugetlb/hugemmap/hugemmap37
/hugetlb/hugemmap/hugemmap38
+/hugetlb/hugemmap/hugemmap42
/hugetlb/hugeshmat/hugeshmat01
/hugetlb/hugeshmat/hugeshmat02
/hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/Makefile b/testcases/kernel/mem/hugetlb/hugemmap/Makefile
index 6e72e7009..662b9c49d 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/Makefile
+++ b/testcases/kernel/mem/hugetlb/hugemmap/Makefile
@@ -12,3 +12,5 @@ CFLAGS_no_stack_prot := $(filter-out -fstack-clash-protection, $(CFLAGS))
hugemmap06: CFLAGS+=-pthread
hugemmap34: CFLAGS=$(CFLAGS_no_stack_prot)
+hugemmap42: LDLIBS+=-lpthread
+hugemmap42: CFLAGS+=-pthread
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
new file mode 100644
index 000000000..b1e091753
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap42.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2005-2006, 2026 IBM Corporation
+ * Author: David Gibson & Adam Litke
+ */
+
+/*\
+ * This test is designed to detect a kernel allocation race introduced
+ * with hugepage demand-faulting. The problem is that no lock is held
+ * between allocating a hugepage and instantiating it in the
+ * pagetables or page cache index. In between the two, the (huge)
+ * page is cleared, so there's substantial time. Thus two processes
+ * can race instantiating the (same) last available hugepage - one
+ * will fail on the allocation, and thus cause an OOM fault even
+ * though the page it actually wants is being instantiated by the
+ * other racing process.
+ */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include "tst_test.h"
+#include "tst_safe_pthread.h"
+#include "hugetlb.h"
+
+#define MNTPOINT "hugetlbfs/"
+
+static char *str_op;
+static int child1, child2, race_type;
+static int fd_sync = -1;
+static int fd_race = -1;
+static void *p_race = MAP_FAILED;
+static void *p_sync = MAP_FAILED;
+
+struct racer_info {
+ void *p;
+ int cpu;
+ int status;
+};
+
+static int one_racer(void *p, int cpu)
+{
+ volatile int *pi = p;
+ cpu_set_t *cpuset;
+ size_t mask_size;
+ int err;
+
+ cpuset = CPU_ALLOC(cpu + 1);
+ if (!cpuset)
+ tst_brk(TBROK | TERRNO, "CPU_ALLOC() failed");
+
+ mask_size = CPU_ALLOC_SIZE(cpu + 1);
+
+ /* Split onto different CPUs to encourage the race */
+ CPU_ZERO_S(mask_size, cpuset);
+ CPU_SET_S(cpu, mask_size, cpuset);
+
+ err = sched_setaffinity(getpid(), mask_size, cpuset);
+ CPU_FREE(cpuset);
+ if (err == -1)
+ tst_brk(TBROK | TERRNO, "sched_setaffinity() failed");
+
+ /* Wait for parent to signal both racers to start */
+ TST_CHECKPOINT_WAIT(0);
+
+ /* Fault in the hugepage - triggers the race */
+ *pi = 1;
+
+ return 0;
+}
+
+static void proc_racer(void *p, int cpu)
+{
+ exit(one_racer(p, cpu));
+}
+
+static void *thread_racer(void *info)
+{
+ struct racer_info *ri = info;
+
+ ri->status = one_racer(ri->p, ri->cpu);
+ return ri;
+}
+
+static void check_online_cpus(int online_cpus[], int nr_cpus_needed)
+{
+ cpu_set_t cpuset;
+ int total_cpus, cpu_idx;
+
+ CPU_ZERO(&cpuset);
+
+ total_cpus = get_nprocs_conf();
+
+ if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuset) == -1)
+ tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");
+
+ tst_res(TINFO, "Online CPUs needed: %d, available: %d",
+ nr_cpus_needed, CPU_COUNT(&cpuset));
+
+ if (CPU_COUNT(&cpuset) < nr_cpus_needed)
+ tst_brk(TCONF, "At least %d online CPUs are required",
+ nr_cpus_needed);
+
+ cpu_idx = 0;
+ for (int i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
+ if (CPU_ISSET(i, &cpuset))
+ online_cpus[cpu_idx++] = i;
+ }
+
+ if (cpu_idx < nr_cpus_needed)
+ tst_brk(TBROK, "Unable to find enough online CPUs");
+}
+
+static void run_race(int rtype)
+{
+ void *tret1, *tret2;
+ int status1 = 0, status2 = 0;
+ int online_cpus[2];
+ long hpage_size;
+ pthread_t thread1, thread2;
+
+ check_online_cpus(online_cpus, 2);
+
+ hpage_size = tst_get_hugepage_size();
+
+ /* Get a new file for the final page */
+ fd_race = tst_creat_unlinked(MNTPOINT, 0, 0600);
+ tst_res(TINFO, "Mapping final page..");
+
+ p_race = SAFE_MMAP(NULL, hpage_size, PROT_READ | PROT_WRITE,
+ rtype, fd_race, 0);
+
+ if (rtype == MAP_SHARED) {
+ child1 = SAFE_FORK();
+ if (child1 == 0)
+ proc_racer(p_race, online_cpus[0]);
+
+ child2 = SAFE_FORK();
+ if (child2 == 0)
+ proc_racer(p_race, online_cpus[1]);
+
+ /* Wake both children to start the race simultaneously */
+ TST_CHECKPOINT_WAKE2(0, 2);
+
+ SAFE_WAITPID(child1, &status1, 0);
+ child1 = 0;
+ tst_res(TINFO, "Child 1 status: %x", status1);
+
+ SAFE_WAITPID(child2, &status2, 0);
+ child2 = 0;
+ tst_res(TINFO, "Child 2 status: %x", status2);
+
+ if (WIFSIGNALED(status1))
+ tst_res(TFAIL, "Child 1 killed by signal %s",
+ strsignal(WTERMSIG(status1)));
+ if (WIFSIGNALED(status2))
+ tst_res(TFAIL, "Child 2 killed by signal %s",
+ strsignal(WTERMSIG(status2)));
+ } else {
+ struct racer_info ri1 = {
+ .p = p_race,
+ .cpu = online_cpus[0],
+ .status = -1,
+ };
+ struct racer_info ri2 = {
+ .p = p_race,
+ .cpu = online_cpus[1],
+ .status = -1,
+ };
+
+ SAFE_PTHREAD_CREATE(&thread1, NULL, thread_racer, &ri1);
+ SAFE_PTHREAD_CREATE(&thread2, NULL, thread_racer, &ri2);
+
+ /* Wake both threads to start the race simultaneously */
+ TST_CHECKPOINT_WAKE2(0, 2);
+
+ SAFE_PTHREAD_JOIN(thread1, &tret1);
+ if (tret1 != &ri1)
+ tst_res(TFAIL, "Thread 1 returned %p not %p, killed?",
+ tret1, &ri1);
+
+ SAFE_PTHREAD_JOIN(thread2, &tret2);
+ if (tret2 != &ri2)
+ tst_res(TFAIL, "Thread 2 returned %p not %p, killed?",
+ tret2, &ri2);
+
+ status1 = ri1.status;
+ status2 = ri2.status;
+ }
+
+ if (status1 != 0)
+ tst_res(TFAIL, "Racer 1 terminated with code %d", status1);
+
+ if (status2 != 0)
+ tst_res(TFAIL, "Racer 2 terminated with code %d", status2);
+
+ if (status1 == 0 && status2 == 0)
+ tst_res(TPASS, "Test completed successfully");
+
+ SAFE_MUNMAP(p_race, hpage_size);
+ p_race = MAP_FAILED;
+
+ SAFE_CLOSE(fd_race);
+ fd_race = -1;
+}
+
+static void run_test(void)
+{
+ unsigned long totpages;
+ long hpage_size;
+
+ totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+ if (totpages < 2)
+ tst_brk(TCONF, "Not enough free hugepages, need at least 2");
+
+ hpage_size = tst_get_hugepage_size();
+
+ tst_res(TINFO, "Instantiating..");
+
+ fd_sync = tst_creat_unlinked(MNTPOINT, 0, 0600);
+
+ tst_res(TINFO, "Mapping %lu/%lu pages..", totpages - 1, totpages);
+ p_sync = SAFE_MMAP(NULL, (totpages - 1) * hpage_size,
+ PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
+
+ run_race(race_type);
+
+ SAFE_MUNMAP(p_sync, (totpages - 1) * hpage_size);
+ p_sync = MAP_FAILED;
+
+ SAFE_CLOSE(fd_sync);
+ fd_sync = -1;
+}
+
+static void setup(void)
+{
+ if (str_op) {
+ if (strcmp(str_op, "shared") == 0)
+ race_type = MAP_SHARED;
+ else if (strcmp(str_op, "private") == 0)
+ race_type = MAP_PRIVATE;
+ else
+ tst_brk(TBROK,
+ "Invalid parameter: use -m <private|shared>");
+ } else {
+ /* Default to shared if no option is passed */
+ race_type = MAP_SHARED;
+ }
+}
+
+static void cleanup(void)
+{
+ if (p_race != MAP_FAILED)
+ SAFE_MUNMAP(p_race, tst_get_hugepage_size());
+
+ if (fd_race >= 0)
+ SAFE_CLOSE(fd_race);
+
+ if (p_sync != MAP_FAILED) {
+ unsigned long totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
+
+ SAFE_MUNMAP(p_sync, totpages * tst_get_hugepage_size());
+ }
+
+ if (fd_sync >= 0)
+ SAFE_CLOSE(fd_sync);
+
+ if (child1 > 0) {
+ if (kill(child1, 0) == 0)
+ SAFE_KILL(child1, SIGKILL);
+ }
+
+ if (child2 > 0) {
+ if (kill(child2, 0) == 0)
+ SAFE_KILL(child2, SIGKILL);
+ }
+}
+
+static struct tst_test test = {
+ .options = (struct tst_option[]) {
+ {"m:", &str_op, "Type of mmap() mapping <private|shared>"},
+ {NULL, NULL, NULL}
+ },
+ .needs_root = 1,
+ .mntpoint = MNTPOINT,
+ .needs_hugetlbfs = 1,
+ .needs_tmpdir = 1,
+ .setup = setup,
+ .cleanup = cleanup,
+ .test_all = run_test,
+ .hugepages = {2, TST_NEEDS},
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .min_cpus = 2,
+};
--
2.52.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
2026-08-18 14:31 [LTP] [PATCH v6] " Samir Mulani
@ 2026-08-18 20:27 ` linuxtestproject.agent
2026-08-21 17:01 ` Samir M
0 siblings, 1 reply; 6+ messages in thread
From: linuxtestproject.agent @ 2026-08-18 20:27 UTC (permalink / raw)
To: Samir Mulani; +Cc: ltp
Hi Samir,
On August 18, 2026, Samir Mulani wrote:
> hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
> Migrate the alloc-instantiate-race.c test from libhugetlbfs [1] to LTP
> as hugemmap36.
Could this be corrected to hugemmap42, which is the test added by this
patch?
> +hugemmap42 hugemmap42
Could a second runtest entry exercise "-m private"? Without an option,
setup() defaults to MAP_SHARED, so the new pthread path is not run by the
hugetlb suite.
> + err = sched_setaffinity(getpid(), mask_size, cpuset);
Could this pass 0 as the pid? sched_setaffinity() applies the mask to the
thread ID supplied in pid. In the MAP_PRIVATE path, getpid() identifies the
thread-group leader for both pthreads, so both racers change the main
thread's affinity and remain unpinned.
> + p_sync = SAFE_MMAP(NULL, (totpages - 1) * hpage_size,
> + PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
> +
> + run_race(race_type);
Could each hugepage in p_sync be written before run_race()? mmap() without
MAP_POPULATE does not fault these pages in. Consequently all free hugepages
remain available to the racers instead of only the final page, and the
allocation race is not exercised. The source test explicitly touches each
page for this reason.
> + if (p_sync != MAP_FAILED) {
> + unsigned long totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
> +
> + SAFE_MUNMAP(p_sync, totpages * tst_get_hugepage_size());
> + }
Could the exact length passed to mmap() be saved and reused here? The
current free-page count is not the mapping length. On an abort with the
current code it is one page larger, and munmap() may remove an adjacent
mapping. After the pages are faulted in, a pre-existing hugepage pool can
instead make it smaller and leave part of p_sync mapped.
> + {NULL, NULL, NULL}
Could this use the standard empty sentinel "{}"? check-hugemmap42 reports
LTP-005 for this options array.
Verdict - Needs revision
---
Note:
The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.
Regards,
LTP AI Reviewer
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
2026-08-18 20:27 ` [LTP] " linuxtestproject.agent
@ 2026-08-21 17:01 ` Samir M
0 siblings, 0 replies; 6+ messages in thread
From: Samir M @ 2026-08-21 17:01 UTC (permalink / raw)
To: linuxtestproject.agent; +Cc: ltp
On 19/08/26 1:57 am, linuxtestproject.agent@gmail.com wrote:
> Hi Samir,
>
> On August 18, 2026, Samir Mulani wrote:
>> hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs
>> Migrate the alloc-instantiate-race.c test from libhugetlbfs [1] to LTP
>> as hugemmap36.
> Could this be corrected to hugemmap42, which is the test added by this
> patch?
Yes, fixed the commit message. It now correctly says hugemmap42. updated
in patch v7
>> +hugemmap42 hugemmap42
> Could a second runtest entry exercise "-m private"? Without an option,
> setup() defaults to MAP_SHARED, so the new pthread path is not run by the
> hugetlb suite.
Yes, added a second entry in runtest/hugetlb: updated in patch v7
>> + err = sched_setaffinity(getpid(), mask_size, cpuset);
> Could this pass 0 as the pid? sched_setaffinity() applies the mask to the
> thread ID supplied in pid. In the MAP_PRIVATE path, getpid() identifies the
> thread-group leader for both pthreads, so both racers change the main
> thread's affinity and remain unpinned.
Yes, fixed. Changed getpid() to 0.
With getpid(), both pthreads return the same thread-group ID, so both
were pinning the main thread instead of themselves. Using 0 means "apply
to the calling thread", which is correct for both the fork and pthread
cases.
>> + p_sync = SAFE_MMAP(NULL, (totpages - 1) * hpage_size,
>> + PROT_READ | PROT_WRITE, MAP_SHARED, fd_sync, 0);
>> +
>> + run_race(race_type);
> Could each hugepage in p_sync be written before run_race()? mmap() without
> MAP_POPULATE does not fault these pages in. Consequently all free hugepages
> remain available to the racers instead of only the final page, and the
> allocation race is not exercised. The source test explicitly touches each
> page for this reason.
Yes, fixed. Added a loop that writes one byte to each sync page before
calling run_race():
for (unsigned long i = 0; i < totpages - 1; i++) {
volatile char *cp = (volatile char *)p_sync + i * hpage_size;
*cp = 0;
}
Without this, mmap() does not actually take hugepages from the pool (no
MAP_POPULATE), so all pages were still free when the race started and
the race condition was never triggered. This was the most important fix.
>> + if (p_sync != MAP_FAILED) {
>> + unsigned long totpages = SAFE_READ_MEMINFO(MEMINFO_HPAGE_FREE);
>> +
>> + SAFE_MUNMAP(p_sync, totpages * tst_get_hugepage_size());
>> + }
> Could the exact length passed to mmap() be saved and reused here? The
> current free-page count is not the mapping length. On an abort with the
> current code it is one page larger, and munmap() may remove an adjacent
> mapping. After the pages are faulted in, a pre-existing hugepage pool can
> instead make it smaller and leave part of p_sync mapped.
Yes, fixed. Added a global sync_mapping_size, set it at mmap() time
and reused it in both run_test() and cleanup(). This avoids using
SAFE_READ_MEMINFO() in cleanup() which can return a different value
than what was actually mapped
>> + {NULL, NULL, NULL}
> Could this use the standard empty sentinel "{}"? check-hugemmap42 reports
> LTP-005 for this options array.
Yes, fixed. Changed {NULL, NULL, NULL} to {}.
>
> Verdict - Needs revision
>
> ---
> Note:
>
> The agent can sometimes produce false positives although often its
> findings are genuine. If you find issues with the review, please
> comment this email or ignore the suggestions.
>
> Regards,
> LTP AI Reviewer
v7 patch will follow shortly.
Regards,
Samir
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-10 19:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 12:12 [LTP] [PATCH v7] hugemmap: Migrate alloc-instantiate-race test from libhugetlbfs Samir Mulani
2026-08-21 18:39 ` [LTP] " linuxtestproject.agent
2026-08-31 13:34 ` [LTP] [PATCH v7] " Cyril Hrubis
2026-09-10 19:13 ` samir
-- strict thread matches above, loose matches on Subject: below --
2026-08-18 14:31 [LTP] [PATCH v6] " Samir Mulani
2026-08-18 20:27 ` [LTP] " linuxtestproject.agent
2026-08-21 17:01 ` Samir M
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox