lkml.org 
[lkml]   [2009]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: [patch] Re: scheduler oddity [bug?]
    Ingo Molnar wrote:
    > * Mike Galbraith <efault@gmx.de> wrote:
    >


    ...
    >> OK, I've not seen any problem indications yet, so find patchlet below.
    >>
    >> However! Balazs has stated that this problem is _not_ present in .git,
    >> and that..
    >>
    >> commit 38736f475071b80b66be28af7b44c854073699cc
    >> Author: Gautham R Shenoy <ego@in.ibm.com>
    >> Date: Sat Sep 6 14:50:23 2008 +0530
    >>
    >> ..is what fixed it. Willy Tarreau verified this as being the case on
    >> his HW as well. It is present in .git with my HW.
    >>
    >> I see it as a problem, but it's your call. Dunno if I'd apply it or
    >> hold back, given these conflicting reports.
    >>
    >
    > I think we still want it - as the purpose of the overlap metric
    > is to measure reality. If preemption causes overlap in execution
    > we should not ignore that.
    >

    I'm sure it's wrong. The only call to dequeue with a non-zero sleep
    value is in deactivate_task. All the rest have zero sleep. The section
    of code identified by Mike in his patchlet should be moved for purpose
    of clarity. It also hilights the symmetry between queue_task and
    dequeue_task:

    --- sched.c 2009-02-21 09:09:34.000000000 +1030
    +++ sched.c.dn 2009-03-09 20:13:51.000000000 +1030
    @@ -1647,12 +1647,6 @@

    static void dequeue_task(struct rq *rq, struct task_struct *p, int sleep)
    {
    - if (sleep && p->se.last_wakeup) {
    - update_avg(&p->se.avg_overlap,
    - p->se.sum_exec_runtime - p->se.last_wakeup);
    - p->se.last_wakeup = 0;
    - }
    -
    sched_info_dequeued(p);
    p->sched_class->dequeue_task(rq, p, sleep);
    p->se.on_rq = 0;
    @@ -1724,6 +1718,12 @@
    if (task_contributes_to_load(p))
    rq->nr_uninterruptible++;

    + if (sleep && p->se.last_wakeup) {
    + update_avg(&p->se.avg_overlap,
    + p->se.sum_exec_runtime - p->se.last_wakeup);
    + p->se.last_wakeup = 0;
    + }
    +
    dequeue_task(rq, p, sleep);
    dec_nr_running(rq);
    }


    Having done that, it makes sense to entirely remove dequeue_task 's
    sleep parameter, and replicate all three lines in deactivate_task:

    --- sched.c.dn 2009-03-09 20:41:13.000000000 +1030
    +++ sched.c.dn2 2009-03-09 20:41:30.000000000 +1030
    @@ -1645,10 +1645,10 @@
    p->se.on_rq = 1;
    }

    -static void dequeue_task(struct rq *rq, struct task_struct *p, int sleep)
    +static void dequeue_task(struct rq *rq, struct task_struct *p)
    {
    sched_info_dequeued(p);
    - p->sched_class->dequeue_task(rq, p, sleep);
    + p->sched_class->dequeue_task(rq, p, 0);
    p->se.on_rq = 0;
    }

    @@ -1724,7 +1724,11 @@
    p->se.last_wakeup = 0;
    }

    - dequeue_task(rq, p, sleep);
    + /*dequeue_task(rq, p, sleep);*/
    + sched_info_dequeued(p);
    + p->sched_class->dequeue_task(rq, p, sleep);
    + p->se.on_rq = 0;
    +
    dec_nr_running(rq);
    }

    @@ -4848,7 +4852,7 @@
    on_rq = p->se.on_rq;
    running = task_current(rq, p);
    if (on_rq)
    - dequeue_task(rq, p, 0);
    + dequeue_task(rq, p);
    if (running)
    p->sched_class->put_prev_task(rq, p);

    @@ -4897,7 +4901,7 @@
    }
    on_rq = p->se.on_rq;
    if (on_rq)
    - dequeue_task(rq, p, 0);
    + dequeue_task(rq, p);

    p->static_prio = NICE_TO_PRIO(nice);
    set_load_weight(p);
    @@ -8637,7 +8641,7 @@
    on_rq = tsk->se.on_rq;

    if (on_rq)
    - dequeue_task(rq, tsk, 0);
    + dequeue_task(rq, tsk);
    if (unlikely(running))
    tsk->sched_class->put_prev_task(rq, tsk);




    \
     
     \ /
      Last update: 2009-03-09 11:19    [W:0.026 / U:119.540 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site