Date: Fri, 8 Apr 2016 12:25:10 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: xlpang@redhat.com, linux-kernel@vger.kernel.org,
        Juri Lelli <juri.lelli@arm.com>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] sched/deadline/rtmutex: Fix a PI crash for deadline
 tasks
Message-ID: <20160408122510.15978179@gandalf.local.home>
In-Reply-To: <20160405092954.GC24771@twins.programming.kicks-ass.net>
References: <1459508418-25577-1-git-send-email-xlpang@redhat.com>
	<20160401113827.GQ3430@twins.programming.kicks-ass.net>
	<56FE685E.6080001@redhat.com>
	<19912883-8AB1-4DFD-A0E1-F23057785243@infradead.org>
	<56FE78E0.5060504@redhat.com>
	<20160401215143.GB2906@worktop>
	<57037974.1020002@redhat.com>
	<20160405091954.GI3448@twins.programming.kicks-ass.net>
	<20160405092954.GC24771@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1929
Lines: 57

On Tue, 5 Apr 2016 11:29:54 +0200
Peter Zijlstra <peterz@infradead.org> wrote:


> --
>  kernel/locking/rtmutex.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 3e746607abe5..1896baf28e9c 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1390,11 +1390,21 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
>  	} else {
>  		bool deboost = slowfn(lock, &wake_q);
>  
> -		wake_up_q(&wake_q);
> -
> -		/* Undo pi boosting if necessary: */
> +		/*
> +		 * Undo pi boosting (if necessary) and wake top waiter.
> +		 *
> +		 * We should deboost before waking the high-prio task such that
> +		 * we don't run two tasks with the 'same' state. This however
> +		 * can lead to prio-inversion if we would get preempted after
> +		 * the deboost but before waking our high-prio task, hence the
> +		 * preempt_disable.
> +		 */
> +		preempt_disable();
>  		if (deboost)
>  			rt_mutex_adjust_prio(current);
> +
> +		wake_up_q(&wake_q);
> +		preempt_enable();
>  	}
>  }
>  

So the preempt_disable() is to allow us to set current back to its
normal priority first before waking up the other task because we don't
want two tasks at the same priority?

Just remember, calling preempt_disable() is semantically the same as
setting your priority as the highest task on the CPU. Thus the above
fix is to set the one task to the highest priority so that we can
deboost it when we remove the highest priority (preempt_enable()),
after we wake up the task that will be lower priority then current
until current calls preempt_enable().

This will of course keep a task that is higher in priority than both
current and the waking task from running till this is all completed.

What's the point of swapping deboost and the wake up again?

Maybe I'm missing something, what exactly do you mean by "same state"?

-- Steve