[patch RT 1/4] rtmutex: Make lock_killable work

[patch RT 1/4] rtmutex: Make lock_killable work

[Thomas Gleixner]

[-- Attachment #1: rtmutex-Make-lock_killable-work.patch --]
[-- Type: text/plain, Size: 1011 bytes --]

Locking an rt mutex killable does not work because signal handling is
restricted to TASK_INTERRUPTIBLE.

Use signal_pending_state() unconditionaly.

Cc: rt-stable@vger.kernel.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/locking/rtmutex.c |   19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1633,18 +1633,13 @@ static int __sched
 		if (try_to_take_rt_mutex(lock, current, waiter))
 			break;
 
-		/*
-		 * TASK_INTERRUPTIBLE checks for signals and
-		 * timeout. Ignored otherwise.
-		 */
-		if (unlikely(state == TASK_INTERRUPTIBLE)) {
-			/* Signal pending? */
-			if (signal_pending(current))
-				ret = -EINTR;
-			if (timeout && !timeout->task)
-				ret = -ETIMEDOUT;
-			if (ret)
-				break;
+		if (timeout && !timeout->task) {
+			ret = -ETIMEDOUT;
+			break;
+		}
+		if (signal_pending_state(state, current)) {
+			ret = -EINTR;
+			break;
 		}
 
 		if (ww_ctx && ww_ctx->acquired > 0) {

Re: [patch RT 1/4] rtmutex: Make lock_killable work

[Steven Rostedt]

On Sat, 01 Apr 2017 12:50:59 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> Locking an rt mutex killable does not work because signal handling is
> restricted to TASK_INTERRUPTIBLE.
> 
> Use signal_pending_state() unconditionaly.

Does this mean rt mutex killable is not INTERRUPTIBLE? because the
change log seems to just assume that.

In other words, mortals reading this have no idea what you are talking
about ;-)

-- Steve

> 
> Cc: rt-stable@vger.kernel.org
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/locking/rtmutex.c |   19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1633,18 +1633,13 @@ static int __sched
>  		if (try_to_take_rt_mutex(lock, current, waiter))
>  			break;
>  
> -		/*
> -		 * TASK_INTERRUPTIBLE checks for signals and
> -		 * timeout. Ignored otherwise.
> -		 */
> -		if (unlikely(state == TASK_INTERRUPTIBLE)) {
> -			/* Signal pending? */
> -			if (signal_pending(current))
> -				ret = -EINTR;
> -			if (timeout && !timeout->task)
> -				ret = -ETIMEDOUT;
> -			if (ret)
> -				break;
> +		if (timeout && !timeout->task) {
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (signal_pending_state(state, current)) {
> +			ret = -EINTR;
> +			break;
>  		}
>  
>  		if (ww_ctx && ww_ctx->acquired > 0) {
> 

Re: [patch RT 1/4] rtmutex: Make lock_killable work

[Peter Zijlstra]

On Mon, Apr 03, 2017 at 11:21:26AM -0400, Steven Rostedt wrote:
> On Sat, 01 Apr 2017 12:50:59 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > Locking an rt mutex killable does not work because signal handling is
> > restricted to TASK_INTERRUPTIBLE.
> > 
> > Use signal_pending_state() unconditionaly.
> 
> Does this mean rt mutex killable is not INTERRUPTIBLE? because the
> change log seems to just assume that.

> > -		if (unlikely(state == TASK_INTERRUPTIBLE)) {

#define TASK_KILLABLE           (TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)


I don't think we need to consider people who don't know where to find
the TASK_state definitions.

Re: [patch RT 1/4] rtmutex: Make lock_killable work

[Steven Rostedt]

On Tue, 4 Apr 2017 09:13:02 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Apr 03, 2017 at 11:21:26AM -0400, Steven Rostedt wrote:
> > On Sat, 01 Apr 2017 12:50:59 +0200
> > Thomas Gleixner <tglx@linutronix.de> wrote:
> >   
> > > Locking an rt mutex killable does not work because signal handling is
> > > restricted to TASK_INTERRUPTIBLE.
> > > 
> > > Use signal_pending_state() unconditionaly.  
> > 
> > Does this mean rt mutex killable is not INTERRUPTIBLE? because the
> > change log seems to just assume that.  
> 
> > > -		if (unlikely(state == TASK_INTERRUPTIBLE)) {  
> 
> #define TASK_KILLABLE           (TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)
> 
> 
> I don't think we need to consider people who don't know where to find
> the TASK_state definitions.

No where in the change log did it mention TASK_KILLABLE. It only talked
about "rt mutex killable". Yeah, I can figure this out, but that
doesn't change that the fact that it was a weak change log.

Thomas has yelled at me for some of my change logs in the past that
were better than this ;-)

-- Steve