Messages in this thread | | | Date | Wed, 10 Apr 2013 09:59:11 +0200 | Subject | Re: [PATCH v4] sched: fix wrong rq's runnable_avg update with rt tasks | From | Vincent Guittot <> |
| |
On 10 April 2013 09:26, Peter Zijlstra <peterz@infradead.org> wrote: > On Tue, 2013-04-09 at 11:06 +0200, Vincent Guittot wrote: >> >> +void idle_enter(struct rq *this_rq) >> >> +{ >> >> + update_rq_runnable_avg(this_rq, 1); >> >> +} > >> >> +void idle_exit(struct rq *this_rq) >> >> +{ >> >> + update_rq_runnable_avg(this_rq, 0); >> >> +} >> > >> > These seem like fairly unfortunate names to expose to the global >> > namespace, why not expose update_rq_runnable_avg() instead? >> >> Just to gather in one place all cfs actions that should be done when >> exiting idle even if we only have update_rq_runnable_avg right now. I >> have distinguished that from idle_balance because this sequence can't >> generate extra context switch like idle_balance and they would finally >> not be called in the same time > > OK, but could we then please give then more scheduler specific names? > It just seems to me that idle_enter/idle_exit() are very obvious > function names for unrelated things. > > How about calling it idle_{enter,exit}_fair; so that once other classes > grow hooks we can do something like:
My primary goal was to align with idle_balance name but idle_{enter,exit}_fair is better
In the same way, should we change idle_balance to idle_balance_fair ?
and since we don't have Steve's irq constraint anymore, we could move idle_balance in the beginning of the function pick_next_task_fair ? We will not have spurious switch context and we will remove fair function from __schedule function
Vincent > > static void pre_schedule_idle(struct rq *rq, struct task_struct *p) > { > struct sched_class *class; > > for_each_class(class) { > if (class->idle_enter) > class->idle_enter(rq); > } > } > > or whatnot.. >
| |