Re: [PATCH v3 1/2] cgroup/rstat: Tracking cgroup-level niced CPU time

[Michal Koutný]

On Mon, Sep 23, 2024 at 07:20:05AM GMT, Joshua Hahn <joshua.hahnjy@xxxxxxxxx> wrote:
>  static void cgroup_base_stat_flush(struct cgroup *cgrp, int cpu)
> @@ -535,7 +537,10 @@ void __cgroup_account_cputime_field(struct cgroup *cgrp,
>  
>  	switch (index) {
>  	case CPUTIME_USER:
> +		rstatc->bstat.cputime.utime += delta_exec;
> +		break;
>  	case CPUTIME_NICE:
> +		rstatc->bstat.ntime += delta_exec;
>  		rstatc->bstat.cputime.utime += delta_exec;
>  		break;

Nit: slightly better diffstat is possible with fallthrough:

        rstatc = cgroup_base_stat_cputime_account_begin(cgrp, &flags);

        switch (index) {
-       case CPUTIME_USER:
        case CPUTIME_NICE:
+               rstatc->bstat.ntime += delta_exec;
+               fallthrough;
+       case CPUTIME_USER:
                rstatc->bstat.cputime.utime += delta_exec;
                break;
        case CPUTIME_SYSTEM:

> @@ -622,16 +629,19 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq)
...
>  
>  	seq_printf(seq, "usage_usec %llu\n"
>  		   "user_usec %llu\n"
> -		   "system_usec %llu\n",
> -		   usage, utime, stime);
> +			 "system_usec %llu\n"
> +			 "nice_usec %llu\n",
> +			 usage, utime, stime, ntime);

This seems to be different whitespace alignment than user_usec above.

(Implementation looks good, I only have some remarks to the concept,
reply to cover letter.)

Michal

Attachment: signature.asc
Description: PGP signature