Ingo Molnar and group,
If their is a single RT task on this rq, then why not just reset the
timeslice and return??? Just MAYBE decreasing re-scheduling
overhead..
Thus,,,,
After
p->time_slice = static_prio_timeslice(p->static_prio);
Why isn't their a check like
if (rq->nr_running == 1)
return;
Which world remove the need for any recheduling
or requeue'ing...
Mitchell Erblich
FYI: below is believed to be a snap of the current/ orig func
-----------------------------
+static void task_tick_rt(struct rq *rq, struct task_struct *p)
+{
+ /*
+ * RR tasks need a special form of timeslice management.
+ * FIFO tasks have no timeslices.
+ */
+ if (p->policy != SCHED_RR)
+ return;
+
+ if (--p->time_slice)
+ return;
+
+ p->time_slice = static_prio_timeslice(p->static_prio);
+ set_tsk_need_resched(p);
+
+ /* put it at the end of the queue: */
+ requeue_task_rt(rq, p);
+}
* Mitchell Erblich <[email protected]> wrote:
> After
> p->time_slice = static_prio_timeslice(p->static_prio);
>
> Why isn't their a check like
> if (rq->nr_running == 1)
> return;
>
> Which world remove the need for any recheduling or requeue'ing...
your change is a possible optimization, but this is a pretty rare
codepath because the overwhelming majority of RT apps uses SCHED_FIFO.
Plus, the time_slice going down to 0 is a relatively rare event even for
SCHED_RR tasks. And if we have only a single SCHED_RR task, why is it
SCHED_RR to begin with? So this is on several levels an uncommon
workload and by adding a check like that we'd increase the codesize. But
... no strong feelings against this optimization - if you send a proper
patch we can apply it, it certainly makes sense from a logic POV.
Ingo
On 08/08/07, Ingo Molnar <[email protected]> wrote:
>
> * Mitchell Erblich <[email protected]> wrote:
>
> > After
> > p->time_slice = static_prio_timeslice(p->static_prio);
> >
> > Why isn't their a check like
> > if (rq->nr_running == 1)
> > return;
> >
> > Which world remove the need for any recheduling or requeue'ing...
>
> your change is a possible optimization, but this is a pretty rare
> codepath because the overwhelming majority of RT apps uses SCHED_FIFO.
> Plus, the time_slice going down to 0 is a relatively rare event even for
> SCHED_RR tasks. [ ... ]
I doubt it's a worth optimization. It's a rare codepath and, moreover,
this optimization is sub-optimal. e.g. the rest of tasks that
contributed to 'rq->nr_running > 1' could be SCHED_NORMAL (or of lower
rt_prio) and so resched_task() is useless as well.
I guess, the following thing would do a better job:
- we do need reschedule() _only_ if there are other task on the
_same_ queue (for this prio level) :
(1) if there is a task with a higher prio, resched_task() would be
called in other place;
(2) if there are tasks of lower prio, we don't care.
I may be wrong, but at the first glance it looks like the following
trick would do the job.. no?
p.s. even if it's ok, it's still a rare codepath (although, it adds
some code to the rare path.. so the drawback is only code-size, I
guess).
--- kernel/sched_rt-prev.c 2007-08-09 09:55:10.000000000 +0200
+++ kernel/sched_rt.c 2007-08-09 09:58:53.000000000 +0200
@@ -207,6 +207,11 @@ static void task_tick_rt(struct rq *rq,
return;
p->time_slice = static_prio_timeslice(p->static_prio);
+
+ /* We are the only element on the queue. */
+ if (p->run_list.prev == p->run_list.next)
+ return;
+
set_tsk_need_resched(p);
/* put it at the end of the queue: */
--
Best regards,
Dmitry Adamushko
* Dmitry Adamushko <[email protected]> wrote:
> I guess, the following thing would do a better job:
>
> - we do need reschedule() _only_ if there are other task on the _same_
> queue (for this prio level) :
ah, indeed.
> --- kernel/sched_rt-prev.c 2007-08-09 09:55:10.000000000 +0200
> +++ kernel/sched_rt.c 2007-08-09 09:58:53.000000000 +0200
> @@ -207,6 +207,11 @@ static void task_tick_rt(struct rq *rq,
> return;
>
> p->time_slice = static_prio_timeslice(p->static_prio);
> +
> + /* We are the only element on the queue. */
> + if (p->run_list.prev == p->run_list.next)
> + return;
> +
i've applied your patch - could you please send a SOB line?
Ingo
FYI, that's the patch i applied:
--------------------------->
Subject: sched: optimize task_tick_rt() a bit
From: Dmitry Adamushko <[email protected]>
Mitchell Erblich suggested a change to not requeue SCHED_RR
tasks if there's only a single task on the runqueue, by
checking for rq->nr_running == 1.
provide a more efficient implementation of that, to check that
particular RT priority-queue only.
[ From: [email protected] ]
Also first requeue the task then set need_resched - results
in slightly better machine-instruction ordering. Also clean
up the code a bit.
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched_rt.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Index: linux/kernel/sched_rt.c
===================================================================
--- linux.orig/kernel/sched_rt.c
+++ linux/kernel/sched_rt.c
@@ -207,10 +207,15 @@ static void task_tick_rt(struct rq *rq,
return;
p->time_slice = static_prio_timeslice(p->static_prio);
- set_tsk_need_resched(p);
- /* put it at the end of the queue: */
- requeue_task_rt(rq, p);
+ /*
+ * Requeue to the end of queue if we are not the only element
+ * on the queue:
+ */
+ if (p->run_list.prev != p->run_list.next) {
+ requeue_task_rt(rq, p);
+ set_tsk_need_resched(p);
+ }
}
static struct sched_class rt_sched_class __read_mostly = {
On 09/08/07, Ingo Molnar <[email protected]> wrote:
>
> FYI, that's the patch i applied:
Thanks. Added my SOB below.
>
> --------------------------->
> Subject: sched: optimize task_tick_rt() a bit
> From: Dmitry Adamushko <[email protected]>
>
> Mitchell Erblich suggested a change to not requeue SCHED_RR
> tasks if there's only a single task on the runqueue, by
> checking for rq->nr_running == 1.
>
> provide a more efficient implementation of that, to check that
> particular RT priority-queue only.
>
> [ From: [email protected] ]
>
> Also first requeue the task then set need_resched - results
> in slightly better machine-instruction ordering. Also clean
> up the code a bit.
>
> Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Dmitry Adamushko <[email protected]>
> ---
> kernel/sched_rt.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> Index: linux/kernel/sched_rt.c
> ===================================================================
> --- linux.orig/kernel/sched_rt.c
> +++ linux/kernel/sched_rt.c
> @@ -207,10 +207,15 @@ static void task_tick_rt(struct rq *rq,
> return;
>
> p->time_slice = static_prio_timeslice(p->static_prio);
> - set_tsk_need_resched(p);
>
> - /* put it at the end of the queue: */
> - requeue_task_rt(rq, p);
> + /*
> + * Requeue to the end of queue if we are not the only element
> + * on the queue:
> + */
> + if (p->run_list.prev != p->run_list.next) {
> + requeue_task_rt(rq, p);
> + set_tsk_need_resched(p);
> + }
> }
>
> static struct sched_class rt_sched_class __read_mostly = {
>
--
Best regards,
Dmitry Adamushko