Re: [PATCH v2 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()

[Guixiong Wei]

> From: "Chang S. Bae"<chang.seok.bae@xxxxxxxxx>
> Date:  Thu, Oct 15, 2026, 08:19
> Subject:  Re: [PATCH v2 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()
> To: "Guixiong Wei"<weiguixiong@xxxxxxxxxxxxx>, <x86@xxxxxxxxxx>
> Cc: "Thomas Gleixner"<tglx@xxxxxxxxxx>, "Ingo Molnar"<mingo@xxxxxxxxxx>, "Borislav Petkov"<bp@xxxxxxxxx>, "Dave Hansen"<dave.hansen@xxxxxxxxxxxxxxx>, "H . Peter Anvin"<hpa@xxxxxxxxx>, "Shuah Khan"<shuah@xxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <linux-kselftest@xxxxxxxxxxxxxxx>
> On 10/7/2026 10:59 PM, Guixiong Wei wrote:
> >   
> > -int main(void)
> > +static int count_dynamic_fpstates(void)
> > +{
> > +        char *line = NULL;
> > +        size_t line_size = 0;
> > +        FILE *fp;
> > +        int count = 0;
> > +
> > +        fp = fopen("/proc/vmallocinfo", "r");
> > +        if (!fp)
> > +                return -errno;
> 
> The call site doesn't interprete the error code. Instead, you can simply 
> call fatal_error() here and below on an IO error.
> 

Will do. v3 drops the -errno plumbing and calls fatal_error() on
fopen()/getline()/ferror() failures

> > +
> > +        while (getline(&line, &line_size, fp) >= 0) {
> > +                if (strstr(line, "__xfd_enable_feature") ||
> > +                    strstr(line, "fpstate_realloc"))
> > +                        count++;
> > +        }
> > +
> > +        if (ferror(fp))
> > +                count = -EIO;
> > +
> > +        free(line);
> > +        fclose(fp);
> > +        return count;
> > +}
> 
> > +static void test_exec(void)
> > +{
> > +        char iterations_arg[16];
> > +        char baseline_arg[16];
> > +        int before, after, delta;
> > +        pid_t child;
> > +        int status;
> > +
> > +        printf("[RUN]\tCheck dynamic fpstate cleanup across exec().\n");
> > +
> > +        before = count_dynamic_fpstates();
> > +        if (before < 0) {
> > +                printf("[SKIP]\tCannot read /proc/vmallocinfo: %s\n",
> > +                       strerror(-before));
> > +                return;
> > +        }
> > +
> > +        child = fork();
> > +        if (child < 0)
> > +                fatal_error("fork");
> > +        if (!child) {
> > +                snprintf(iterations_arg, sizeof(iterations_arg), "%d",
> > +                         EXEC_TEST_ITERS);
> > +                snprintf(baseline_arg, sizeof(baseline_arg), "%d", before);
> > +                execl("/proc/self/exe", "amx", EXEC_TEST_ARG, iterations_arg,
> > +                      baseline_arg, NULL);
> > +                fatal_error("exec");
> > +        }
> 
> This nested exec() with passing numbers via arguments looks more 
> complicate than necessary.
> 
> Instead, you could put the fork()/exec() sequence in a loop. Before each 
> exec(), call load_rand_tiledata(), and adjust main() to return 
> immediately from the exec test. I found this also reproduces the 
> increase in /proc/vmallocinfo.
> 
Agreed, much simpler. v3 restructures test_exec() as:

    for (i = 0; i < EXEC_TEST_ITERS; i++) {
            child = fork();
            if (child < 0)
                    fatal_error("fork");
            if (!child) {
                    req_xtiledata_perm();
                    load_rand_tiledata(&stashed_tiles);
                    execl("/proc/self/exe", "amx",
                          EXEC_TEST_ARG, NULL);
                    fatal_error("exec");
            }
            if (waitpid(child, &status, 0) != child)
                    fatal_error("waitpid");
            ...
    }

main() short-circuits when argv[1] == EXEC_TEST_ARG and exits 0, so
the exec'd child does nothing beyond returning. The baseline/final
counts are taken by the parent around the loop. Confirmed on an AMX
host: unfixed kernel reports EXEC_TEST_ITERS leaked allocations,
fixed kernel reports 0.

> > +
> > +        if (waitpid(child, &status, 0) != child)
> > +                fatal_error("waitpid");
> > +        if (WIFEXITED(status) && WEXITSTATUS(status) == KSFT_SKIP) {
> > +                printf("[SKIP]\tDynamic fpstate allocation was not isolated.\n");
> > +                return;
> > +        }
> > +        if (!WIFEXITED(status) || WEXITSTATUS(status))
> > +                fatal_error("exec test child");
> > +
> > +        after = count_dynamic_fpstates();
> > +        if (after < 0) {
> > +                printf("[SKIP]\tCannot read /proc/vmallocinfo after exec(): %s\n",
> > +                       strerror(-after));
> > +                return;
> > +        }
> > +
> > +        delta = after - before;
> > +        if (!delta) {
> > +                printf("[OK]\tDynamic fpstate allocations were freed across exec().\n");
> > +                return;
> > +        }
> > +
> > +        if (delta == EXEC_TEST_ITERS)
> > +                errx(1, "[FAIL]\texec() leaked %d dynamic fpstate allocations",
> > +                     delta);
> 
> errx() terminates the entire test, preventing subsequent tests from 
> running. Unless this failure makes it unsafe or meaningless to continue, 
> why not report the failure with printf() and let the remaining tests run?
> 

Will do. v3 replaces the errx() with a printf("[FAIL]\t...") and
returns from test_exec() so later subtests still execute. The
failure is still visible in kselftest output.

> > +
> > +        printf("[SKIP]\tDynamic fpstate vmalloc usage changed concurrently.\n");
> > +}
> 
> Thanks,
> Chang
> 
Thanks,
Guixiong