* [PATCH v3] selftests/cgroup: add PSI pressure trigger and validation tests
@ 2026-08-13 13:37 Tao Cui
2026-08-19 5:47 ` Suren Baghdasaryan
0 siblings, 1 reply; 3+ messages in thread
From: Tao Cui @ 2026-08-13 13:37 UTC (permalink / raw)
To: Michal Koutný, Suren Baghdasaryan
Cc: Tejun Heo, Johannes Weiner, Shuah Khan, cgroups, linux-kselftest,
linux-kernel, Ziyang Men, Tao Cui, cui.tao
From: Tao Cui <cuitao@kylinos.cn>
The cgroup selftests have no PSI coverage. Add test_psi.c: per-resource
trigger smoke tests (one trigger per fd, IRQ full-only), a
cgroup.pressure hide/show toggle test, and a CPU-pressure trigger test
using over-subscription. Skips when PSI is disabled or a resource is
absent.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
Changes since v2 (Suren Baghdasaryan, Michal Koutny review):
- Restructure the trigger test into per-resource cases (io, memory, cpu,
irq) so a failure points at the specific resource; irq is skipped when
/proc/pressure/irq is absent.
- Spawn the CPU hogs with cg_run_nowait() instead of open-coding fork(),
and arm the trigger with a 2s window so unprivileged users can set it.
- Address the remaining review comments on cleanup and robustness:
guard teardown with a "created" flag, use cg_read_strcmp() instead of
atoi(), report strerror() on errors, and fix the unused-parameter and
sign-compare nits.
Changes since v1 (Michal Koutny, sashiko review):
- Keep trigger tests smoke-level; switch the firing test from memory to
CPU pressure; drop churn_memory().
- Keep the runner out of the cgroup; add PSI/IRQ skip-guards and a
.gitignore entry.
Link: https://lore.kernel.org/all/20260724025826.504586-1-cui.tao@linux.dev/
Link: https://lore.kernel.org/all/20260728083742.2359320-1-cui.tao@linux.dev/
---
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 | 296 ++++++++++++++++++++++
4 files changed, 300 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..cfb412f61458
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_psi.c
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <poll.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/limits.h>
+
+#include "kselftest.h"
+#include "cgroup_util.h"
+
+#define PSI_POLL_TIMEOUT_MS 5000
+
+/* PSI triggers are written with a trailing NUL the kernel parser expects. */
+static ssize_t write_trigger(int fd, const char *trigger)
+{
+ return write(fd, trigger, strlen(trigger) + 1);
+}
+
+static int pressure_open(const char *resource)
+{
+ char path[PATH_MAX];
+ int fd;
+
+ snprintf(path, sizeof(path), "/proc/pressure/%s", resource);
+ fd = open(path, O_RDWR);
+ if (fd < 0)
+ ksft_print_msg("open %s: %s\n", path, strerror(errno));
+ return fd;
+}
+
+/*
+ * /proc/pressure/<resource> accepts exactly one trigger per file
+ * descriptor. For io, memory and cpu verify that a "some" trigger arms
+ * and that a second trigger on the same fd is rejected with EBUSY. For
+ * irq, which only tracks "full", verify that "some" is rejected and
+ * "full" arms. irq is optional -- it only exists with IRQ-time
+ * accounting -- so a missing /proc/pressure/irq is SKIP, not FAIL.
+ */
+static int test_proc_trigger(const char *resource, bool full_only)
+{
+ int fd, ret = KSFT_FAIL;
+
+ fd = pressure_open(resource);
+ if (fd < 0)
+ return full_only ? KSFT_SKIP : KSFT_FAIL;
+
+ if (!full_only) {
+ if (write_trigger(fd, "some 150000 2000000") <= 0) {
+ ksft_print_msg("%s: 'some' trigger rejected: %s\n",
+ resource, strerror(errno));
+ goto out;
+ }
+ if (write_trigger(fd, "full 150000 2000000") != -1 ||
+ errno != EBUSY) {
+ ksft_print_msg("%s: second trigger not EBUSY\n",
+ resource);
+ goto out;
+ }
+ } else {
+ if (write_trigger(fd, "some 150000 2000000") != -1) {
+ ksft_print_msg("irq: 'some' trigger unexpectedly accepted\n");
+ goto out;
+ }
+ if (write_trigger(fd, "full 150000 2000000") <= 0) {
+ ksft_print_msg("irq: 'full' trigger rejected: %s\n",
+ strerror(errno));
+ goto out;
+ }
+ }
+
+ ret = KSFT_PASS;
+out:
+ close(fd);
+ return ret;
+}
+
+/*
+ * cgroup.pressure gates visibility of the per-resource *.pressure files
+ * inside a cgroup: writing 0 hides them, writing 1 shows them again.
+ * Drive one hide/show cycle and check that memory.pressure appears and
+ * disappears along with it.
+ */
+static int test_cgroup_pressure_toggle(const char *root)
+{
+ char buf[BUF_SIZE];
+ char *cg = NULL;
+ int ret = KSFT_FAIL, created = 0;
+
+ cg = cg_name(root, "psi_toggle_test");
+ if (!cg)
+ goto cleanup;
+ if (cg_create(cg)) {
+ ksft_print_msg("cg_create: %s\n", strerror(errno));
+ goto cleanup;
+ }
+ created = 1;
+
+ if (cg_write(cg, "cgroup.pressure", "0")) {
+ ksft_print_msg("write cgroup.pressure=0: %s\n", strerror(errno));
+ goto cleanup;
+ }
+ if (cg_read_strcmp(cg, "cgroup.pressure", "0\n")) {
+ ksft_print_msg("cgroup.pressure readback != 0\n");
+ goto cleanup;
+ }
+ if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) >= 0) {
+ ksft_print_msg("memory.pressure readable while hidden\n");
+ goto cleanup;
+ }
+
+ if (cg_write(cg, "cgroup.pressure", "1")) {
+ ksft_print_msg("write cgroup.pressure=1: %s\n", strerror(errno));
+ goto cleanup;
+ }
+ if (cg_read_strcmp(cg, "cgroup.pressure", "1\n")) {
+ ksft_print_msg("cgroup.pressure readback != 1\n");
+ goto cleanup;
+ }
+ if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) < 0) {
+ ksft_print_msg("memory.pressure unreadable after enabling\n");
+ goto cleanup;
+ }
+
+ ret = KSFT_PASS;
+cleanup:
+ if (created)
+ cg_destroy(cg);
+ free(cg);
+ return ret;
+}
+
+/* A child that burns CPU forever; stopped by cg_killall() in the parent. */
+static int hog_cpu(const char *cgroup, void *arg)
+{
+ for (;;)
+ ;
+ return 0;
+}
+
+/*
+ * Arm a "some" trigger on a cgroup's cpu.pressure, oversubscribe the
+ * cgroup with more spinning hogs than there are CPUs, and check that the
+ * trigger fires once the cgroup stalls on CPU.
+ */
+static int test_cgroup_trigger_fire(const char *root)
+{
+ char *cg = NULL, *cpupress = NULL;
+ int fd = -1, ret = KSFT_FAIL, created = 0, i;
+ long ncpus;
+
+ cg = cg_name(root, "psi_trigger_test");
+ if (!cg)
+ goto cleanup;
+ if (cg_create(cg)) {
+ ksft_print_msg("cg_create: %s\n", strerror(errno));
+ goto cleanup;
+ }
+ created = 1;
+
+ 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;
+ }
+
+ /*
+ * 1usec threshold over a 2s window: any CPU stall fires it. The 2s
+ * window is the smallest unprivileged users are allowed to arm.
+ */
+ if (write_trigger(fd, "some 1 2000000") <= 0) {
+ ksft_print_msg("arm trigger: %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+ if (ncpus <= 0) {
+ ksft_print_msg("sysconf(_SC_NPROCESSORS_ONLN) returned %ld\n",
+ ncpus);
+ goto cleanup;
+ }
+
+ /* ncpus+1 hogs guarantee CPU contention inside the cgroup. */
+ for (i = 0; i < ncpus + 1; i++) {
+ if (cg_run_nowait(cg, hog_cpu, NULL) < 0) {
+ ksft_print_msg("spawn hog %d: %s\n", i, strerror(errno));
+ goto cleanup;
+ }
+ }
+
+ struct pollfd pfd = { .fd = fd, .events = POLLPRI };
+
+ switch (poll(&pfd, 1, PSI_POLL_TIMEOUT_MS)) {
+ case -1:
+ ksft_print_msg("poll: %s\n", strerror(errno));
+ goto cleanup;
+ case 0:
+ ksft_print_msg("trigger did not fire (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;
+ }
+
+cleanup:
+ if (fd >= 0)
+ close(fd);
+ if (created) {
+ cg_killall(cg);
+ cg_destroy(cg);
+ }
+ free(cpupress);
+ free(cg);
+ return ret;
+}
+
+struct psi_proc_test {
+ const char *name;
+ const char *resource;
+ bool full_only;
+};
+static const struct psi_proc_test proc_tests[] = {
+ { "proc_trigger_io", "io", false },
+ { "proc_trigger_memory", "memory", false },
+ { "proc_trigger_cpu", "cpu", false },
+ { "proc_trigger_irq", "irq", true },
+};
+
+struct psi_cg_test {
+ const char *name;
+ int (*fn)(const char *root);
+};
+static const struct psi_cg_test cg_tests[] = {
+ { "cgroup_pressure_toggle", test_cgroup_pressure_toggle },
+ { "cgroup_trigger_fire", test_cgroup_trigger_fire },
+};
+
+int main(int argc, char **argv)
+{
+ char root[PATH_MAX];
+ int psi_fd, i;
+
+ ksft_print_header();
+ ksft_set_plan(ARRAY_SIZE(proc_tests) + ARRAY_SIZE(cg_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 disabled on the cmdline). */
+ psi_fd = open("/proc/pressure/memory", O_RDONLY);
+ if (psi_fd < 0)
+ ksft_exit_skip("PSI unavailable (CONFIG_PSI=n or psi=0)\n");
+ close(psi_fd);
+
+ for (i = 0; i < ARRAY_SIZE(proc_tests); i++) {
+ switch (test_proc_trigger(proc_tests[i].resource,
+ proc_tests[i].full_only)) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", proc_tests[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", proc_tests[i].name);
+ break;
+ default:
+ ksft_test_result_fail("%s\n", proc_tests[i].name);
+ break;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(cg_tests); i++) {
+ switch (cg_tests[i].fn(root)) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", cg_tests[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", cg_tests[i].name);
+ break;
+ default:
+ ksft_test_result_fail("%s\n", cg_tests[i].name);
+ break;
+ }
+ }
+
+ ksft_finished();
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] selftests/cgroup: add PSI pressure trigger and validation tests
2026-08-13 13:37 [PATCH v3] selftests/cgroup: add PSI pressure trigger and validation tests Tao Cui
@ 2026-08-19 5:47 ` Suren Baghdasaryan
2026-08-24 8:48 ` Tao Cui
0 siblings, 1 reply; 3+ messages in thread
From: Suren Baghdasaryan @ 2026-08-19 5:47 UTC (permalink / raw)
To: Tao Cui
Cc: Michal Koutný, Tejun Heo, Johannes Weiner, Shuah Khan,
cgroups, linux-kselftest, linux-kernel, Ziyang Men, Tao Cui
On Thu, Aug 13, 2026 at 6:37 AM Tao Cui <cui.tao@linux.dev> wrote:
>
> From: Tao Cui <cuitao@kylinos.cn>
>
> The cgroup selftests have no PSI coverage. Add test_psi.c: per-resource
> trigger smoke tests (one trigger per fd, IRQ full-only), a
> cgroup.pressure hide/show toggle test, and a CPU-pressure trigger test
> using over-subscription. Skips when PSI is disabled or a resource is
> absent.
>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>
> ---
> Changes since v2 (Suren Baghdasaryan, Michal Koutny review):
> - Restructure the trigger test into per-resource cases (io, memory, cpu,
> irq) so a failure points at the specific resource; irq is skipped when
> /proc/pressure/irq is absent.
> - Spawn the CPU hogs with cg_run_nowait() instead of open-coding fork(),
> and arm the trigger with a 2s window so unprivileged users can set it.
> - Address the remaining review comments on cleanup and robustness:
> guard teardown with a "created" flag, use cg_read_strcmp() instead of
> atoi(), report strerror() on errors, and fix the unused-parameter and
> sign-compare nits.
>
> Changes since v1 (Michal Koutny, sashiko review):
> - Keep trigger tests smoke-level; switch the firing test from memory to
> CPU pressure; drop churn_memory().
> - Keep the runner out of the cgroup; add PSI/IRQ skip-guards and a
> .gitignore entry.
>
> Link: https://lore.kernel.org/all/20260724025826.504586-1-cui.tao@linux.dev/
> Link: https://lore.kernel.org/all/20260728083742.2359320-1-cui.tao@linux.dev/
> ---
> 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 | 296 ++++++++++++++++++++++
> 4 files changed, 300 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..cfb412f61458
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/test_psi.c
> @@ -0,0 +1,296 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <poll.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <linux/limits.h>
> +
> +#include "kselftest.h"
> +#include "cgroup_util.h"
> +
> +#define PSI_POLL_TIMEOUhas this check T_MS 5000
> +
> +/* PSI triggers are written with a trailing NUL the kernel parser expects. */
> +static ssize_t write_trigger(int fd, const char *trigger)
> +{
> + return write(fd, trigger, strlen(trigger) + 1);
> +}
> +
> +static int pressure_open(const char *resource)
> +{
> + char path[PATH_MAX];
> + int fd;
> +
> + snprintf(path, sizeof(path), "/proc/pressure/%s", resource);
> + fd = open(path, O_RDWR);
> + if (fd < 0)
> + ksft_print_msg("open %s: %s\n", path, strerror(errno));
> + return fd;
> +}
> +
> +/*
> + * /proc/pressure/<resource> accepts exactly one trigger per file
> + * descriptor. For io, memory and cpu verify that a "some" trigger arms
> + * and that a second trigger on the same fd is rejected with EBUSY. For
> + * irq, which only tracks "full", verify that "some" is rejected and
> + * "full" arms. irq is optional -- it only exists with IRQ-time
> + * accounting -- so a missing /proc/pressure/irq is SKIP, not FAIL.
> + */
> +static int test_proc_trigger(const char *resource, bool full_only)
> +{
> + int fd, ret = KSFT_FAIL;
> +
> + fd = pressure_open(resource);
> + if (fd < 0)
> + return full_only ? KSFT_SKIP : KSFT_FAIL;
> +
> + if (!full_only) {
> + if (write_trigger(fd, "some 150000 2000000") <= 0) {
> + ksft_print_msg("%s: 'some' trigger rejected: %s\n",
> + resource, strerror(errno));
> + goto out;
> + }
> + if (write_trigger(fd, "full 150000 2000000") != -1 ||
> + errno != EBUSY) {
> + ksft_print_msg("%s: second trigger not EBUSY\n",
> + resource);
> + goto out;
> + }
> + } else {
> + if (write_trigger(fd, "some 150000 2000000") != -1) {
> + ksft_print_msg("irq: 'some' trigger unexpectedly accepted\n");
> + goto out;
> + }
> + if (write_trigger(fd, "full 150000 2000000") <= 0) {
> + ksft_print_msg("irq: 'full' trigger rejected: %s\n",
> + strerror(errno));
> + goto out;
> + }
> + }
> +
> + ret = KSFT_PASS;
> +out:
> + close(fd);
> + return ret;
You don't really need "ret" variable. Just "return KSFT_FAIL;" here is enough.
> +}
> +
> +/*
> + * cgroup.pressure gates visibility of the per-resource *.pressure files
> + * inside a cgroup: writing 0 hides them, writing 1 shows them again.
> + * Drive one hide/show cycle and check that memory.pressure appears and
> + * disappears along with it.
> + */
> +static int test_cgroup_pressure_toggle(const char *root)
> +{
> + char buf[BUF_SIZE];
> + char *cg = NULL;
> + int ret = KSFT_FAIL, created = 0;
Again, you don't need "ret" variable here.
> +
> + cg = cg_name(root, "psi_toggle_test");
> + if (!cg)
> + goto cleanup;
> + if (cg_create(cg)) {
> + ksft_print_msg("cg_create: %s\n", strerror(errno));
> + goto cleanup;
> + }
> + created = 1;
Nope. The way we usually do the cleanup is by jumping to the
appropriate label instead of adding extra state flags. In this case
that would look something like this:
cg = cg_name(root, "psi_toggle_test");
if (!cg)
return KSFT_FAIL;
if (cg_create(cg)) {
ksft_print_msg("cg_create: %s\n", strerror(errno));
goto free_cg;
}
if (cg_write(cg, "cgroup.pressure", "0")) {
ksft_print_msg("write cgroup.pressure=0: %s\n", strerror(errno));
goto cleanup;
}
...
return KSFT_PASS;
cleanup:
cg_destroy(cg);
free_cg:
free(cg);
return KSFT_FAIL;
}
> +
> + if (cg_write(cg, "cgroup.pressure", "0")) {
> + ksft_print_msg("write cgroup.pressure=0: %s\n", strerror(errno));
> + goto cleanup;
> + }
> + if (cg_read_strcmp(cg, "cgroup.pressure", "0\n")) {
> + ksft_print_msg("cgroup.pressure readback != 0\n");
> + goto cleanup;
> + }
> + if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) >= 0) {
> + ksft_print_msg("memory.pressure readable while hidden\n");
> + goto cleanup;
> + }
> +
> + if (cg_write(cg, "cgroup.pressure", "1")) {
> + ksft_print_msg("write cgroup.pressure=1: %s\n", strerror(errno));
> + goto cleanup;
> + }
> + if (cg_read_strcmp(cg, "cgroup.pressure", "1\n")) {
> + ksft_print_msg("cgroup.pressure readback != 1\n");
> + goto cleanup;
> + }
> + if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) < 0) {
> + ksft_print_msg("memory.pressure unreadable after enabling\n");
> + goto cleanup;
> + }
> +
> + ret = KSFT_PASS;
> +cleanup:
> + if (created)
> + cg_destroy(cg);
> + free(cg);
> + return ret;
> +}
> +
> +/* A child that burns CPU forever; stopped by cg_killall() in the parent. */
> +static int hog_cpu(const char *cgroup, void *arg)
> +{
> + for (;;)
> + ;
This loop looks strange. Maybe:
for (;;) {}
> + return 0;
> +}
> +
> +/*
> + * Arm a "some" trigger on a cgroup's cpu.pressure, oversubscribe the
> + * cgroup with more spinning hogs than there are CPUs, and check that the
> + * trigger fires once the cgroup stalls on CPU.
> + */
> +static int test_cgroup_trigger_fire(const char *root)
> +{
> + char *cg = NULL, *cpupress = NULL;
> + int fd = -1, ret = KSFT_FAIL, created = 0, i;
Please define each variable on a separate line.
> + long ncpus;
> +
> + cg = cg_name(root, "psi_trigger_test");
> + if (!cg)
> + goto cleanup;
> + if (cg_create(cg)) {
> + ksft_print_msg("cg_create: %s\n", strerror(errno));
> + goto cleanup;
> + }
> + created = 1;
Same as above. Jump to the appropriate label.
> +
> + 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;
> + }
> +
> + /*
> + * 1usec threshold over a 2s window: any CPU stall fires it. The 2s
> + * window is the smallest unprivileged users are allowed to arm.
> + */
> + if (write_trigger(fd, "some 1 2000000") <= 0) {
> + ksft_print_msg("arm trigger: %s\n", strerror(errno));
> + goto cleanup;
> + }
> +
> + ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> + if (ncpus <= 0) {
I don't think this syscall will ever return 0, so you just need to
check for -1. Also reporting -1 in the error log isn't very
informative. Instead report strerror(errno).
> + ksft_print_msg("sysconf(_SC_NPROCESSORS_ONLN) returned %ld\n",
> + ncpus);
> + goto cleanup;
> + }
> +
> + /* ncpus+1 hogs guarantee CPU contention inside the cgroup. */
> + for (i = 0; i < ncpus + 1; i++) {
> + if (cg_run_nowait(cg, hog_cpu, NULL) < 0) {
> + ksft_print_msg("spawn hog %d: %s\n", i, strerror(errno));
> + goto cleanup;
> + }
> + }
> +
> + struct pollfd pfd = { .fd = fd, .events = POLLPRI };
> +
> + switch (poll(&pfd, 1, PSI_POLL_TIMEOUT_MS)) {
> + case -1:
> + ksft_print_msg("poll: %s\n", strerror(errno));
> + goto cleanup;
So, goto cleanup here but break everywhere else? Consistency would be
nice, so please use break here as well.
> + case 0:
> + ksft_print_msg("trigger did not fire (could not induce CPU pressure)\n");
> + ret = KSFT_SKIP;
This means your trigger timed out. Why is this treated as KSFT_SKIP
and not as KSFT_FAIL?
> + break;
> + default:
> + if (pfd.revents & POLLPRI)
> + ret = KSFT_PASS;
> + else
> + ksft_print_msg("poll returned 0x%x\n", pfd.revents);
> + break;
> + }
> +
> +cleanup:
> + if (fd >= 0)
> + close(fd);
> + if (created) {
> + cg_killall(cg);
> + cg_destroy(cg);
> + }
> + free(cpupress);
> + free(cg);
> + return ret;
> +}
> +
> +struct psi_proc_test {
> + const char *name;
> + const char *resource;
> + bool full_only;
> +};
> +static const struct psi_proc_test proc_tests[] = {
> + { "proc_trigger_io", "io", false },
> + { "proc_trigger_memory", "memory", false },
> + { "proc_trigger_cpu", "cpu", false },
> + { "proc_trigger_irq", "irq", true },
> +};
> +
> +struct psi_cg_test {
> + const char *name;
> + int (*fn)(const char *root);
> +};
> +static const struct psi_cg_test cg_tests[] = {
> + { "cgroup_pressure_toggle", test_cgroup_pressure_toggle },
> + { "cgroup_trigger_fire", test_cgroup_trigger_fire },
> +};
> +
> +int main(int argc, char **argv)
> +{
> + char root[PATH_MAX];
> + int psi_fd, i;
> +
> + ksft_print_header();
> + ksft_set_plan(ARRAY_SIZE(proc_tests) + ARRAY_SIZE(cg_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 disabled on the cmdline). */
> + psi_fd = open("/proc/pressure/memory", O_RDONLY);
> + if (psi_fd < 0)
> + ksft_exit_skip("PSI unavailable (CONFIG_PSI=n or psi=0)\n");
> + close(psi_fd);
> +
> + for (i = 0; i < ARRAY_SIZE(proc_tests); i++) {
> + switch (test_proc_trigger(proc_tests[i].resource,
> + proc_tests[i].full_only)) {
> + case KSFT_PASS:
> + ksft_test_result_pass("%s\n", proc_tests[i].name);
> + break;
> + case KSFT_SKIP:
> + ksft_test_result_skip("%s\n", proc_tests[i].name);
> + break;
> + default:
> + ksft_test_result_fail("%s\n", proc_tests[i].name);
> + break;
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(cg_tests); i++) {
> + switch (cg_tests[i].fn(root)) {
> + case KSFT_PASS:
> + ksft_test_result_pass("%s\n", cg_tests[i].name);
> + break;
> + case KSFT_SKIP:
> + ksft_test_result_skip("%s\n", cg_tests[i].name);
> + break;
> + default:
> + ksft_test_result_fail("%s\n", cg_tests[i].name);
> + break;
> + }
> + }
Please convert this to use
https://docs.kernel.org/dev-tools/kselftest.html#test-harness as I
suggested before. This code will look much simpler. Even if some other
tests are not using it, there is no reason for new tests not to take
advantage of the existing harness. You will find planty of examples
which use them.
> +
> + ksft_finished();
> +}
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] selftests/cgroup: add PSI pressure trigger and validation tests
2026-08-19 5:47 ` Suren Baghdasaryan
@ 2026-08-24 8:48 ` Tao Cui
0 siblings, 0 replies; 3+ messages in thread
From: Tao Cui @ 2026-08-24 8:48 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: cui.tao, Michal Koutný, Tejun Heo, Johannes Weiner,
Shuah Khan, cgroups, linux-kselftest, linux-kernel, Ziyang Men,
Tao Cui
Hi, Suren
在 2026/8/19 13:47, Suren Baghdasaryan 写道:
> On Thu, Aug 13, 2026 at 6:37 AM Tao Cui <cui.tao@linux.dev> wrote:
>>
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> The cgroup selftests have no PSI coverage. Add test_psi.c: per-resource
>> trigger smoke tests (one trigger per fd, IRQ full-only), a
>> cgroup.pressure hide/show toggle test, and a CPU-pressure trigger test
>> using over-subscription. Skips when PSI is disabled or a resource is
>> absent.
>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>>
>> ---
>> Changes since v2 (Suren Baghdasaryan, Michal Koutny review):
>> - Restructure the trigger test into per-resource cases (io, memory, cpu,
>> irq) so a failure points at the specific resource; irq is skipped when
>> /proc/pressure/irq is absent.
>> - Spawn the CPU hogs with cg_run_nowait() instead of open-coding fork(),
>> and arm the trigger with a 2s window so unprivileged users can set it.
>> - Address the remaining review comments on cleanup and robustness:
>> guard teardown with a "created" flag, use cg_read_strcmp() instead of
>> atoi(), report strerror() on errors, and fix the unused-parameter and
>> sign-compare nits.
>>
>> Changes since v1 (Michal Koutny, sashiko review):
>> - Keep trigger tests smoke-level; switch the firing test from memory to
>> CPU pressure; drop churn_memory().
>> - Keep the runner out of the cgroup; add PSI/IRQ skip-guards and a
>> .gitignore entry.
>>
>> Link: https://lore.kernel.org/all/20260724025826.504586-1-cui.tao@linux.dev/
>> Link: https://lore.kernel.org/all/20260728083742.2359320-1-cui.tao@linux.dev/
>> ---
>> 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 | 296 ++++++++++++++++++++++
>> 4 files changed, 300 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..cfb412f61458
>> --- /dev/null
>> +++ b/tools/testing/selftests/cgroup/test_psi.c
>> @@ -0,0 +1,296 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#define _GNU_SOURCE
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <poll.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <linux/limits.h>
>> +
>> +#include "kselftest.h"
>> +#include "cgroup_util.h"
>> +
>> +#define PSI_POLL_TIMEOUhas this check T_MS 5000
>> +
>> +/* PSI triggers are written with a trailing NUL the kernel parser expects. */
>> +static ssize_t write_trigger(int fd, const char *trigger)
>> +{
>> + return write(fd, trigger, strlen(trigger) + 1);
>> +}
>> +
>> +static int pressure_open(const char *resource)
>> +{
>> + char path[PATH_MAX];
>> + int fd;
>> +
>> + snprintf(path, sizeof(path), "/proc/pressure/%s", resource);
>> + fd = open(path, O_RDWR);
>> + if (fd < 0)
>> + ksft_print_msg("open %s: %s\n", path, strerror(errno));
>> + return fd;
>> +}
>> +
>> +/*
>> + * /proc/pressure/<resource> accepts exactly one trigger per file
>> + * descriptor. For io, memory and cpu verify that a "some" trigger arms
>> + * and that a second trigger on the same fd is rejected with EBUSY. For
>> + * irq, which only tracks "full", verify that "some" is rejected and
>> + * "full" arms. irq is optional -- it only exists with IRQ-time
>> + * accounting -- so a missing /proc/pressure/irq is SKIP, not FAIL.
>> + */
>> +static int test_proc_trigger(const char *resource, bool full_only)
>> +{
>> + int fd, ret = KSFT_FAIL;
>> +
>> + fd = pressure_open(resource);
>> + if (fd < 0)
>> + return full_only ? KSFT_SKIP : KSFT_FAIL;
>> +
>> + if (!full_only) {
>> + if (write_trigger(fd, "some 150000 2000000") <= 0) {
>> + ksft_print_msg("%s: 'some' trigger rejected: %s\n",
>> + resource, strerror(errno));
>> + goto out;
>> + }
>> + if (write_trigger(fd, "full 150000 2000000") != -1 ||
>> + errno != EBUSY) {
>> + ksft_print_msg("%s: second trigger not EBUSY\n",
>> + resource);
>> + goto out;
>> + }
>> + } else {
>> + if (write_trigger(fd, "some 150000 2000000") != -1) {
>> + ksft_print_msg("irq: 'some' trigger unexpectedly accepted\n");
>> + goto out;
>> + }
>> + if (write_trigger(fd, "full 150000 2000000") <= 0) {
>> + ksft_print_msg("irq: 'full' trigger rejected: %s\n",
>> + strerror(errno));
>> + goto out;
>> + }
>> + }
>> +
>> + ret = KSFT_PASS;
>> +out:
>> + close(fd);
>> + return ret;
>
> You don't really need "ret" variable. Just "return KSFT_FAIL;" here is enough.
>
Done. The harness conversion took care of this one: the switch/case
reporting is gone and the failure paths are just ASSERTs now.
>> +}
>> +
>> +/*
>> + * cgroup.pressure gates visibility of the per-resource *.pressure files
>> + * inside a cgroup: writing 0 hides them, writing 1 shows them again.
>> + * Drive one hide/show cycle and check that memory.pressure appears and
>> + * disappears along with it.
>> + */
>> +static int test_cgroup_pressure_toggle(const char *root)
>> +{
>> + char buf[BUF_SIZE];
>> + char *cg = NULL;
>> + int ret = KSFT_FAIL, created = 0;
>
> Again, you don't need "ret" variable here.
>
>> +
>> + cg = cg_name(root, "psi_toggle_test");
>> + if (!cg)
>> + goto cleanup;
>> + if (cg_create(cg)) {
>> + ksft_print_msg("cg_create: %s\n", strerror(errno));
>> + goto cleanup;
>> + }
>> + created = 1;
>
> Nope. The way we usually do the cleanup is by jumping to the
> appropriate label instead of adding extra state flags. In this case
> that would look something like this:
>
> cg = cg_name(root, "psi_toggle_test");
> if (!cg)
> return KSFT_FAIL;
>
> if (cg_create(cg)) {
> ksft_print_msg("cg_create: %s\n", strerror(errno));
> goto free_cg;
> }
>
> if (cg_write(cg, "cgroup.pressure", "0")) {
> ksft_print_msg("write cgroup.pressure=0: %s\n", strerror(errno));
> goto cleanup;
> }
> ...
> return KSFT_PASS;
> cleanup:
> cg_destroy(cg);
> free_cg:
> free(cg);
> return KSFT_FAIL;
> }
>
You're right, that reads better. I ended up going one step further and
moving the teardown into FIXTURE_TEARDOWN() instead: the cgroup is
created inside the test and self->cg is NULL until then, so the
teardown is a no-op for the tests that don't create one. That way
there are no labels and no state flags left.
>> +
>> + if (cg_write(cg, "cgroup.pressure", "0")) {
>> + ksft_print_msg("write cgroup.pressure=0: %s\n", strerror(errno));
>> + goto cleanup;
>> + }
>> + if (cg_read_strcmp(cg, "cgroup.pressure", "0\n")) {
>> + ksft_print_msg("cgroup.pressure readback != 0\n");
>> + goto cleanup;
>> + }
>> + if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) >= 0) {
>> + ksft_print_msg("memory.pressure readable while hidden\n");
>> + goto cleanup;
>> + }
>> +
>> + if (cg_write(cg, "cgroup.pressure", "1")) {
>> + ksft_print_msg("write cgroup.pressure=1: %s\n", strerror(errno));
>> + goto cleanup;
>> + }
>> + if (cg_read_strcmp(cg, "cgroup.pressure", "1\n")) {
>> + ksft_print_msg("cgroup.pressure readback != 1\n");
>> + goto cleanup;
>> + }
>> + if (cg_read(cg, "memory.pressure", buf, sizeof(buf)) < 0) {
>> + ksft_print_msg("memory.pressure unreadable after enabling\n");
>> + goto cleanup;
>> + }
>> +
>> + ret = KSFT_PASS;
>> +cleanup:
>> + if (created)
>> + cg_destroy(cg);
>> + free(cg);
>> + return ret;
>> +}
>> +
>> +/* A child that burns CPU forever; stopped by cg_killall() in the parent. */
>> +static int hog_cpu(const char *cgroup, void *arg)
>> +{
>> + for (;;)
>> + ;
>
> This loop looks strange. Maybe:
>
> for (;;) {}
>
Fixed.
>> + return 0;
>> +}
>> +
>> +/*
>> + * Arm a "some" trigger on a cgroup's cpu.pressure, oversubscribe the
>> + * cgroup with more spinning hogs than there are CPUs, and check that the
>> + * trigger fires once the cgroup stalls on CPU.
>> + */
>> +static int test_cgroup_trigger_fire(const char *root)
>> +{
>> + char *cg = NULL, *cpupress = NULL;
>> + int fd = -1, ret = KSFT_FAIL, created = 0, i;
>
> Please define each variable on a separate line.
>
Fixed.
>> + long ncpus;
>> +
>> + cg = cg_name(root, "psi_trigger_test");
>> + if (!cg)
>> + goto cleanup;
>> + if (cg_create(cg)) {
>> + ksft_print_msg("cg_create: %s\n", strerror(errno));
>> + goto cleanup;
>> + }
>> + created = 1;
>
> Same as above. Jump to the appropriate label.
>
>> +
>> + 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;
>> + }
>> +
>> + /*
>> + * 1usec threshold over a 2s window: any CPU stall fires it. The 2s
>> + * window is the smallest unprivileged users are allowed to arm.
>> + */
>> + if (write_trigger(fd, "some 1 2000000") <= 0) {
>> + ksft_print_msg("arm trigger: %s\n", strerror(errno));
>> + goto cleanup;
>> + }
>> +
>> + ncpus = sysconf(_SC_NPROCESSORS_ONLN);
>> + if (ncpus <= 0) {
>
> I don't think this syscall will ever return 0, so you just need to
> check for -1. Also reporting -1 in the error log isn't very
> informative. Instead report strerror(errno).
>
Agreed, now checks only for -1 and reports strerror(errno) via TH_LOG.
>> + ksft_print_msg("sysconf(_SC_NPROCESSORS_ONLN) returned %ld\n",
>> + ncpus);
>> + goto cleanup;
>> + }
>> +
>> + /* ncpus+1 hogs guarantee CPU contention inside the cgroup. */
>> + for (i = 0; i < ncpus + 1; i++) {
>> + if (cg_run_nowait(cg, hog_cpu, NULL) < 0) {
>> + ksft_print_msg("spawn hog %d: %s\n", i, strerror(errno));
>> + goto cleanup;
>> + }
>> + }
>> +
>> + struct pollfd pfd = { .fd = fd, .events = POLLPRI };
>> +
>> + switch (poll(&pfd, 1, PSI_POLL_TIMEOUT_MS)) {
>> + case -1:
>> + ksft_print_msg("poll: %s\n", strerror(errno));
>> + goto cleanup;
>
> So, goto cleanup here but break everywhere else? Consistency would be
> nice, so please use break here as well.
>
The switch is gone along with the ret bookkeeping, so this is moot
now: poll() collapsed into a single ASSERT_EQ(1, poll(...)).
>> + case 0:
>> + ksft_print_msg("trigger did not fire (could not induce CPU pressure)\n");
>> + ret = KSFT_SKIP;
>
> This means your trigger timed out. Why is this treated as KSFT_SKIP
> and not as KSFT_FAIL?
Fair point, I agree. With ncpus+1 hogs and a 1usec threshold over a 2s
window the cgroup must stall, so if the trigger doesn't fire something
is actually broken. I made it a FAIL.
>
>> + break;
>> + default:
>> + if (pfd.revents & POLLPRI)
>> + ret = KSFT_PASS;
>> + else
>> + ksft_print_msg("poll returned 0x%x\n", pfd.revents);
>> + break;
>> + }
>> +
>> +cleanup:
>> + if (fd >= 0)
>> + close(fd);
>> + if (created) {
>> + cg_killall(cg);
>> + cg_destroy(cg);
>> + }
>> + free(cpupress);
>> + free(cg);
>> + return ret;
>> +}
>> +
>> +struct psi_proc_test {
>> + const char *name;
>> + const char *resource;
>> + bool full_only;
>> +};
>> +static const struct psi_proc_test proc_tests[] = {
>> + { "proc_trigger_io", "io", false },
>> + { "proc_trigger_memory", "memory", false },
>> + { "proc_trigger_cpu", "cpu", false },
>> + { "proc_trigger_irq", "irq", true },
>> +};
>> +
>> +struct psi_cg_test {
>> + const char *name;
>> + int (*fn)(const char *root);
>> +};
>> +static const struct psi_cg_test cg_tests[] = {
>> + { "cgroup_pressure_toggle", test_cgroup_pressure_toggle },
>> + { "cgroup_trigger_fire", test_cgroup_trigger_fire },
>> +};
>> +
>> +int main(int argc, char **argv)
>> +{
>> + char root[PATH_MAX];
>> + int psi_fd, i;
>> +
>> + ksft_print_header();
>> + ksft_set_plan(ARRAY_SIZE(proc_tests) + ARRAY_SIZE(cg_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 disabled on the cmdline). */
>> + psi_fd = open("/proc/pressure/memory", O_RDONLY);
>> + if (psi_fd < 0)
>> + ksft_exit_skip("PSI unavailable (CONFIG_PSI=n or psi=0)\n");
>> + close(psi_fd);
>> +
>> + for (i = 0; i < ARRAY_SIZE(proc_tests); i++) {
>> + switch (test_proc_trigger(proc_tests[i].resource,
>> + proc_tests[i].full_only)) {
>> + case KSFT_PASS:
>> + ksft_test_result_pass("%s\n", proc_tests[i].name);
>> + break;
>> + case KSFT_SKIP:
>> + ksft_test_result_skip("%s\n", proc_tests[i].name);
>> + break;
>> + default:
>> + ksft_test_result_fail("%s\n", proc_tests[i].name);
>> + break;
>> + }
>> + }
>> +
>> + for (i = 0; i < ARRAY_SIZE(cg_tests); i++) {
>> + switch (cg_tests[i].fn(root)) {
>> + case KSFT_PASS:
>> + ksft_test_result_pass("%s\n", cg_tests[i].name);
>> + break;
>> + case KSFT_SKIP:
>> + ksft_test_result_skip("%s\n", cg_tests[i].name);
>> + break;
>> + default:
>> + ksft_test_result_fail("%s\n", cg_tests[i].name);
>> + break;
>> + }
>> + }
>
> Please convert this to use
> https://docs.kernel.org/dev-tools/kselftest.html#test-harness as I
> suggested before. This code will look much simpler. Even if some other
> tests are not using it, there is no reason for new tests not to take
> advantage of the existing harness. You will find planty of examples
> which use them.
>
Converted, thanks for the pointer. Each case is a TEST_F(psi, ...) now;
the cgroup v2 and PSI availability checks live in FIXTURE_SETUP() and
SKIP there, and the hog killing and cgroup destruction live in
FIXTURE_TEARDOWN(). It does make the code simpler, as you predicted.
The hogs also set PR_SET_PDEATHSIG on themselves now, so an interrupted
run doesn't leave orphans pinning every CPU.
With that the file shrinks from 296 to 206 lines. I tested it on a few
setups (PSI on and off, with and without /proc/pressure/irq): 6/6 pass
where everything is available, clean skips otherwise.
Thanks a lot for the review and for your patience with all the iterations.
Thanks,
Tao>> +
>> + ksft_finished();
>> +}
>> --
>> 2.43.0
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-24 8:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 13:37 [PATCH v3] selftests/cgroup: add PSI pressure trigger and validation tests Tao Cui
2026-08-19 5:47 ` Suren Baghdasaryan
2026-08-24 8:48 ` Tao Cui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).