Messages in this thread | | | Date | Wed, 07 May 2014 18:49:47 +0200 | From | Denys Vlasenko <> | Subject | Re: [PATCH 3/4 v2] nohz: Fix idle/iowait counts going backwards |
| |
On 05/07/2014 04:23 PM, Peter Zijlstra wrote: > On Wed, May 07, 2014 at 03:41:33PM +0200, Denys Vlasenko wrote: >> With this change, "iowait-ness" of every idle period is decided >> at the moment it starts: >> if this CPU's run-queue had tasks waiting on I/O, then this idle >> period's duration will be added to iowait_sleeptime. >> >> This fixes the bug where iowait and/or idle counts could go backwards, >> but iowait accounting is not precise (it can show more iowait >> that there really is). >> > > NAK on this, the thing going backwards is a symptom of the bug, not an > actual bug itself.
This patch does fix that bug.
The bug is that in nohz_stop_idle(), we base the decision to add accumulated time to ts->iowait_sleeptime or to ts->idle_sleeptime on nr_iowait_cpu(cpu):
if (nr_iowait_cpu(smp_processor_id()) > 0) ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime, delta); else ts->idle_sleeptime = ktime_add(ts->idle_sleeptime, delta);
and we use the same nr_iowait_cpu(cpu) to calculate result of get_cpu_iowait_time_us():
if (ts->idle_active && nr_iowait_cpu(cpu) > 0) { delta = ktime_sub(now, ts->idle_entrytime); iowait = ktime_add(ts->iowait_sleeptime, delta); }
This is wrong because nr_iowait_cpu(cpu) is not stable. It could be != 0 in get_cpu_iowait_time_us() but later become 0 when we are in nohz_stop_idle().
We must use consistent logic in these two functions. If nr_iowait_cpu() added something to iowait counter, the same should be done in nohz_stop_idle().
| |