| Message ID | 20200909144122.77210-2-tom.hromatka@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iowait and idle fixes in /proc/stat | expand |
Tom, On Wed, Sep 09 2020 at 08:41, Tom Hromatka wrote: > A customer reported that when a cpu goes offline and then comes back > online, the overall cpu idle and iowait data in /proc/stat decreases. > This is wreaking havoc with their cpu usage calculations. for a changelog it's pretty irrelevant whether a customer reported something or not. Fact is that this happens and you fail to explain WHY it happens, i.e. because the values are cleared when the CPU goes down and therefore the accounting starts over from 0 when the CPU comes online again. Describing this is much more useful than showing random numbers before and after. > --- a/kernel/time/tick-sched.c > +++ b/kernel/time/tick-sched.c > @@ -1375,13 +1375,22 @@ void tick_setup_sched_timer(void) > void tick_cancel_sched_timer(int cpu) > { > struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu); > + ktime_t idle_sleeptime, iowait_sleeptime; > > # ifdef CONFIG_HIGH_RES_TIMERS > if (ts->sched_timer.base) > hrtimer_cancel(&ts->sched_timer); > # endif > > + /* save off and restore the idle_sleeptime and the iowait_sleeptime > + * to avoid discontinuities and ensure that they are monotonically > + * increasing > + */ /* * Please use sane multiline comment style and not the above * abomination. */ Also please explain what this 'monotonically increasing' thing is about. Without consulting the changelog it's hard to figure out what that means. Comments are valuable but only when they make actually sense on their own. Something like the below perhaps? /* * Preserve idle and iowait sleep times accross a CPU offline/online * sequence as they are accumulative. */ Hmm? > + idle_sleeptime = ts->idle_sleeptime; > + iowait_sleeptime = ts->iowait_sleeptime; > memset(ts, 0, sizeof(*ts)); > + ts->idle_sleeptime = idle_sleeptime; > + ts->iowait_sleeptime = iowait_sleeptime; > } Thanks, tglx
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 3e2dc9b8858c..8103bad7bbd6 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -1375,13 +1375,22 @@ void tick_setup_sched_timer(void) void tick_cancel_sched_timer(int cpu) { struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu); + ktime_t idle_sleeptime, iowait_sleeptime; # ifdef CONFIG_HIGH_RES_TIMERS if (ts->sched_timer.base) hrtimer_cancel(&ts->sched_timer); # endif + /* save off and restore the idle_sleeptime and the iowait_sleeptime + * to avoid discontinuities and ensure that they are monotonically + * increasing + */ + idle_sleeptime = ts->idle_sleeptime; + iowait_sleeptime = ts->iowait_sleeptime; memset(ts, 0, sizeof(*ts)); + ts->idle_sleeptime = idle_sleeptime; + ts->iowait_sleeptime = iowait_sleeptime; } #endif
A customer reported that when a cpu goes offline and then comes back online, the overall cpu idle and iowait data in /proc/stat decreases. This is wreaking havoc with their cpu usage calculations. Prior to this patch: user nice system idle iowait cpu 1390748 636 209444 9802206 19598 cpu1 178384 75 24545 1392450 3025 take cpu1 offline and bring it back online user nice system idle iowait cpu 1391209 636 209682 8453440 16595 cpu1 178440 75 24572 627 0 To prevent this, do not clear the idle and iowait times for the cpu that has come back online. With this patch: user nice system idle iowait cpu 129913 17 17590 166512 704 cpu1 15916 3 2395 20989 47 take cpu1 offline and bring it back online user nice system idle iowait cpu 130089 17 17686 184625 711 cpu1 15942 3 2401 23088 47 Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> --- kernel/time/tick-sched.c | 9 +++++++++ 1 file changed, 9 insertions(+)