2007-11-23 11:45:47

by Nikanth Karthikesan

[permalink] [raw]
Subject: [PATCH] sched: minor optimization

As an optimization, if all tasks are in the fair class, the next task is
directly picked from fair_sched_class. But, if it returns no task we go
through again from sched_class_highest which could be avoided and
instead return the idle task directly.


Signed-off-by : Nikanth Karthikesan <[email protected]>
---
kernel/sched.c | 2 ++
1 file changed, 2 insertions(+)

Index: b/kernel/sched.c
===================================================================
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3449,6 +3449,8 @@ pick_next_task(struct rq *rq, struct tas
p = fair_sched_class.pick_next_task(rq);
if (likely(p))
return p;
+ else /* rq->nr_running is zero */
+ return idle_sched_class.pick_next_task(rq);
}

class = sched_class_highest;


2007-11-23 12:18:30

by Dmitry Adamushko

[permalink] [raw]
Subject: Re: [PATCH] sched: minor optimization

On 24/11/2007, Nikanth Karthikesan <[email protected]> wrote:
> As an optimization, if all tasks are in the fair class, the next task is
> directly picked from fair_sched_class. But, if it returns no task we go
> through again from sched_class_highest which could be avoided and
> instead return the idle task directly.

The only legitimate possibility of having the fair_sched_class
returning no task in this case is when 'rq->nr_running ==
rq->cfs.nr_running == 0'.

iow, a possible optimization would be just the following check :

if (rq->nr_running == 0)
return idle_sched_class.pick_next_task(rq);

at the beginning of pick_next_task().

(or maybe put it at the beginning of the
if (likely(rq->nr_running == rq->cfs.nr_running)) {} block as we
already have 'likely()' there).


--
Best regards,
Dmitry Adamushko

2007-11-23 13:25:15

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [PATCH] sched: minor optimization

> On Fri, Nov 23, 2007 at 5:48 PM, "Dmitry Adamushko"
<[email protected]> wrote:
> The only legitimate possibility of having the fair_sched_class
> returning no task in this case is when 'rq->nr_running ==
> rq->cfs.nr_running == 0'.

Yes, I think so.

> iow, a possible optimization would be just the following check :
>
> if (rq->nr_running == 0)
> return idle_sched_class.pick_next_task(rq);
> at the beginning of pick_next_task().
>
> (or maybe put it at the beginning of the
> if (likely(rq->nr_running == rq->cfs.nr_running)) {} block as we
> already have 'likely()' there).
>

But that might add a test before the case we want to optimize the most.
I just thought of taking advantage of a case where we know
rq->nr_running==0, instead of throwing away that information.

Thanks
Nikanth