On Thu, Apr 06, 2017 at 02:17:28PM +0200, Peter Zijlstra wrote: > 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.
Aha, "must not", OK. I interpretted "cannot" as "is incapable of failing". So let's use something like that for the comment: /* * We updated the user value and are committed to completing the unlock, we must * not fail. */ Wow... English. I tried a few versions, but cannot, may not, etc. all have doublt meanings. :-) -- Darren Hart VMware Open Source Technology Center
