* Re: [PATCH] selftests/mm: transhuge-stress: check duration inside page loop
2026-07-20 10:31 [PATCH] selftests/mm: transhuge-stress: check duration inside page loop Sarthak Sharma
@ 2026-07-21 13:28 ` David Hildenbrand (Arm)
0 siblings, 0 replies; 2+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-21 13:28 UTC (permalink / raw)
To: Sarthak Sharma, Andrew Morton
Cc: Lorenzo Stoakes, Zi Yan, Baolin Wang, Liam R . Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Mark Brown, linux-mm, linux-kselftest, linux-kernel,
Aishwarya TCV
On 7/20/26 12:31, Sarthak Sharma wrote:
> transhuge-stress.c currently scans memory equal to RAM by default,
> in jumps of hugepage size, trying to allocate and split a THP at
> each step. After the entire range is covered, it checks whether
> elapsed time is greater than the duration. But it is possible
> that the THP work it does takes time more than the duration itself,
> causing the test to exceed the CI timeout.
>
> Check the elapsed time after each THP operation. If the duration
> has elapsed, break and end the test. Adjust the statistics
> calculations to accommodate partially completed passes. Also,
> reject sizes which are smaller than a huge page.
>
> Reported-by: Aishwarya TCV <aishwarya.tcv@arm.com>
> Suggested-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> ---
> Tested with -d 5 on a machine with about 250 GiB RAM.
That is certainly an improvement for running the test, but see below on the
overall test: within 5 seconds on that machine, the test is probably completely
useless to run.
IOW, 5 wasted seconds :)
>
> Time taken before the patch = 4m26.793s
> Time taken after the patch = 0m5.198s
> ---
> tools/testing/selftests/mm/transhuge-stress.c | 30 ++++++++++++++-----
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/transhuge-stress.c b/tools/testing/selftests/mm/transhuge-stress.c
> index 8eb0c5630e7e..a0f9fc900360 100644
> --- a/tools/testing/selftests/mm/transhuge-stress.c
> +++ b/tools/testing/selftests/mm/transhuge-stress.c
> @@ -35,6 +35,7 @@ int main(int argc, char **argv)
> size_t map_len;
> int pagemap_fd;
> int duration = 0;
> + double elapsed = 0;
>
> ksft_print_header();
>
> @@ -60,8 +61,6 @@ int main(int argc, char **argv)
> len = atoll(argv[i]) << 20;
> }
>
> - ksft_set_plan(1);
> -
> if (name) {
> backing_fd = open(name, O_RDWR);
> if (backing_fd == -1)
> @@ -78,6 +77,10 @@ int main(int argc, char **argv)
> ksft_exit_fail_msg("open pagemap\n");
>
> len -= len % HPAGE_SIZE;
> + if (len == 0)
> + ksft_exit_fail_msg("len must be at least %d MiB\n",
> + HPAGE_SIZE >> 20);
> +
> ptr = mmap(NULL, len + HPAGE_SIZE, PROT_RW, mmap_flags, backing_fd, 0);
> if (ptr == MAP_FAILED)
> ksft_exit_fail_msg("initial mmap");
> @@ -91,6 +94,7 @@ int main(int argc, char **argv)
> if (!map)
> ksft_exit_fail_msg("map malloc\n");
>
> + ksft_set_plan(1);
> clock_gettime(CLOCK_MONOTONIC, &start);
>
> while (1) {
> @@ -125,16 +129,28 @@ int main(int argc, char **argv)
> /* split transhuge page, keep last page */
> if (madvise(p, HPAGE_SIZE - psize(), MADV_DONTNEED))
> ksft_exit_fail_msg("MADV_DONTNEED");
> +
> + if (duration > 0) {
> + clock_gettime(CLOCK_MONOTONIC, &b);
> + elapsed = b.tv_sec - start.tv_sec +
> + (b.tv_nsec - start.tv_nsec) / 1000000000.;
> + if (elapsed >= duration)
> + break;
> + }
> }
> clock_gettime(CLOCK_MONOTONIC, &b);
> s = b.tv_sec - a.tv_sec + (b.tv_nsec - a.tv_nsec) / 1000000000.;
>
> ksft_print_msg("%.3f s/loop, %.3f ms/page, %10.3f MiB/s\t"
> - "%4d succeed, %4d failed, %4d different pages\n",
> - s, s * 1000 / (len >> HPAGE_SHIFT), len / s / (1 << 20),
> - nr_succeed, nr_failed, nr_pages);
> -
> - if (duration > 0 && b.tv_sec - start.tv_sec >= duration) {
> + "%4d succeed, %4d failed, %4d different pages\n",
> + s, s * 1000 / (nr_failed + nr_succeed),
> + s > 0 ?
> + (double)(nr_failed + nr_succeed) *
> + (HPAGE_SIZE >> 20) / s :
> + 0.0,
> + nr_succeed, nr_failed, nr_pages);
> +
> + if (duration > 0 && elapsed >= duration) {
> ksft_test_result_pass("Completed\n");
> ksft_finished();
> }
Looking into this test here, this is actually the type of test we don't want as
a selftests.
a) It never fails on actual THP allocation issues, so the test can never fail
(only trigger splats etc).
b) It relies on magic input parameters nobody ever sets. Setting the value too
small (e.g., to 5), will not test anything on a bigger machine, really. Even
the default of "20" from run_vmtests.sh is likely insufficient.
There an option to set a file when really, nobody does that. We don't want
random testing tools to be part of our automated selftests.
c) It measures and prints things nobody ever checks
I think we should rethink this test as a whole: what could it look like as a
proper selftests, whose sole purpose is to be a selftest that passes or fails,
reliably.
For example, we could
a) Allocate as many THPs until it fails
b) Split all THPs, keeping only a single page
c) Retry allocating THPs and make sure that we succeed in allocating the
majority. If not, fail.
While making sure that the system is configured properly.
To avoid long runtimes, we could consider skipping the tests on larger machines
(e.g., > 10s) where allocation would just take too long.
That would be more deterministic than what we have here.
--
Cheers,
David
^ permalink raw reply [flat|nested] 2+ messages in thread