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 <[email protected]>
> Suggested-by: Dev Jain <[email protected]>
> Signed-off-by: Sarthak Sharma <[email protected]>
> ---
> 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

Reply via email to