lkml.org 
[lkml]   [2012]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2] fix idle ticks in cpu summary line of /proc/stat
Hi Thomas,

On Thu, 29 Mar 2012 12:42:22 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 13 Mar 2012, Michal Hocko wrote:
>
> > OK, so the updated version of the patch looks like this. I am sorry but
> > I had time to only compile test this...
> > ---
> > >From d12247f14c5f8b00ae97a87442f62e49227a759b Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.cz>
> > Date: Mon, 12 Mar 2012 13:11:38 +0100
> > Subject: [PATCH] nohz: fix idle ticks in cpu summary line of /proc/stat
> >
> > Git commit 09a1d34f8535ecf9 "nohz: Make idle/iowait counter update
> > conditional" introduced a bug in regard to cpu hotplug. The effect is
> > that the number of idle ticks in the cpu summary line in /proc/stat is
> > still counting ticks for offline cpus.
> >
> > Reproduction is easy, just start a workload that keeps all cpus busy,
> > switch off one or more cpus and then watch the idle field in top.
> > On a dual-core with one cpu 100% busy and one offline cpu you will get
> > something like this:
> >
> > %Cpu(s): 48.7 us, 1.3 sy, 0.0 ni, 50.0 id, 0.0 wa, 0.0 hi, 0.0 si, 0.0 st
> >
> > The problem is that an offline cpu still has ts->idle_active == 1.
> > To fix this we should make sure that the cpu is online when calling
> > get_cpu_idle_time_us and get_cpu_iowait_time_us.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> > Reported-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
>
> Martin, does that solve the problem for you ?

Yes, the patch does solve the immediate problem. I just tested if again
on x86 and s390, works fine. I would like to have another change though.
The idle_sleep calculation in the scheduler is fine for x86 but for s390
the arch_idle_time delivers a much better precision. A patch would look
like this:
--
Subject: [PATCH] use arch_idle_time for idle and iowait times if available

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Git commit a25cac5198d4ff28 "proc: Consider NO_HZ when printing idle and
iowait times" changes the code for /proc/stat to use get_cpu_idle_time_us
and get_cpu_iowait_time_us if the system is running with nohz enabled.
For architectures which define arch_idle_time (currently s390 only)
this is a change for the worse. The result of arch_idle_time is supposed
to be the exact sleep time of the target cpu and should be used instead
of the value kept by the scheduler.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
fs/proc/stat.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)

--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -18,9 +18,30 @@
#ifndef arch_irq_stat
#define arch_irq_stat() 0
#endif
-#ifndef arch_idle_time
-#define arch_idle_time(cpu) 0
-#endif
+
+#ifdef arch_idle_time
+
+static cputime64_t get_idle_time(int cpu)
+{
+ cputime64_t idle;
+
+ idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+ if (cpu_online(cpu) && !nr_iowait_cpu(cpu))
+ idle += arch_idle_time(cpu);
+ return idle;
+}
+
+static cputime64_t get_iowait_time(int cpu)
+{
+ cputime64_t iowait;
+
+ iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+ if (cpu_online(cpu) && nr_iowait_cpu(cpu))
+ iowait += arch_idle_time(cpu);
+ return iowait;
+}
+
+#else

static u64 get_idle_time(int cpu)
{
@@ -29,11 +50,10 @@ static u64 get_idle_time(int cpu)
if (cpu_online(cpu))
idle_time = get_cpu_idle_time_us(cpu, NULL);

- if (idle_time == -1ULL) {
+ if (idle_time == -1ULL)
/* !NO_HZ or cpu offline so we can rely on cpustat.idle */
idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
- idle += arch_idle_time(cpu);
- } else
+ else
idle = usecs_to_cputime64(idle_time);

return idle;
@@ -55,6 +75,8 @@ static u64 get_iowait_time(int cpu)
return iowait;
}

+#endif
+
static int show_stat(struct seq_file *p, void *v)
{
int i, j;
--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.



\
 
 \ /
  Last update: 2012-03-30 12:25    [W:0.147 / U:2.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site