From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dmitry Adamushko" Subject: Re: [PATCH 1/2] sched: push rt tasks only if newly activated tasks have been added Date: Wed, 23 Apr 2008 13:20:54 +0200 Message-ID: References: <20080421180747.4560.67794.stgit@novell1.haskins.net> <20080421181015.4560.7658.stgit@novell1.haskins.net> <480ED122.BA47.005A.0@novell.com> <480EDD2F.BA47.005A.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: mingo@elte.hu, "Steven Rostedt" , 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 To: "Gregory Haskins" Return-path: Received: from wx-out-0506.google.com ([66.249.82.234]:39287 "EHLO wx-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752424AbYDWLVC (ORCPT ); Wed, 23 Apr 2008 07:21:02 -0400 Received: by wx-out-0506.google.com with SMTP id h31so2157937wxd.4 for ; Wed, 23 Apr 2008 04:20:58 -0700 (PDT) In-Reply-To: <480EDD2F.BA47.005A.0@novell.com> Content-Disposition: inline Sender: linux-rt-users-owner@vger.kernel.org List-ID: 2008/4/23 Gregory Haskins : > >>> On Wed, Apr 23, 2008 at 6:23 AM, in message > , "Dmitry > > Adamushko" wrote: > > > 2008/4/23 Gregory Haskins : > >> [ ... ] > >> > >> 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? No, it's better indeed to delay push_rt_tasks() until post_rt_schedule() when 'current' (that's a task to be preempted) is also available for potential migration. Not considering "current" (as it's running) in task_wake_up_rt() is a key of the problem which I illustrated in my first message. I mean, we set rq->rt.pushed = 1 and that kind of means "nothing to be push off here any more" but "current" was out of our consideration. humm... a quick idea (should think more on it): (1) select_task_rq_rt() + task_wake_up_rt() should consider only 'p' (a newly woken up task) for pushing off a cpu; i.e. task_wake_up_rt() calls push_rt_task(..., p) and _never_ sets up rq->rt.pushed; the main point where 'pushed' is done : (2) post_schedule_rt() here we call push_rt_tasks() and set up rq->rt.pushed to 1. heh, I can be missing something important... need to verify different scenarios wrt this algo. > > 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). Agreed. And as I noticed above, ex-current is also available for migration policies at this point + the most eligible task has just got control (and it's the only task that shouldn't be considered for migration). -- Best regards, Dmitry Adamushko