* [PATCH v3 -next] cgroup/rstat: add force idle show helper
@ 2024-07-04 0:50 Chen Ridong
2024-07-04 1:14 ` Waiman Long
0 siblings, 1 reply; 3+ messages in thread
From: Chen Ridong @ 2024-07-04 0:50 UTC (permalink / raw)
To: tj, lizefan.x, hannes, longman; +Cc: cgroups, linux-kernel
In the function cgroup_base_stat_cputime_show, there are five
instances of #ifdef, which makes the code not concise.
To address this, add the function cgroup_force_idle_show
to make the code more succinct.
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
kernel/cgroup/rstat.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index fb8b49437573..1568558e36e5 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -594,49 +594,45 @@ static void root_cgroup_cputime(struct cgroup_base_stat *bstat)
}
}
+
+static void cgroup_force_idle_show(struct seq_file *seq, struct cgroup_base_stat *bstat)
+{
+#ifdef CONFIG_SCHED_CORE
+ u64 forceidle_time = bstat->forceidle_sum;
+
+ do_div(forceidle_time, NSEC_PER_USEC);
+ seq_printf(seq, "core_sched.force_idle_usec %llu\n", forceidle_time);
+#endif
+}
+
void cgroup_base_stat_cputime_show(struct seq_file *seq)
{
struct cgroup *cgrp = seq_css(seq)->cgroup;
u64 usage, utime, stime;
- struct cgroup_base_stat bstat;
-#ifdef CONFIG_SCHED_CORE
- u64 forceidle_time;
-#endif
if (cgroup_parent(cgrp)) {
cgroup_rstat_flush_hold(cgrp);
usage = cgrp->bstat.cputime.sum_exec_runtime;
cputime_adjust(&cgrp->bstat.cputime, &cgrp->prev_cputime,
&utime, &stime);
-#ifdef CONFIG_SCHED_CORE
- forceidle_time = cgrp->bstat.forceidle_sum;
-#endif
cgroup_rstat_flush_release(cgrp);
} else {
- root_cgroup_cputime(&bstat);
- usage = bstat.cputime.sum_exec_runtime;
- utime = bstat.cputime.utime;
- stime = bstat.cputime.stime;
-#ifdef CONFIG_SCHED_CORE
- forceidle_time = bstat.forceidle_sum;
-#endif
+ root_cgroup_cputime(&cgrp->bstat);
+ usage = cgrp->bstat.cputime.sum_exec_runtime;
+ utime = cgrp->bstat.cputime.utime;
+ stime = cgrp->bstat.cputime.stime;
}
do_div(usage, NSEC_PER_USEC);
do_div(utime, NSEC_PER_USEC);
do_div(stime, NSEC_PER_USEC);
-#ifdef CONFIG_SCHED_CORE
- do_div(forceidle_time, NSEC_PER_USEC);
-#endif
seq_printf(seq, "usage_usec %llu\n"
"user_usec %llu\n"
"system_usec %llu\n",
usage, utime, stime);
-#ifdef CONFIG_SCHED_CORE
- seq_printf(seq, "core_sched.force_idle_usec %llu\n", forceidle_time);
-#endif
+ cgroup_force_idle_show(seq, &cgrp->bstat);
}
/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3 -next] cgroup/rstat: add force idle show helper
2024-07-04 0:50 [PATCH v3 -next] cgroup/rstat: add force idle show helper Chen Ridong
@ 2024-07-04 1:14 ` Waiman Long
2024-07-04 1:22 ` chenridong
0 siblings, 1 reply; 3+ messages in thread
From: Waiman Long @ 2024-07-04 1:14 UTC (permalink / raw)
To: Chen Ridong, tj, lizefan.x, hannes; +Cc: cgroups, linux-kernel
On 7/3/24 20:50, Chen Ridong wrote:
> In the function cgroup_base_stat_cputime_show, there are five
> instances of #ifdef, which makes the code not concise.
> To address this, add the function cgroup_force_idle_show
> to make the code more succinct.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
> kernel/cgroup/rstat.c | 36 ++++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
> index fb8b49437573..1568558e36e5 100644
> --- a/kernel/cgroup/rstat.c
> +++ b/kernel/cgroup/rstat.c
> @@ -594,49 +594,45 @@ static void root_cgroup_cputime(struct cgroup_base_stat *bstat)
> }
> }
>
> +
> +static void cgroup_force_idle_show(struct seq_file *seq, struct cgroup_base_stat *bstat)
> +{
> +#ifdef CONFIG_SCHED_CORE
> + u64 forceidle_time = bstat->forceidle_sum;
> +
> + do_div(forceidle_time, NSEC_PER_USEC);
> + seq_printf(seq, "core_sched.force_idle_usec %llu\n", forceidle_time);
> +#endif
> +}
> +
> void cgroup_base_stat_cputime_show(struct seq_file *seq)
> {
> struct cgroup *cgrp = seq_css(seq)->cgroup;
> u64 usage, utime, stime;
> - struct cgroup_base_stat bstat;
> -#ifdef CONFIG_SCHED_CORE
> - u64 forceidle_time;
> -#endif
>
> if (cgroup_parent(cgrp)) {
> cgroup_rstat_flush_hold(cgrp);
> usage = cgrp->bstat.cputime.sum_exec_runtime;
> cputime_adjust(&cgrp->bstat.cputime, &cgrp->prev_cputime,
> &utime, &stime);
> -#ifdef CONFIG_SCHED_CORE
> - forceidle_time = cgrp->bstat.forceidle_sum;
> -#endif
> cgroup_rstat_flush_release(cgrp);
> } else {
> - root_cgroup_cputime(&bstat);
> - usage = bstat.cputime.sum_exec_runtime;
> - utime = bstat.cputime.utime;
> - stime = bstat.cputime.stime;
> -#ifdef CONFIG_SCHED_CORE
> - forceidle_time = bstat.forceidle_sum;
> -#endif
> + root_cgroup_cputime(&cgrp->bstat);
> + usage = cgrp->bstat.cputime.sum_exec_runtime;
> + utime = cgrp->bstat.cputime.utime;
> + stime = cgrp->bstat.cputime.stime;
Just a simple comment saying that you are reusing the root cgroup bstat
which is otherwise not being used will help readability.
Other than that, the change LGTM.
Cheers,
Longman
> }
>
> do_div(usage, NSEC_PER_USEC);
> do_div(utime, NSEC_PER_USEC);
> do_div(stime, NSEC_PER_USEC);
> -#ifdef CONFIG_SCHED_CORE
> - do_div(forceidle_time, NSEC_PER_USEC);
> -#endif
>
> seq_printf(seq, "usage_usec %llu\n"
> "user_usec %llu\n"
> "system_usec %llu\n",
> usage, utime, stime);
>
> -#ifdef CONFIG_SCHED_CORE
> - seq_printf(seq, "core_sched.force_idle_usec %llu\n", forceidle_time);
> -#endif
> + cgroup_force_idle_show(seq, &cgrp->bstat);
> }
>
> /* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3 -next] cgroup/rstat: add force idle show helper
2024-07-04 1:14 ` Waiman Long
@ 2024-07-04 1:22 ` chenridong
0 siblings, 0 replies; 3+ messages in thread
From: chenridong @ 2024-07-04 1:22 UTC (permalink / raw)
To: Waiman Long, tj, lizefan.x, hannes; +Cc: cgroups, linux-kernel
On 2024/7/4 9:14, Waiman Long wrote:
>
> On 7/3/24 20:50, Chen Ridong wrote:
>> In the function cgroup_base_stat_cputime_show, there are five
>> instances of #ifdef, which makes the code not concise.
>> To address this, add the function cgroup_force_idle_show
>> to make the code more succinct.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>> kernel/cgroup/rstat.c | 36 ++++++++++++++++--------------------
>> 1 file changed, 16 insertions(+), 20 deletions(-)
>>
>> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
>> index fb8b49437573..1568558e36e5 100644
>> --- a/kernel/cgroup/rstat.c
>> +++ b/kernel/cgroup/rstat.c
>> @@ -594,49 +594,45 @@ static void root_cgroup_cputime(struct
>> cgroup_base_stat *bstat)
>> }
>> }
>> +
>> +static void cgroup_force_idle_show(struct seq_file *seq, struct
>> cgroup_base_stat *bstat)
>> +{
>> +#ifdef CONFIG_SCHED_CORE
>> + u64 forceidle_time = bstat->forceidle_sum;
>> +
>> + do_div(forceidle_time, NSEC_PER_USEC);
>> + seq_printf(seq, "core_sched.force_idle_usec %llu\n",
>> forceidle_time);
>> +#endif
>> +}
>> +
>> void cgroup_base_stat_cputime_show(struct seq_file *seq)
>> {
>> struct cgroup *cgrp = seq_css(seq)->cgroup;
>> u64 usage, utime, stime;
>> - struct cgroup_base_stat bstat;
>> -#ifdef CONFIG_SCHED_CORE
>> - u64 forceidle_time;
>> -#endif
>> if (cgroup_parent(cgrp)) {
>> cgroup_rstat_flush_hold(cgrp);
>> usage = cgrp->bstat.cputime.sum_exec_runtime;
>> cputime_adjust(&cgrp->bstat.cputime, &cgrp->prev_cputime,
>> &utime, &stime);
>> -#ifdef CONFIG_SCHED_CORE
>> - forceidle_time = cgrp->bstat.forceidle_sum;
>> -#endif
>> cgroup_rstat_flush_release(cgrp);
>> } else {
>> - root_cgroup_cputime(&bstat);
>> - usage = bstat.cputime.sum_exec_runtime;
>> - utime = bstat.cputime.utime;
>> - stime = bstat.cputime.stime;
>> -#ifdef CONFIG_SCHED_CORE
>> - forceidle_time = bstat.forceidle_sum;
>> -#endif
>> + root_cgroup_cputime(&cgrp->bstat);
>> + usage = cgrp->bstat.cputime.sum_exec_runtime;
>> + utime = cgrp->bstat.cputime.utime;
>> + stime = cgrp->bstat.cputime.stime;
>
> Just a simple comment saying that you are reusing the root cgroup bstat
> which is otherwise not being used will help readability.
>
> Other than that, the change LGTM.
Thank you, Longman. I will do that.
Regards,
Ridong
>
> Cheers,
> Longman
>
>> }
>> do_div(usage, NSEC_PER_USEC);
>> do_div(utime, NSEC_PER_USEC);
>> do_div(stime, NSEC_PER_USEC);
>> -#ifdef CONFIG_SCHED_CORE
>> - do_div(forceidle_time, NSEC_PER_USEC);
>> -#endif
>> seq_printf(seq, "usage_usec %llu\n"
>> "user_usec %llu\n"
>> "system_usec %llu\n",
>> usage, utime, stime);
>> -#ifdef CONFIG_SCHED_CORE
>> - seq_printf(seq, "core_sched.force_idle_usec %llu\n",
>> forceidle_time);
>> -#endif
>> + cgroup_force_idle_show(seq, &cgrp->bstat);
>> }
>> /* Add bpf kfuncs for cgroup_rstat_updated() and
>> cgroup_rstat_flush() */
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-04 1:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-04 0:50 [PATCH v3 -next] cgroup/rstat: add force idle show helper Chen Ridong
2024-07-04 1:14 ` Waiman Long
2024-07-04 1:22 ` chenridong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox