On 01/17/2014 03:09 PM, Peter Zijlstra wrote:

On Fri, Jan 17, 2014 at 10:04:04AM +0100, Daniel Lezcano wrote:

...

• schedstat_inc(rq, sched_goidle);

+#ifdef CONFIG_SMP

• /* Trigger the post schedule to do an idle_enter for CFS */
• rq->post_schedule = 1;

+#endif

• return rq->idle;

Urgh, that retains the stupid idle crap like it is.

I've not yet tested this, but is there a reason something like the below couldn't work?

I tested it by running hackbench and it seems to work as expected.

---

Subject: sched: Clean up idle task SMP logic From: Peter Zijlstra peterz@infradead.org Date: Fri Jan 17 14:54:02 CET 2014

The idle post_schedule hook is just a vile waste of time, fix it proper.

Signed-off-by: Peter Zijlstra peterz@infradead.org

kernel/sched/fair.c | 5 +++-- kernel/sched/idle_task.c | 21 ++++++--------------- 2 files changed, 9 insertions(+), 17 deletions(-)

Index: linux-2.6/kernel/sched/fair.c

--- linux-2.6.orig/kernel/sched/fair.c +++ linux-2.6/kernel/sched/fair.c @@ -2416,7 +2416,8 @@ void idle_exit_fair(struct rq *this_rq) update_rq_runnable_avg(this_rq, 0); }

-#else +#else /* CONFIG_SMP */

• static inline void update_entity_load_avg(struct sched_entity *se, int update_cfs_rq) {} static inline void update_rq_runnable_avg(struct rq *rq, int runnable) {}

@@ -2428,7 +2429,7 @@ static inline void dequeue_entity_load_a int sleep) {} static inline void update_cfs_rq_blocked_load(struct cfs_rq *cfs_rq, int force_update) {} -#endif +#endif /* CONFIG_SMP */

static void enqueue_sleeper(struct cfs_rq *cfs_rq, struct sched_entity *se) { Index: linux-2.6/kernel/sched/idle_task.c =================================================================== --- linux-2.6.orig/kernel/sched/idle_task.c +++ linux-2.6/kernel/sched/idle_task.c @@ -13,18 +13,8 @@ select_task_rq_idle(struct task_struct * { return task_cpu(p); /* IDLE tasks as never migrated */ }

-static void pre_schedule_idle(struct rq *rq, struct task_struct *prev) -{

• idle_exit_fair(rq);
• rq_last_tick_reset(rq);

-}

-static void post_schedule_idle(struct rq *rq) -{

• idle_enter_fair(rq);

-} #endif /* CONFIG_SMP */

• /*
  • Idle tasks are unconditionally rescheduled:
  */

@@ -37,8 +27,7 @@ static struct task_struct *pick_next_tas { schedstat_inc(rq, sched_goidle); #ifdef CONFIG_SMP

• /* Trigger the post schedule to do an idle_enter for CFS */
• rq->post_schedule = 1;

• idle_enter_fair(rq); #endif return rq->idle; }

@@ -58,6 +47,10 @@ dequeue_task_idle(struct rq *rq, struct

static void put_prev_task_idle(struct rq *rq, struct task_struct *prev) { +#ifdef CONFIG_SMP

• idle_exit_fair(rq);
• rq_last_tick_reset(rq);

+#endif }

static void task_tick_idle(struct rq *rq, struct task_struct *curr, int queued) @@ -101,8 +94,6 @@ const struct sched_class idle_sched_clas

#ifdef CONFIG_SMP .select_task_rq = select_task_rq_idle,

• .pre_schedule = pre_schedule_idle,

• .post_schedule = post_schedule_idle, #endif

  .set_curr_task = set_curr_task_idle,

On Fri, Jan 17, 2014 at 10:04:04AM +0100, Daniel Lezcano wrote:

@@ -2679,11 +2715,8 @@ need_resched: pre_schedule(rq, prev);

• if (unlikely(!rq->nr_running))

• rq->idle_stamp = idle_balance(rq) ? 0 : rq_clock(rq);
  

• put_prev_task(rq, prev);
• next = pick_next_task(rq);

• next = pick_next_task_or_idle(rq); clear_tsk_need_resched(prev); clear_preempt_need_resched(); rq->skip_clock_update = 0;

I have vague memories that we need to have idle_balance() before put_prev_task(), but I can't recollect why this would be so.

That said, if I resurrect these patches:

https://lkml.org/lkml/2012/6/14/271

I suppose we could write something like:

struct task_struct *pick_next_task(struct rq *rq, struct task_struct *prev) { const struct sched_class *class; struct task_struct *p;

again: if (likely(rq->nr_running)) {

if (likely(rq->nr_running == rq->cfs.h_nr_running)) return fair_sched_class.pick_next_task(rq, prev);

for_each_class(class) { p = class->pick_next_task(rq, prev); if (p) return p; } }

if (idle_balance(rq)) goto again;

rq->idle_stamp = rq_clock(rq);

return idle_sched_class.pick_next_task(rq, prev); }

Which keeps idle_balance() before put_prev_task(), and by using idle_sched_clas.pick_next_task() doesn't rape the idle class interface like you did :-)

From: Peter Zijlstra peterz@infradead.org

With the previous patches, we have no ambiguity on going to idle. So we can return directly the idle task instead of looking up all the domains which will in any case return the idle_task.

Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- kernel/sched/core.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 76f0075..b5237dc 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2563,23 +2563,33 @@ pick_next_task(struct rq *rq, struct task_struct *prev) const struct sched_class *class; struct task_struct *p;

+again: + if (likely(rq->nr_running)) { + /* + * Optimization: we know that if all tasks are in + * the fair class we can call that function directly: + */ + if (likely(rq->nr_running == rq->cfs.h_nr_running)) + return fair_sched_class.pick_next_task(rq, prev); + + for_each_class(class) { + p = class->pick_next_task(rq, prev); + if (p) + return p; + } + } + /* - * Optimization: we know that if all tasks are in - * the fair class we can call that function directly: + * If there is a task balanced on this cpu, pick the next task, + * otherwise fall in the optimization by picking the idle task + * directly. */ - if (likely(rq->nr_running == rq->cfs.h_nr_running)) { - p = fair_sched_class.pick_next_task(rq, prev); - if (likely(p)) - return p; - } + if (idle_balance(rq)) + goto again;

- for_each_class(class) { - p = class->pick_next_task(rq, prev); - if (p) - return p; - } + rq->idle_stamp = rq_clock(rq);

- BUG(); /* the idle class will always have a runnable task */ + return idle_sched_class.pick_next_task(rq, prev); }

/* @@ -2672,9 +2682,6 @@ need_resched:

pre_schedule(rq, prev);

- if (unlikely(!rq->nr_running)) - rq->idle_stamp = idle_balance(rq) ? 0 : rq_clock(rq); - if (prev->on_rq || rq->skip_clock_update < 0) update_rq_clock(rq);