Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1491275 > unrolled thread

[PATCH -v2 0/9] PI and assorted failings

Started byPeter Zijlstra <peterz@infradead.org>
First post2016-09-26 14:50 +0200
Last post2016-09-26 14:50 +0200
Articles 16 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH -v2 0/9] PI and assorted failings Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
    [PATCH -v2 5/9] rtmutex: Clean up Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
      Re: [PATCH -v2 5/9] rtmutex: Clean up Steven Rostedt <rostedt@goodmis.org> - 2016-09-26 18:20 +0200
        Re: [PATCH -v2 5/9] rtmutex: Clean up Thomas Gleixner <tglx@linutronix.de> - 2016-09-29 17:00 +0200
    [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
      Re: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the  dl_runtime/dl_period update Steven Rostedt <rostedt@goodmis.org> - 2016-09-26 18:10 +0200
      Re: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the  dl_runtime/dl_period update Thomas Gleixner <tglx@linutronix.de> - 2016-09-29 17:00 +0200
    [PATCH -v2 8/9] rtmutex: Fix PI chain order integrity Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
    [PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio() Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
      Re: [PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio() Steven Rostedt <rostedt@goodmis.org> - 2016-09-26 19:00 +0200
    [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio() Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
      Re: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio() Steven Rostedt <rostedt@goodmis.org> - 2016-09-26 19:10 +0200
        Re: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio() Peter Zijlstra <peterz@infradead.org> - 2016-09-27 09:50 +0200
    [PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock() Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200
      Re: [PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock() Thomas Gleixner <tglx@linutronix.de> - 2016-09-29 17:00 +0200
    [PATCH -v2 9/9] rtmutex: Fix more prio comparisons Peter Zijlstra <peterz@infradead.org> - 2016-09-26 14:50 +0200

#1491275 — [PATCH -v2 0/9] PI and assorted failings

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 0/9] PI and assorted failings
Message-ID<slDw5-3SS-3@gated-at.bofh.it>
OK, long time since I posted, but I finally found these patches again.
Hopefully I addressed all feedback from last time (sorry if I missed anything).

Aside from the stuff I posted during review last time, I restructured the first
two patches to get rid of that unparsable changelog.

I folded part of the second patch into the first patch and rewrote the
Changelog for the first patch.

Now the first patch fully restructures the unlock+deboost path and the second
patch only adds the pi_top_task pointer cache. Where previously the first patch
did half the unlock+deboost restructure and the second patch did the rest and
added that cache.

If there are no objections, I'll queue the stuff to finally get rid of these
patches (and not 'loose' them again).

[toc] | [next] | [standalone]


#1491276 — [PATCH -v2 5/9] rtmutex: Clean up

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 5/9] rtmutex: Clean up
Message-ID<slDw5-3SS-9@gated-at.bofh.it>
In reply to#1491275
Previous patches changed the meaning of the return value of
rt_mutex_slowunlock(); update comments and code to reflect this.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/futex.c                  |   12 ++++++------
 kernel/locking/rtmutex.c        |   20 +++++++++-----------
 kernel/locking/rtmutex_common.h |    2 +-
 3 files changed, 16 insertions(+), 18 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1261,7 +1261,7 @@ static int wake_futex_pi(u32 __user *uad
 	struct futex_pi_state *pi_state = this->pi_state;
 	u32 uninitialized_var(curval), newval;
 	WAKE_Q(wake_q);
-	bool deboost;
+	bool postunlock;
 	int ret = 0;
 
 	if (!pi_state)
@@ -1327,17 +1327,17 @@ static int wake_futex_pi(u32 __user *uad
 
 	raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock);
 
-	deboost = rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
+	postunlock = rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
 
 	/*
 	 * First unlock HB so the waiter does not spin on it once he got woken
-	 * up. Second wake up the waiter before the priority is adjusted. If we
-	 * deboost first (and lose our higher priority), then the task might get
-	 * scheduled away before the wake up can take place.
+	 * up. Then wakeup the waiter by calling rt_mutex_postunlock(). Priority
+	 * is already adjusted and preemption is disabled to avoid inversion.
 	 */
 	spin_unlock(&hb->lock);
 
-	rt_mutex_postunlock(&wake_q, deboost);
+	if (postunlock)
+		rt_mutex_postunlock(&wake_q);
 
 	return 0;
 }
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1254,7 +1254,8 @@ static inline int rt_mutex_slowtrylock(s
 
 /*
  * Slow path to release a rt-mutex.
- * Return whether the current task needs to undo a potential priority boosting.
+ *
+ * Return whether the current task needs to call rt_mutex_postunlock().
  */
 static bool __sched rt_mutex_slowunlock(struct rt_mutex *lock,
 					struct wake_q_head *wake_q)
@@ -1327,7 +1328,7 @@ static bool __sched rt_mutex_slowunlock(
 
 	raw_spin_unlock_irqrestore(&lock->wait_lock, flags);
 
-	/* check PI boosting */
+	/* call rt_mutex_postunlock() */
 	return true;
 }
 
@@ -1378,15 +1379,14 @@ rt_mutex_fasttrylock(struct rt_mutex *lo
 }
 
 /*
- * Undo pi boosting (if necessary) and wake top waiter.
+ * Performs the wakeup of the the top-waiter and re-enables preemption.
  */
-void rt_mutex_postunlock(struct wake_q_head *wake_q, bool deboost)
+void rt_mutex_postunlock(struct wake_q_head *wake_q)
 {
 	wake_up_q(wake_q);
 
 	/* Pairs with preempt_disable() in rt_mutex_slowunlock() */
-	if (deboost)
-		preempt_enable();
+	preempt_enable();
 }
 
 /**
@@ -1489,9 +1489,8 @@ void __sched rt_mutex_unlock(struct rt_m
 		rt_mutex_deadlock_account_unlock(current);
 
 	} else {
-		bool deboost = rt_mutex_slowunlock(lock, &wake_q);
-
-		rt_mutex_postunlock(&wake_q, deboost);
+		if (rt_mutex_slowunlock(lock, &wake_q))
+			rt_mutex_postunlock(&wake_q);
 	}
 }
 EXPORT_SYMBOL_GPL(rt_mutex_unlock);
@@ -1500,8 +1499,7 @@ EXPORT_SYMBOL_GPL(rt_mutex_unlock);
  * rt_mutex_futex_unlock - Futex variant of rt_mutex_unlock
  * @lock: the rt_mutex to be unlocked
  *
- * Returns: true/false indicating whether priority adjustment is
- * required or not.
+ * Returns: true/false indicating whether we should call rt_mutex_postunlock().
  */
 bool __sched rt_mutex_futex_unlock(struct rt_mutex *lock,
 				   struct wake_q_head *wqh)
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -111,7 +111,7 @@ extern int rt_mutex_finish_proxy_lock(st
 extern int rt_mutex_timed_futex_lock(struct rt_mutex *l, struct hrtimer_sleeper *to);
 extern bool rt_mutex_futex_unlock(struct rt_mutex *lock,
 				  struct wake_q_head *wqh);
-extern void rt_mutex_postunlock(struct wake_q_head *wake_q, bool deboost);
+extern void rt_mutex_postunlock(struct wake_q_head *wake_q);
 extern void rt_mutex_adjust_prio(struct task_struct *task);
 
 #ifdef CONFIG_DEBUG_RT_MUTEXES

[toc] | [prev] | [next] | [standalone]


#1491396 — Re: [PATCH -v2 5/9] rtmutex: Clean up

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-09-26 18:20 +0200
SubjectRe: [PATCH -v2 5/9] rtmutex: Clean up
Message-ID<slGNj-64r-1@gated-at.bofh.it>
In reply to#1491276
On Mon, 26 Sep 2016 14:32:18 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> Previous patches changed the meaning of the return value of
> rt_mutex_slowunlock(); update comments and code to reflect this.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/futex.c                  |   12 ++++++------
>  kernel/locking/rtmutex.c        |   20 +++++++++-----------
>  kernel/locking/rtmutex_common.h |    2 +-
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -1261,7 +1261,7 @@ static int wake_futex_pi(u32 __user *uad
>  	struct futex_pi_state *pi_state = this->pi_state;
>  	u32 uninitialized_var(curval), newval;
>  	WAKE_Q(wake_q);
> -	bool deboost;
> +	bool postunlock;
>  	int ret = 0;
>  
>  	if (!pi_state)
> @@ -1327,17 +1327,17 @@ static int wake_futex_pi(u32 __user *uad
>  
>  	raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock);
>  
> -	deboost = rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
> +	postunlock = rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
>  
>  	/*
>  	 * First unlock HB so the waiter does not spin on it once he got woken
> -	 * up. Second wake up the waiter before the priority is adjusted. If we
> -	 * deboost first (and lose our higher priority), then the task might get
> -	 * scheduled away before the wake up can take place.
> +	 * up. Then wakeup the waiter by calling rt_mutex_postunlock(). Priority
> +	 * is already adjusted and preemption is disabled to avoid inversion.

Can we specify here that preemption is only disabled if
rt_mutex_futex_unlock() returns true, and will be enabled again with
rt_mutex_postunlock().


>  	 */
>  	spin_unlock(&hb->lock);
>  
> -	rt_mutex_postunlock(&wake_q, deboost);
> +	if (postunlock)
> +		rt_mutex_postunlock(&wake_q);
>  
>  	return 0;
>  }
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1254,7 +1254,8 @@ static inline int rt_mutex_slowtrylock(s
>  
>  /*
>   * Slow path to release a rt-mutex.
> - * Return whether the current task needs to undo a potential priority boosting.
> + *
> + * Return whether the current task needs to call rt_mutex_postunlock().
>   */
>  static bool __sched rt_mutex_slowunlock(struct rt_mutex *lock,
>  					struct wake_q_head *wake_q)
> @@ -1327,7 +1328,7 @@ static bool __sched rt_mutex_slowunlock(
>  
>  	raw_spin_unlock_irqrestore(&lock->wait_lock, flags);
>  
> -	/* check PI boosting */
> +	/* call rt_mutex_postunlock() */

Can we rephrase this to "A call to rt_mutex_postunlock() is required".

>  	return true;
>  }
>  
> @@ -1378,15 +1379,14 @@ rt_mutex_fasttrylock(struct rt_mutex *lo
>  }
>  
>  /*
> - * Undo pi boosting (if necessary) and wake top waiter.
> + * Performs the wakeup of the the top-waiter and re-enables preemption.
>   */
> -void rt_mutex_postunlock(struct wake_q_head *wake_q, bool deboost)
> +void rt_mutex_postunlock(struct wake_q_head *wake_q)
>  {
>  	wake_up_q(wake_q);
>  
>  	/* Pairs with preempt_disable() in rt_mutex_slowunlock() */
> -	if (deboost)
> -		preempt_enable();
> +	preempt_enable();
>  }
>  
>  /**
> @@ -1489,9 +1489,8 @@ void __sched rt_mutex_unlock(struct rt_m
>  		rt_mutex_deadlock_account_unlock(current);
>  
>  	} else {
> -		bool deboost = rt_mutex_slowunlock(lock, &wake_q);
> -
> -		rt_mutex_postunlock(&wake_q, deboost);
> +		if (rt_mutex_slowunlock(lock, &wake_q))
> +			rt_mutex_postunlock(&wake_q);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(rt_mutex_unlock);
> @@ -1500,8 +1499,7 @@ EXPORT_SYMBOL_GPL(rt_mutex_unlock);
>   * rt_mutex_futex_unlock - Futex variant of rt_mutex_unlock
>   * @lock: the rt_mutex to be unlocked
>   *
> - * Returns: true/false indicating whether priority adjustment is
> - * required or not.
> + * Returns: true/false indicating whether we should call rt_mutex_postunlock().

Can this be rephrased to: "Returns true if preemption has been
disabled and a call to rt_mutex_postunlock() is required (which will
re-enable preemption)"

-- Steve


>   */
>  bool __sched rt_mutex_futex_unlock(struct rt_mutex *lock,
>  				   struct wake_q_head *wqh)
> --- a/kernel/locking/rtmutex_common.h
> +++ b/kernel/locking/rtmutex_common.h
> @@ -111,7 +111,7 @@ extern int rt_mutex_finish_proxy_lock(st
>  extern int rt_mutex_timed_futex_lock(struct rt_mutex *l, struct hrtimer_sleeper *to);
>  extern bool rt_mutex_futex_unlock(struct rt_mutex *lock,
>  				  struct wake_q_head *wqh);
> -extern void rt_mutex_postunlock(struct wake_q_head *wake_q, bool deboost);
> +extern void rt_mutex_postunlock(struct wake_q_head *wake_q);
>  extern void rt_mutex_adjust_prio(struct task_struct *task);
>  
>  #ifdef CONFIG_DEBUG_RT_MUTEXES
> 

[toc] | [prev] | [next] | [standalone]


#1493525 — Re: [PATCH -v2 5/9] rtmutex: Clean up

FromThomas Gleixner <tglx@linutronix.de>
Date2016-09-29 17:00 +0200
SubjectRe: [PATCH -v2 5/9] rtmutex: Clean up
Message-ID<smKYx-5qg-11@gated-at.bofh.it>
In reply to#1491396
On Mon, 26 Sep 2016, Steven Rostedt wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> Can this be rephrased to: "Returns true if preemption has been
> disabled and a call to rt_mutex_postunlock() is required (which will
> re-enable preemption)"

I agree with Steven that the comments should be rephrased.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1491278 — [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update
Message-ID<slDw5-3SS-15@gated-at.bofh.it>
In reply to#1491275
Currently dl tasks will actually return at the very beginning
of rt_mutex_adjust_prio_chain() in !detect_deadlock cases:

    if (waiter->prio == task->prio) {
        if (!detect_deadlock)
            goto out_unlock_pi; // out here
        else
            requeue = false;
    }

As the deadline value of blocked deadline tasks(waiters) without
changing their sched_class(thus prio doesn't change) never changes,
this seems reasonable, but it actually misses the chance of updating
rt_mutex_waiter's "dl_runtime(period)_copy" if a waiter updates its
deadline parameters(dl_runtime, dl_period) or boosted waiter changes
to !deadline class.

Thus, force deadline task not out by adding the !dl_prio() condition.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Xunlei Pang <xlpang@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1460633827-345-7-git-send-email-xlpang@redhat.com
---
 kernel/locking/rtmutex.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -488,7 +488,7 @@ static int rt_mutex_adjust_prio_chain(st
 	 * enabled we continue, but stop the requeueing in the chain
 	 * walk.
 	 */
-	if (waiter->prio == task->prio) {
+	if (waiter->prio == task->prio && !dl_task(task)) {
 		if (!detect_deadlock)
 			goto out_unlock_pi;
 		else

[toc] | [prev] | [next] | [standalone]


#1491389 — Re: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-09-26 18:10 +0200
SubjectRe: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update
Message-ID<slGDE-5XZ-9@gated-at.bofh.it>
In reply to#1491278
On Mon, 26 Sep 2016 14:32:16 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> Currently dl tasks will actually return at the very beginning
> of rt_mutex_adjust_prio_chain() in !detect_deadlock cases:
> 
>     if (waiter->prio == task->prio) {
>         if (!detect_deadlock)
>             goto out_unlock_pi; // out here
>         else
>             requeue = false;
>     }
> 
> As the deadline value of blocked deadline tasks(waiters) without
> changing their sched_class(thus prio doesn't change) never changes,
> this seems reasonable, but it actually misses the chance of updating
> rt_mutex_waiter's "dl_runtime(period)_copy" if a waiter updates its
> deadline parameters(dl_runtime, dl_period) or boosted waiter changes
> to !deadline class.
> 
> Thus, force deadline task not out by adding the !dl_prio() condition.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org.

-- Steve

> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: http://lkml.kernel.org/r/1460633827-345-7-git-send-email-xlpang@redhat.com

[toc] | [prev] | [next] | [standalone]


#1493528 — Re: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update

FromThomas Gleixner <tglx@linutronix.de>
Date2016-09-29 17:00 +0200
SubjectRe: [PATCH -v2 3/9] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update
Message-ID<smKYx-5qg-23@gated-at.bofh.it>
In reply to#1491278
On Mon, 26 Sep 2016, Peter Zijlstra wrote:

> Currently dl tasks will actually return at the very beginning
> of rt_mutex_adjust_prio_chain() in !detect_deadlock cases:
> 
>     if (waiter->prio == task->prio) {
>         if (!detect_deadlock)
>             goto out_unlock_pi; // out here
>         else
>             requeue = false;
>     }
> 
> As the deadline value of blocked deadline tasks(waiters) without
> changing their sched_class(thus prio doesn't change) never changes,
> this seems reasonable, but it actually misses the chance of updating
> rt_mutex_waiter's "dl_runtime(period)_copy" if a waiter updates its
> deadline parameters(dl_runtime, dl_period) or boosted waiter changes
> to !deadline class.
> 
> Thus, force deadline task not out by adding the !dl_prio() condition.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[toc] | [prev] | [next] | [standalone]


#1491280 — [PATCH -v2 8/9] rtmutex: Fix PI chain order integrity

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 8/9] rtmutex: Fix PI chain order integrity
Message-ID<slDw6-3SS-23@gated-at.bofh.it>
In reply to#1491275
rt_mutex_waiter::prio is a copy of task_struct::prio which is updated
during the PI chain walk, such that the PI chain order isn't messed up
by (asynchronous) task state updates.

Currently rt_mutex_waiter_less() uses task state for deadline tasks;
this is broken, since the task state can, as said above, change
asynchronously, causing the RB tree order to change without actual
tree update -> FAIL.

Fix this by also copying the deadline into the rt_mutex_waiter state
and updating it along with its prio field.

Ideally we would also force PI chain updates whenever DL tasks update
their deadline parameter, but for first approximation this is less
broken than it was.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/locking/rtmutex.c        |   29 +++++++++++++++++++++++++++--
 kernel/locking/rtmutex_common.h |    1 +
 2 files changed, 28 insertions(+), 2 deletions(-)

--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -172,8 +172,7 @@ rt_mutex_waiter_less(struct rt_mutex_wai
 	 * then right waiter has a dl_prio() too.
 	 */
 	if (dl_prio(left->prio))
-		return dl_time_before(left->task->dl.deadline,
-				      right->task->dl.deadline);
+		return dl_time_before(left->deadline, right->deadline);
 
 	return 0;
 }
@@ -584,7 +583,26 @@ static int rt_mutex_adjust_prio_chain(st
 
 	/* [7] Requeue the waiter in the lock waiter tree. */
 	rt_mutex_dequeue(lock, waiter);
+
+	/*
+	 * Update the waiter prio fields now that we're dequeued.
+	 *
+	 * These values can have changed through either:
+	 *
+	 *   sys_sched_set_scheduler() / sys_sched_setattr()
+	 *
+	 * or
+	 *
+	 *   DL CBS enforcement advancing the effective deadline.
+	 *
+	 * Even though pi_waiters also uses these fields, and that tree is only
+	 * updated in [11], we can do this here, since we hold [L], which
+	 * serializes all pi_waiters access and rb_erase() does not care about
+	 * the values of the node being removed.
+	 */
 	waiter->prio = task->prio;
+	waiter->deadline = task->dl.deadline;
+
 	rt_mutex_enqueue(lock, waiter);
 
 	/* [8] Release the task */
@@ -711,6 +729,8 @@ static int rt_mutex_adjust_prio_chain(st
 static int try_to_take_rt_mutex(struct rt_mutex *lock, struct task_struct *task,
 				struct rt_mutex_waiter *waiter)
 {
+	lockdep_assert_held(&lock->wait_lock);
+
 	/*
 	 * Before testing whether we can acquire @lock, we set the
 	 * RT_MUTEX_HAS_WAITERS bit in @lock->owner. This forces all
@@ -838,6 +858,8 @@ static int task_blocks_on_rt_mutex(struc
 	struct rt_mutex *next_lock;
 	int chain_walk = 0, res;
 
+	lockdep_assert_held(&lock->wait_lock);
+
 	/*
 	 * Early deadlock detection. We really don't want the task to
 	 * enqueue on itself just to untangle the mess later. It's not
@@ -855,6 +877,7 @@ static int task_blocks_on_rt_mutex(struc
 	waiter->task = task;
 	waiter->lock = lock;
 	waiter->prio = task->prio;
+	waiter->deadline = task->dl.deadline;
 
 	/* Get the top priority waiter on the lock */
 	if (rt_mutex_has_waiters(lock))
@@ -972,6 +995,8 @@ static void remove_waiter(struct rt_mute
 	struct task_struct *owner = rt_mutex_owner(lock);
 	struct rt_mutex *next_lock;
 
+	lockdep_assert_held(&lock->wait_lock);
+
 	raw_spin_lock(&current->pi_lock);
 	rt_mutex_dequeue(lock, waiter);
 	current->pi_blocked_on = NULL;
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -33,6 +33,7 @@ struct rt_mutex_waiter {
 	struct rt_mutex		*deadlock_lock;
 #endif
 	int prio;
+	u64 deadline;
 };
 
 /*

[toc] | [prev] | [next] | [standalone]


#1491281 — [PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio()
Message-ID<slDw6-3SS-25@gated-at.bofh.it>
In reply to#1491275
With the introduction of SCHED_DEADLINE the whole notion that priority
is a single number is gone, therefore the @prio argument to
rt_mutex_setprio() doesn't make sense anymore.

So rework the code to pass a pi_task instead.

Note this also fixes a problem with pi_top_task caching; previously we
would not set the pointer (call rt_mutex_update_top_task) if the
priority didn't change, this could lead to a stale pointer.

As for the XXX, I think its fine to use pi_task->prio, because if it
differs from waiter->prio, a PI chain update is immenent.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/sched/rt.h        |   21 +-------
 kernel/locking/rtmutex.c        |  105 +++++++++++-----------------------------
 kernel/locking/rtmutex_common.h |    1 
 kernel/sched/core.c             |   66 ++++++++++++++++++++-----
 4 files changed, 88 insertions(+), 105 deletions(-)

--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -16,28 +16,17 @@ static inline int rt_task(struct task_st
 }
 
 #ifdef CONFIG_RT_MUTEXES
-extern int rt_mutex_getprio(struct task_struct *p);
-extern void rt_mutex_setprio(struct task_struct *p, int prio);
-extern int rt_mutex_get_effective_prio(struct task_struct *task, int newprio);
-extern void rt_mutex_update_top_task(struct task_struct *p);
-extern struct task_struct *rt_mutex_get_top_task(struct task_struct *task);
+static inline struct task_struct *rt_mutex_get_top_task(struct task_struct *p)
+{
+	return p->pi_top_task;
+}
+extern void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task);
 extern void rt_mutex_adjust_pi(struct task_struct *p);
 static inline bool tsk_is_pi_blocked(struct task_struct *tsk)
 {
 	return tsk->pi_blocked_on != NULL;
 }
 #else
-static inline int rt_mutex_getprio(struct task_struct *p)
-{
-	return p->normal_prio;
-}
-
-static inline int rt_mutex_get_effective_prio(struct task_struct *task,
-					      int newprio)
-{
-	return newprio;
-}
-
 static inline struct task_struct *rt_mutex_get_top_task(struct task_struct *task)
 {
 	return NULL;
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -256,61 +256,16 @@ rt_mutex_dequeue_pi(struct task_struct *
 	RB_CLEAR_NODE(&waiter->pi_tree_entry);
 }
 
-void rt_mutex_update_top_task(struct task_struct *p)
+static void rt_mutex_adjust_prio(struct task_struct *p)
 {
-	if (!task_has_pi_waiters(p)) {
-		p->pi_top_task = NULL;
-		return;
-	}
+	struct task_struct *pi_task = NULL;
 
-	p->pi_top_task = task_top_pi_waiter(p)->task;
-}
-
-/*
- * Calculate task priority from the waiter tree priority
- *
- * Return task->normal_prio when the waiter tree is empty or when
- * the waiter is not allowed to do priority boosting
- */
-int rt_mutex_getprio(struct task_struct *task)
-{
-	if (likely(!task_has_pi_waiters(task)))
-		return task->normal_prio;
+	lockdep_assert_held(&p->pi_lock);
 
-	return min(task_top_pi_waiter(task)->prio,
-		   task->normal_prio);
-}
+	if (task_has_pi_waiters(p))
+		pi_task = task_top_pi_waiter(p)->task;
 
-struct task_struct *rt_mutex_get_top_task(struct task_struct *task)
-{
-	return task->pi_top_task;
-}
-
-/*
- * Called by sched_setscheduler() to get the priority which will be
- * effective after the change.
- */
-int rt_mutex_get_effective_prio(struct task_struct *task, int newprio)
-{
-	struct task_struct *top_task = rt_mutex_get_top_task(task);
-
-	if (!top_task)
-		return newprio;
-
-	return min(top_task->prio, newprio);
-}
-
-/*
- * Adjust the priority of a task, after its pi_waiters got modified.
- *
- * This can be both boosting and unboosting. task->pi_lock must be held.
- */
-static void __rt_mutex_adjust_prio(struct task_struct *task)
-{
-	int prio = rt_mutex_getprio(task);
-
-	if (task->prio != prio || dl_prio(prio))
-		rt_mutex_setprio(task, prio);
+	rt_mutex_setprio(p, pi_task);
 }
 
 /*
@@ -670,7 +625,7 @@ static int rt_mutex_adjust_prio_chain(st
 		 */
 		rt_mutex_dequeue_pi(task, prerequeue_top_waiter);
 		rt_mutex_enqueue_pi(task, waiter);
-		__rt_mutex_adjust_prio(task);
+		rt_mutex_adjust_prio(task);
 
 	} else if (prerequeue_top_waiter == waiter) {
 		/*
@@ -686,7 +641,7 @@ static int rt_mutex_adjust_prio_chain(st
 		rt_mutex_dequeue_pi(task, waiter);
 		waiter = rt_mutex_top_waiter(lock);
 		rt_mutex_enqueue_pi(task, waiter);
-		__rt_mutex_adjust_prio(task);
+		rt_mutex_adjust_prio(task);
 	} else {
 		/*
 		 * Nothing changed. No need to do any priority
@@ -896,7 +851,7 @@ static int task_blocks_on_rt_mutex(struc
 		return -EDEADLK;
 
 	raw_spin_lock(&task->pi_lock);
-	__rt_mutex_adjust_prio(task);
+	rt_mutex_adjust_prio(task);
 	waiter->task = task;
 	waiter->lock = lock;
 	waiter->prio = task->prio;
@@ -918,7 +873,7 @@ static int task_blocks_on_rt_mutex(struc
 		rt_mutex_dequeue_pi(owner, top_waiter);
 		rt_mutex_enqueue_pi(owner, waiter);
 
-		__rt_mutex_adjust_prio(owner);
+		rt_mutex_adjust_prio(owner);
 		if (owner->pi_blocked_on)
 			chain_walk = 1;
 	} else if (rt_mutex_cond_detect_deadlock(waiter, chwalk)) {
@@ -970,13 +925,14 @@ static void mark_wakeup_next_waiter(stru
 	waiter = rt_mutex_top_waiter(lock);
 
 	/*
-	 * Remove it from current->pi_waiters. We do not adjust a
-	 * possible priority boost right now. We execute wakeup in the
-	 * boosted mode and go back to normal after releasing
-	 * lock->wait_lock.
+	 * Remove it from current->pi_waiters and deboost.
+	 *
+	 * We must in fact deboost here in order to ensure we call
+	 * rt_mutex_setprio() to update p->pi_top_task before the
+	 * task unblocks.
 	 */
 	rt_mutex_dequeue_pi(current, waiter);
-	__rt_mutex_adjust_prio(current);
+	rt_mutex_adjust_prio(current);
 
 	/*
 	 * As we are waking up the top waiter, and the waiter stays
@@ -988,9 +944,19 @@ static void mark_wakeup_next_waiter(stru
 	 */
 	lock->owner = (void *) RT_MUTEX_HAS_WAITERS;
 
-	raw_spin_unlock(&current->pi_lock);
-
+	/*
+	 * We deboosted before waking the top waiter task such that we don't
+	 * run two tasks with the 'same' priority (and ensure the
+	 * p->pi_top_task pointer points to a blocked task). This however can
+	 * lead to priority inversion if we would get preempted after the
+	 * deboost but before waking our donor task, hence the preempt_disable()
+	 * before unlock.
+	 *
+	 * Pairs with preempt_enable() in rt_mutex_postunlock();
+	 */
+	preempt_disable();
 	wake_q_add(wake_q, waiter->task);
+	raw_spin_unlock(&current->pi_lock);
 }
 
 /*
@@ -1025,7 +991,7 @@ static void remove_waiter(struct rt_mute
 	if (rt_mutex_has_waiters(lock))
 		rt_mutex_enqueue_pi(owner, rt_mutex_top_waiter(lock));
 
-	__rt_mutex_adjust_prio(owner);
+	rt_mutex_adjust_prio(owner);
 
 	/* Store the lock on which owner is blocked or NULL */
 	next_lock = task_blocked_on_lock(owner);
@@ -1064,8 +1030,7 @@ void rt_mutex_adjust_pi(struct task_stru
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
 
 	waiter = task->pi_blocked_on;
-	if (!waiter || (waiter->prio == task->prio &&
-			!dl_prio(task->prio))) {
+	if (!waiter || (waiter->prio == task->prio && !dl_prio(task->prio))) {
 		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
 		return;
 	}
@@ -1316,16 +1281,6 @@ static bool __sched rt_mutex_slowunlock(
 	 */
 	mark_wakeup_next_waiter(wake_q, lock);
 
-	/*
-	 * We should deboost before waking the top waiter task such that
-	 * we don't run two tasks with the 'same' priority. 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 before unlock. Pairs with preempt_enable() in
-	 * rt_mutex_postunlock();
-	 */
-	preempt_disable();
-
 	raw_spin_unlock_irqrestore(&lock->wait_lock, flags);
 
 	/* call rt_mutex_postunlock() */
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -112,7 +112,6 @@ extern int rt_mutex_timed_futex_lock(str
 extern bool rt_mutex_futex_unlock(struct rt_mutex *lock,
 				  struct wake_q_head *wqh);
 extern void rt_mutex_postunlock(struct wake_q_head *wake_q);
-extern void rt_mutex_adjust_prio(struct task_struct *task);
 
 #ifdef CONFIG_DEBUG_RT_MUTEXES
 # include "rtmutex-debug.h"
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3546,10 +3546,25 @@ EXPORT_SYMBOL(default_wake_function);
 
 #ifdef CONFIG_RT_MUTEXES
 
+static inline int __rt_effective_prio(struct task_struct *pi_task, int prio)
+{
+	if (pi_task)
+		prio = min(prio, pi_task->prio);
+
+	return prio;
+}
+
+static inline int rt_effective_prio(struct task_struct *p, int prio)
+{
+	struct task_struct *pi_task = rt_mutex_get_top_task(p);
+
+	return __rt_effective_prio(pi_task, prio);
+}
+
 /*
  * rt_mutex_setprio - set the current priority of a task
- * @p: task
- * @prio: prio value (kernel-internal form)
+ * @p: task to boost
+ * @pi_task: donor task
  *
  * This function changes the 'effective' priority of a task. It does
  * not touch ->normal_prio like __setscheduler().
@@ -3557,16 +3572,40 @@ EXPORT_SYMBOL(default_wake_function);
  * Used by the rt_mutex code to implement priority inheritance
  * logic. Call site only calls if the priority of the task changed.
  */
-void rt_mutex_setprio(struct task_struct *p, int prio)
+void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task)
 {
-	int oldprio, queued, running, queue_flag = DEQUEUE_SAVE | DEQUEUE_MOVE;
+	int prio, oldprio, queued, running, queue_flag = DEQUEUE_SAVE | DEQUEUE_MOVE;
 	const struct sched_class *prev_class;
 	struct rq_flags rf;
 	struct rq *rq;
 
-	BUG_ON(prio > MAX_PRIO);
+	/* XXX used to be waiter->prio, not waiter->task->prio */
+	prio = __rt_effective_prio(pi_task, p->normal_prio);
+
+	/*
+	 * If nothing changed; bail early.
+	 */
+	if (p->pi_top_task == pi_task && prio == p->prio && !dl_prio(prio))
+		return;
 
 	rq = __task_rq_lock(p, &rf);
+	/*
+	 * Set under pi_lock && rq->lock, such that the value can be used under
+	 * either lock.
+	 *
+	 * Note that there is loads of tricky to make this pointer cache work
+	 * right. rt_mutex_slowunlock()+rt_mutex_postunlock() work together to
+	 * ensure a task is de-boosted (pi_task is set to NULL) before the
+	 * task is allowed to run again (and can exit). This ensures the pointer
+	 * points to a blocked task -- which guaratees the task is present.
+	 */
+	p->pi_top_task = pi_task;
+
+	/*
+	 * For FIFO/RR we only need to set prio, if that matches we're done.
+	 */
+	if (prio == p->prio && !dl_prio(prio))
+		goto out_unlock;
 
 	/*
 	 * Idle task boosting is a nono in general. There is one
@@ -3586,9 +3625,7 @@ void rt_mutex_setprio(struct task_struct
 		goto out_unlock;
 	}
 
-	rt_mutex_update_top_task(p);
-
-	trace_sched_pi_setprio(p, prio);
+	trace_sched_pi_setprio(p, prio); /* broken */
 	oldprio = p->prio;
 
 	if (oldprio == prio)
@@ -3612,7 +3649,6 @@ void rt_mutex_setprio(struct task_struct
 	 *          running task
 	 */
 	if (dl_prio(prio)) {
-		struct task_struct *pi_task = rt_mutex_get_top_task(p);
 		if (!dl_prio(p->normal_prio) ||
 		    (pi_task && dl_entity_preempt(&pi_task->dl, &p->dl))) {
 			p->dl.dl_boosted = 1;
@@ -3649,6 +3685,11 @@ void rt_mutex_setprio(struct task_struct
 	balance_callback(rq);
 	preempt_enable();
 }
+#else
+static inline int rt_effective_prio(struct task_struct *p, int prio)
+{
+	return prio;
+}
 #endif
 
 void set_user_nice(struct task_struct *p, long nice)
@@ -3887,10 +3928,9 @@ static void __setscheduler(struct rq *rq
 	 * Keep a potential priority boosting if called from
 	 * sched_setscheduler().
 	 */
+	p->prio = normal_prio(p);
 	if (keep_boost)
-		p->prio = rt_mutex_get_effective_prio(p, normal_prio(p));
-	else
-		p->prio = normal_prio(p);
+		p->prio = rt_effective_prio(p, p->prio);
 
 	if (dl_prio(p->prio))
 		p->sched_class = &dl_sched_class;
@@ -4177,7 +4217,7 @@ static int __sched_setscheduler(struct t
 		 * the runqueue. This will be done when the task deboost
 		 * itself.
 		 */
-		new_effective_prio = rt_mutex_get_effective_prio(p, newprio);
+		new_effective_prio = rt_effective_prio(p, newprio);
 		if (new_effective_prio == oldprio)
 			queue_flags &= ~DEQUEUE_MOVE;
 	}

[toc] | [prev] | [next] | [standalone]


#1491441 — Re: [PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio()

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-09-26 19:00 +0200
SubjectRe: [PATCH -v2 6/9] sched/rtmutex: Refactor rt_mutex_setprio()
Message-ID<slHq2-6hN-27@gated-at.bofh.it>
In reply to#1491281
On Mon, 26 Sep 2016 14:32:19 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> @@ -3586,9 +3625,7 @@ void rt_mutex_setprio(struct task_struct
>  		goto out_unlock;
>  	}
>  
> -	rt_mutex_update_top_task(p);
> -
> -	trace_sched_pi_setprio(p, prio);
> +	trace_sched_pi_setprio(p, prio); /* broken */

Not a very comforting comment to see during a review ;-)

I'll hold off giving a full review of this patch while in the airport.
My mind needs a bit more "quiet" to do this one.

-- Steve

>  	oldprio = p->prio;
>  
>  	if (oldprio == prio)

[toc] | [prev] | [next] | [standalone]


#1491282 — [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()
Message-ID<slDw6-3SS-27@gated-at.bofh.it>
In reply to#1491275
Pass the PI donor task, instead of a numerical priority.

Numerical priorities are not sufficient to describe state ever since
SCHED_DEADLINE.

Annotate all sched tracepoints that are currently broken; fixing them
will bork userspace. *hate*.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/trace/events/sched.h |   16 +++++++++-------
 kernel/sched/core.c          |    2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -70,7 +70,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_templat
 	TP_fast_assign(
 		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
 		__entry->pid		= p->pid;
-		__entry->prio		= p->prio;
+		__entry->prio		= p->prio; /* XXX SCHED_DEADLINE */
 		__entry->success	= 1; /* rudiment, kill when possible */
 		__entry->target_cpu	= task_cpu(p);
 	),
@@ -147,6 +147,7 @@ TRACE_EVENT(sched_switch,
 		memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN);
 		__entry->next_pid	= next->pid;
 		__entry->next_prio	= next->prio;
+		/* XXX SCHED_DEADLINE */
 	),
 
 	TP_printk("prev_comm=%s prev_pid=%d prev_prio=%d prev_state=%s%s ==> next_comm=%s next_pid=%d next_prio=%d",
@@ -181,7 +182,7 @@ TRACE_EVENT(sched_migrate_task,
 	TP_fast_assign(
 		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
 		__entry->pid		= p->pid;
-		__entry->prio		= p->prio;
+		__entry->prio		= p->prio; /* XXX SCHED_DEADLINE */
 		__entry->orig_cpu	= task_cpu(p);
 		__entry->dest_cpu	= dest_cpu;
 	),
@@ -206,7 +207,7 @@ DECLARE_EVENT_CLASS(sched_process_templa
 	TP_fast_assign(
 		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
 		__entry->pid		= p->pid;
-		__entry->prio		= p->prio;
+		__entry->prio		= p->prio; /* XXX SCHED_DEADLINE */
 	),
 
 	TP_printk("comm=%s pid=%d prio=%d",
@@ -253,7 +254,7 @@ TRACE_EVENT(sched_process_wait,
 	TP_fast_assign(
 		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
 		__entry->pid		= pid_nr(pid);
-		__entry->prio		= current->prio;
+		__entry->prio		= current->prio; /* XXX SCHED_DEADLINE */
 	),
 
 	TP_printk("comm=%s pid=%d prio=%d",
@@ -413,9 +414,9 @@ DEFINE_EVENT(sched_stat_runtime, sched_s
  */
 TRACE_EVENT(sched_pi_setprio,
 
-	TP_PROTO(struct task_struct *tsk, int newprio),
+	TP_PROTO(struct task_struct *tsk, struct task_struct *pi_task),
 
-	TP_ARGS(tsk, newprio),
+	TP_ARGS(tsk, pi_task),
 
 	TP_STRUCT__entry(
 		__array( char,	comm,	TASK_COMM_LEN	)
@@ -428,7 +429,8 @@ TRACE_EVENT(sched_pi_setprio,
 		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
 		__entry->pid		= tsk->pid;
 		__entry->oldprio	= tsk->prio;
-		__entry->newprio	= newprio;
+		__entry->newprio	= pi_task ? pi_task->prio : tsk->prio;
+		/* XXX SCHED_DEADLINE bits missing */
 	),
 
 	TP_printk("comm=%s pid=%d oldprio=%d newprio=%d",
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3634,7 +3634,7 @@ void rt_mutex_setprio(struct task_struct
 		goto out_unlock;
 	}
 
-	trace_sched_pi_setprio(p, prio); /* broken */
+	trace_sched_pi_setprio(p, pi_task);
 	oldprio = p->prio;
 
 	if (oldprio == prio)

[toc] | [prev] | [next] | [standalone]


#1491447 — Re: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-09-26 19:10 +0200
SubjectRe: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()
Message-ID<slHzH-6zZ-11@gated-at.bofh.it>
In reply to#1491282
On Mon, 26 Sep 2016 14:32:20 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> Pass the PI donor task, instead of a numerical priority.
> 
> Numerical priorities are not sufficient to describe state ever since
> SCHED_DEADLINE.
> 
> Annotate all sched tracepoints that are currently broken; fixing them
> will bork userspace. *hate*.

Perhaps display -1 for SCHED_DEADLINE tasks?

-- Steve

> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Reviewed-by: Steven Rostedt <rostedt@goodmis.org.

-- Steve

[toc] | [prev] | [next] | [standalone]


#1491738 — Re: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-27 09:50 +0200
SubjectRe: [PATCH -v2 7/9] sched,tracing: Update trace_sched_pi_setprio()
Message-ID<slVjk-6IZ-39@gated-at.bofh.it>
In reply to#1491447
On Mon, Sep 26, 2016 at 01:04:30PM -0400, Steven Rostedt wrote:
> On Mon, 26 Sep 2016 14:32:20 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Pass the PI donor task, instead of a numerical priority.
> > 
> > Numerical priorities are not sufficient to describe state ever since
> > SCHED_DEADLINE.
> > 
> > Annotate all sched tracepoints that are currently broken; fixing them
> > will bork userspace. *hate*.
> 
> Perhaps display -1 for SCHED_DEADLINE tasks?

I think it already does that, but that's plenty useless.

[toc] | [prev] | [next] | [standalone]


#1491283 — [PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock()

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock()
Message-ID<slDw6-3SS-29@gated-at.bofh.it>
In reply to#1491275
There is but a single user and the previous patch mandates slowfn must
be rt_mutex_slowunlock(), so fold the lot together and save a few
lines.

Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/locking/rtmutex.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1377,24 +1377,6 @@ rt_mutex_fasttrylock(struct rt_mutex *lo
 	return slowfn(lock);
 }
 
-static inline void
-rt_mutex_fastunlock(struct rt_mutex *lock,
-		    bool (*slowfn)(struct rt_mutex *lock,
-				   struct wake_q_head *wqh))
-{
-	WAKE_Q(wake_q);
-
-	if (likely(rt_mutex_cmpxchg_release(lock, current, NULL))) {
-		rt_mutex_deadlock_account_unlock(current);
-
-	} else {
-		bool deboost = slowfn(lock, &wake_q);
-
-		rt_mutex_postunlock(&wake_q, deboost);
-	}
-}
-
-
 /*
  * Undo pi boosting (if necessary) and wake top waiter.
  */
@@ -1501,7 +1483,16 @@ EXPORT_SYMBOL_GPL(rt_mutex_trylock);
  */
 void __sched rt_mutex_unlock(struct rt_mutex *lock)
 {
-	rt_mutex_fastunlock(lock, rt_mutex_slowunlock);
+	WAKE_Q(wake_q);
+
+	if (likely(rt_mutex_cmpxchg_release(lock, current, NULL))) {
+		rt_mutex_deadlock_account_unlock(current);
+
+	} else {
+		bool deboost = rt_mutex_slowunlock(lock, &wake_q);
+
+		rt_mutex_postunlock(&wake_q, deboost);
+	}
 }
 EXPORT_SYMBOL_GPL(rt_mutex_unlock);
 

[toc] | [prev] | [next] | [standalone]


#1493529 — Re: [PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock()

FromThomas Gleixner <tglx@linutronix.de>
Date2016-09-29 17:00 +0200
SubjectRe: [PATCH -v2 4/9] rtmutex: Remove rt_mutex_fastunlock()
Message-ID<smKYx-5qg-27@gated-at.bofh.it>
In reply to#1491283
On Mon, 26 Sep 2016, Peter Zijlstra wrote:

> There is but a single user and the previous patch mandates slowfn must
> be rt_mutex_slowunlock(), so fold the lot together and save a few
> lines.

We might have to bring that back for RT, but ok :)

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[toc] | [prev] | [next] | [standalone]


#1491284 — [PATCH -v2 9/9] rtmutex: Fix more prio comparisons

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-26 14:50 +0200
Subject[PATCH -v2 9/9] rtmutex: Fix more prio comparisons
Message-ID<slDw6-3SS-33@gated-at.bofh.it>
In reply to#1491275
There was a pure ->prio comparison left in try_to_wake_rt_mutex(),
convert it to use rt_mutex_waiter_less(), noting that greater-or-equal
is not-less (both in kernel priority view).

This necessitated the introduction of cmp_task() which creates a
pointer to an unnamed stack variable of struct rt_mutex_waiter type to
compare against tasks.

With this, we can now also create and employ rt_mutex_waiter_equal().

Reviewed-and-tested-by: Juri Lelli <juri.lelli@arm.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/locking/rtmutex.c |   32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -158,6 +158,12 @@ static inline bool unlock_rt_mutex_safe(
 }
 #endif
 
+/*
+ * Only use with rt_mutex_waiter_{less,equal}()
+ */
+#define cmp_task(p)	\
+	&(struct rt_mutex_waiter){ .prio = (p)->prio, .deadline = (p)->dl.deadline }
+
 static inline int
 rt_mutex_waiter_less(struct rt_mutex_waiter *left,
 		     struct rt_mutex_waiter *right)
@@ -177,6 +183,25 @@ rt_mutex_waiter_less(struct rt_mutex_wai
 	return 0;
 }
 
+static inline int
+rt_mutex_waiter_equal(struct rt_mutex_waiter *left,
+		      struct rt_mutex_waiter *right)
+{
+	if (left->prio != right->prio)
+		return 0;
+
+	/*
+	 * If both waiters have dl_prio(), we check the deadlines of the
+	 * associated tasks.
+	 * If left waiter has a dl_prio(), and we didn't return 0 above,
+	 * then right waiter has a dl_prio() too.
+	 */
+	if (dl_prio(left->prio))
+		return left->deadline == right->deadline;
+
+	return 1;
+}
+
 static void
 rt_mutex_enqueue(struct rt_mutex *lock, struct rt_mutex_waiter *waiter)
 {
@@ -487,7 +512,7 @@ static int rt_mutex_adjust_prio_chain(st
 	 * enabled we continue, but stop the requeueing in the chain
 	 * walk.
 	 */
-	if (waiter->prio == task->prio && !dl_task(task)) {
+	if (rt_mutex_waiter_equal(waiter, cmp_task(task))) {
 		if (!detect_deadlock)
 			goto out_unlock_pi;
 		else
@@ -790,7 +815,8 @@ static int try_to_take_rt_mutex(struct r
 			 * the top waiter priority (kernel view),
 			 * @task lost.
 			 */
-			if (task->prio >= rt_mutex_top_waiter(lock)->prio)
+			if (!rt_mutex_waiter_less(cmp_task(task),
+						  rt_mutex_top_waiter(lock)))
 				return 0;
 
 			/*
@@ -1055,7 +1081,7 @@ void rt_mutex_adjust_pi(struct task_stru
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
 
 	waiter = task->pi_blocked_on;
-	if (!waiter || (waiter->prio == task->prio && !dl_prio(task->prio))) {
+	if (!waiter || rt_mutex_waiter_equal(waiter, cmp_task(task))) {
 		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
 		return;
 	}

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web