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
>
