* [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently.
@ 2020-04-20 7:04 Muchun Song
2020-05-19 18:44 ` [tip: sched/core] sched/cpuacct: Fix charge cpuacct.usage_sys tip-bot2 for Muchun Song
2021-03-02 17:43 ` [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Michal Koutný
0 siblings, 2 replies; 4+ messages in thread
From: Muchun Song @ 2020-04-20 7:04 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
rostedt, bsegall, mgorman, mingo
Cc: linux-kernel, Muchun Song
The user_mode(task_pt_regs(tsk)) always return true for
user thread, and false for kernel thread. So it means that
the cpuacct.usage_sys is the time that kernel thread uses
not the time that thread uses in the kernel mode. We can
try get_irq_regs() first, if it is NULL, then we can fall
back to task_pt_regs().
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
Changes in v2:
1. we use get_irq_regs() first, if it's NULL, fall back to task_pt_regs()
kernel/sched/cpuacct.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 6448b0438ffb2..941c28cf97384 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -5,6 +5,7 @@
* Based on the work by Paul Menage (menage@google.com) and Balbir Singh
* (balbir@in.ibm.com).
*/
+#include <asm/irq_regs.h>
#include "sched.h"
/* Time spent by the tasks of the CPU accounting group executing in ... */
@@ -339,7 +340,7 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
{
struct cpuacct *ca;
int index = CPUACCT_STAT_SYSTEM;
- struct pt_regs *regs = task_pt_regs(tsk);
+ struct pt_regs *regs = get_irq_regs() ? : task_pt_regs(tsk);
if (regs && user_mode(regs))
index = CPUACCT_STAT_USER;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip: sched/core] sched/cpuacct: Fix charge cpuacct.usage_sys
2020-04-20 7:04 [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Muchun Song
@ 2020-05-19 18:44 ` tip-bot2 for Muchun Song
2021-03-02 17:43 ` [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Michal Koutný
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot2 for Muchun Song @ 2020-05-19 18:44 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Muchun Song, Peter Zijlstra (Intel), x86, LKML
The following commit has been merged into the sched/core branch of tip:
Commit-ID: dbe9337109c2705f08e6a00392f991eb2d2570a5
Gitweb: https://git.kernel.org/tip/dbe9337109c2705f08e6a00392f991eb2d2570a5
Author: Muchun Song <songmuchun@bytedance.com>
AuthorDate: Mon, 20 Apr 2020 15:04:53 +08:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 May 2020 20:34:14 +02:00
sched/cpuacct: Fix charge cpuacct.usage_sys
The user_mode(task_pt_regs(tsk)) always return true for
user thread, and false for kernel thread. So it means that
the cpuacct.usage_sys is the time that kernel thread uses
not the time that thread uses in the kernel mode. We can
try get_irq_regs() first, if it is NULL, then we can fall
back to task_pt_regs().
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200420070453.76815-1-songmuchun@bytedance.com
---
kernel/sched/cpuacct.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 6448b04..941c28c 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -5,6 +5,7 @@
* Based on the work by Paul Menage (menage@google.com) and Balbir Singh
* (balbir@in.ibm.com).
*/
+#include <asm/irq_regs.h>
#include "sched.h"
/* Time spent by the tasks of the CPU accounting group executing in ... */
@@ -339,7 +340,7 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
{
struct cpuacct *ca;
int index = CPUACCT_STAT_SYSTEM;
- struct pt_regs *regs = task_pt_regs(tsk);
+ struct pt_regs *regs = get_irq_regs() ? : task_pt_regs(tsk);
if (regs && user_mode(regs))
index = CPUACCT_STAT_USER;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently.
2020-04-20 7:04 [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Muchun Song
2020-05-19 18:44 ` [tip: sched/core] sched/cpuacct: Fix charge cpuacct.usage_sys tip-bot2 for Muchun Song
@ 2021-03-02 17:43 ` Michal Koutný
2021-03-03 3:39 ` [External] " Muchun Song
1 sibling, 1 reply; 4+ messages in thread
From: Michal Koutný @ 2021-03-02 17:43 UTC (permalink / raw)
To: Muchun Song
Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
rostedt, bsegall, mgorman, mingo, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]
Hello.
(Sorry for necroposting, found this upstream reference only now.)
On Mon, Apr 20, 2020 at 03:04:53PM +0800, Muchun Song <songmuchun@bytedance.com> wrote:
> /* Time spent by the tasks of the CPU accounting group executing in ... */
> @@ -339,7 +340,7 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
> {
> struct cpuacct *ca;
> int index = CPUACCT_STAT_SYSTEM;
> - struct pt_regs *regs = task_pt_regs(tsk);
> + struct pt_regs *regs = get_irq_regs() ? : task_pt_regs(tsk);
I've read the discussion in [1] but I don't think this approach is
correct either (and I don't know what is better :-/).
I only have a qualitative proof:
host:~ # uname -r
5.10.16-1-default
host:~ # systemd-run -p CPUAccounting=yes sh -c 'time sh -c "i=0 ; while [ \"\$i\" -lt 10000 ] ; do i=\$((\$i+1)) ; cat /proc/slabinfo >/dev/null ; done" ; sleep inf'
Running as unit: run-r101b9f53efcb4d2a9bfb65feb6f120ca.service
host:~ # cat /sys/fs/cgroup/cpuacct/system.slice/run-r101b9f53efcb4d2a9bfb65feb6f120ca.service/cpuacct.usage{,_user,_sys}
16138535165
14332580468
1805954697
(See that sys/user ~ 0.1)
host:~ # journalctl -u run-r101b9f53efcb4d2a9bfb65feb6f120ca.service
-- Logs begin at Tue 2021-03-02 18:06:41 CET, end at Tue 2021-03-02 18:27:45 CET. --
Mar 02 18:27:29 host systemd[1]: Started /usr/bin/sh -c time sh -c "i=0 ; while [ \"\$i\" -lt 10000 ] ; do i=\$((\$i+1)) ; cat /proc/slabinfo >/dev/null ; done" ; sleep inf.
Mar 02 18:27:45 host sh[19117]: real 0m15.543s
Mar 02 18:27:45 host sh[19117]: user 0m10.752s
Mar 02 18:27:45 host sh[19117]: sys 0m5.379s
(See that sys/user ~ 0.5)
host:~ # cat /sys/fs/cgroup/cpuacct/system.slice/run-r101b9f53efcb4d2a9bfb65feb6f120ca.service/cpuacct.stat
user 415
system 1209
(See that sys/user ~ 3.0 :-o)
The expectation is that significant amount of the loop is spent in
kernel (dumping slabinfo). I can't tell which of the ratios fits the
reality best but the cpuacct.usage_sys still seems too low.
Michal
[1] https://lore.kernel.org/lkml/20200416141833.50663-1-songmuchun@bytedance.com/
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently.
2021-03-02 17:43 ` [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Michal Koutný
@ 2021-03-03 3:39 ` Muchun Song
0 siblings, 0 replies; 4+ messages in thread
From: Muchun Song @ 2021-03-03 3:39 UTC (permalink / raw)
To: Michal Koutný
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
dietmar.eggemann, Steven Rostedt, Benjamin Segall, Mel Gorman,
Ingo Molnar, LKML
On Wed, Mar 3, 2021 at 1:43 AM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hello.
>
> (Sorry for necroposting, found this upstream reference only now.)
>
> On Mon, Apr 20, 2020 at 03:04:53PM +0800, Muchun Song <songmuchun@bytedance.com> wrote:
> > /* Time spent by the tasks of the CPU accounting group executing in ... */
> > @@ -339,7 +340,7 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
> > {
> > struct cpuacct *ca;
> > int index = CPUACCT_STAT_SYSTEM;
> > - struct pt_regs *regs = task_pt_regs(tsk);
> > + struct pt_regs *regs = get_irq_regs() ? : task_pt_regs(tsk);
> I've read the discussion in [1] but I don't think this approach is
> correct either (and I don't know what is better :-/).
Yes. It didn't solve the problem completely. I am trying to
count the sys time as much as possible, but the sys time here
still can be lower than the real situation. I didn't think of a better
solution.
>
> I only have a qualitative proof:
>
> host:~ # uname -r
> 5.10.16-1-default
>
> host:~ # systemd-run -p CPUAccounting=yes sh -c 'time sh -c "i=0 ; while [ \"\$i\" -lt 10000 ] ; do i=\$((\$i+1)) ; cat /proc/slabinfo >/dev/null ; done" ; sleep inf'
> Running as unit: run-r101b9f53efcb4d2a9bfb65feb6f120ca.service
>
> host:~ # cat /sys/fs/cgroup/cpuacct/system.slice/run-r101b9f53efcb4d2a9bfb65feb6f120ca.service/cpuacct.usage{,_user,_sys}
> 16138535165
> 14332580468
> 1805954697
>
> (See that sys/user ~ 0.1)
>
> host:~ # journalctl -u run-r101b9f53efcb4d2a9bfb65feb6f120ca.service
> -- Logs begin at Tue 2021-03-02 18:06:41 CET, end at Tue 2021-03-02 18:27:45 CET. --
> Mar 02 18:27:29 host systemd[1]: Started /usr/bin/sh -c time sh -c "i=0 ; while [ \"\$i\" -lt 10000 ] ; do i=\$((\$i+1)) ; cat /proc/slabinfo >/dev/null ; done" ; sleep inf.
> Mar 02 18:27:45 host sh[19117]: real 0m15.543s
> Mar 02 18:27:45 host sh[19117]: user 0m10.752s
> Mar 02 18:27:45 host sh[19117]: sys 0m5.379s
>
> (See that sys/user ~ 0.5)
>
> host:~ # cat /sys/fs/cgroup/cpuacct/system.slice/run-r101b9f53efcb4d2a9bfb65feb6f120ca.service/cpuacct.stat
> user 415
> system 1209
>
> (See that sys/user ~ 3.0 :-o)
>
> The expectation is that significant amount of the loop is spent in
> kernel (dumping slabinfo). I can't tell which of the ratios fits the
> reality best but the cpuacct.usage_sys still seems too low.
>
>
> Michal
>
> [1] https://lore.kernel.org/lkml/20200416141833.50663-1-songmuchun@bytedance.com/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-03 11:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 7:04 [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Muchun Song
2020-05-19 18:44 ` [tip: sched/core] sched/cpuacct: Fix charge cpuacct.usage_sys tip-bot2 for Muchun Song
2021-03-02 17:43 ` [PATCH v2] sched/cpuacct: Fix charge cpuacct.usage_sys incorrently Michal Koutný
2021-03-03 3:39 ` [External] " Muchun Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).