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

[Thomas Gleixner <tglx@linutronix.de>]

On Tue, 27 May 2014, Steven Rostedt wrote:
> On Thu, 22 May 2014 03:25:39 -0000
> Thomas Gleixner <tglx@linutronix.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