Re: [PATCH v3 2/2] cgroup/rstat: Selftests for niced CPU statistics

[Michal Koutný]

On Mon, Sep 23, 2024 at 07:20:06AM GMT, Joshua Hahn <joshua.hah...@gmail.com> 
wrote:
> +/*
> + * Creates a nice process that consumes CPU and checks that the elapsed
> + * usertime in the cgroup is close to the expected time.
> + */
> +static int test_cpucg_nice(const char *root)
> +{
> +     int ret = KSFT_FAIL;
> +     int status;
> +     long user_usec, nice_usec;
> +     long usage_seconds = 2;
> +     long expected_nice_usec = usage_seconds * USEC_PER_SEC;
> +     char *cpucg;
> +     pid_t pid;
> +
> +     cpucg = cg_name(root, "cpucg_test");
> +     if (!cpucg)
> +             goto cleanup;
> +
> +     if (cg_create(cpucg))
> +             goto cleanup;
> +
> +     user_usec = cg_read_key_long(cpucg, "cpu.stat", "user_usec");
> +     nice_usec = cg_read_key_long(cpucg, "cpu.stat", "nice_usec");
> +     if (user_usec != 0 || nice_usec != 0)
> +             goto cleanup;

Can you please distinguish a check between non-zero nice_usec and
non-existent nice_usec (KSFT_FAIL vs KSFT_SKIP)? So that the selftest is
usable on older kernels too.

> +
> +     /*
> +      * We fork here to create a new process that can be niced without
> +      * polluting the nice value of other selftests
> +      */
> +     pid = fork();
> +     if (pid < 0) {
> +             goto cleanup;
> +     } else if (pid == 0) {
> +             struct cpu_hog_func_param param = {
> +                     .nprocs = 1,
> +                     .ts = {
> +                             .tv_sec = usage_seconds,
> +                             .tv_nsec = 0,
> +                     },
> +                     .clock_type = CPU_HOG_CLOCK_PROCESS,
> +             };
> +
> +             /* Try to keep niced CPU usage as constrained to hog_cpu as 
> possible */
> +             nice(1);
> +             cg_run(cpucg, hog_cpus_timed, (void *)&param);

Notice that cg_run() does fork itself internally.
So you can call hog_cpus_timed(cpucg, (void *)&param) directly, no
need for the fork with cg_run(). (Alternatively substitute fork in this
test with the fork in cg_run() but with extension of cpu_hog_func_params
with the nice value.)


Thanks,
Michal

signature.asc
Description: PGP signature