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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tdwyqv3qqbecnxhps5qkdux2atei7ydbduvx5uzm5q6ouw52y6@lqx2bity5fci>
Date: Thu, 26 Sep 2024 20:10:21 +0200
From: Michal Koutný <mkoutny@...e.com>
To: Joshua Hahn <joshua.hahnjy@...il.com>
Cc: tj@...nel.org, cgroups@...r.kernel.org, hannes@...xchg.org, 
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, lizefan.x@...edance.com, 
	shuah@...nel.org
Subject: Re: [PATCH v3 1/2] cgroup/rstat: Tracking cgroup-level niced CPU time

On Mon, Sep 23, 2024 at 07:20:05AM GMT, Joshua Hahn <joshua.hahnjy@...il.com> 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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)