public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gregory Haskins" <ghaskins@novell.com>
To: "Dmitry Adamushko" <dmitry.adamushko@gmail.com>
Cc: <mingo@elte.hu>, "Steven Rostedt" <rostedt@goodmis.org>,
	<chinang.ma@intel.com>, <suresh.b.siddha@intel.com>,
	<arjan@linux.intel.com>, <willy@linux.intel.com>,
	<linux-kernel@vger.kernel.org>, <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH 1/2] sched: push rt tasks only if newly activated tasks have been added
Date: Wed, 23 Apr 2008 04:54:39 -0600	[thread overview]
Message-ID: <480EDD2F.BA47.005A.0@novell.com> (raw)
In-Reply-To: <b647ffbd0804230323s20d25697qcf80ca9996e143a7@mail.gmail.com>

>>> On Wed, Apr 23, 2008 at  6:23 AM, in message
<b647ffbd0804230323s20d25697qcf80ca9996e143a7@mail.gmail.com>, "Dmitry
Adamushko" <dmitry.adamushko@gmail.com> wrote: 
> 2008/4/23 Gregory Haskins <ghaskins@novell.com>:
>> [ ... ]
>>
>>  I think we can simplify this further.  We really only need to push here if 
> we are not going to reschedule anytime soon (probably white-space damaged):
>>
>>
>>  --- a/kernel/sched_rt.c
>>
>> +++ b/kernel/sched_rt.c
>>  @@ -1058,11 +1058,14 @@ static void post_schedule_rt(struct rq *rq)
>>         }
>>   }
>>
>>  -
>>  +/*
>>  + * If we are not running and we are not going to reschedule soon, we 
> should
>>  + * try to push tasks away now
>>  + */
>>
>>  static void task_wake_up_rt(struct rq *rq, struct task_struct *p)
>>   {
>>         if (!task_running(rq, p) &&
>>  -           (p->prio >= rq->rt.highest_prio) &&
>>  +           !test_tsk_thread_flag(rq->curr, TIF_NEED_RESCHED) &&
>>             rq->rt.overloaded)
>>                 push_rt_tasks(rq);
>>   }
> 
> 
> It's somewhat suboptimal as it doesn't guarantee that 'p' gets control next.

No it doesn't, but I don't see that as a requirement to work properly.  All we really need is to make sure push_rt_tasks() will be executed in the near future, and a reschedule will do that.  Perhaps I am missing something?

> 
> e.g. 2 tasks (T0 and T1) have been woken up before an actual
> re-schedule takes place. Even if T1 is of lower prio than T0,
> task_wake_up_rt() will see the NEED_RESCHED flag and bail out while it
> would make sense at this moment to push T1 off this cpu.

I think this is "ok".  IMO, we really want to prioritize the push operation from post_schedule() over task_wake_up() because it is the most accurate (since the scheduling decision would be atomic to the rq->lock).  This way we avoid pushing the wrong task away before the schedule has a chance to run.  I think your alg probably approximates this same accuracy, albeit in a more expensive way.  The one thing I can say is an advantage to your approach is that it potentially would migrate the task faster (though it shouldn't be by very much, since the resched is technically pending).  This argument is further compounded by the fact that you would have to subtract the extra overhead of running pick_next_task() et. al. from the gain of avoiding the wait for the reschedule, of course.  This would errode some of the lower-latency argument.

Hmmm...  Im not sure which I like better...simplicity is often nice IMO, but I think either could work.

> 
> 
> p.s. hope you are better today. get well! :-)

Thanks!  I am feeling much better today, though I don't think I am fully out of the woods yet.

> 




  reply	other threads:[~2008-04-23 10:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-21 18:10 [PATCH 0/2] sched: refreshes Gregory Haskins
2008-04-21 18:10 ` [PATCH 1/2] sched: push rt tasks only if newly activated tasks have been added Gregory Haskins
2008-04-22 15:30   ` Dmitry Adamushko
2008-04-22 15:51     ` Steven Rostedt
2008-04-23  8:05       ` Dmitry Adamushko
2008-04-23  9:53         ` Dmitry Adamushko
2008-04-23 10:03           ` Gregory Haskins
2008-04-23 10:23             ` Dmitry Adamushko
2008-04-23 10:54               ` Gregory Haskins [this message]
2008-04-23 11:20                 ` Dmitry Adamushko
2008-04-22 16:38     ` Gregory Haskins
2008-04-23 11:13       ` [RFC PATCH 0/2] sched fixes for suboptimal balancing Gregory Haskins
2008-04-23 11:13         ` [PATCH 1/2] sched: fix RT task-wakeup logic Gregory Haskins
2008-04-23 12:54           ` Steven Rostedt
2008-04-23 14:29           ` Dmitry Adamushko
2008-04-24 11:56           ` Gregory Haskins
2008-04-28 16:30             ` [(RESEND) PATCH] " Gregory Haskins
2008-04-29 14:35               ` Ingo Molnar
2008-04-23 11:13         ` [PATCH 2/2] sched: prioritize non-migratable tasks over migratable ones Gregory Haskins
2008-04-23 12:58           ` Steven Rostedt
2008-04-23 13:11             ` [PATCH 2/2] sched: prioritize non-migratable tasks over migratableones Gregory Haskins
2008-04-28 18:55         ` [RFC PATCH 0/2] sched fixes for suboptimal balancing Ingo Molnar
2008-04-21 18:10 ` [PATCH 2/2] sched: Use a 2-d bitmap for searching lowest-pri CPU Gregory Haskins
2008-04-21 19:33 ` [PATCH 0/2] sched: refreshes Ingo Molnar

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=480EDD2F.BA47.005A.0@novell.com \
    --to=ghaskins@novell.com \
    --cc=arjan@linux.intel.com \
    --cc=chinang.ma@intel.com \
    --cc=dmitry.adamushko@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=suresh.b.siddha@intel.com \
    --cc=willy@linux.intel.com \
    /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