Re: [PATCH v2] selftests/cgroup: add PSI pressure trigger and validation tests

On Tue, Jul 28, 2026 at 1:38 AM Tao Cui <[email protected]> wrote:
>
> From: Tao Cui <[email protected]>
>
> The cgroup selftests have no PSI coverage. Add test_psi.c: trigger
> smoke tests (one per fd, IRQ full-only), cgroup.pressure toggle, and a
> CPU-pressure trigger test using over-subscription. Skips when PSI is
> disabled or a resource is absent.
>
> Signed-off-by: Tao Cui <[email protected]>
> ---
> Changes since v1 (Michal Koutny, sashiko review):
> - Trim errno checks to smoke level; loosen toggle range checks.
> - Switch trigger test from memory to CPU pressure (deterministic, no SKIP).
> - churn_memory() removed -- sysconf/shared-helper point is moot.
> - Runner stays out of the cgroup; hogs reaped via cg_killall().
> - Add PSI/IRQ skip-guards, zero-init buffers, .gitignore entry.
> ---
>  tools/testing/selftests/cgroup/.gitignore |   1 +
>  tools/testing/selftests/cgroup/Makefile   |   2 +
>  tools/testing/selftests/cgroup/config     |   1 +
>  tools/testing/selftests/cgroup/test_psi.c | 280 ++++++++++++++++++++++
>  4 files changed, 284 insertions(+)
>  create mode 100644 tools/testing/selftests/cgroup/test_psi.c
>
> diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
> index 952e4448bf07..ce2b907c57ea 100644
> --- a/tools/testing/selftests/cgroup/.gitignore
> +++ b/tools/testing/selftests/cgroup/.gitignore
> @@ -8,5 +8,6 @@ test_kill
>  test_kmem
>  test_memcontrol
>  test_pids
> +test_psi
>  test_zswap
>  wait_inotify
> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index e01584c2189a..a8c69e37332a 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -16,6 +16,7 @@ TEST_GEN_PROGS += test_kill
>  TEST_GEN_PROGS += test_kmem
>  TEST_GEN_PROGS += test_memcontrol
>  TEST_GEN_PROGS += test_pids
> +TEST_GEN_PROGS += test_psi
>  TEST_GEN_PROGS += test_zswap
>
>  LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> @@ -32,4 +33,5 @@ $(OUTPUT)/test_kill: $(LIBCGROUP_O)
>  $(OUTPUT)/test_kmem: $(LIBCGROUP_O)
>  $(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
>  $(OUTPUT)/test_pids: $(LIBCGROUP_O)
> +$(OUTPUT)/test_psi: $(LIBCGROUP_O)
>  $(OUTPUT)/test_zswap: $(LIBCGROUP_O)
> diff --git a/tools/testing/selftests/cgroup/config b/tools/testing/selftests/cgroup/config
> index 39f979690dd3..8a3ef479e83d 100644
> --- a/tools/testing/selftests/cgroup/config
> +++ b/tools/testing/selftests/cgroup/config
> @@ -4,3 +4,4 @@ CONFIG_CGROUP_FREEZER=y
>  CONFIG_CGROUP_SCHED=y
>  CONFIG_MEMCG=y
>  CONFIG_PAGE_COUNTER=y
> +CONFIG_PSI=y
> diff --git a/tools/testing/selftests/cgroup/test_psi.c b/tools/testing/selftests/cgroup/test_psi.c
> new file mode 100644
> index 000000000000..51dc35e26013
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/test_psi.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <poll.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <linux/limits.h>
> +
> +#include "kselftest.h"
> +#include "cgroup_util.h"
> +
> +/* How long to wait for the CPU-pressure trigger before giving up. */
> +#define PSI_POLL_TIMEOUT_MS    5000
> +
> +static int pressure_open(const char *resource)
> +{
> +       char path[PATH_MAX];
> +
> +       snprintf(path, sizeof(path), "/proc/pressure/%s", resource);
> +       return open(path, O_RDWR);
> +}
> +
> +/* Write a trigger descriptor, including the trailing NUL the parser expects. */
> +static ssize_t write_trigger(int fd, const char *trigger)
> +{
> +       return write(fd, trigger, strlen(trigger) + 1);
> +}
> +
> +/* Trigger smoke test: second on same fd -> EBUSY; IRQ rejects "some". */

Please write more descriptive comments for your tests. I have to guess
what you mean by this.

> +static int test_proc_triggers(const char *root)

Why do you pass root here if you are not using it? AI glitch?

> +{
> +       static const char *const resources[] = { "io", "memory", "cpu" };
> +       int ret = KSFT_FAIL;
> +       int fd = -1;
> +       int i;
> +
> +       (void)root;
> +
> +       for (i = 0; i < (int)ARRAY_SIZE(resources); i++) {
> +               fd = pressure_open(resources[i]);
> +               if (fd < 0) {
> +                       ksft_print_msg("open /proc/pressure/%s: %s\n",
> +                                      resources[i], strerror(errno));

You could move these ksft_print_msg() into pressure_open() and make
that function a bit more useful while making the code here simpler.

> +                       goto cleanup;
> +               }
> +
> +               errno = 0;

Why are you resetting the errno? If the next syscall fails, it will
reset the errno and if it succeeds, you won't be using errno anyway.
I'm confused.

> +               if (write_trigger(fd, "some 150000 2000000") <= 0) {
> +                       ksft_print_msg("%s: 'some' trigger rejected: %s\n",
> +                                      resources[i], strerror(errno));
> +                       goto cleanup;
> +               }
> +               /* A second trigger on the same fd must fail with EBUSY. */
> +               errno = 0;
> +               if (write_trigger(fd, "full 150000 2000000") != -1 || errno != EBUSY) {
> +                       ksft_print_msg("%s: second trigger expected EBUSY, got %s\n",
> +                                      resources[i], strerror(errno));
> +                       goto cleanup;
> +               }
> +
> +               close(fd);
> +               fd = -1;

When you jump to cleanup label you always need to close the fd. You
could simplify the flow if you remove all these "fd = -1;" and change
the end of the function to be:

        return KSFT_PASS
cleanup:
        close(fd);
        return ret;
}

> +       }
> +
> +       /* IRQ is full-only, and only exists with CONFIG_IRQ_TIME_ACCOUNTING. */

It would be better if you split this test into separate tests for
"io", "memory", "cpu" and "irq" and if "irq" is not available you can
return KSFT_SKIP for that test only. This way when a test fails the
user will know exactly what failed.

> +       fd = pressure_open("irq");
> +       if (fd >= 0) {
> +               errno = 0;
> +               if (write_trigger(fd, "some 150000 1000000") != -1) {
> +                       ksft_print_msg("irq 'some': expected failure, got %s\n",
> +                                      strerror(errno));
> +                       goto cleanup;
> +               }
> +               close(fd);
> +               fd = -1;
> +       }
> +
> +       ret = KSFT_PASS;
> +cleanup:
> +       if (fd >= 0)
> +               close(fd);
> +       return ret;
> +}
> +
> +/* cgroup.pressure 0/1 hides/shows the *.pressure files and round-trips. */

