lkml.org 
[lkml]   [2011]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH] Re: [PATCH] sched: Buggy comparison in check_preempt_tick
    From
    Date
    Going through my mailbox, I see this remains unaddressed.  I chose the
    keep it option, but whack it and revisit later is also viable.

    On Tue, 2010-12-28 at 06:48 +0100, Mike Galbraith wrote:
    > On Sun, 2010-12-26 at 08:23 +0100, Mike Galbraith wrote:
    >
    > > But anyway..
    > >
    > > echo NO_WAKEUP_PREEMPT > sched_features
    > > echo NO_TESTME > sched_features
    > > two hogs running on isolcpu 3, pid 6890 at nice -2
    > >
    > > while sleep 1; do grep 'pert.*6890' /proc/sched_debug; done
    > >
    > > runnable tasks:
    > > task PID tree-key switches prio
    > > -------------------------------------------------------
    > > R pert 6890 50201.071851 7453 118
    > > R pert 6890 50596.171290 7513 118 +60
    > > R pert 6890 50991.265264 7572 118 +59
    > > R pert 6890 51383.781965 7631 118 +59
    > > pert 6890 51781.463129 7691 118 +60
    > >
    > > echo TESTME > sched_features
    > > pert 6890 126881.306733 18977 118
    > > R pert 6890 127273.825719 19036 118 +59
    > > R pert 6890 127668.928218 19095 118 +59
    > > R pert 6890 128064.031372 19154 118 +59
    > > R pert 6890 128459.134339 19213 118 +59
    > >
    > > ...with a compute load, the thing should be a noop, and appears to be so
    > > (with busted compare fixed anyway;). You have to be well overloaded for
    > > buddies to kick in these days, so it's probably pretty hard to get
    > > enough spread for the thing to fire.
    >
    > I did a bit more testing yesterday with wakeup loads. There's enough
    > spread for the test to nudge things a few [0..4] times per second/core.
    >
    > I'd either fix the comparison, and let it keep on nudging once in a
    > while, or whack the whole thing.

    sched: fix signed unsigned comparison in check_preempt_tick()

    signed unsigned comparison may lead to superfluous resched if leftmost
    is right of the current task, wasting a few cycles, and inadvertently
    _lengthening_ the current task's slice.

    Reported-by: Venkatesh Pallipadi <venki@google.com>
    Signed-off-by: Mike Galbraith <efault@gmx.de>

    ---
    kernel/sched_fair.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    Index: linux-2.6.37.git/kernel/sched_fair.c
    ===================================================================
    --- linux-2.6.37.git.orig/kernel/sched_fair.c
    +++ linux-2.6.37.git/kernel/sched_fair.c
    @@ -872,7 +872,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq
    struct sched_entity *se = __pick_next_entity(cfs_rq);
    s64 delta = curr->vruntime - se->vruntime;

    - if (delta > ideal_runtime)
    + if (delta > (s64)ideal_runtime)
    resched_task(rq_of(cfs_rq)->curr);
    }
    }



    \
     
     \ /
      Last update: 2011-01-05 05:43    [W:0.024 / U:0.088 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site