On Thu, Aug 6, 2026 at 9:52 AM Michal Koutný <[email protected]> wrote: > > Hi, > thanks for continuing with this.
Thanks for adding these tests and sorry I was not able to review them yet. I'll try to review them this weekend and will provide my feedback. Thanks, Suren. > > On Tue, Jul 28, 2026 at 04:37:42PM +0800, Tao Cui <[email protected]> wrote: > > +/* Trigger smoke test: second on same fd -> EBUSY; IRQ rejects "some". */ > > +static int test_proc_triggers(const char *root) > > +{ > > + static const char *const resources[] = { "io", "memory", "cpu" }; > > + int ret = KSFT_FAIL; > > + int fd = -1; > > + int i; > > + > > + (void)root; > > WTH? > > > + > > + for (i = 0; i < (int)ARRAY_SIZE(resources); i++) { > > This (int) cast is quite uncommon, look around. > > > +/* 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; > > + > > + 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; > > + } > > When I see this, could you increase the window size to 2000000 (so that > it has potential to run for unprivileged users)? > > > + > > + ncpus = sysconf(_SC_NPROCESSORS_ONLN); > > + if (ncpus <= 0) > > + 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); > > + } > > This part could be replaced with generalized hog_cpus_timed() from > test_cpu.c (after move to cgroup_util.c). > > > +int main(int argc, char **argv) > > +{ > > + char root[PATH_MAX]; > > + int mempress_fd; > > + int i; > > + > > + (void)argc; > > What's up with this? > > > + > > + 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"); > > The memory controller is unnecessary now, right? > > > Regards, > Michal

