From: Darren Hart <dvhart@infradead.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: tglx@linutronix.de, mingo@kernel.org, juri.lelli@arm.com,
rostedt@goodmis.org, xlpang@redhat.com, bigeasy@linutronix.de,
linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com,
jdesfossez@efficios.com, bristot@redhat.com
Subject: Re: [PATCH -v6 07/13] futex: Rework inconsistent rt_mutex/futex_q state
Date: Wed, 5 Apr 2017 14:58:58 -0700 [thread overview]
Message-ID: <20170405215858.GC13494@fury> (raw)
In-Reply-To: <20170322104151.850383690@infradead.org>
On Wed, Mar 22, 2017 at 11:35:54AM +0100, Peter Zijlstra wrote:
> There is a weird state in the futex_unlock_pi() path when it
> interleaves with a concurrent futex_lock_pi() at the point where it
> drops hb->lock.
>
> In this case, it can happen that the rt_mutex wait_list and the
> futex_q disagree on pending waiters, in particular rt_mutex will find
> no pending waiters where futex_q thinks there are.
>
> In this case the rt_mutex unlock code cannot assign an owner.
>
> What the current code does in this case is use the futex_q waiter that
> got us here; however when the rt_mutex_timed_futex_lock() has already
> failed; this leaves things in a weird state, resulting in much
> head-aches in fixup_owner().
>
> Simplify all this by changing wake_futex_pi() to return -EAGAIN when
> this situation occurs. This then gives the futex_lock_pi() code the
> opportunity to continue and the retried futex_unlock_pi() will now
> observe a coherent state.
>
> The only problem is that this breaks RT timeliness guarantees. That
> is, consider the following scenario:
>
> T1 and T2 are both pinned to CPU0. prio(T2) > prio(T1)
>
> CPU0
>
> T1
> lock_pi()
> queue_me() <- Waiter is visible
>
> preemption
>
> T2
> unlock_pi()
> loops with -EAGAIN forever
>
> Which is undesirable for PI primitives. Future patches will rectify
> this. For now we want to get rid of the fixup magic.
Errrrm... OK... I don't like the idea of having this broken after this commit,
but until I internalize the remaining 5 (that number has never seemed quite so
dauntingly large before... 5...) I can't comment on the alternative. I suppose
having it documented in the commit log means anyone backporting only up to this
point gets what they deserve.
A good patch *removing* code from futex.c is always nice though !
--
Darren Hart
VMware Open Source Technology Center
next prev parent reply other threads:[~2017-04-05 21:59 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-22 10:35 [PATCH -v6 00/13] The arduous story of FUTEX_UNLOCK_PI Peter Zijlstra
2017-03-22 10:35 ` [PATCH -v6 01/13] futex: Cleanup variable names for futex_top_waiter() Peter Zijlstra
2017-03-23 18:19 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-03-24 21:11 ` [PATCH -v6 01/13] " Darren Hart
2017-03-22 10:35 ` [PATCH -v6 02/13] futex: Use smp_store_release() in mark_wake_futex() Peter Zijlstra
2017-03-23 18:19 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-03-24 21:16 ` [PATCH -v6 02/13] " Darren Hart
2017-03-22 10:35 ` [PATCH -v6 03/13] futex: Remove rt_mutex_deadlock_account_*() Peter Zijlstra
2017-03-23 18:20 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-03-24 21:29 ` [PATCH -v6 03/13] " Darren Hart
2017-03-24 21:31 ` Darren Hart
2017-03-22 10:35 ` [PATCH -v6 04/13] futex,rt_mutex: Provide futex specific rt_mutex API Peter Zijlstra
2017-03-23 18:20 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-03-25 0:37 ` [PATCH -v6 04/13] " Darren Hart
2017-04-06 12:15 ` Peter Zijlstra
2017-04-06 17:02 ` Darren Hart
2017-04-05 15:02 ` Darren Hart
2017-04-06 12:17 ` Peter Zijlstra
2017-04-06 17:08 ` Darren Hart
2017-03-22 10:35 ` [PATCH -v6 05/13] futex: Change locking rules Peter Zijlstra
2017-03-23 18:21 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-05 21:18 ` [PATCH -v6 05/13] " Darren Hart
2017-04-06 12:28 ` Peter Zijlstra
2017-04-06 15:58 ` Joe Perches
2017-04-06 17:21 ` Darren Hart
2017-03-22 10:35 ` [PATCH -v6 06/13] futex: Cleanup refcounting Peter Zijlstra
2017-03-23 18:21 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-05 21:29 ` [PATCH -v6 06/13] " Darren Hart
2017-03-22 10:35 ` [PATCH -v6 07/13] futex: Rework inconsistent rt_mutex/futex_q state Peter Zijlstra
2017-03-23 18:22 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-05 21:58 ` Darren Hart [this message]
2017-03-22 10:35 ` [PATCH -v6 08/13] futex: Pull rt_mutex_futex_unlock() out from under hb->lock Peter Zijlstra
2017-03-23 18:22 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-05 23:52 ` [PATCH -v6 08/13] " Darren Hart
2017-04-06 12:42 ` Peter Zijlstra
2017-04-06 17:42 ` Darren Hart
2017-03-22 10:35 ` [PATCH -v6 09/13] futex,rt_mutex: Introduce rt_mutex_init_waiter() Peter Zijlstra
2017-03-23 18:23 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-05 23:57 ` [PATCH -v6 09/13] " Darren Hart
2017-03-22 10:35 ` [PATCH -v6 10/13] futex,rt_mutex: Restructure rt_mutex_finish_proxy_lock() Peter Zijlstra
2017-03-23 18:23 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-07 23:30 ` [PATCH -v6 10/13] " Darren Hart
2017-04-07 23:35 ` Darren Hart
2017-03-22 10:35 ` [PATCH -v6 11/13] futex: Rework futex_lock_pi() to use rt_mutex_*_proxy_lock() Peter Zijlstra
2017-03-23 18:24 ` [tip:locking/core] " tip-bot for Peter Zijlstra
2017-04-08 0:55 ` [PATCH -v6 11/13] " Darren Hart
2017-04-10 15:51 ` alexander.levin
2017-04-10 16:03 ` Thomas Gleixner
2017-04-14 9:30 ` [tip:locking/core] futex: Avoid freeing an active timer tip-bot for Thomas Gleixner
2017-03-22 10:35 ` [PATCH -v6 12/13] futex: futex_unlock_pi() determinism Peter Zijlstra
2017-03-23 18:24 ` [tip:locking/core] futex: Futex_unlock_pi() determinism tip-bot for Peter Zijlstra
2017-04-08 1:27 ` [PATCH -v6 12/13] futex: futex_unlock_pi() determinism Darren Hart
2017-03-22 10:36 ` [PATCH -v6 13/13] futex: futex_lock_pi() vs PREEMPT_RT_FULL Peter Zijlstra
2017-03-23 18:25 ` [tip:locking/core] futex: Drop hb->lock before enqueueing on the rtmutex tip-bot for Peter Zijlstra
2017-04-08 2:26 ` [PATCH -v6 13/13] futex: futex_lock_pi() vs PREEMPT_RT_FULL Darren Hart
2017-04-08 5:22 ` Mike Galbraith
2017-04-10 8:43 ` Sebastian Andrzej Siewior
2017-04-10 9:08 ` Peter Zijlstra
2017-04-10 16:05 ` Darren Hart
2017-03-24 1:45 ` [PATCH -v6 00/13] The arduous story of FUTEX_UNLOCK_PI Darren Hart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170405215858.GC13494@fury \
--to=dvhart@infradead.org \
--cc=bigeasy@linutronix.de \
--cc=bristot@redhat.com \
--cc=jdesfossez@efficios.com \
--cc=juri.lelli@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=xlpang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.