lkml.org 
[lkml]   [2013]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 2/4] nohz: Synchronize sleep time stats with seqlock
    Date
    When some call site uses get_cpu_*_time_us() to read a sleeptime
    stat, it deduces the total sleeptime by adding the pending time
    to the last sleeptime snapshot if the CPU target is idle.

    Namely this sums up to:

    sleeptime = ts($CPU)->idle_sleeptime;
    if (ts($CPU)->idle_active)
    sleeptime += NOW() - ts($CPU)->idle_entrytime

    But this only works if idle_sleeptime, idle_entrytime and idle_active are
    read and updated under some disciplined order.

    Lets consider the following scenario:

    CPU 0 CPU 1

    (seq 1) ts(CPU 0)->idle_active = 1
    ts(CPU 0)->idle_entrytime = NOW()

    (seq 2) sleeptime = NOW() - ts(CPU 0)->idle_entrytime
    ts(CPU 0)->idle_sleeptime += sleeptime sleeptime = ts(CPU 0)->idle_sleeptime;
    if (ts(CPU 0)->idle_active)
    ts(CPU 0)->idle_entrytime = NOW() sleeptime += NOW() - ts(CPU 0)->idle_entrytime

    The resulting value of sleeptime in CPU 1 can vary depending of some
    ordering scenario:

    * If it sees the value of idle_entrytime after seq 1 and the value of idle_sleeptime
    after seq 2, the value of sleeptime will be buggy because it accounts the delta twice,
    so it will be too high.

    * If it sees the value of idle_entrytime after seq 2 and the value of idle_sleeptime
    after seq 1, the value of sleeptime will be buggy because it misses the delta, so it
    will be too low.

    * If it sees the value of idle_entrytime and idle_sleeptime, both as seen after seq 1 or 2,
    the value will be correct.

    Some more tricky scenario can also happen if idle_active value is read from a former sequence.

    Hence we must honour the following constraints:

    - idle_sleeptime, idle_active and idle_entrytime must be updated and read
    under some correctly enforced SMP ordering

    - The three variable values as read by CPU 1 must belong to the same update
    sequences from CPU 0. The update sequences must be delimited such that the
    resulting three values after a sequence completion produce a coherent result
    together when read from the CPU 1.

    - We need to prevent from fetching middle-state sequence values.

    The ideal solution to implement this synchronization is to use a seqcount. Lets
    use one here around these three values to enforce sequence synchronization between
    updates and read.

    This fixes a reported bug where non-monotonic sleeptime stats are returned by /proc/stat
    when it is frequently read. And potential cpufreq governor bugs.

    Reported-by: Fernando Luis Vazquez Cao <fernando_b1@lab.ntt.co.jp>
    Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Fernando Luis Vazquez Cao <fernando_b1@lab.ntt.co.jp>
    Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Arjan van de Ven <arjan@linux.intel.com>
    ---
    include/linux/tick.h | 2 ++
    kernel/time/tick-sched.c | 37 +++++++++++++++++++++++++------------
    2 files changed, 27 insertions(+), 12 deletions(-)

    diff --git a/include/linux/tick.h b/include/linux/tick.h
    index 62bd8b7..49f9720 100644
    --- a/include/linux/tick.h
    +++ b/include/linux/tick.h
    @@ -10,6 +10,7 @@
    #include <linux/irqflags.h>
    #include <linux/percpu.h>
    #include <linux/hrtimer.h>
    +#include <linux/seqlock.h>

    #ifdef CONFIG_GENERIC_CLOCKEVENTS

    @@ -70,6 +71,7 @@ struct tick_sched {
    unsigned long next_jiffies;
    ktime_t idle_expires;
    int do_timer_last;
    + seqcount_t sleeptime_seq;
    };

    extern void __init tick_init(void);
    diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
    index ede0405..f7fc27b 100644
    --- a/kernel/time/tick-sched.c
    +++ b/kernel/time/tick-sched.c
    @@ -414,6 +414,7 @@ static void tick_nohz_stop_idle(int cpu, ktime_t now)
    ktime_t delta;

    /* Updates the per cpu time idle statistics counters */
    + write_seqcount_begin(&ts->sleeptime_seq);
    delta = ktime_sub(now, ts->idle_entrytime);
    if (nr_iowait_cpu(cpu) > 0)
    ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime, delta);
    @@ -421,6 +422,7 @@ static void tick_nohz_stop_idle(int cpu, ktime_t now)
    ts->idle_sleeptime = ktime_add(ts->idle_sleeptime, delta);
    ts->idle_entrytime = now;
    ts->idle_active = 0;
    + write_seqcount_end(&ts->sleeptime_seq);

    sched_clock_idle_wakeup_event(0);
    }
    @@ -429,8 +431,11 @@ static ktime_t tick_nohz_start_idle(int cpu, struct tick_sched *ts)
    {
    ktime_t now = ktime_get();

    + write_seqcount_begin(&ts->sleeptime_seq);
    ts->idle_entrytime = now;
    ts->idle_active = 1;
    + write_seqcount_end(&ts->sleeptime_seq);
    +
    sched_clock_idle_sleep_event();
    return now;
    }
    @@ -453,6 +458,7 @@ u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time)
    {
    struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu);
    ktime_t now, idle;
    + unsigned int seq;

    if (!tick_nohz_enabled)
    return -1;
    @@ -461,12 +467,15 @@ u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time)
    if (last_update_time)
    *last_update_time = ktime_to_us(now);

    - if (ts->idle_active && !nr_iowait_cpu(cpu)) {
    - ktime_t delta = ktime_sub(now, ts->idle_entrytime);
    - idle = ktime_add(ts->idle_sleeptime, delta);
    - } else {
    - idle = ts->idle_sleeptime;
    - }
    + do {
    + seq = read_seqcount_begin(&ts->sleeptime_seq);
    + if (ts->idle_active && !nr_iowait_cpu(cpu)) {
    + ktime_t delta = ktime_sub(now, ts->idle_entrytime);
    + idle = ktime_add(ts->idle_sleeptime, delta);
    + } else {
    + idle = ts->idle_sleeptime;
    + }
    + } while (read_seqcount_retry(&ts->sleeptime_seq, seq));

    return ktime_to_us(idle);

    @@ -491,6 +500,7 @@ u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
    {
    struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu);
    ktime_t now, iowait;
    + unsigned int seq;

    if (!tick_nohz_enabled)
    return -1;
    @@ -499,12 +509,15 @@ u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
    if (last_update_time)
    *last_update_time = ktime_to_us(now);

    - if (ts->idle_active && nr_iowait_cpu(cpu) > 0) {
    - ktime_t delta = ktime_sub(now, ts->idle_entrytime);
    - iowait = ktime_add(ts->iowait_sleeptime, delta);
    - } else {
    - iowait = ts->iowait_sleeptime;
    - }
    + do {
    + seq = read_seqcount_begin(&ts->sleeptime_seq);
    + if (ts->idle_active && nr_iowait_cpu(cpu) > 0) {
    + ktime_t delta = ktime_sub(now, ts->idle_entrytime);
    + iowait = ktime_add(ts->iowait_sleeptime, delta);
    + } else {
    + iowait = ts->iowait_sleeptime;
    + }
    + } while (read_seqcount_retry(&ts->sleeptime_seq, seq));

    return ktime_to_us(iowait);
    }
    --
    1.7.5.4


    \
     
     \ /
      Last update: 2013-08-16 18:01    [W:2.481 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site