lkml.org 
[lkml]   [2008]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/3] sched: terminate newidle balancing once atleastone task has moved over
From
Date
On Tue, 2008-06-24 at 10:55 -0600, Gregory Haskins wrote:
> >>> On Tue, Jun 24, 2008 at 9:31 AM, in message <1214314273.4351.34.camel@twins>,
> Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, 2008-06-24 at 07:18 -0600, Gregory Haskins wrote:
> >> >>> On Tue, Jun 24, 2008 at 6:13 AM, in message
> > <1214302406.4351.23.camel@twins>,
> >> Peter Zijlstra <peterz@infradead.org> wrote:
> >> > On Mon, 2008-06-23 at 17:04 -0600, Gregory Haskins wrote:
> >> >> Inspired by Peter Zijlstra.
> >> >>
> >> >> Signed-off-by: Gregory Haskins <ghaskins@novell.com>
> >> >> ---
> >> >>
> >> >> kernel/sched.c | 4 ++++
> >> >> 1 files changed, 4 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/kernel/sched.c b/kernel/sched.c
> >> >> index 3efbbc5..c8e8520 100644
> >> >> --- a/kernel/sched.c
> >> >> +++ b/kernel/sched.c
> >> >> @@ -2775,6 +2775,10 @@ static int move_tasks(struct rq *this_rq, int
> >> > this_cpu, struct rq *busiest,
> >> >> max_load_move - total_load_moved,
> >> >> sd, idle, all_pinned, &this_best_prio);
> >> >> class = class->next;
> >> >> +
> >> >> + if (idle == CPU_NEWLY_IDLE && this_rq->nr_running)
> >> >> + break;
> >> >> +
> >> >> } while (class && max_load_move > total_load_moved);
> >> >>
> >> >> return total_load_moved > 0;
> >> >
> >> >
> >> > right,.. uhm, except that you forgot all the other fixes and
> >> > generalizations I had,..
> >>
> >> Heh...well I intentionally simplified it, but perhaps that is out of
> > ignorance. I did say "inspired by" ;)
> >>
> >> >
> >> > The LB_START/LB_COMPLETE stuff is needed to fix CFS load balancing. It
> >> > now always iterates the first sysctl_sched_nr_migrate tasks, and if it
> >> > doesn't find any there, just gives up - which isn't too big of a problem
> >> > with it set to 32, but if you drop it to 2/4 stuff starts valing apart.
> >> >
> >> > And the break I had here, only checks classes above and equal to the
> >> > current class.
> >> >
> >> > This again is needed when you have more classes.
> >>
> >> Im not sure I understand/agree here (unless you plan on having a class below
> > sched_idle()??)
> >>
> >> The fact that we are going NEWLYIDLE to me implies that all the other
> > classes are
> >> "above or equal". And rq->nr_running approximates all the per-class vtable
> > work
> >> that you had done to probe the higher classes. We currently only hit this
> > code when
> >> rq->nr_running == 0, so rq->nr_running !=0 seems like a logical termination
> >> condition.
> >>
> >> I guess what I am not clear on is: "when would we be NEWLYIDLE in a higher
> > class,
> >> yet have tasks populated in lower classes such at nr_running is non-zero".
> >> Additionally, even if we have that condition (e.g. with something like the
> > EDF work you
> >> are doing, perhaps?), shouldn't we patch the advanced form of this logic
> > when the rest
> >> of the code goes in? For now, this seems like the most straight forward way
> > to
> >> accomplish the goal. But I could be missing something ;)
> >
> > The thing I'm worried about - but it might be unfounded and is certainly
> > so now - is that suppose we have:
> >
> > EDF
> > FIFO/RR
> > SOFTRT
> > OTHER
> > IDLE
> >
> > and we've just done FIFO/RR (which is a nop) and and some interrupt woke
> > an OTHER task while we dropped for lockbreak.
> >
> > At this point your logic would bail out and start running the OTHER
> > task, even though we might have found a SOFTRQ task to run had we
> > bothered to look.
> >
>
> Ok, now I think I understand your concern. But I think you may be worrying about
> this at the wrong level. I would think we should be doing something similar to the
> post-balance patch I submitted a while back. It basically iterated through each class,
> giving each an opportunity to pull tasks over in its own way. The difference there
> was that I was doing it post-schedule to deal with that locking issue. We could
> take the same idea and do it where we pre_schedule() today.
>
> I think the f_b_g() et. al. is really SCHED_OTHER specific, and probably always will be.
> Lets just formalize that. Perhaps we should move all the LB code to sched_fair and set
> something like what I proposed up. Thoughts?

Right,. generalizing f_b_g() isn't something we should consider, its
plenty impossible to understand already.

OK, moving everything into _fair sounds like the right approach.



\
 
 \ /
  Last update: 2008-06-24 21:47    [W:0.067 / U:0.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site