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


Groups > linux.kernel > #1592444 > unrolled thread

[PATCH -v5 00/14] the saga of FUTEX_UNLOCK_PI wobbles continues

Started byPeter Zijlstra <peterz@infradead.org>
First post2017-03-04 11:10 +0100
Last post2017-03-13 10:20 +0100
Articles 10 on this page of 30 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH -v5 00/14] the saga of FUTEX_UNLOCK_PI wobbles continues Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
    [PATCH -v5 02/14] futex: Add missing error handling to FUTEX_REQUEUE_PI Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
    [PATCH -v5 04/14] futex: Use smp_store_release() in mark_wake_futex() Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
    [PATCH -v5 10/14] futex: Pull rt_mutex_futex_unlock() out from under hb->lock Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
      Re: [PATCH -v5 10/14] futex: Pull rt_mutex_futex_unlock() out from  under hb->lock Thomas Gleixner <tglx@linutronix.de> - 2017-03-07 19:20 +0100
        Re: [PATCH -v5 10/14] futex: Pull rt_mutex_futex_unlock() out from  under hb->lock Peter Zijlstra <peterz@infradead.org> - 2017-03-07 19:50 +0100
    [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
      Re: [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Thomas Gleixner <tglx@linutronix.de> - 2017-03-07 15:40 +0100
        Re: [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Peter Zijlstra <peterz@infradead.org> - 2017-03-07 19:50 +0100
        Re: [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Peter Zijlstra <peterz@infradead.org> - 2017-03-13 10:30 +0100
          Re: [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Thomas Gleixner <tglx@linutronix.de> - 2017-03-13 15:30 +0100
            Re: [PATCH -v5 14/14] futex: futex_unlock_pi() determinism Peter Zijlstra <peterz@infradead.org> - 2017-03-13 16:20 +0100
    [PATCH -v5 11/14] futex,rt_mutex: Introduce rt_mutex_init_waiter() Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
    [PATCH -v5 07/14] futex: Change locking rules Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:10 +0100
      Re: [PATCH -v5 07/14] futex: Change locking rules Thomas Gleixner <tglx@linutronix.de> - 2017-03-07 14:50 +0100
        Re: [PATCH -v5 07/14] futex: Change locking rules Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-03-07 18:20 +0100
          Re: [PATCH -v5 07/14] futex: Change locking rules Peter Zijlstra <peterz@infradead.org> - 2017-03-07 19:10 +0100
    [PATCH -v5 06/14] futex,rt_mutex: Provide futex specific rt_mutex API Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:50 +0100
    [PATCH -v5 05/14] futex: Remove rt_mutex_deadlock_account_*() Peter Zijlstra <peterz@infradead.org> - 2017-03-04 11:50 +0100
    [PATCH -v5 08/14] futex: Cleanup refcounting Peter Zijlstra <peterz@infradead.org> - 2017-03-04 12:50 +0100
    [PATCH -v5 03/14] futex: Cleanup variable names for futex_top_waiter() Peter Zijlstra <peterz@infradead.org> - 2017-03-04 12:50 +0100
    [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock() Peter Zijlstra <peterz@infradead.org> - 2017-03-04 12:50 +0100
      Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure  rt_mutex_finish_proxy_lock() Thomas Gleixner <tglx@linutronix.de> - 2017-03-07 15:50 +0100
        Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure  rt_mutex_finish_proxy_lock() Thomas Gleixner <tglx@linutronix.de> - 2017-03-07 19:50 +0100
        Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure  rt_mutex_finish_proxy_lock() Peter Zijlstra <peterz@infradead.org> - 2017-03-07 19:50 +0100
      [PATCH] futex: move debug_rt_mutex_free_waiter() further down Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-03-08 16:40 +0100
        Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-03-08 16:40 +0100
          Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:40 +0100
        Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:30 +0100
        Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down Peter Zijlstra <peterz@infradead.org> - 2017-03-13 10:20 +0100

Page 2 of 2 — ← Prev page 1 [2]


#1592469 — [PATCH -v5 03/14] futex: Cleanup variable names for futex_top_waiter()

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-04 12:50 +0100
Subject[PATCH -v5 03/14] futex: Cleanup variable names for futex_top_waiter()
Message-ID<thgmd-6M-9@gated-at.bofh.it>
In reply to#1592444
futex_top_waiter() returns the top-waiter on the pi_mutex. Assinging
this to a variable 'match' totally obscures the code.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/futex.c |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1120,14 +1120,14 @@ static int attach_to_pi_owner(u32 uval,
 static int lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
 			   union futex_key *key, struct futex_pi_state **ps)
 {
-	struct futex_q *match = futex_top_waiter(hb, key);
+	struct futex_q *top_waiter = futex_top_waiter(hb, key);
 
 	/*
 	 * If there is a waiter on that futex, validate it and
 	 * attach to the pi_state when the validation succeeds.
 	 */
-	if (match)
-		return attach_to_pi_state(uval, match->pi_state, ps);
+	if (top_waiter)
+		return attach_to_pi_state(uval, top_waiter->pi_state, ps);
 
 	/*
 	 * We are the first waiter - try to look up the owner based on
@@ -1174,7 +1174,7 @@ static int futex_lock_pi_atomic(u32 __us
 				struct task_struct *task, int set_waiters)
 {
 	u32 uval, newval, vpid = task_pid_vnr(task);
-	struct futex_q *match;
+	struct futex_q *top_waiter;
 	int ret;
 
 	/*
@@ -1200,9 +1200,9 @@ static int futex_lock_pi_atomic(u32 __us
 	 * Lookup existing state first. If it exists, try to attach to
 	 * its pi_state.
 	 */
-	match = futex_top_waiter(hb, key);
-	if (match)
-		return attach_to_pi_state(uval, match->pi_state, ps);
+	top_waiter = futex_top_waiter(hb, key);
+	if (top_waiter)
+		return attach_to_pi_state(uval, top_waiter->pi_state, ps);
 
 	/*
 	 * No waiter and user TID is 0. We are here because the
@@ -1292,11 +1292,11 @@ static void mark_wake_futex(struct wake_
 	q->lock_ptr = NULL;
 }
 
-static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this,
+static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *top_waiter,
 			 struct futex_hash_bucket *hb)
 {
 	struct task_struct *new_owner;
-	struct futex_pi_state *pi_state = this->pi_state;
+	struct futex_pi_state *pi_state = top_waiter->pi_state;
 	u32 uninitialized_var(curval), newval;
 	DEFINE_WAKE_Q(wake_q);
 	bool deboost;
@@ -1317,11 +1317,11 @@ static int wake_futex_pi(u32 __user *uad
 
 	/*
 	 * It is possible that the next waiter (the one that brought
-	 * this owner to the kernel) timed out and is no longer
+	 * top_waiter owner to the kernel) timed out and is no longer
 	 * waiting on the lock.
 	 */
 	if (!new_owner)
-		new_owner = this->task;
+		new_owner = top_waiter->task;
 
 	/*
 	 * We pass it to the next owner. The WAITERS bit is always
@@ -2631,7 +2631,7 @@ static int futex_unlock_pi(u32 __user *u
 	u32 uninitialized_var(curval), uval, vpid = task_pid_vnr(current);
 	union futex_key key = FUTEX_KEY_INIT;
 	struct futex_hash_bucket *hb;
-	struct futex_q *match;
+	struct futex_q *top_waiter;
 	int ret;
 
 retry:
@@ -2655,9 +2655,9 @@ static int futex_unlock_pi(u32 __user *u
 	 * all and we at least want to know if user space fiddled
 	 * with the futex value instead of blindly unlocking.
 	 */
-	match = futex_top_waiter(hb, &key);
-	if (match) {
-		ret = wake_futex_pi(uaddr, uval, match, hb);
+	top_waiter = futex_top_waiter(hb, &key);
+	if (top_waiter) {
+		ret = wake_futex_pi(uaddr, uval, top_waiter, hb);
 		/*
 		 * In case of success wake_futex_pi dropped the hash
 		 * bucket lock.

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


#1592470 — [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-04 12:50 +0100
Subject[PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()
Message-ID<thgmd-6M-11@gated-at.bofh.it>
In reply to#1592444
With the ultimate goal of keeping rt_mutex wait_list and futex_q
waiters consistent we want to split 'rt_mutex_futex_lock()' into finer
parts, such that only the actual blocking can be done without hb->lock
held.

This means we need to split rt_mutex_finish_proxy_lock() into two
parts, one that does the blocking and one that does remove_waiter()
when we fail to acquire.

When we do acquire, we can safely remove ourselves, since there is no
concurrency on the lock owner.

This means that, except for futex_lock_pi(), all wait_list
modifications are done with both hb->lock and wait_lock held.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/futex.c                  |    5 +++-
 kernel/locking/rtmutex.c        |   47 ++++++++++++++++++++++++++++++++++------
 kernel/locking/rtmutex_common.h |    8 ++++--
 3 files changed, 49 insertions(+), 11 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3012,10 +3012,13 @@ static int futex_wait_requeue_pi(u32 __u
 		 */
 		WARN_ON(!q.pi_state);
 		pi_mutex = &q.pi_state->pi_mutex;
-		ret = rt_mutex_finish_proxy_lock(pi_mutex, to, &rt_waiter);
+		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
 		debug_rt_mutex_free_waiter(&rt_waiter);
 
 		spin_lock(q.lock_ptr);
+		if (ret && !rt_mutex_cleanup_proxy_lock(pi_mutex, &rt_waiter))
+			ret = 0;
+
 		/*
 		 * Fixup the pi_state owner and possibly acquire the lock if we
 		 * haven't already.
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1747,21 +1747,23 @@ struct task_struct *rt_mutex_next_owner(
 }
 
 /**
- * rt_mutex_finish_proxy_lock() - Complete lock acquisition
+ * rt_mutex_wait_proxy_lock() - Wait for lock acquisition
  * @lock:		the rt_mutex we were woken on
  * @to:			the timeout, null if none. hrtimer should already have
  *			been started.
  * @waiter:		the pre-initialized rt_mutex_waiter
  *
- * Complete the lock acquisition started our behalf by another thread.
+ * Wait for the the lock acquisition started on our behalf by
+ * rt_mutex_start_proxy_lock(). Upon failure, the caller must call
+ * rt_mutex_cleanup_proxy_lock().
  *
  * Returns:
  *  0 - success
  * <0 - error, one of -EINTR, -ETIMEDOUT
  *
- * Special API call for PI-futex requeue support
+ * Special API call for PI-futex support
  */
-int rt_mutex_finish_proxy_lock(struct rt_mutex *lock,
+int rt_mutex_wait_proxy_lock(struct rt_mutex *lock,
 			       struct hrtimer_sleeper *to,
 			       struct rt_mutex_waiter *waiter)
 {
@@ -1774,9 +1776,6 @@ int rt_mutex_finish_proxy_lock(struct rt
 	/* sleep on the mutex */
 	ret = __rt_mutex_slowlock(lock, TASK_INTERRUPTIBLE, to, waiter);
 
-	if (unlikely(ret))
-		remove_waiter(lock, waiter);
-
 	/*
 	 * try_to_take_rt_mutex() sets the waiter bit unconditionally. We might
 	 * have to fix that up.
@@ -1787,3 +1786,37 @@ int rt_mutex_finish_proxy_lock(struct rt
 
 	return ret;
 }
+
+/**
+ * rt_mutex_cleanup_proxy_lock() - Cleanup failed lock acquisition
+ * @lock:		the rt_mutex we were woken on
+ * @waiter:		the pre-initialized rt_mutex_waiter
+ *
+ * Clean up the failed lock acquisition as per rt_mutex_wait_proxy_lock().
+ *
+ * Returns:
+ *  true  - did the cleanup, we done.
+ *  false - we acquired the lock after rt_mutex_wait_proxy_lock() returned,
+ *          caller should disregards its return value.
+ *
+ * Special API call for PI-futex support
+ */
+bool rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock,
+				 struct rt_mutex_waiter *waiter)
+{
+	bool cleanup = false;
+
+	raw_spin_lock_irq(&lock->wait_lock);
+	/*
+	 * If we acquired the lock, no cleanup required.
+	 */
+	if (rt_mutex_owner(lock) != current) {
+		remove_waiter(lock, waiter);
+		fixup_rt_mutex_waiters(lock);
+		cleanup = true;
+	}
+	raw_spin_unlock_irq(&lock->wait_lock);
+
+	return cleanup;
+}
+
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -106,9 +106,11 @@ extern void rt_mutex_proxy_unlock(struct
 extern int rt_mutex_start_proxy_lock(struct rt_mutex *lock,
 				     struct rt_mutex_waiter *waiter,
 				     struct task_struct *task);
-extern int rt_mutex_finish_proxy_lock(struct rt_mutex *lock,
-				      struct hrtimer_sleeper *to,
-				      struct rt_mutex_waiter *waiter);
+extern int rt_mutex_wait_proxy_lock(struct rt_mutex *lock,
+			       struct hrtimer_sleeper *to,
+			       struct rt_mutex_waiter *waiter);
+extern bool rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock,
+				 struct rt_mutex_waiter *waiter);
 
 extern int rt_mutex_timed_futex_lock(struct rt_mutex *l, struct hrtimer_sleeper *to);
 extern int rt_mutex_futex_trylock(struct rt_mutex *l);

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


#1594310 — Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()

FromThomas Gleixner <tglx@linutronix.de>
Date2017-03-07 15:50 +0100
SubjectRe: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()
Message-ID<tioB5-pr-37@gated-at.bofh.it>
In reply to#1592470
On Sat, 4 Mar 2017, Peter Zijlstra wrote:
> +/**
> + * rt_mutex_cleanup_proxy_lock() - Cleanup failed lock acquisition
> + * @lock:		the rt_mutex we were woken on
> + * @waiter:		the pre-initialized rt_mutex_waiter
> + *
> + * Clean up the failed lock acquisition as per rt_mutex_wait_proxy_lock().
> + *
> + * Returns:
> + *  true  - did the cleanup, we done.
> + *  false - we acquired the lock after rt_mutex_wait_proxy_lock() returned,
> + *          caller should disregards its return value.

Hmm. How would that happen? Magic owner assignement to a non waiter? The
callsite only calls here in the failed case.

I must be missing something

Thanks,

	tglx

> + *
> + * Special API call for PI-futex support
> + */
> +bool rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock,
> +				 struct rt_mutex_waiter *waiter)
> +{
> +	bool cleanup = false;
> +
> +	raw_spin_lock_irq(&lock->wait_lock);
> +	/*
> +	 * If we acquired the lock, no cleanup required.
> +	 */
> +	if (rt_mutex_owner(lock) != current) {
> +		remove_waiter(lock, waiter);
> +		fixup_rt_mutex_waiters(lock);
> +		cleanup = true;
> +	}
> +	raw_spin_unlock_irq(&lock->wait_lock);
> +
> +	return cleanup;
> +}

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


#1594518 — Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()

FromThomas Gleixner <tglx@linutronix.de>
Date2017-03-07 19:50 +0100
SubjectRe: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()
Message-ID<tislk-35L-25@gated-at.bofh.it>
In reply to#1594310
On Tue, 7 Mar 2017, Peter Zijlstra wrote:

> On Tue, Mar 07, 2017 at 03:18:46PM +0100, Thomas Gleixner wrote:
> > On Sat, 4 Mar 2017, Peter Zijlstra wrote:
> > > +/**
> > > + * rt_mutex_cleanup_proxy_lock() - Cleanup failed lock acquisition
> > > + * @lock:		the rt_mutex we were woken on
> > > + * @waiter:		the pre-initialized rt_mutex_waiter
> > > + *
> > > + * Clean up the failed lock acquisition as per rt_mutex_wait_proxy_lock().
> > > + *
> > > + * Returns:
> > > + *  true  - did the cleanup, we done.
> > > + *  false - we acquired the lock after rt_mutex_wait_proxy_lock() returned,
> > > + *          caller should disregards its return value.
> > 
> > Hmm. How would that happen? Magic owner assignement to a non waiter? The
> > callsite only calls here in the failed case.
> 
> Ah, but until the remove_waiter() below, we _still_ are a waiter, and
> thus can get assigned ownership.
> 
> > > + *
> > > + * Special API call for PI-futex support
> > > + */
> > > +bool rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock,
> > > +				 struct rt_mutex_waiter *waiter)
> > > +{
> > > +	bool cleanup = false;
> > > +
> > > +	raw_spin_lock_irq(&lock->wait_lock);
> > > +	/*
> > > +	 * If we acquired the lock, no cleanup required.
> > > +	 */
> > > +	if (rt_mutex_owner(lock) != current) {
> > > +		remove_waiter(lock, waiter);
> 
> See, up till this point, we still a waiter and any unlock can see us
> being one.

Hmm, true. So the comments should explain that

Thanks,

	tglx

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


#1594522 — Re: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-07 19:50 +0100
SubjectRe: [PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()
Message-ID<tislk-35L-27@gated-at.bofh.it>
In reply to#1594310
On Tue, Mar 07, 2017 at 03:18:46PM +0100, Thomas Gleixner wrote:
> On Sat, 4 Mar 2017, Peter Zijlstra wrote:
> > +/**
> > + * rt_mutex_cleanup_proxy_lock() - Cleanup failed lock acquisition
> > + * @lock:		the rt_mutex we were woken on
> > + * @waiter:		the pre-initialized rt_mutex_waiter
> > + *
> > + * Clean up the failed lock acquisition as per rt_mutex_wait_proxy_lock().
> > + *
> > + * Returns:
> > + *  true  - did the cleanup, we done.
> > + *  false - we acquired the lock after rt_mutex_wait_proxy_lock() returned,
> > + *          caller should disregards its return value.
> 
> Hmm. How would that happen? Magic owner assignement to a non waiter? The
> callsite only calls here in the failed case.

Ah, but until the remove_waiter() below, we _still_ are a waiter, and
thus can get assigned ownership.

> > + *
> > + * Special API call for PI-futex support
> > + */
> > +bool rt_mutex_cleanup_proxy_lock(struct rt_mutex *lock,
> > +				 struct rt_mutex_waiter *waiter)
> > +{
> > +	bool cleanup = false;
> > +
> > +	raw_spin_lock_irq(&lock->wait_lock);
> > +	/*
> > +	 * If we acquired the lock, no cleanup required.
> > +	 */
> > +	if (rt_mutex_owner(lock) != current) {
> > +		remove_waiter(lock, waiter);

See, up till this point, we still a waiter and any unlock can see us
being one.

> > +		fixup_rt_mutex_waiters(lock);
> > +		cleanup = true;
> > +	}
> > +	raw_spin_unlock_irq(&lock->wait_lock);
> > +
> > +	return cleanup;
> > +}

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


#1595283 — [PATCH] futex: move debug_rt_mutex_free_waiter() further down

FromSebastian Andrzej Siewior <bigeasy@linutronix.de>
Date2017-03-08 16:40 +0100
Subject[PATCH] futex: move debug_rt_mutex_free_waiter() further down
Message-ID<tiLQZ-8we-7@gated-at.bofh.it>
In reply to#1592470
Without this, futex_requeue_pi_signal_restart will trigger

|kernel BUG at locking/rtmutex_common.h:55!
|Call Trace:
| rt_mutex_cleanup_proxy_lock+0x54/0x90
| futex_wait_requeue_pi.constprop.21+0x387/0x4d0
| do_futex+0x289/0xbf0
|RIP: remove_waiter+0x157/0x170 RSP: ffffc90000e0fbe0

with BUG 2222222222222222 != pointer once this patch is applied.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/futex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 00ec4a01d3f5..73abfe0da4d0 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3046,11 +3046,11 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
 		WARN_ON(!q.pi_state);
 		pi_mutex = &q.pi_state->pi_mutex;
 		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
-		debug_rt_mutex_free_waiter(&rt_waiter);
 
 		spin_lock(q.lock_ptr);
 		if (ret && !rt_mutex_cleanup_proxy_lock(pi_mutex, &rt_waiter))
 			ret = 0;
+		debug_rt_mutex_free_waiter(&rt_waiter);
 
 		/*
 		 * Fixup the pi_state owner and possibly acquire the lock if we
-- 
2.11.0

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


#1595295 — Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down

FromSebastian Andrzej Siewior <bigeasy@linutronix.de>
Date2017-03-08 16:40 +0100
SubjectRe: [PATCH] futex: move debug_rt_mutex_free_waiter() further down
Message-ID<tiLR1-8we-45@gated-at.bofh.it>
In reply to#1595283
On 2017-03-08 16:29:02 [+0100], To Peter Zijlstra wrote:
> Without this, futex_requeue_pi_signal_restart will trigger
> 
> |kernel BUG at locking/rtmutex_common.h:55!
> |Call Trace:
> | rt_mutex_cleanup_proxy_lock+0x54/0x90
> | futex_wait_requeue_pi.constprop.21+0x387/0x4d0
> | do_futex+0x289/0xbf0
> |RIP: remove_waiter+0x157/0x170 RSP: ffffc90000e0fbe0
> 
> with BUG 2222222222222222 != pointer once this patch is applied.

My wording is wrong. This BUG_ON() statement described here in this
patch (together with the test case mentioned) will trigger once

 "[PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()"

is applied.

Sebastian

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


#1595356 — Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-03-08 17:40 +0100
SubjectRe: [PATCH] futex: move debug_rt_mutex_free_waiter() further down
Message-ID<tiMN3-GJ-5@gated-at.bofh.it>
In reply to#1595295
On Wed, 8 Mar 2017 16:37:32 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> On 2017-03-08 16:29:02 [+0100], To Peter Zijlstra wrote:
> > Without this, futex_requeue_pi_signal_restart will trigger
> > 
> > |kernel BUG at locking/rtmutex_common.h:55!
> > |Call Trace:
> > | rt_mutex_cleanup_proxy_lock+0x54/0x90
> > | futex_wait_requeue_pi.constprop.21+0x387/0x4d0
> > | do_futex+0x289/0xbf0
> > |RIP: remove_waiter+0x157/0x170 RSP: ffffc90000e0fbe0
> > 
> > with BUG 2222222222222222 != pointer once this patch is applied.  
> 
> My wording is wrong. This BUG_ON() statement described here in this
> patch (together with the test case mentioned) will trigger once
> 
>  "[PATCH -v5 12/14] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock()"
> 
> is applied.
>

Now I read this. Ignore my last email.

-- Steve

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


#1595334 — Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-03-08 17:30 +0100
SubjectRe: [PATCH] futex: move debug_rt_mutex_free_waiter() further down
Message-ID<tiMDo-Dj-19@gated-at.bofh.it>
In reply to#1595283
On Wed, 8 Mar 2017 16:29:02 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> Without this, futex_requeue_pi_signal_restart will trigger
> 
> |kernel BUG at locking/rtmutex_common.h:55!
> |Call Trace:
> | rt_mutex_cleanup_proxy_lock+0x54/0x90
> | futex_wait_requeue_pi.constprop.21+0x387/0x4d0
> | do_futex+0x289/0xbf0
> |RIP: remove_waiter+0x157/0x170 RSP: ffffc90000e0fbe0
> 
> with BUG 2222222222222222 != pointer once this patch is applied.

This sentence makes no sense. It's a no-sensetence ;-)

-- Steve

> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/futex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index 00ec4a01d3f5..73abfe0da4d0 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -3046,11 +3046,11 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
>  		WARN_ON(!q.pi_state);
>  		pi_mutex = &q.pi_state->pi_mutex;
>  		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
> -		debug_rt_mutex_free_waiter(&rt_waiter);
>  
>  		spin_lock(q.lock_ptr);
>  		if (ret && !rt_mutex_cleanup_proxy_lock(pi_mutex, &rt_waiter))
>  			ret = 0;
> +		debug_rt_mutex_free_waiter(&rt_waiter);
>  
>  		/*
>  		 * Fixup the pi_state owner and possibly acquire the lock if we

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


#1599116 — Re: [PATCH] futex: move debug_rt_mutex_free_waiter() further down

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-13 10:20 +0100
SubjectRe: [PATCH] futex: move debug_rt_mutex_free_waiter() further down
Message-ID<tkuj1-6OF-65@gated-at.bofh.it>
In reply to#1595283
On Wed, Mar 08, 2017 at 04:29:02PM +0100, Sebastian Andrzej Siewior wrote:

>  kernel/futex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index 00ec4a01d3f5..73abfe0da4d0 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -3046,11 +3046,11 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
>  		WARN_ON(!q.pi_state);
>  		pi_mutex = &q.pi_state->pi_mutex;
>  		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
> -		debug_rt_mutex_free_waiter(&rt_waiter);
>  
>  		spin_lock(q.lock_ptr);
>  		if (ret && !rt_mutex_cleanup_proxy_lock(pi_mutex, &rt_waiter))
>  			ret = 0;
> +		debug_rt_mutex_free_waiter(&rt_waiter);
>  
>  		/*
>  		 * Fixup the pi_state owner and possibly acquire the lock if we
> 

Thanks, folded.

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web