mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: 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: [RFC][PATCH 4/4] futex: Rewrite FUTEX_UNLOCK_PI
Date: Fri, 25 Nov 2016 20:13:24 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1611252007230.3602@nanos> (raw)
In-Reply-To: <20161125100300.GH3174@twins.programming.kicks-ass.net>

On Fri, 25 Nov 2016, Peter Zijlstra wrote:
> On Fri, Nov 25, 2016 at 10:23:26AM +0100, Peter Zijlstra wrote:
> > On Thu, Nov 24, 2016 at 07:58:07PM +0100, Peter Zijlstra wrote:
> > 
> > > OK, so clearly I'm confused. So let me try again.
> > > 
> > > LOCK_PI, does in one function: lookup_pi_state, and fixup_owner. If
> > > fixup_owner fails with -EAGAIN, we can redo the pi_state lookup.
> > > 
> > > The requeue stuff, otoh, has one each. REQUEUE_WAIT has fixup_owner(),
> > > CMP_REQUEUE has lookup_pi_state. Therefore, fixup_owner failing with
> > > -EAGAIN leaves us dead in the water. There's nothing to go back to to
> > > retry.
> > > 
> > > So far, so 'good', right?
> > > 
> > > Now, as far as I understand this requeue stuff, we have 2 futexes, an
> > > inner futex and an outer futex. The inner futex is always 'locked' and
> > > serves as a collection pool for waiting threads.

Yes.
 
> > > The requeue crap picks one (or more) waiters from the inner futex and
> > > sticks them on the outer futex, which gives them a chance to run.

No. The chance to ran can get only one, if it can acquire the futex in user
space directly. If the futex is locked, it is queued on the outer
futex/rtmutex and gets to run when the outer futex is unlocked by the owner.
 
> > > So WAIT_REQUEUE blocks on the inner futex, but knows that if it ever
> > > gets woken, it will be on the outer futex, and hence needs to
> > > fixup_owner if the futex and rt_mutex state got out of sync.

It can be woken on the inner one as well (signal/timeout), but that does
not affect the outer futex, so it's not interesting.

> > > CMP_REQUEUEUEUE picks the one (or more) waiters of the inner futex and
> > > sticks them on the outer futex.
> > > 
> > > So far, so 'good' ?
> > > 
> > > The thing I'm not entire sure on is what happens with the outer futex,
> > > do we first LOCK_PI it before doing CMP_REQUEUE, giving us waiters, and
> > > then UNLOCK_PI to let them rip? Or do we just CMP_REQUEUE and then let
> > > whoever wins finish with UNLOCK_PI?
> > > 
> > > 
> > > In any case, I don't think it matters much, either way we can race
> > > betwen the 'last' UNLOCK_PI and getting rt_mutex waiters and then hit
> > > the &init_task funny state, such that WAIT_REQUEUE waking hits EAGAIN
> > > and we're 'stuck'.

Right, that would be exceptionally bad.
 
> > > Now, if we always CMP_REQUEUE to a locked outer futex, then we cannot
> > > know, at CMP_REQUEUE time, who will win and cannot fix up.
> > 
> > OTOH, if we always first LOCK_PI before doing CMP_REQUEUE, I don't think
> > we can hit the funny state, LOCK_PI will have fixed it up for us.
> > 
> > So the question is, do we mandate LOCK_PI before CMP_REQUEUE?
> 
> Going by futex_requeue(), the first thing it does; after validation and
> getting hbs locked, is futex_proxy_trylock_atomic(), which per the
> comment above it will attempt to acquire uaddr2.
> 
> So no such mandate, otherwise that op would not exist and we'd only need
> to validate that uaddr2 was 'current'.

Correct. It can be done with uaddr2 locked or not. That's why we try to
take it directly first.

Thanks,

	tglx

  reply	other threads:[~2016-11-25 19:16 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-03  9:12 [RFC][PATCH 0/4] FUTEX_UNLOCK_PI wobbles Peter Zijlstra
2016-10-03  9:12 ` [RFC][PATCH 1/4] futex: Cleanup variable names for futex_top_waiter() Peter Zijlstra
2016-10-03 14:15   ` Steven Rostedt
2016-10-05  3:58   ` Davidlohr Bueso
2016-10-03  9:12 ` [RFC][PATCH 2/4] futex: Use smp_store_release() in mark_wake_futex() Peter Zijlstra
2016-10-03 14:19   ` Steven Rostedt
2016-10-05  3:57   ` Davidlohr Bueso
2016-10-05  6:20     ` Peter Zijlstra
2016-10-03  9:12 ` [RFC][PATCH 3/4] futex: Remove rt_mutex_deadlock_account_*() Peter Zijlstra
2016-10-03  9:34   ` Peter Zijlstra
2016-10-03 14:25   ` Steven Rostedt
2016-10-05  1:08   ` Davidlohr Bueso
2016-10-05  7:29   ` Sebastian Andrzej Siewior
2016-10-03  9:12 ` [RFC][PATCH 4/4] futex: Rewrite FUTEX_UNLOCK_PI Peter Zijlstra
2016-10-03 15:36   ` Steven Rostedt
2016-10-03 15:44     ` Peter Zijlstra
2016-10-03 15:45     ` Peter Zijlstra
2016-10-03 16:23       ` Steven Rostedt
2016-10-05  7:41   ` Sebastian Andrzej Siewior
2016-10-05  8:09     ` Peter Zijlstra
2016-10-05  8:21       ` Sebastian Andrzej Siewior
2016-10-05  8:32         ` Peter Zijlstra
2016-10-06 10:29   ` Peter Zijlstra
2016-10-07 11:21   ` Peter Zijlstra
2016-10-08 15:53     ` Thomas Gleixner
2016-10-08 16:55       ` Peter Zijlstra
2016-10-08 17:06         ` Thomas Gleixner
2016-10-10 10:17         ` Thomas Gleixner
2016-10-10 11:40           ` Peter Zijlstra
2016-10-21 12:27           ` Peter Zijlstra
2016-10-27 20:36             ` Thomas Gleixner
2016-11-23 19:20               ` Peter Zijlstra
2016-11-24 16:52                 ` Peter Zijlstra
2016-11-24 17:56                   ` Thomas Gleixner
2016-11-24 18:58                     ` Peter Zijlstra
2016-11-25  9:23                       ` Peter Zijlstra
2016-11-25 10:03                         ` Peter Zijlstra
2016-11-25 19:13                           ` Thomas Gleixner [this message]
2016-11-25 14:09               ` Peter Zijlstra
2016-10-08 18:22     ` Thomas Gleixner
2016-10-09 11:17     ` Thomas Gleixner
2016-10-10 14:06       ` Peter Zijlstra
2016-10-05  1:02 ` [RFC][PATCH 0/4] FUTEX_UNLOCK_PI wobbles Davidlohr Bueso
2016-10-05  6:20   ` Peter Zijlstra
2016-10-05  7:26     ` Sebastian Andrzej Siewior
2016-10-05 16:04     ` Davidlohr Bueso

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=alpine.DEB.2.20.1611252007230.3602@nanos \
    --to=tglx@linutronix.de \
    --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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®