Re: [PATCH -v6 04/13] futex,rt_mutex: Provide futex specific rt_mutex API

[Peter Zijlstra]

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.