rtmutex: Code clarification and optimization

There is no point to keep the task ref across the check for lock owner. Drop the ref before that, so the protection context is clear.

Found while documenting the chain walk.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +3 -2

@@ -411,6 +411,8 @@ static int rt_mutex_adjust_prio_chain(st
 
 	/* Release the task */
 	raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+	put_task_struct(task);
+
 	if (!rt_mutex_owner(lock)) {
 		/*
 		 * If the requeue above changed the top waiter, then we need

@@ -420,9 +422,8 @@ static int rt_mutex_adjust_prio_chain(st
 		if (top_waiter != rt_mutex_top_waiter(lock))
 			wake_up_process(rt_mutex_top_waiter(lock)->task);
 		raw_spin_unlock(&lock->wait_lock);
-		goto out_put_task;
+		return 0;
 	}
-	put_task_struct(task);
 
 	/* Grab the next task */
 	task = rt_mutex_owner(lock);

Quoted 3 lines · On Mon, 09 Jun 2014 20:28:06 -0000 Thomas Gleixner <[email protected]> wrote:

There is no point to keep the task ref across the check for lock owner. Drop the ref before that, so the protection context is clear.

Quoted 1 line

Found while documenting the chain walk.

This looks fine, I just hate the subject. I don't see how it is 'deobfuscating" the chain walk. How about:

rtmutex: No need to keep task ref when checking lock ownership

Reviewed-by Steven Rostedt <[email protected]>

There is no point to keep the task ref across the check for lock owner. Drop the ref before that, so the protection context is clear.

Found while documenting the chain walk.

Signed-off-by Thomas Gleixner <[email protected]>

There is no point to keep the task ref across the check for lock owner. Drop the ref before that, so the protection context is clear.

Found while documenting the chain walk.

Signed-off-by Thomas Gleixner <[email protected]>

Add a separate local variable for the boost/deboost logic to make the code more readable. Add comments where appropriate.

Add a separate local variable for the boost/deboost logic to make the code more readable. Add comments where appropriate.

Signed-off-by Thomas Gleixner <[email protected]>

Add a separate local variable for the boost/deboost logic to make the code more readable. Add comments where appropriate.

Signed-off-by Thomas Gleixner <[email protected]>

Add commentry to document the chain walk and the protection mechanisms and their scope.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +14 -0

+ *	@task					[A]
+ *	@orig_lock if != NULL			@top_task is blocked on it
+ *	@next_lock				Unprotected. Cannot be
+ *						dereferenced. Only used for
+ *						comparison.
+ *	@orig_waiter if != NULL			@top_task is blocked on it
+ *	@top_task				current, or in case of proxy
+ *						locking protected by calling
+ *						code
+ * again:
+ *	loop_sanity_check();
+ * retry:
+ *	lock(task->pi_lock);			[A] acquire [B]
+ *	waiter = task->pi_blocked_on;		[B]

kernel/locking/rtmutex.c +17 -0

+ *	lock(task->pi_lock);			[C] acquire [B]
+ *	requeue_pi_waiter(task, waiters(lock));	[B] + [C]
+ *	check_exit_conditions();		[B] + [C]
+ *	unlock(task->pi_lock);			drop [B]
+ *	unlock(lock->wait_lock);		drop [C]
+ *	goto again;
  */
 static int rt_mutex_adjust_prio_chain(struct task_struct *task,
 				      int deadlock_detect,

@@ -326,6 +367,12 @@ static int rt_mutex_adjust_prio_chain(st
 
 		return -EDEADLK;
 	}
+
+	/*
+	 * We are fully preemptible here and only hold the refcount on
+	 * @task. So everything can have changed under us since the
+	 * caller or our own code below (goto retry) dropped all locks.
+	 */
  retry:
 	/*
 	 * Task can not go away as we did a get_task() before !

@@ -383,6 +430,11 @@ static int rt_mutex_adjust_prio_chain(st
 	if (!detect_deadlock && waiter->prio == task->prio)
 		goto out_unlock_pi;
 
+	/*
+	 * We need to trylock here as we are holding task->pi_lock,
+	 * which is the reverse lock order versus the other rtmutex
+	 * operations.
+	 */
 	lock = waiter->lock;
 	if (!raw_spin_trylock(&lock->wait_lock)) {
 		raw_spin_unlock_irqrestore(&task->pi_lock, flags);

Quoted 13 lines · On Mon, 09 Jun 2014 20:28:08 -0000 Thomas Gleixner <[email protected]> wrote:

Add commentry to document the chain walk and the protection mechanisms and their scope.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c static inline struct rt_mutex *task_bloc · line 293

Quoted 9 lines

kernel/locking/rtmutex.c +6 -0

@@ -285,6 +285,47 @@ static inline struct rt_mutex *task_bloc
  * @top_task:	the current top waiter
  *
  * Returns 0 or -EDEADLK.
+ *
+ * Chain walk basics and protection scope
+ *
+ * [A] refcount on task
+ * [B] task->pi_lock held
+ * [C] rtmutex->lock held

A,B, C is rather meaningless, and requires constant looking back up at the key. Perhaps [R],[P] and [L]

[R] refcount on task (get_task_struct)
[P] task->pi_lock held
[L] rtmutex->lock held

That way we can associate R being refcount, P being pi_lock and L being lock. Easier to remember.

kernel/locking/rtmutex.c static inline struct rt_mutex *task_bloc · line 295

Quoted 2 lines

kernel/locking/rtmutex.c +2 -0

+ *
+ * call()					Protected by

"call()"?

Quoted 14 lines

kernel/locking/rtmutex.c +14 -0

+ *	@task					[A]
+ *	@orig_lock if != NULL			@top_task is blocked on it
+ *	@next_lock				Unprotected. Cannot be
+ *						dereferenced. Only used for
+ *						comparison.
+ *	@orig_waiter if != NULL			@top_task is blocked on it
+ *	@top_task				current, or in case of proxy
+ *						locking protected by calling
+ *						code
+ * again:
+ *	loop_sanity_check();
+ * retry:
+ *	lock(task->pi_lock);			[A] acquire [B]
+ *	waiter = task->pi_blocked_on;		[B]

kernel/locking/rtmutex.c static inline struct rt_mutex *task_bloc · line 319

Quoted 10 lines

kernel/locking/rtmutex.c +10 -0

+ *	check_exit_conditions();		[B]
+ *	lock = waiter->lock;			[B]
+ *	if (!try_lock(lock->wait_lock)) {	[B] try to acquire [C]
+ *		unlock(task->pi_lock);		drop [B]
+ *		goto retry;
+ *	}
+ *	check_exit_conditions();		[B] + [C]
+ *	requeue_lock_waiter(lock, waiter);	[B] + [C]
+ *	unlock(task->pi_lock);			drop [B]
+ *	drop_task_ref(task);			drop [A]

Maybe just state "put_task_struct()", less abstractions.

kernel/locking/rtmutex.c static inline struct rt_mutex *task_bloc · line 322

Quoted 3 lines

kernel/locking/rtmutex.c +3 -0

+ *	check_exit_conditions();		[C]
+ *	task = owner(lock);			[C]
+ *	get_task_ref(task);			[C] acquire [A]

get_task_struct()

-- Steve

Quoted 7 lines

kernel/locking/rtmutex.c +6 -0

+ *	lock(task->pi_lock);			[C] acquire [B]
+ *	requeue_pi_waiter(task, waiters(lock));	[B] + [C]
+ *	check_exit_conditions();		[B] + [C]
+ *	unlock(task->pi_lock);			drop [B]
+ *	unlock(lock->wait_lock);		drop [C]
+ *	goto again;
  */

Quoted 18 lines

kernel/locking/rtmutex.c +6 -0

 static int rt_mutex_adjust_prio_chain(struct task_struct *task,
 				      int deadlock_detect,

@@ -326,6 +367,12 @@ static int rt_mutex_adjust_prio_chain(st
 
 		return -EDEADLK;
 	}
+
+	/*
+	 * We are fully preemptible here and only hold the refcount on
+	 * @task. So everything can have changed under us since the
+	 * caller or our own code below (goto retry) dropped all locks.
+	 */
  retry:
 	/*
 	 * Task can not go away as we did a get_task() before !

@@ -383,6 +430,11 @@ static int rt_mutex_adjust_prio_chain(st
 	if (!detect_deadlock && waiter->prio == task->prio)

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 440

Quoted 10 lines

kernel/locking/rtmutex.c +5 -0

 		goto out_unlock_pi;
 
+	/*
+	 * We need to trylock here as we are holding task->pi_lock,
+	 * which is the reverse lock order versus the other rtmutex
+	 * operations.
+	 */
 	lock = waiter->lock;
 	if (!raw_spin_trylock(&lock->wait_lock)) {
 		raw_spin_unlock_irqrestore(&task->pi_lock, flags);

.

Add commentry to document the chain walk and the protection mechanisms and their scope.

Signed-off-by Thomas Gleixner <[email protected]>

Exit right away, when the removed waiter was not the top prioriy waiter on the lock. Get rid of the extra indent level.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +1 -1

@@ -780,7 +780,7 @@ static void remove_waiter(struct rt_mute
 {
 	int first = (waiter == rt_mutex_top_waiter(lock));
 	struct task_struct *owner = rt_mutex_owner(lock);
-	struct rt_mutex *next_lock = NULL;
+	struct rt_mutex *next_lock;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&current->pi_lock, flags);

kernel/locking/rtmutex.c +9 -15

-	if (!owner)
+	if (!owner || !first)
 		return;
 
-	if (first) {
+	raw_spin_lock_irqsave(&owner->pi_lock, flags);
 
-		raw_spin_lock_irqsave(&owner->pi_lock, flags);
+	rt_mutex_dequeue_pi(owner, waiter);
 
-		rt_mutex_dequeue_pi(owner, waiter);
+	if (rt_mutex_has_waiters(lock))
+		rt_mutex_enqueue_pi(owner, rt_mutex_top_waiter(lock));
 
-		if (rt_mutex_has_waiters(lock)) {
-			struct rt_mutex_waiter *next;
+	__rt_mutex_adjust_prio(owner);
 
-			next = rt_mutex_top_waiter(lock);
-			rt_mutex_enqueue_pi(owner, next);
-		}
-		__rt_mutex_adjust_prio(owner);
+	/* Store the lock on which owner is blocked or NULL */
+	next_lock = task_blocked_on_lock(owner);
 
-		/* Store the lock on which owner is blocked or NULL */
-		next_lock = task_blocked_on_lock(owner);
-
-		raw_spin_unlock_irqrestore(&owner->pi_lock, flags);
-	}
+	raw_spin_unlock_irqrestore(&owner->pi_lock, flags);
 
 	if (!next_lock)
 		return;

Quoted 22 lines · On Mon, 09 Jun 2014 20:28:08 -0000 Thomas Gleixner <[email protected]> wrote:

Exit right away, when the removed waiter was not the top prioriy waiter on the lock. Get rid of the extra indent level.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +1 -1

@@ -780,7 +780,7 @@ static void remove_waiter(struct rt_mute
 {
 	int first = (waiter == rt_mutex_top_waiter(lock));
 	struct task_struct *owner = rt_mutex_owner(lock);
-	struct rt_mutex *next_lock = NULL;
+	struct rt_mutex *next_lock;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&current->pi_lock, flags);

kernel/locking/rtmutex.c static void remove_waiter(struct rt_mute · line 790

Quoted 3 lines

kernel/locking/rtmutex.c +0 -0

@@ -788,28 +788,22 @@ static void remove_waiter(struct rt_mute
 	current->pi_blocked_on = NULL;
 	raw_spin_unlock_irqrestore(&current->pi_lock, flags);
 

Add comment here, something like...

	/*
	 * Only update priority if this task was the highest priority
	 * task waiting on the lock, and there is an owner to update.
	 */

Rest looks good.

Reviewed-by Steven Rostedt <[email protected]>

Quoted 22 lines · On Mon, 09 Jun 2014 20:28:08 -0000 Thomas Gleixner <[email protected]> wrote:

Exit right away, when the removed waiter was not the top prioriy waiter on the lock. Get rid of the extra indent level.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +1 -1

@@ -780,7 +780,7 @@ static void remove_waiter(struct rt_mute
 {
      int first = (waiter == rt_mutex_top_waiter(lock));
      struct task_struct *owner = rt_mutex_owner(lock);
-     struct rt_mutex *next_lock = NULL;
+     struct rt_mutex *next_lock;
      unsigned long flags;
 
      raw_spin_lock_irqsave(&current->pi_lock, flags);

kernel/locking/rtmutex.c static void remove_waiter(struct rt_mute · line 790

Quoted 3 lines

kernel/locking/rtmutex.c +0 -0

@@ -788,28 +788,22 @@ static void remove_waiter(struct rt_mute
      current->pi_blocked_on = NULL;
      raw_spin_unlock_irqrestore(&current->pi_lock, flags);
 

Add comment here, something like...

/*
* Only update priority if this task was the highest priority
* task waiting on the lock, and there is an owner to update. */

Rest looks good.

Reviewed-by Steven Rostedt <[email protected]>

-- Steve

Quoted 34 lines

kernel/locking/rtmutex.c +9 -15

-     if (!owner)
+     if (!owner || !first)
              return;
 
-     if (first) {
+     raw_spin_lock_irqsave(&owner->pi_lock, flags);
 
-             raw_spin_lock_irqsave(&owner->pi_lock, flags);
+     rt_mutex_dequeue_pi(owner, waiter);
 
-             rt_mutex_dequeue_pi(owner, waiter);
+     if (rt_mutex_has_waiters(lock))
+             rt_mutex_enqueue_pi(owner, rt_mutex_top_waiter(lock));
 
-             if (rt_mutex_has_waiters(lock)) {
-                     struct rt_mutex_waiter *next;
+     __rt_mutex_adjust_prio(owner);
 
-                     next = rt_mutex_top_waiter(lock);
-                     rt_mutex_enqueue_pi(owner, next);
-             }
-             __rt_mutex_adjust_prio(owner);
+     /* Store the lock on which owner is blocked or NULL */
+     next_lock = task_blocked_on_lock(owner);
 
-             /* Store the lock on which owner is blocked or NULL */
-             next_lock = task_blocked_on_lock(owner);
-
-             raw_spin_unlock_irqrestore(&owner->pi_lock, flags);
-     }
+     raw_spin_unlock_irqrestore(&owner->pi_lock, flags);
 
      if (!next_lock)
              return;

Exit right away, when the removed waiter was not the top prioriy waiter on the lock. Get rid of the extra indent level.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +15 -7

+/*
+ * Futex variant to allow full deadlock detection.
+ */
+int __rt_mutex_timed_lock(struct rt_mutex *lock,
+			  struct hrtimer_sleeper *timeout)
+{
+	might_sleep();
+
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+				       rt_mutex_slowlock);
+}
+
 /**
  * rt_mutex_timed_lock - lock a rt_mutex interruptible
  *			the timeout structure is provided
  *			by the caller
  *
- * @lock: 		the rt_mutex to be locked
+ * @lock:		the rt_mutex to be locked
  * @timeout:		timeout structure or NULL (no timeout)
- * @detect_deadlock:	deadlock detection on/off
  *
  * Returns:
- *  0 		on success
- * -EINTR 	when interrupted by a signal
+ *  0		on success
+ * -EINTR	when interrupted by a signal
  * -ETIMEDOUT	when the timeout expired
- * -EDEADLK	when the lock would deadlock (when deadlock detection is on)
  */
 int
-rt_mutex_timed_lock(struct rt_mutex *lock, struct hrtimer_sleeper *timeout,
-		    int detect_deadlock)

kernel/locking/rtmutex.c void __sched rt_mutex_lock(struct rt_mut · line 1124

Quoted 11 lines · On Mon, 09 Jun 2014 20:28:09 -0000 Thomas Gleixner <[email protected]> wrote:

kernel/locking/rtmutex.c +11 -0

+/*
+ * Futex variant to allow full deadlock detection.
+ */
+int __rt_mutex_timed_lock(struct rt_mutex *lock,
+			  struct hrtimer_sleeper *timeout)
+{
+	might_sleep();
+
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+				       rt_mutex_slowlock);
+}

Quoted 26 lines

kernel/locking/rtmutex.c +5 -7

+
 /**
  * rt_mutex_timed_lock - lock a rt_mutex interruptible
  *			the timeout structure is provided
  *			by the caller
  *
- * @lock: 		the rt_mutex to be locked
+ * @lock:		the rt_mutex to be locked
  * @timeout:		timeout structure or NULL (no timeout)
- * @detect_deadlock:	deadlock detection on/off
  *
  * Returns:
- *  0 		on success
- * -EINTR 	when interrupted by a signal
+ *  0		on success
+ * -EINTR	when interrupted by a signal
  * -ETIMEDOUT	when the timeout expired
- * -EDEADLK	when the lock would deadlock (when deadlock detection is on)
  */
 int
-rt_mutex_timed_lock(struct rt_mutex *lock, struct hrtimer_sleeper *timeout,
-		    int detect_deadlock)
+rt_mutex_timed_lock(struct rt_mutex *lock, struct hrtimer_sleeper *timeout)
 {
 	might_sleep();

kernel/locking/rtmutex.c void __sched rt_mutex_lock(struct rt_mut · line 1147

Quoted 7 lines

kernel/locking/rtmutex.c +2 -2

-	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout,
-				       detect_deadlock, rt_mutex_slowlock);
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+				       rt_mutex_slowlock);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_timed_lock);

I must be missing something. What's the difference between the above and the futex variant one? They both do the exact same thing:

int foo(struct rt_mutex *lock, struct hrtimer_sleeper *timeout)

{
	might_sleep();

	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timout,
				       0, rt_mutex_slowlock);
}

??

-- Steve
.

Quoted 34 lines · On Mon, 09 Jun 2014 20:28:09 -0000 Thomas Gleixner <[email protected]> wrote:

kernel/locking/rtmutex.c +15 -7

+/*
+ * Futex variant to allow full deadlock detection.
+ */
+int __rt_mutex_timed_lock(struct rt_mutex *lock,
+			  struct hrtimer_sleeper *timeout)
+{
+	might_sleep();
+
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+				       rt_mutex_slowlock);
+}
+
 /**
  * rt_mutex_timed_lock - lock a rt_mutex interruptible
  *			the timeout structure is provided
  *			by the caller
  *
- * @lock: 		the rt_mutex to be locked
+ * @lock:		the rt_mutex to be locked
  * @timeout:		timeout structure or NULL (no timeout)
- * @detect_deadlock:	deadlock detection on/off
  *
  * Returns:
- *  0 		on success
- * -EINTR 	when interrupted by a signal
+ *  0		on success
+ * -EINTR	when interrupted by a signal
  * -ETIMEDOUT	when the timeout expired
- * -EDEADLK	when the lock would deadlock (when deadlock detection is on)
  */
 int
-rt_mutex_timed_lock(struct rt_mutex *lock, struct hrtimer_sleeper *timeout,
-		    int detect_deadlock)

kernel/locking/rtmutex.c void __sched rt_mutex_lock(struct rt_mut · line 1147

Quoted 10 lines

kernel/locking/rtmutex.c +3 -2

+rt_mutex_timed_lock(struct rt_mutex *lock, struct hrtimer_sleeper *timeout)
 {
 	might_sleep();
 
-	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout,
-				       detect_deadlock, rt_mutex_slowlock);
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+				       rt_mutex_slowlock);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_timed_lock);

I must be missing something. What's the difference between the above and the futex variant one? They both do the exact same thing:

int foo(struct rt_mutex *lock, struct hrtimer_sleeper *timeout)

{
	might_sleep();

	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timout,
				       0, rt_mutex_slowlock);
}

The conditions under which deadlock detection is conducted are unclear and undocumented.

Add constants instead of using 0/1 and provide a selection function which hides the additional debug dependency from the calling code.

Add comments where needed.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex-debug.c  |    5 +-
 kernel/locking/rtmutex-debug.h  |    7 ++--
 kernel/locking/rtmutex.c        |   69 +++++++++++++++++++++++++++-------------
 kernel/locking/rtmutex.h        |    7 +++-
 kernel/locking/rtmutex_common.h |   15 ++++++++
 5 files changed, 75 insertions(+), 28 deletions(-)

Index: tip/kernel/locking/rtmutex-debug.c
===================================================================

kernel/locking/rtmutex-debug.c +0 -0

@@ -66,12 +66,13 @@ void rt_mutex_debug_task_free(struct tas
  * the deadlock. We print when we return. act_waiter can be NULL in
  * case of a remove waiter operation.

The conditions under which deadlock detection is conducted are unclear and undocumented.

Add constants instead of using 0/1 and provide a selection function which hides the additional debug dependency from the calling code.

Add comments where needed.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex-debug.c  |    5 +-
 kernel/locking/rtmutex-debug.h  |    7 ++--
 kernel/locking/rtmutex.c        |   69 +++++++++++++++++++++++++++-------------
 kernel/locking/rtmutex.h        |    7 +++-
 kernel/locking/rtmutex_common.h |   15 ++++++++
 5 files changed, 75 insertions(+), 28 deletions(-)

Index: tip/kernel/locking/rtmutex-debug.c
===================================================================

kernel/locking/rtmutex-debug.c +3 -2

@@ -66,12 +66,13 @@ void rt_mutex_debug_task_free(struct tas
  * the deadlock. We print when we return. act_waiter can be NULL in
  * case of a remove waiter operation.
  */
-void debug_rt_mutex_deadlock(int detect, struct rt_mutex_waiter *act_waiter,
+void debug_rt_mutex_deadlock(enum rtmutex_chainwalk chwalk,
+			     struct rt_mutex_waiter *act_waiter,
 			     struct rt_mutex *lock)
 {
 	struct task_struct *task;
 
-	if (!debug_locks || detect || !act_waiter)
+	if (!debug_locks || chwalk || !act_waiter)
 		return;
 
 	task = rt_mutex_owner(act_waiter->lock);

Index: tip/kernel/locking/rtmutex-debug.h
===================================================================

kernel/locking/rtmutex-debug.h +4 -3

@@ -20,14 +20,15 @@ extern void debug_rt_mutex_unlock(struct
 extern void debug_rt_mutex_proxy_lock(struct rt_mutex *lock,
 				      struct task_struct *powner);
 extern void debug_rt_mutex_proxy_unlock(struct rt_mutex *lock);
-extern void debug_rt_mutex_deadlock(int detect, struct rt_mutex_waiter *waiter,
+extern void debug_rt_mutex_deadlock(enum rtmutex_chainwalk chwalk,
+				    struct rt_mutex_waiter *waiter,
 				    struct rt_mutex *lock);
 extern void debug_rt_mutex_print_deadlock(struct rt_mutex_waiter *waiter);
 # define debug_rt_mutex_reset_waiter(w)			\
 	do { (w)->deadlock_lock = NULL; } while (0)
 
-static inline int debug_rt_mutex_detect_deadlock(struct rt_mutex_waiter *waiter,
-						 int detect)
+static inline bool debug_rt_mutex_detect_deadlock(struct rt_mutex_waiterr *waiter,
+						  enum rtmutex_chainwalk walk)
 {
 	return (waiter != NULL);
 }

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +11 -0

@@ -256,6 +256,25 @@ static void rt_mutex_adjust_prio(struct
 }
 
 /*
+ * Deadlock detection is conditional:
+ *
+ * If CONFIG_DEBUG_RT_MUTEXES=n, deadlock detection is only conducted
+ * if the detect argument is == RT_MUTEX_FULL_CHAINWALK.
+ *
+ * If CONFIG_DEBUG_RT_MUTEXES=y, deadlock detection is always
+ * conducted independent of the detect argument.
+ *
+ * If the waiter argument is NULL this indicates the deboost path and
+ * deadlock detection is disabled independent of the detect argument
+ * and the config settings.

kernel/locking/rtmutex.c +28 -22

 int max_lock_depth = 1024;

@@ -328,7 +347,7 @@ static inline struct rt_mutex *task_bloc
  *	goto again;
  */
 static int rt_mutex_adjust_prio_chain(struct task_struct *task,
-				      int deadlock_detect,
+				      enum rtmutex_chainwalk chwalk,
 				      struct rt_mutex *orig_lock,
 				      struct rt_mutex *next_lock,
 				      struct rt_mutex_waiter *orig_waiter,

@@ -336,12 +355,12 @@ static int rt_mutex_adjust_prio_chain(st
 {
 	struct rt_mutex_waiter *waiter, *top_waiter = orig_waiter;
 	struct rt_mutex_waiter *prerequeue_top_waiter;
-	int detect_deadlock, ret = 0, depth = 0;
+	int ret = 0, depth = 0;
 	struct rt_mutex *lock;
+	bool detect_deadlock;
 	unsigned long flags;
 
-	detect_deadlock = debug_rt_mutex_detect_deadlock(orig_waiter,
-							 deadlock_detect);
+	detect_deadlock = rt_mutex_cond_detect_deadlock(orig_waiter, chwalk);
 
 	/*
 	 * The (de)boosting is a step by step approach with a lot of

@@ -449,7 +468,7 @@ static int rt_mutex_adjust_prio_chain(st
 	 * walk, we detected a deadlock.
 	 */
 	if (lock == orig_lock || rt_mutex_owner(lock) == top_task) {
-		debug_rt_mutex_deadlock(deadlock_detect, orig_waiter, lock);
+		debug_rt_mutex_deadlock(chwalk, orig_waiter, lock);
 		raw_spin_unlock(&lock->wait_lock);
 		ret = -EDEADLK;
 		goto out_unlock_pi;

@@ -659,7 +678,7 @@ static int try_to_take_rt_mutex(struct r
 static int task_blocks_on_rt_mutex(struct rt_mutex *lock,
 				   struct rt_mutex_waiter *waiter,
 				   struct task_struct *task,
-				   int detect_deadlock)
+				   enum rtmutex_chainwalk chwalk)
 {
 	struct task_struct *owner = rt_mutex_owner(lock);
 	struct rt_mutex_waiter *top_waiter = waiter;

@@ -705,7 +724,7 @@ static int task_blocks_on_rt_mutex(struc
 		__rt_mutex_adjust_prio(owner);
 		if (owner->pi_blocked_on)
 			chain_walk = 1;
-	} else if (debug_rt_mutex_detect_deadlock(waiter, detect_deadlock)) {
+	} else if (rt_mutex_cond_detect_deadlock(waiter, chwalk)) {
 		chain_walk = 1;
 	}
 

@@ -730,7 +749,7 @@ static int task_blocks_on_rt_mutex(struc
 
 	raw_spin_unlock(&lock->wait_lock);
 
-	res = rt_mutex_adjust_prio_chain(owner, detect_deadlock, lock,
+	res = rt_mutex_adjust_prio_chain(owner, chwalk, lock,
 					 next_lock, waiter, task);
 
 	raw_spin_lock(&lock->wait_lock);

@@ -813,7 +832,8 @@ static void remove_waiter(struct rt_mute
 
 	raw_spin_unlock(&lock->wait_lock);
 
-	rt_mutex_adjust_prio_chain(owner, 0, lock, next_lock, NULL, current);
+	rt_mutex_adjust_prio_chain(owner, RT_MUTEX_MIN_CHAINWALK, lock,
+				   next_lock, NULL, current);
 
 	raw_spin_lock(&lock->wait_lock);
 }

@@ -843,7 +863,8 @@ void rt_mutex_adjust_pi(struct task_stru
 	/* gets dropped in rt_mutex_adjust_prio_chain()! */
 	get_task_struct(task);
 
-	rt_mutex_adjust_prio_chain(task, 0, NULL, next_lock, NULL, task);
+	rt_mutex_adjust_prio_chain(task, RT_MUTEX_MIN_CHAINWALK, NULL, NULL,
+				   NULL, task);
 }
 
 /**

@@ -921,7 +942,7 @@ static void rt_mutex_handle_deadlock(int
 static int __sched
 rt_mutex_slowlock(struct rt_mutex *lock, int state,
 		  struct hrtimer_sleeper *timeout,
-		  int detect_deadlock)
+		  enum rtmutex_chainwalk chwalk)
 {
 	struct rt_mutex_waiter waiter;
 	int ret = 0;

@@ -947,7 +968,7 @@ rt_mutex_slowlock(struct rt_mutex *lock,
 			timeout->task = NULL;
 	}
 
-	ret = task_blocks_on_rt_mutex(lock, &waiter, current, detect_deadlock);
+	ret = task_blocks_on_rt_mutex(lock, &waiter, current, chwalk);
 
 	if (likely(!ret))
 		ret = __rt_mutex_slowlock(lock, state, timeout, &waiter);

@@ -956,7 +977,7 @@ rt_mutex_slowlock(struct rt_mutex *lock,
 
 	if (unlikely(ret)) {
 		remove_waiter(lock, &waiter);
-		rt_mutex_handle_deadlock(ret, detect_deadlock, &waiter);
+		rt_mutex_handle_deadlock(ret, chwalk, &waiter);
 	}
 
 	/*

@@ -1037,27 +1058,28 @@ static inline int
 rt_mutex_fastlock(struct rt_mutex *lock, int state,
 		  int (*slowfn)(struct rt_mutex *lock, int state,
 				struct hrtimer_sleeper *timeout,
-				int detect_deadlock))
+				enum rtmutex_chainwalk chwalk))
 {
 	if (likely(rt_mutex_cmpxchg(lock, NULL, current))) {
 		rt_mutex_deadlock_account_lock(lock, current);
 		return 0;
 	} else
-		return slowfn(lock, state, NULL, 0);
+		return slowfn(lock, state, NULL, RT_MUTEX_MIN_CHAINWALK);
 }
 
 static inline int
 rt_mutex_timed_fastlock(struct rt_mutex *lock, int state,
-			struct hrtimer_sleeper *timeout, int detect_deadlock,
+			struct hrtimer_sleeper *timeout,
+			enum rtmutex_chainwalk chwalk,
 			int (*slowfn)(struct rt_mutex *lock, int state,
 				      struct hrtimer_sleeper *timeout,
-				      int detect_deadlock))
+				      enum rtmutex_chainwalk chwalk))
 {
-	if (!detect_deadlock && likely(rt_mutex_cmpxchg(lock, NULL, current))) {
+	if (!chwalk && likely(rt_mutex_cmpxchg(lock, NULL, current))) {
 		rt_mutex_deadlock_account_lock(lock, current);
 		return 0;
 	} else
-		return slowfn(lock, state, timeout, detect_deadlock);
+		return slowfn(lock, state, timeout, chwalk);
 }
 
 static inline int

@@ -1119,7 +1141,8 @@ int __rt_mutex_timed_lock(struct rt_mute
 {
 	might_sleep();
 
-	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout,
+				       RT_MUTEX_FULL_CHAINWALK,
 				       rt_mutex_slowlock);
 }
 

@@ -1141,7 +1164,8 @@ rt_mutex_timed_lock(struct rt_mutex *loc
 {
 	might_sleep();
 
-	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout, 0,
+	return rt_mutex_timed_fastlock(lock, TASK_INTERRUPTIBLE, timeout,
+				       RT_MUTEX_MIN_CHAINWALK,
 				       rt_mutex_slowlock);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_timed_lock);

@@ -1270,7 +1294,8 @@ int rt_mutex_start_proxy_lock(struct rt_
 	}
 
 	/* We enforce deadlock detection for futexes */
-	ret = task_blocks_on_rt_mutex(lock, waiter, task, 1);
+	ret = task_blocks_on_rt_mutex(lock, waiter, task,
+				      RT_MUTEX_FULL_CHAINWALK);
 
 	if (ret && !rt_mutex_owner(lock)) {
 		/*

Index: tip/kernel/locking/rtmutex.h
===================================================================

kernel/locking/rtmutex.h +6 -1

@@ -22,10 +22,15 @@
 #define debug_rt_mutex_init(m, n)			do { } while (0)
 #define debug_rt_mutex_deadlock(d, a ,l)		do { } while (0)
 #define debug_rt_mutex_print_deadlock(w)		do { } while (0)
-#define debug_rt_mutex_detect_deadlock(w,d)		(d)
 #define debug_rt_mutex_reset_waiter(w)			do { } while (0)
 
 static inline void rt_mutex_print_deadlock(struct rt_mutex_waiter *w)
 {
 	WARN(1, "rtmutex deadlock detected\n");
 }
+
+static inline bool debug_rt_mutex_detect_deadlock(struct rt_mutex_waiter *w,
+						  enum rtmutex_chainwalk walk)
+{
+	return walk == RT_MUTEX_FULL_CHAINWALK;
+}

Index: tip/kernel/locking/rtmutex_common.h
===================================================================

kernel/locking/rtmutex_common.h +15 -0

@@ -102,6 +102,21 @@ static inline struct task_struct *rt_mut
 }
 
 /*
+ * Constants for rt mutex functions which have a selectable deadlock
+ * detection.
+ *
+ * RT_MUTEX_MIN_CHAINWALK:	Stops the lock chain walk when there are
+ *				no further PI adjustments to be made.
+ *
+ * RT_MUTEX_FULL_CHAINWALK:	Invoke deadlock detection with a full
+ *				walk of the lock chain.
+ */
+enum rtmutex_chainwalk {
+	RT_MUTEX_MIN_CHAINWALK,
+	RT_MUTEX_FULL_CHAINWALK,
+};
+
+/*
  * PI-futex support (proxy locking functions, etc.):
  */
 extern struct task_struct *rt_mutex_next_owner(struct rt_mutex *lock);

In case the dead lock detector is enabled we follow the lock chain to the end in rt_mutex_adjust_prio_chain, even if we could stop earlier due to the priority/waiter constellation.

But once we are not longer the top priority waiter in a certain step or the task holding the lock has already the same priority then there is no point in dequeing and enqueing along the lock chain as there is no change at all.

So stop the requeueing at this point.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   61 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 7 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +21 -7

@@ -359,6 +359,7 @@ static int rt_mutex_adjust_prio_chain(st
 	struct rt_mutex *lock;
 	bool detect_deadlock;
 	unsigned long flags;
+	bool requeue = true;
 
 	detect_deadlock = rt_mutex_cond_detect_deadlock(orig_waiter, chwalk);
 

@@ -436,18 +437,31 @@ static int rt_mutex_adjust_prio_chain(st
 			goto out_unlock_pi;
 		/*
 		 * If deadlock detection is off, we stop here if we
-		 * are not the top pi waiter of the task.
+		 * are not the top pi waiter of the task. If deadlock
+		 * detection is enabled we continue, but stop the
+		 * requeueing in the chain walk.
 		 */
-		if (!detect_deadlock && top_waiter != task_top_pi_waiter(task))
-			goto out_unlock_pi;
+		if (top_waiter != task_top_pi_waiter(task)) {
+			if (!detect_deadlock)
+				goto out_unlock_pi;
+			else
+				requeue = false;
+		}
 	}
 
 	/*
-	 * When deadlock detection is off then we check, if further
-	 * priority adjustment is necessary.
+	 * If the waiter priority is the same as the task priority
+	 * then there is no further priority adjustment necessary.  If
+	 * deadlock detection is off, we stop the chain walk. If its
+	 * enabled we continue, but stop the requeueing in the chain
+	 * walk.
 	 */
-	if (!detect_deadlock && waiter->prio == task->prio)
-		goto out_unlock_pi;
+	if (waiter->prio == task->prio) {
+		if (!detect_deadlock)
+			goto out_unlock_pi;
+		else
+			requeue = false;
+	}
 
 	/*
 	 * We need to trylock here as we are holding task->pi_lock,

kernel/locking/rtmutex.c +16 -0

+	 * the minimum chain walk checks here.
+	 */
+	if (!requeue) {
+		/* Release the task */
+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		put_task_struct(task);
+
+		/* If there is no owner of the lock, end of chain. */
+		if (!rt_mutex_owner(lock)) {
+			raw_spin_unlock(&lock->wait_lock);
+			return 0;
+		}
+
+		/* Grab the next task, i.e. owner of @lock */
+		task = rt_mutex_owner(lock);
+		get_task_struct(task);

kernel/locking/rtmutex.c +4 -0

+		goto again;
+	}
+
+	/*
 	 * Store the current top waiter before doing the requeue
 	 * operation on @lock. We need it for the boost/deboost
 	 * decision below.

Quoted 68 lines · On Mon, 09 Jun 2014 20:28:10 -0000 Thomas Gleixner <[email protected]> wrote:

In case the dead lock detector is enabled we follow the lock chain to the end in rt_mutex_adjust_prio_chain, even if we could stop earlier due to the priority/waiter constellation.

But once we are not longer the top priority waiter in a certain step or the task holding the lock has already the same priority then there is no point in dequeing and enqueing along the lock chain as there is no change at all.

So stop the requeueing at this point.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   61 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 7 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +21 -7

@@ -359,6 +359,7 @@ static int rt_mutex_adjust_prio_chain(st
      struct rt_mutex *lock;
      bool detect_deadlock;
      unsigned long flags;
+     bool requeue = true;
 
      detect_deadlock = rt_mutex_cond_detect_deadlock(orig_waiter, chwalk);
 

@@ -436,18 +437,31 @@ static int rt_mutex_adjust_prio_chain(st
                      goto out_unlock_pi;
              /*
               * If deadlock detection is off, we stop here if we
-              * are not the top pi waiter of the task.
+              * are not the top pi waiter of the task. If deadlock
+              * detection is enabled we continue, but stop the
+              * requeueing in the chain walk.
               */
-             if (!detect_deadlock && top_waiter != task_top_pi_waiter(task))
-                     goto out_unlock_pi;
+             if (top_waiter != task_top_pi_waiter(task)) {
+                     if (!detect_deadlock)
+                             goto out_unlock_pi;
+                     else
+                             requeue = false;
+             }
      }
 
      /*
-      * When deadlock detection is off then we check, if further
-      * priority adjustment is necessary.
+      * If the waiter priority is the same as the task priority
+      * then there is no further priority adjustment necessary.  If
+      * deadlock detection is off, we stop the chain walk. If its
+      * enabled we continue, but stop the requeueing in the chain
+      * walk.
       */
-     if (!detect_deadlock && waiter->prio == task->prio)
-             goto out_unlock_pi;
+     if (waiter->prio == task->prio) {
+             if (!detect_deadlock)
+                     goto out_unlock_pi;
+             else
+                     requeue = false;
+     }
 
      /*
       * We need to trylock here as we are holding task->pi_lock,

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 493

Quoted 5 lines

kernel/locking/rtmutex.c +2 -0

@@ -475,6 +489,39 @@ static int rt_mutex_adjust_prio_chain(st
      }
 
      /*
+      * If we just follow the lock chain for deadlock detection, no
+      * need to do all the requeue operations. We avoid a truckload

s/We/To/

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 494

Quoted 1 line

kernel/locking/rtmutex.c +1 -0

+      * of conditinals around the various places below and just do

s/ and/, /

Quoted 16 lines

kernel/locking/rtmutex.c +16 -0

+      * the minimum chain walk checks here.
+      */
+     if (!requeue) {
+             /* Release the task */
+             raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+             put_task_struct(task);
+
+             /* If there is no owner of the lock, end of chain. */
+             if (!rt_mutex_owner(lock)) {
+                     raw_spin_unlock(&lock->wait_lock);
+                     return 0;
+             }
+
+             /* Grab the next task, i.e. owner of @lock */
+             task = rt_mutex_owner(lock);
+             get_task_struct(task);

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 520

Quoted 10 lines

kernel/locking/rtmutex.c +10 -0

+             raw_spin_lock_irqsave(&task->pi_lock, flags);
+
+             /* Store whether owner is blocked itself and drop locks */
+             next_lock = task_blocked_on(task);
+             raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+             raw_spin_unlock(&lock->wait_lock);
+
+             /* If owner is not blocked, end of chain. */
+             if (!next_lock)
+                     goto out_put_task;

On the loop back around, have something like:

        if (top_waiter) {
                if (!task_has_pi_waiters(task))
                        goto out_unlock_pi;

                if (!requeue &&
                    top_waiter != task_top_pi_waiter(task)) {
                        if (!detect_deadlock)
                                goto out_unlock_pi;
                        else
                                requeue = false;
                }
        }

??

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 527

Quoted 8 lines

kernel/locking/rtmutex.c +4 -0

+             goto again;
+     }
+
+     /*
       * Store the current top waiter before doing the requeue
       * operation on @lock. We need it for the boost/deboost
       * decision below.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 493

Quoted 3 lines · On Mon, 09 Jun 2014 20:28:10 -0000 Thomas Gleixner <[email protected]> wrote:

kernel/locking/rtmutex.c +2 -0

 	/*
+	 * If we just follow the lock chain for deadlock detection, no
+	 * need to do all the requeue operations. We avoid a truckload

s/We/To/

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 494

Quoted 1 line

kernel/locking/rtmutex.c +1 -0

+	 * of conditinals around the various places below and just do

s/ and/, /

Quoted 16 lines

kernel/locking/rtmutex.c +16 -0

+	 * the minimum chain walk checks here.
+	 */
+	if (!requeue) {
+		/* Release the task */
+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		put_task_struct(task);
+
+		/* If there is no owner of the lock, end of chain. */
+		if (!rt_mutex_owner(lock)) {
+			raw_spin_unlock(&lock->wait_lock);
+			return 0;
+		}
+
+		/* Grab the next task, i.e. owner of @lock */
+		task = rt_mutex_owner(lock);
+		get_task_struct(task);

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 520

Quoted 10 lines

kernel/locking/rtmutex.c +10 -0

+		raw_spin_lock_irqsave(&task->pi_lock, flags);
+
+		/* Store whether owner is blocked itself and drop locks */
+		next_lock = task_blocked_on(task);
+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		raw_spin_unlock(&lock->wait_lock);
+
+		/* If owner is not blocked, end of chain. */
+		if (!next_lock)
+			goto out_put_task;

On the loop back around, have something like:

	if (top_waiter) {
		if (!task_has_pi_waiters(task))
			goto out_unlock_pi;

		if (!requeue &&
		    top_waiter != task_top_pi_waiter(task)) {
			if (!detect_deadlock)
				goto out_unlock_pi;
			else
				requeue = false;
		}

Quoted 5 lines

On the loop back around, have something like:

	if (top_waiter) {
		if (!task_has_pi_waiters(task))
			goto out_unlock_pi;

The task has at least one pi waiter.

Quoted 7 lines

		if (!requeue &&
		    top_waiter != task_top_pi_waiter(task)) {
			if (!detect_deadlock)
				goto out_unlock_pi;
			else
				requeue = false;
		}

Errm? if requeue is off we are in deadlock detection chainwalk mode. So all we care about is whether task is blocked on next_lock or not.

Quoted 16 lines · On Tue, 10 Jun 2014 19:41:39 +0200 (CEST) Thomas Gleixner <[email protected]>…

Quoted 5 lines

On the loop back around, have something like:

	if (top_waiter) {
		if (!task_has_pi_waiters(task))
			goto out_unlock_pi;

The task has at least one pi waiter.

Quoted 7 lines

		if (!requeue &&
		    top_waiter != task_top_pi_waiter(task)) {
			if (!detect_deadlock)
				goto out_unlock_pi;
			else
				requeue = false;
		}

Quoted 3 lines

Errm? if requeue is off we are in deadlock detection chainwalk mode. So all we care about is whether task is blocked on next_lock or not.

Actually that was a typo on my part. That should have been:

	if (requeue &&
		...

As we don't need to read the task_top_pi_waiter() again.

In case the dead lock detector is enabled we follow the lock chain to the end in rt_mutex_adjust_prio_chain, even if we could stop earlier due to the priority/waiter constellation.

But once we are not longer the top priority waiter in a certain step or the task holding the lock has already the same priority then there is no point in dequeing and enqueing along the lock chain as there is no change at all.

So stop the requeueing at this point.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/locking/rtmutex.c |   61 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 7 deletions(-)

Index: tip/kernel/locking/rtmutex.c
===================================================================

kernel/locking/rtmutex.c +8 -3

@@ -359,6 +359,7 @@ static int rt_mutex_adjust_prio_chain(st
 	struct rt_mutex *lock;
 	bool detect_deadlock;
 	unsigned long flags;
+	bool requeue = true;
 
 	detect_deadlock = rt_mutex_cond_detect_deadlock(orig_waiter, chwalk);
 

@@ -436,18 +437,31 @@ static int rt_mutex_adjust_prio_chain(st
 			goto out_unlock_pi;
 		/*
 		 * If deadlock detection is off, we stop here if we
-		 * are not the top pi waiter of the task.
+		 * are not the top pi waiter of the task. If deadlock
+		 * detection is enabled we continue, but stop the
+		 * requeueing in the chain walk.
 		 */
-		if (!detect_deadlock && top_waiter != task_top_pi_waiter(task))
-			goto out_unlock_pi;
+		if (top_waiter != task_top_pi_waiter(task)) {
+			if (!detect_deadlock)
+				goto out_unlock_pi;
+			else

kernel/locking/rtmutex.c +21 -2

+	 * enabled we continue, but stop the requeueing in the chain
+	 * walk.
 	 */
-	if (!detect_deadlock && waiter->prio == task->prio)
-		goto out_unlock_pi;
+	if (waiter->prio == task->prio) {
+		if (!detect_deadlock)
+			goto out_unlock_pi;
+		else
+			requeue = false;
+	}
 
 	/*
 	 * We need to trylock here as we are holding task->pi_lock,

@@ -475,6 +489,39 @@ static int rt_mutex_adjust_prio_chain(st
 	}
 
 	/*
+	 * If we just follow the lock chain for deadlock detection, no
+	 * need to do all the requeue operations. We avoid a truckload
+	 * of conditinals around the various places below and just do
+	 * the minimum chain walk checks here.
+	 */
+	if (!requeue) {
+		/* Release the task */
+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		put_task_struct(task);
+
+		/* If there is no owner of the lock, end of chain. */
+		if (!rt_mutex_owner(lock)) {
+			raw_spin_unlock(&lock->wait_lock);

kernel/locking/rtmutex.c +10 -0

+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		raw_spin_unlock(&lock->wait_lock);
+
+		/* If owner is not blocked, end of chain. */
+		if (!next_lock)
+			goto out_put_task;
+		goto again;
+	}
+
+	/*
 	 * Store the current top waiter before doing the requeue
 	 * operation on @lock. We need it for the boost/deboost
 	 * decision below.

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 514

Quoted 2 lines

kernel/locking/rtmutex.c +2 -0

+		/* Store whether owner is blocked itself and drop locks */
+		next_lock = task_blocked_on(task);

task_blocked_on(task) is not clear to me, the recipient of the return is the only clue that hints at what the function does.

Quoted 15 lines

kernel/locking/rtmutex.c +10 -0

+		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+		raw_spin_unlock(&lock->wait_lock);
+
+		/* If owner is not blocked, end of chain. */
+		if (!next_lock)
+			goto out_put_task;
+		goto again;
+	}
+
+	/*
 	 * Store the current top waiter before doing the requeue
 	 * operation on @lock. We need it for the boost/deboost
 	 * decision below.

Quoted 4 lines · On Tue, 10 Jun 2014 09:57:25 -0500 "Brad Mouring" <[email protected]> wrote:

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 514

Quoted 2 lines

kernel/locking/rtmutex.c +2 -0

+		/* Store whether owner is blocked itself and drop locks */
+		next_lock = task_blocked_on(task);

task_blocked_on(task) is not clear to me, the recipient of the return is the only clue that hints at what the function does.

Well, this is more than confusing, it's the only user, all other users are task_blocked_on_lock(), and this causes the code not to compile.

Quoted 9 lines · Steven Rostedt

Quoted 4 lines · On Tue, 10 Jun 2014 09:57:25 -0500 "Brad Mouring" <[email protected]> wrote:

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 514

Quoted 2 lines

kernel/locking/rtmutex.c +2 -0

+		/* Store whether owner is blocked itself and drop locks */
+		next_lock = task_blocked_on(task);

task_blocked_on(task) is not clear to me, the recipient of the return is the only clue that hints at what the function does.

Quoted 2 lines

Well, this is more than confusing, it's the only user, all other users are task_blocked_on_lock(), and this causes the code not to compile.

Grr, yes.

Quoted 11 lines · On Tue, 10 Jun 2014 19:43:16 +0200 (CEST) Thomas Gleixner <[email protected]>…

Quoted 9 lines · Steven Rostedt

Quoted 4 lines · On Tue, 10 Jun 2014 09:57:25 -0500 "Brad Mouring" <[email protected]> wrote:

kernel/locking/rtmutex.c static int rt_mutex_adjust_prio_chain(st · line 514

Quoted 2 lines

kernel/locking/rtmutex.c +2 -0

+		/* Store whether owner is blocked itself and drop locks */
+		next_lock = task_blocked_on(task);

task_blocked_on(task) is not clear to me, the recipient of the return is the only clue that hints at what the function does.

Quoted 4 lines

Quoted 2 lines

Well, this is more than confusing, it's the only user, all other users are task_blocked_on_lock(), and this causes the code not to compile.

Grr, yes.

Luckily you are not posting this to that grumpy IRQ maintainer. He'd shoot some frozen sharks your way if you sent him patches like this ;-)

This is another round of clarification and optimization patches for the rtmutex code.

When I looked last week into the dead lock detector I spent some time to figure out how the various arguments and local variables are protected and whats the scope of the various protection algorithms, so I added documentation for this as well.

I also changed the last patch which avoids the requeueing in case of the deadlock detection only chain walk. Instead of adding tons of conditional to the boost/deboost code path I simply have a separate code path for it.

So what branch do these apply to? I tried latest Linus master and it didn't apply.

Quoted 2 lines · On Mon, 9 Jun 2014 20:27:13 -0400 Steven Rostedt <[email protected]> wrote:

So what branch do these apply to? I tried latest Linus master and it didn't apply.

OK, they seem to be on tip/locking/urgent. Not yet in mainline.

-- Steve
.