On Wed, Apr 05, 2017 at 08:02:17AM -0700, Darren Hart wrote:
> > @@ -1364,20 +1364,18 @@ static int wake_futex_pi(u32 __user *uad
> > pi_state->owner = new_owner;
> > raw_spin_unlock(&new_owner->pi_lock);
> >
> > /*
> > + * We've updated the uservalue, this unlock cannot fail.
>
> It isn't clear to me what I should understand from this new comment. How does
> the value of the uval affect whether or not the pi_state->pi_mutex can be
> unlocked or not? Or are you noting that we've set FUTEX_WAITIERS so any valid
> userspace operations will be forced intot he kernel and can't race with us
> since
> we hold the hb->lock? With futexes, I think it's important that we be very
> explicit in our comment blocks.
The critical point is that once you've modified uval we must not fail;
there is no way to undo things thereafter.
> > */
> > + deboost = __rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
> > +
> > + raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock);
> > spin_unlock(&hb->lock);
> > +
> > + if (deboost) {
> > + wake_up_q(&wake_q);
>
> Is moving wake_up_q under deboost related to this change or is it just an
> optimization since there is no need to wake unless we are deboosting
> ourselves -
> which was true before as well?
>
> If this is due to the rt_mutex_futex* API, I haven't made the connection.
It's how rt_mutex does wakeups, note that later patches clean this up.
