lkml.org 
[lkml]   [2015]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] sched: fix lose fair sleeper bonus in switch_to_fair()
    On Tue, Sep 08, 2015 at 06:48:43AM +0800, Wanpeng Li wrote:
    > On 9/7/15 10:02 PM, Peter Zijlstra wrote:
    > >Please always Cc at least the person who wrote the lines you modify.
    > >
    > >On Mon, Sep 07, 2015 at 05:45:20PM +0800, Wanpeng Li wrote:
    > >>The sleeper task will be normalized when moved from fair_sched_class, in
    > >>order that vruntime will be adjusted either the task is running or sleeping
    > >>when moved back. The nomalization in switch_to_fair for sleep task will
    > >>result in lose fair sleeper bonus in place_entity() once the vruntime -
    > >>cfs_rq->min_vruntime is big when moved from fair_sched_class.
    > >>
    > >>This patch fix it by adjusting vruntime just during migrating as original
    > >>codes since the vruntime of the task has usually NOT been normalized in
    > >>this case.

    > >Sorry, I cannot follow that at all. Maybe its me being sleep deprived,
    > >but could you try that again?
    >
    > When changing away from the fair class while sleeping, relative vruntime is
    > calculated to handle the case sleep when moved from fair_sched_class and
    > running when moved to fair_sched_class.

    That, or the task being migrated to a different cgroup / cpu while being
    outside of the fair class.

    Also, the 'relative vruntime' as you call it, is an approximation for
    lag. Because we do not compute the 0-lag point (too expensive) we use
    min_vruntime as a conservative approximation.

    Lag is something you can transfer between runqueues, so that is the
    natural state for something that is not associated with a rq.

    > The absolute vruntime will be
    > calculated in enqueue_entity() either the task is running or sleeping when
    > moved back.

    Incorrect, enqueue_entity() will only do that conditionally, in the
    other cases it will assume se->vruntime is already absolute as you call
    it.

    attach_task_cfs_rq() must deal with the other cases.

    > The fair sleeper bonus should be gained in place_entity() if the
    > task is still sleeping.

    And this is still true. place_entity() assumes 'absolute vruntime', no
    matter how it got there. If we went to relative/lag, someone needs to go
    back to 'absolute'.

    > However, after recent commit ( 23ec30ddd7c1306:
    > 'sched: add two functions for att(det)aching a task to(from) a cfs_rq'), the
    > absolute vruntime will be calculated in switched_to_fair(),

    Also, conditionally, to complement the other places.

    > so the
    > max_vruntime() which is called in place_entity() will select the absolute
    > vruntime which is calculated in switched_to_fair() as the se->vruntime and
    > lose the fair sleeper bonus.

    You cannot loose your sleeper bonus by going to and from relative/lag
    (or rather you can, but that's due to min_vruntime being a poor
    substitute for the 0-lag point). But since we do this for all rq
    transfers we should not make exemptions.


    Afaict, the only possibly place for a bug to be here is
    vruntime_normalized(), if that somehow gets the conditions wrong we
    could fail-to/incorrectly subtract/add min_vruntime, creating a mess.


    \
     
     \ /
      Last update: 2015-09-08 12:01    [W:2.116 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site