Re: [PATCH 1/2] sched: push rt tasks only if newly activated tasks have been added

["Dmitry Adamushko" <dmitry.adamushko@gmail.com>]

2008/4/23 Gregory Haskins <ghaskins@novell.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?

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