sched/cputime: Fix NO_HZ_FULL getrusage() monotonicity regression
PATCH 1/1 sched/cputime: Fix NO_HZ_FULL getrusage() monotonicity regression
Hi Peter,
While running ltp, the fates decided it was time for me to encounter the roughly 1 out of every 10 call failure below. As much as I run ltp, I'm a bit surprised that I (or anyone else) haven't met this before, but then the fates are known to be a tad fickle.
getrusage04 0 TINFO : Expected timers granularity is 4000 us getrusage04 0 TINFO : Using 1 as multiply factor for max [us]time increment (1000+4000us)! getrusage04 0 TINFO : utime: 0us; stime: 179us getrusage04 0 TINFO : utime: 3751us; stime: 0us getrusage04 1 TFAIL : getrusage04.c:133: stime increased > 5000us:
When applying the full rtime to either stime or utime, do not overwrite the previously tallied value.
--- kernel/sched/cputime.c | 2 ++ 1 file changed, 2 insertions(+)
kernel/sched/cputime.c +2 -0
@@ -608,11 +608,13 @@ static void cputime_adjust(struct task_cif (utime ==0 ) { stime = rtime; + utime = prev->utime; goto update; }if (stime ==0 ) { utime = rtime; + stime = prev->stime; goto update; }
Peter Zijlstra
maintainerQuoted 28 lines · Mike Galbraith
Hi Peter,
While running ltp, the fates decided it was time for me to encounter the roughly 1 out of every 10 call failure below. As much as I run ltp, I'm a bit surprised that I (or anyone else) haven't met this before, but then the fates are known to be a tad fickle.
getrusage04 0 TINFO : Expected timers granularity is 4000 us getrusage04 0 TINFO : Using 1 as multiply factor for max [us]time increment (1000+4000us)! getrusage04 0 TINFO : utime: 0us; stime: 179us getrusage04 0 TINFO : utime: 3751us; stime: 0us getrusage04 1 TFAIL : getrusage04.c:133: stime increased > 5000us:When applying the full rtime to either stime or utime, do not overwrite the previously tallied value.
Fixes 9d7fb0427648 ("sched/cputime: Guarantee stime + utime == rtime")Signed-off-by Mike Galbraith <[email protected]>Cc [email protected] # 4.3+--- kernel/sched/cputime.c | 2 ++ 1 file changed, 2 insertions(+)kernel/sched/cputime.c +0
-0@@ -608,11 +608,13 @@ static void cputime_adjust(struct task_cif (utime ==0 ) {
kernel/sched/cputime.c static void cputime_adjust(struct task_c · line 619
Quoted 10 lines
kernel/sched/cputime.c +2
-0stime = rtime; + utime = prev->utime; goto update; }if (stime ==0 ) { utime = rtime; + stime = prev->stime; goto update; }
This cannot be right; it violates that utime+stime==rtime. Let me try and figure out what actually happens.
Peter Zijlstra
maintainerQuoted 34 lines · Peter Zijlstra
Quoted 33 lines · Mike Galbraith
Hi Peter,
While running ltp, the fates decided it was time for me to encounter the roughly 1 out of every 10 call failure below. As much as I run ltp, I'm a bit surprised that I (or anyone else) haven't met this before, but then the fates are known to be a tad fickle.
getrusage04 0 TINFO : Expected timers granularity is 4000 us getrusage04 0 TINFO : Using 1 as multiply factor for max [us]time increment (1000+4000us)! getrusage04 0 TINFO : utime: 0us; stime: 179us getrusage04 0 TINFO : utime: 3751us; stime: 0us getrusage04 1 TFAIL : getrusage04.c:133: stime increased > 5000us:When applying the full rtime to either stime or utime, do not overwrite the previously tallied value.
Fixes 9d7fb0427648 ("sched/cputime: Guarantee stime + utime == rtime")Signed-off-by Mike Galbraith <[email protected]>Cc [email protected] # 4.3+--- kernel/sched/cputime.c | 2 ++ 1 file changed, 2 insertions(+)kernel/sched/cputime.c +1
-0@@ -608,11 +608,13 @@ static void cputime_adjust(struct task_cif (utime ==0 ) { stime = rtime; + utime = prev->utime; goto update; }
Quoted 8 lines
kernel/sched/cputime.c static void cputime_adjust(struct task_c · line 619
Quoted 5 lines
kernel/sched/cputime.c +1
-0if (stime ==0 ) { utime = rtime; + stime = prev->stime; goto update; }This cannot be right; it violates that utime+stime==rtime. Let me try and figure out what actually happens.
Any idea where your [us]time are coming from? Do you end up in the vtime_accounting_enabled() path or not?
Quoted 2 lines · Peter Zijlstra
Any idea where your [us]time are coming from? Do you end up in the
vtime_accounting_enabled()path or not?
No, I'm not booting with nohz_full=.
-Mike
Quoted 12 lines · Peter Zijlstra
Quoted 10 lines · Mike Galbraith
kernel/sched/cputime.c +1
-0@@ -608,11 +608,13 @@ static void cputime_adjust(struct task_cif (utime ==0 ) { stime = rtime; + utime = prev->utime; goto update; }
Quoted 8 lines
kernel/sched/cputime.c static void cputime_adjust(struct task_c · line 619
Quoted 5 lines
kernel/sched/cputime.c +1
-0if (stime ==0 ) { utime = rtime; + stime = prev->stime; goto update; }This cannot be right; it violates that utime+stime==rtime. Let me try and figure out what actually happens.
How about this instead.
sched/cputime: Fix NO_HZ_FULL getrusage() monotonicity regression
Roughly 10% of the time, ltp testcase getrusage04 fails:
getrusage04 0 TINFO : Expected timers granularity is 4000 us getrusage04 0 TINFO : Using 1 as multiply factor for max [us]time increment (1000+4000us)! getrusage04 0 TINFO : utime: 0us; stime: 179us getrusage04 0 TINFO : utime: 3751us; stime: 0us getrusage04 1 TFAIL : getrusage04.c:133: stime increased > 5000us:
If ->sum_exec_runtime has moved beyond the rtime of ->prev_cputime, but no time has as yet been accounted to the task, bail.
--- kernel/sched/cputime.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/sched/cputime.c +7 -0
@@ -606,6 +606,13 @@ static void cputime_adjust(struct task_c stime = curr->stime; utime = curr->utime; +/* +* sum_exec_runtime has moved, but nothing has yet been +* accounted to the task, there's nothing to update. +*/ +if (utime + stime ==0 ) + goto out; +if (utime ==0 ) { stime = rtime; goto update;
Peter Zijlstra
maintainerQuoted 22 lines · Mike Galbraith
sched/cputime: Fix
NO_HZ_FULLgetrusage()monotonicity regressionRoughly 10% of the time, ltp testcase getrusage04 fails:
getrusage04 0 TINFO : Expected timers granularity is 4000 us getrusage04 0 TINFO : Using 1 as multiply factor for max [us]time increment (1000+4000us)! getrusage04 0 TINFO : utime: 0us; stime: 179us getrusage04 0 TINFO : utime: 3751us; stime: 0us getrusage04 1 TFAIL : getrusage04.c:133: stime increased > 5000us:If ->sum_exec_runtime has moved beyond the rtime of ->prev_cputime, but no time has as yet been accounted to the task, bail.
Fixes 9d7fb0427648 ("sched/cputime: Guarantee stime + utime == rtime")Signed-off-by Mike Galbraith <[email protected]>Cc [email protected] # 4.3+--- kernel/sched/cputime.c | 7 +++++++ 1 file changed, 7 insertions(+)
kernel/sched/cputime.c static void cputime_adjust(struct task_c · line 614
Quoted 9 lines
kernel/sched/cputime.c +6
-0@@ -606,6 +606,13 @@ static void cputime_adjust(struct task_c stime = curr->stime; utime = curr->utime; +/* +* sum_exec_runtime has moved, but nothing has yet been +* accounted to the task, there's nothing to update. +*/ +if (utime + stime ==0 ) + goto out;
urgh...
Valid scenario.. not sure about the solution though. This would mean the task has _no_ running time if it forever dodges the tick, which would be bad.
Does something like so cure things too?
--- kernel/sched/cputime.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
kernel/sched/cputime.c +10 -5
@@ -614,19 +614,25 @@ static void cputime_adjust(struct task_cputime *curr, stime = curr->stime; utime = curr->utime; -if (utime ==0 ) { - stime = rtime; +/* +* If either stime or both stime and utime are 0, assume all runtime is +* userspace. Once a task gets some ticks, the monotonicy code at +* 'update' will ensure things converge to the observed ratio. +*/ +if (stime ==0 ) { + utime = rtime; goto update; } -if (stime ==0 ) { - utime = rtime; +if (utime ==0 ) { + stime = rtime; goto update; } stime =scale_stime ((__force u64 )stime, (__force u64 )rtime, (__force u64)(stime + utime)); +update:/* * Make sure stime doesn't go backwards; this preserves monotonicity * for utime because rtime is monotonic.
@@ -649,7 +655,6 @@ static void cputime_adjust(struct task_cputime *curr, stime = rtime - utime; } -update: prev->stime = stime; prev->utime = utime; out:
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Mike Galbraith
+> > > > /*
+> > > > * sum_exec_runtime has moved, but nothing has yet been +> > > > * accounted to the task, there's nothing to update. +> > > > */+> > > > if (utime + stime == 0)
+> > > > > > goto out;
Quoted 7 lines
urgh...
Valid scenario.. not sure about the solution though. This would mean the task has _no_ running time if it forever dodges the tick, which would be bad.
Does something like so cure things too?
Yeah, it's a happy camper.
Quoted 46 lines
--- kernel/sched/cputime.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)kernel/sched/cputime.c +10
-5@@ -614,19 +614,25 @@ static void cputime_adjust(struct task_cputime *curr, > > stime = curr->stime; > > utime = curr->utime; -> > if (utime == 0) { -> > > stime = rtime; +> > /* +> > * If either stime or both stime and utime are 0, assume all runtime is +> > * userspace. Once a task gets some ticks, the monotonicy code at +> > * 'update' will ensure things converge to the observed ratio. +> > */ +> > if (stime == 0) { +> > > utime = rtime; > > > goto update; > > } -> > if (stime == 0) { -> > > utime = rtime; +> > if (utime == 0) { +> > > stime = rtime; > > > goto update; > > } > > stime = scale_stime((__force u64)stime, (__force u64)rtime, > > > > (__force u64)(stime + utime)); +update: > > /*> > * Make sure stime doesn't go backwards; this preserves monotonicity > > * for utime because rtime is monotonic. @@ -649,7 +655,6 @@ static void cputime_adjust(struct task_cputime *curr, > > > stime = rtime - utime; > > } -update: > > prev->stime = stime; > > prev->utime = utime; out: