public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gregory Haskins" <ghaskins@novell.com>
To: "Nick Piggin" <nickpiggin@yahoo.com.au>
Cc: <mingo@elte.hu>, <rostedt@goodmis.org>, <peterz@infradead.org>,
	<npiggin@suse.de>, <linux-kernel@vger.kernel.org>,
	<linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH 2/3] sched: terminate newidle balancing once at least one task has moved over
Date: Wed, 09 Jul 2008 04:53:56 -0600	[thread overview]
Message-ID: <48746084.BA47.005A.0@novell.com> (raw)
In-Reply-To: <200807091809.52293.nickpiggin@yahoo.com.au>

>>> On Wed, Jul 9, 2008 at  4:09 AM, in message
<200807091809.52293.nickpiggin@yahoo.com.au>, Nick Piggin
<nickpiggin@yahoo.com.au> 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@yahoo.com.au>, Nick Piggin
>>
>> <nickpiggin@yahoo.com.au> wrote:
>> > On Saturday 28 June 2008 06:29, Gregory Haskins wrote:
>> >> Inspired by Peter Zijlstra.
>> >>
>> >> Signed-off-by: Gregory Haskins <ghaskins@novell.com>
>> >
>> > 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.

> 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.

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.  As a compromise
we could put an #ifdef CONFIG_PREEMPT around this new logic, but I don't think it
is strictly necessary.

Regards,
-Greg

  reply	other threads:[~2008-07-09 11:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-27 20:29 [PATCH 0/3] sched: newidle and RT wake-buddy fixes Gregory Haskins
2008-06-27 20:29 ` [PATCH 1/3] sched: enable interrupts and drop rq-lock during newidle balancing Gregory Haskins
2008-06-27 20:29 ` [PATCH 2/3] sched: terminate newidle balancing once at least one task has moved over Gregory Haskins
2008-07-08  5:00   ` Nick Piggin
2008-07-08 12:37     ` Gregory Haskins
2008-07-09  8:09       ` Nick Piggin
2008-07-09 10:53         ` Gregory Haskins [this message]
2008-07-09 11:17           ` Nick Piggin
2008-07-09 11:53             ` Gregory Haskins
2008-06-27 20:30 ` [PATCH 3/3] sched: add avg-overlap support to RT tasks Gregory Haskins
2008-06-27 20:51 ` [PATCH 0/3] sched: newidle and RT wake-buddy fixes Peter Zijlstra
2008-06-30 12:56   ` Ingo Molnar
2008-06-30 13:15 ` Ingo Molnar
2008-06-30 11:20   ` Gregory Haskins
2008-06-30 14:41   ` Gregory Haskins
2008-06-30 15:01     ` Steven Rostedt
2008-06-30 17:16   ` Gregory Haskins
2008-06-30 18:10     ` Ingo Molnar
2008-07-03 14:41     ` Ingo Molnar
2008-07-03 15:12       ` Ingo Molnar
2008-07-08 12:38         ` Gregory Haskins
2008-07-08 16:45         ` Gregory Haskins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48746084.BA47.005A.0@novell.com \
    --to=ghaskins@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nickpiggin@yahoo.com.au \
    --cc=npiggin@suse.de \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox