lkml.org 
[lkml]   [2012]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] fix idle ticks in cpu summary line of /proc/stat
    On 03/30/2012 03:53 PM, Martin Schwidefsky wrote:

    > 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>


    Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.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;
    >




    \
     
     \ /
      Last update: 2012-03-30 13:19    [W:0.031 / U:42.396 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site