linux-kernel - Re: sched: BUG in load

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 18 Feb 2013 21:34:06 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Sasha Levin <sasha.levin@...cle.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Dave Jones <davej@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: sched: BUG in load_balance

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/