Re: [LTP] cgroup: Add cgroup_burst01 test

[linuxtestproject.agent@gmail.com]

Hi Samir,

On Mon, 28 Sep 2026 07:17:34 +0200, Samir Mulani wrote:
> cgroup: Add cgroup_burst01 test

--- [PATCH 1/2] ---

> cgroup: Add cgroup_burst01 test

The test and binary name is cpu_max_burst01. Update the subject line
to match the test name.

>   TC3 - A worker underruns to build up burst credit, then overloads
>         to consume it; cpu.stat nr_bursts and burst_usec counters
>         must increase. Also checks that burst can be updated while
>         tasks are running.

The test code never updates cpu.max.burst while tasks are running.
Update the commit message or add the missing check to the test.

> diff --git a/runtest/controllers b/runtest/controllers
> --- a/runtest/controllers
> +++ b/runtest/controllers
> @@ -3,6 +3,7 @@ cgroup_core01	cgroup_core01
>  cgroup_core02	cgroup_core02
>  cgroup_core03	cgroup_core03
>  cgroup		cgroup_regression_test.sh
> +cpu_max_burst01  cpu_max_burst01

Use a tab character instead of spaces between the test name and the
binary path.

>  * Test scenarios:
>  *   TC 1 - Default value and write-zero: cpu.max.burst defaults to 0;

Insert a blank line before the list in the documentation comment block.

> 		if (err != EINVAL) {
> 			tst_res(TWARN,
> 				"tc2_burst_exceeds_quota: got errno=%d (%s), expected EINVAL",
> 				err, tst_strerrno(err));
> 		}
> 
> 		if (burst != BURST_US) {
> 			tst_res(TFAIL,
> 				"tc2_burst_exceeds_quota: burst corrupted after rejected write (expected %llu us, got %llu us)",
> 				BURST_US, burst);
> 		} else {
> 			tst_res(TPASS,
> 				"tc2_burst_exceeds_quota: burst=%llu rejected (errno=%d), stored value unchanged (%llu us)",
> 				bad_burst, err, burst);
> 		}

If err != EINVAL, report TFAIL instead of TWARN. As written, receiving an
unexpected errno still reports TPASS when burst remains unchanged.

> 	read_burst_stat(cg_child, &after);

Check the return value of read_burst_stat(). If reading cpu.stat fails,
abort with TBROK rather than proceeding with zeroed stats to TFAIL.

> 	if (cmd_pipe[1] >= 0) {
> 		SAFE_CLOSE(cmd_pipe[1]);
> 		cmd_pipe[1] = -1;
> 	}
> 	if (ack_pipe[0] >= 0) {
> 		SAFE_CLOSE(ack_pipe[0]);
> 		ack_pipe[0] = -1;
> 	}

Use '!= -1' instead of '>= 0' to check file descriptor validity.

--- [PATCH 2/2] ---

> cgroup: Add invalid burst test

Name the test explicitly in the subject line (e.g. cgroup: Add
cpu_max_burst_invalid01 test).

> diff --git a/runtest/controllers b/runtest/controllers
> --- a/runtest/controllers
> +++ b/runtest/controllers
> @@ -4,6 +4,7 @@ cgroup_core02	cgroup_core02
>  cgroup_core03	cgroup_core03
>  cgroup		cgroup_regression_test.sh
>  cpu_max_burst01  cpu_max_burst01
> +cpu_max_burst_invalid01  cpu_max_burst_invalid01

Use a tab character instead of spaces between the test name and the
binary path.

>  * Test scenarios:
>  *   TC 1 - Boundary and sign violations: burst = quota+1 (B) and burst = "-1"

Insert a blank line before the list in the documentation comment block.

> /* MAX_BW_US: kernel max_bw_runtime_us = (1 << BW_SHIFT) * USEC_PER_SEC */
> #define MAX_BW_US	(((uint64_t)1 << 20) * 1000000ULL)

The kernel defines MAX_BW as ((1ULL << (64 - BW_SHIFT)) - 1), which is
(1ULL << 44) - 1. The current calculation is too small and does not exceed
the kernel limit, causing tc2 to always take the TCONF fallback path.

> static int cg_write_str(const struct tst_cg_group *cg,
> 			const char *fname, const char *str)
> {
> 	int fd, saved = 0;
> 	ssize_t n;
> 
> 	fd = SAFE_OPENAT(cg_dirfd(cg), fname, O_WRONLY);
> 	n  = write(fd, str, strlen(str));

Use TST_EXP_FAIL2(write(...), EINVAL) rather than calling bare write()
and propagating errno through helper return values.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp