Re: [PATCH v3] sched/core: add forced idle accounting for cgroups

From: Josh Don
Date: Mon Jun 27 2022 - 18:35:42 EST


Thanks Tejun,

On Mon, Jun 27, 2022 at 2:16 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
>
> Hello,
>
> On Tue, Jun 21, 2022 at 04:44:43PM -0700, Josh Don wrote:
> > 4feee7d1260 previously added per-task forced idle accounting. This patch
> > extends this to also include cgroups.
> >
> > rstat is used for cgroup accounting, except for the root, which uses
> > kcpustat in order to bypass the need for doing an rstat flush when
> > reading root stats.
> >
> > Only cgroup v2 is supported. Similar to the task accounting, the cgroup
> > accounting requires that schedstats is enabled.
>
> Generally looks fine from cgroup side. One nitpick follow.
>
> > + seq_printf(seq, "forceidle_usec %llu\n", forceidle_time);
>
> Would it make sense to namespace the name to reflect the fact that
> this is tied to core scheduling? e.g. something like
> core.force_idle_usec (and yeah, underscore between words, please). I
> kinda hate that the feature is named "core". The word is so
> overloaded.

Sure, although a namespace of "core_sched" would be a bit clearer,
since as you point out "core" is pretty overloaded :)

Lack of underscore for forceidle was to be consistent with
"core_forceidle_sum" being dumped from /proc/pid/sched, but I'm fine
with it either way.

So,

core_sched.force_idle_usec ?

>
> Thanks.
>
> --
> tejun