On Mon, 2013-02-18 at 21:19 -0500, Sasha Levin wrote: > > Only compiled tested: > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 0fcdbff..a31174c 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -5222,9 +5222,9 @@ void idle_balance(int this_cpu, struct rq *this_rq) > > update_rq_runnable_avg(this_rq, 1); > > > > /* > > - * Drop the rq->lock, but keep preempt disabled. > > + * Drop the rq->lock, but keep softirqs disabled. > > */ > > - preempt_disable(); > > + local_bh_disable(); > > raw_spin_unlock_irq(&this_rq->lock); > > > > update_blocked_averages(this_cpu); > > @@ -5253,7 +5253,7 @@ void idle_balance(int this_cpu, struct rq *this_rq) > > rcu_read_unlock(); > > > > raw_spin_lock_irq(&this_rq->lock); > > - preempt_enable(); > > + local_bh_enable(); > > I have to admit, I'm slightly confused with the patch: there's a > raw_spin_lock_irq() > followed by local_bh_enable(). afaik it's illegal to call local_bh_enable() > with > interrupts disabled. >
Bah, you're right. I was trying to enable interrupts without enabling softirqs, but if an interrupt happens here and raises a softirq, it will miss being executed by the local_bh_enabled(). We can keep the preempt disable and only disable local_bh, around the idle_balance(). But still, I'm getting uncomfortable with these patches, they may need more work, and Peter's not too happy with them either. Ingo, Can you revert this and the previous patch, and I'll work with Peter to get something that we can agree on, where we can hopefully remove the idle hooks from the scheduler. Thanks, -- Steve -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
