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

["Michal Koutný" <mkoutny@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

Hi,
thanks for continuing with this.

On Tue, Jul 28, 2026 at 04:37:42PM +0800, Tao Cui <cui.tao@linux.dev> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]