linux-kernel - Re: [patch 1/6] rtmutex: Fix deadlock detector for real

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.DEB.2.02.1405281144150.21720@ionos.tec.linutronix.de>
Date:	Wed, 28 May 2014 11:57:11 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Steven Rostedt <rostedt@...dmis.org>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Lai Jiangshan <laijs@...fujitsu.com>
Subject: Re: [patch 1/6] rtmutex: Fix deadlock detector for real

On Tue, 27 May 2014, Steven Rostedt wrote:
> On Thu, 22 May 2014 03:25:39 -0000
> Thomas Gleixner <tglx@...utronix.de> wrote:
> 
> >  	 * When deadlock detection is off then we check, if further
> >  	 * priority adjustment is necessary.
> >  	 */
> > -	if (!detect_deadlock && waiter->prio == task->prio)
> > -		goto out_unlock_pi;
> > +	if (waiter->prio == task->prio) {
> > +		if (!detect_deadlock)
> > +			goto out_unlock_pi;
> > +	}
> 
> This too.
> 
> Although! if you want to micro-optimize the detect_deadlock case
> where !detect_deadlock is false. You might want to reverse the order.
> That way we don't need to dereference the ->prio for both waiter and
> task before seeing that we don't go to the out_unlock_pi.
> 
> 	if (!detect_deadlock) {
> 		if (waiter->prio == task->prio)
> 			goto out_unlock_pi;
> 	}
> 
> Hmm, or you did it this way for your "don't requeue" patch? Looking at
> that one, it seems you did.
> 
> 	if (waiter->prio == task->prio) {
> 		if (!detect_deadlock)
> 			goto out_unlock_pi;
> 		requeue = false;
> 	}
> 
> Oh well. But for stable maybe have the optimized way? And change it
> back when you add the requeue patch?

Right.

> > @@ -527,6 +538,10 @@ static int task_blocks_on_rt_mutex(struc
> >  	unsigned long flags;
> >  	int chain_walk = 0, res;
> >  
> > +	/* Early deadlock detection */
> > +	if (detect_deadlock && owner == task)
> > +		return -EDEADLK;
> > +
> 
> This is an optimization, right? Does it belong for stable?

It's kind of an optimization, but we really want to avoid enqueueing
the task on its own pi waiter list in the first place. So I rather
keep it for stable as well.

Thanks,

	tglx
--
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/