Re: [PATCH 2/3] sched: terminate newidle balancing once at least one task has moved over

[Nick Piggin]

On Wednesday 09 July 2008 20:53, Gregory Haskins wrote:
> >>> On Wed, Jul 9, 2008 at  4:09 AM, in message
>
> <200807091809.52293.nickpiggin@xxxxxxxxxxxx>, Nick Piggin
>
> <nickpiggin@xxxxxxxxxxxx> wrote:
> > On Tuesday 08 July 2008 22:37, Gregory Haskins wrote:
> >> >>> On Tue, Jul 8, 2008 at  1:00 AM, in message
> >>
> >> <200807081500.18245.nickpiggin@xxxxxxxxxxxx>, Nick Piggin
> >>
> >> <nickpiggin@xxxxxxxxxxxx> wrote:
> >> > On Saturday 28 June 2008 06:29, Gregory Haskins wrote:
> >> >> Inspired by Peter Zijlstra.
> >> >>
> >> >> Signed-off-by: Gregory Haskins <ghaskins@xxxxxxxxxx>
> >> >
> >> > What happened to the feedback I sent about this?
> >> >
> >> > It is still nack from me.
> >>
> >> Ah yes.  Slipped through the cracks...sorry about that.
> >>
> >> What if we did "if (idle == CPU_NEWLY_IDLE && need_resched())" instead?
> >
> > Isn't that exactly the same thing
>
> Not quite.  The former version would break on *any* succesful enqueue (as a
> result of a local move_task as well as a remote wake-up/migration).  The
> latter version will only break on the the remote variety.  You were
> concerned about stopping a move_task operation early because it would
> reduce efficiency, and I do not entirely disagree. However, this really
> only concerns the local type (which have now been removed).
>
> Remote preemptions should (IMO) always break immediately because it would
> have been likely to invalidate the f_b_g() calculation anyway, and
> low-latency requirements dictate its the right thing to do.

I thought this was about newidle balancing? Tasks are always going to
be coming from remote runqueues, aren't they?


> > because any task will preempt the idle thread?
>
> During NEWIDLE this is a preempt-disabled section because we are still in
> the middle of a schedule(). Therefore there will be no involuntary
> preemption and that is why we are concerned with making sure we check for
> voluntary preemption.  The move_tasks() will run to completion without this
> patch.  With this patch it will break the operation if someone tries to
> preempt us.

Firstly, won't the act of pulling tasks set the need_resched condition?

Secondly, even if it does what you say, what exactly would be the difference
between blocking a newly moved task from running and blocking a newly woken
task from running? Either way you introduce the same worst case latency
condition.


> I'll keep an open mind but I am pretty sure this is something we should be
> doing. As far as I can tell, there should be no downside with this second
> version. 

I don't think it has really been thought through that well. So I'm still
against it.

> As a compromise we could put an #ifdef CONFIG_PREEMPT around this 
> new logic, but I don't think it is strictly necessary.

That's not very nice. It's reasonable to run with CONFIG_PREEMPT but not
blindly want to trade latency for throughput.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/