The above comment needs to be expanded to explain what you are
testing. It does not read as a proper English sentence.

> +static int test_cgroup_pressure_toggle(const char *root)
> +{
> +       char buf[BUF_SIZE] = { 0 };
> +       char *cg = NULL;
> +       int ret = KSFT_FAIL;
> +
> +       cg = cg_name(root, "psi_toggle_test");
> +       if (!cg)
> +               goto cleanup;
> +       if (cg_create(cg))
> +               goto cleanup;

Jumping to "cleanup" above results in a call to cg_destroy() while
cg_create() failed. It will try to rmdir() a directory which we never
created.

> +
> +       if (cg_write(cg, "cgroup.pressure", "0")) {
> +               ksft_print_msg("failed to disable cgroup.pressure\n");

Reporting strerror() in these failure logs would be useful.

> +               goto cleanup;
> +       }
> +       if (cg_read(cg, "cgroup.pressure", buf, sizeof(buf)) < 0 || atoi(buf) != 0) {

atoi() will return 0 even on error, so if you want to really check the
value is 0 use a more robust method like strtol() or simply do a
string comparison.

> +               ksft_print_msg("cgroup.pressure=0 readback: '%s'\n", buf);
> +               goto cleanup;
> +       }
> +       if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) >= 0) {
> +               ksft_print_msg("memory.pressure visible while disabled\n");
> +               goto cleanup;
> +       }
> +
> +       if (cg_write(cg, "cgroup.pressure", "1")) {
> +               ksft_print_msg("failed to re-enable cgroup.pressure\n");
> +               goto cleanup;
> +       }
> +       if (cg_read(cg, "cgroup.pressure", buf, sizeof(buf)) < 0 || atoi(buf) != 1) {
> +               ksft_print_msg("cgroup.pressure=1 readback: '%s'\n", buf);
> +               goto cleanup;
> +       }
> +       if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) < 0) {
> +               ksft_print_msg("memory.pressure unreadable while enabled\n");
> +               goto cleanup;
> +       }
> +
> +       ret = KSFT_PASS;
> +cleanup:
> +       if (cg)
> +               cg_destroy(cg);
> +       free(cg);

free(NULL) works but would be cleaner to do this instead:

      if (cg) {
              cg_destroy(cg);
              free(cg);
      }

