lkml.org 
[lkml]   [2016]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 2/3] sched/cputime: Fix prev steal time accouting during cpu hotplug
    From
    Date


    On 08/06/2016 05:05, Wanpeng Li wrote:
    > From: Wanpeng Li <wanpeng.li@hotmail.com>
    >
    > Commit e9532e69b8d1 ("sched/cputime: Fix steal time accounting vs. CPU hotplug")
    > set rq->prev_* to 0 after a cpu hotplug comes back in order to fix the scenario:
    >
    > | steal is smaller than rq->prev_steal_time we end up with an insane large
    > | value which then gets added to rq->prev_steal_time, resulting in a permanent
    > | wreckage of the accounting.
    >
    > However, it is still buggy.
    >
    > rq->prev_steal_time = 0:
    >
    > As Rik pointed out:
    >
    > | setting rq->prev_irq_time to 0 in the guest, and then getting a giant value from
    > | the host, could result in a very large of steal_jiffies.
    >
    > rq->prev_steal_time_rq = 0:
    >
    > | steal = paravirt_steal_clock(cpu_of(rq));
    > | steal -= rq->prev_steal_time_rq;
    > |
    > | if (unlikely(steal > delta))
    > | steal = delta;
    > |
    > | rq->prev_steal_time_rq += steal;
    > | delta -= steal;
    > |
    > | rq->clock_task += delta;
    >
    > steal is a giant value and rq->prev_steal_time_rq is 0, rq->prev_steal_time_rq
    > grows in delta granularity, rq->clock_task can't ramp up until rq->prev_steal_time_rq
    > catches up steal clock since delta value will be 0 after reducing steal time from
    > normal execution time. That's why I obersved that cpuhg/1-12 continue running
    > until rq->prev_steal_time_rq catches up steal clock timestamp.
    >
    > I believe rq->prev_irq_time has similar issue. So this patch fix it by reverting
    > commit e9532e69b8d1.

    This is still very hard to read. I think the justification for this
    patch is much simpler:

    -------
    Commit e9532e69b8d1 ("sched/cputime: Fix steal time accounting vs. CPU
    hotplug") set rq->prev_* to 0 after a cpu hotplug comes back in order to
    fix the case where (after CPU hotplug) steal is smaller than
    rq->prev_steal_time.

    However, this should never happen. steal was only smaller because of the
    KVM-specific bug fixed by the previous patch. Worse, the previous patch
    triggers a bug on CPU hot-unplug/plug operation: because
    rq->prev_steal_time is cleared, all of the CPU's past steal time will be
    accounted again on hot-plug.

    Since the root cause has been fixed, we can just revert commit e9532e69b8d1.
    -------

    Since things are currently pretty broken, I think it's fair to keep the
    two patches separate instead of squashing them.

    Paolo

    \
     
     \ /
      Last update: 2016-06-08 12:21    [W:3.116 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site