> +       return ret;
> +}
> +
> +/* Induce deterministic CPU pressure (more hogs than CPUs). */
> +static int test_cgroup_trigger_fire(const char *root)
> +{
> +       char *cg = NULL, *cpupress = NULL;
> +       int fd = -1, ret = KSFT_FAIL;
> +       struct pollfd pfd;
> +       long ncpus, i;
> +       pid_t pid;
> +
> +       cg = cg_name(root, "psi_trigger_test");
> +       if (!cg)
> +               goto cleanup;
> +       if (cg_create(cg))
> +               goto cleanup;

Same issue with this jump. You will be calling cg_killall(cg) and
cg_destroy(cg) even though cg_create() did not succeed.

> +
> +       cpupress = cg_control(cg, "cpu.pressure");
> +       if (!cpupress)
> +               goto cleanup;
> +       fd = open(cpupress, O_RDWR);
> +       if (fd < 0) {
> +               ksft_print_msg("open cpu.pressure: %s\n", strerror(errno));
> +               goto cleanup;
> +       }
> +
> +       /* 1us threshold in a 1s window: any cpu stall fires it. */
> +       errno = 0;
> +       if (write_trigger(fd, "some 1 1000000") <= 0) {
> +               ksft_print_msg("arming trigger failed: %s\n", strerror(errno));
> +               goto cleanup;
> +       }
> +
> +       ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +       if (ncpus <= 0)

Why is this not treated as a test failure?

> +               ncpus = 1;
> +
> +       pid = fork();
> +       if (pid < 0) {
> +               ksft_print_msg("fork: %s\n", strerror(errno));
> +               goto cleanup;
> +       }
> +       if (pid == 0) {
> +               /* Enter the cgroup, then over-subscribe it with CPU hogs. */
> +               if (cg_enter_current(cg))
> +                       _exit(KSFT_FAIL);
> +               for (i = 0; i < ncpus; i++) {
> +                       if (fork() == 0) {
> +                               for (;;)
> +                                       asm volatile("" ::: "memory");
> +                               _exit(0);
> +                       }
> +               }
> +               for (;;)
> +                       asm volatile("" ::: "memory");  /* child is also a hog */
> +               _exit(0);
> +       }
> +
> +       pfd.fd = fd;
> +       pfd.events = POLLPRI;
> +       switch (poll(&pfd, 1, PSI_POLL_TIMEOUT_MS)) {
> +       case -1:
> +               ksft_print_msg("poll: %s\n", strerror(errno));
> +               break;
> +       case 0:
> +               ksft_print_msg("no trigger event; could not induce cpu pressure\n");
> +               ret = KSFT_SKIP;
> +               break;
> +       default:
> +               if (pfd.revents & POLLPRI)
> +                       ret = KSFT_PASS;
> +               else
> +                       ksft_print_msg("poll returned 0x%x\n", pfd.revents);
> +               break;
> +       }
> +
> +       /* Stop the hogs (bounded, so waitpid can't hang) and reap the child. */
> +       cg_killall(cg);
> +       waitpid(pid, NULL, 0);
> +
> +cleanup:
> +       if (fd >= 0)
> +               close(fd);
> +       if (cg) {
> +               cg_killall(cg);
> +               cg_destroy(cg);
> +       }
> +       free(cpupress);
> +       free(cg);
> +       return ret;
> +}
> +
> +#define TEST(x) { #x, x }
> +
> +struct psi_test {
> +       const char *name;
> +       int (*fn)(const char *root);
> +};
> +
> +static struct psi_test tests[] = {
> +       TEST(test_proc_triggers),
> +       TEST(test_cgroup_pressure_toggle),
> +       TEST(test_cgroup_trigger_fire),

Instead of setting up these tests manually you could use
TEST_HARNESS_MAIN, TEST_F and other helpers from kselftest_harness.h.

> +};
> +
> +int main(int argc, char **argv)
> +{
> +       char root[PATH_MAX];
> +       int mempress_fd;
> +       int i;
> +
> +       (void)argc;
> +
> +       ksft_print_header();
> +       ksft_set_plan(ARRAY_SIZE(tests));
> +
> +       if (cg_find_unified_root(root, sizeof(root), NULL))
> +               ksft_exit_skip("cgroup v2 isn't mounted\n");
> +
> +       /* PSI must be enabled (CONFIG_PSI=y, not default-disabled). */
> +       mempress_fd = open("/proc/pressure/memory", O_RDONLY);
> +       if (mempress_fd < 0)
> +               ksft_exit_skip("PSI unavailable (CONFIG_PSI=n or psi=0)\n");
> +       close(mempress_fd);
> +
> +       if (cg_read_strstr(root, "cgroup.controllers", "memory"))
> +               ksft_exit_skip("memory controller isn't available\n");
> +       if (cg_read_strstr(root, "cgroup.subtree_control", "memory"))
> +               if (cg_write(root, "cgroup.subtree_control", "+memory"))
> +                       ksft_exit_skip("failed to enable memory controller\n");
> +
> +       for (i = 0; i < (int)ARRAY_SIZE(tests); i++) {
> +               switch (tests[i].fn(root)) {
> +               case KSFT_PASS:
> +                       ksft_test_result_pass("%s\n", tests[i].name);
> +                       break;
> +               case KSFT_SKIP:
> +                       ksft_test_result_skip("%s\n", tests[i].name);
> +                       break;
> +               default:
> +                       ksft_test_result_fail("%s\n", tests[i].name);
> +                       break;
> +               }
> +       }
> +
> +       ksft_finished();
> +}
> --
> 2.43.0
>