On Thu, Feb 05, 2015 at 07:10:14PM +0100, Oleg Nesterov wrote: > Let me first say that I simply do not know if PI+robust futex is actually > supposed (or guaranteed) to work.
> Now, if it should work, I 'think' it _should_ work. Afaict the glibc code sees this as a valid combination. > On 02/05, Peter Zijlstra wrote: > > > > So as long as we unhash _last_ I can't see this happening, we'll always > > find the task, the robust list walk doesn't care about PI state. > > and it simply can't take care of PI state. ->pi_state can be NULL by > the time exit_robust_list() is called. > > > But please, if you suspect, share a little more detail on how you see > > this happening, this is not code I've looked at in detail before. > > Heh, I am reading it for the first time ;) So I can be easily wrong. > > But afaics the race/problem is very simple. Suppose a task T locks a PI+robust > mutex and exits. I this case (I presume) sys_futex(uaddr, FUTEX_LOCK_PI) > from another task X must always succeed sooner or later. But > > - X takes queue_lock() and reads *uaddr == T->pid. Need to setup > pi_state and wait. FUTEX_WAITERS is set. > > - T exits and calls handle_futex_death(). This clears FUTEX_TID_MASK > and sets FUTEX_OWNER_DIED, without any lock. > > T->pi_state_list is empty, exit_pi_state_list() does nothing. Right, because T acquired the lock from userspace and there have not yet been any waiters, so there's no pi state. > T goes away or simply sets PF_EXITPIDONE (lets ignore PF_EXITING). > > - X calls attach_to_pi_owner() and futex_find_get_task() returns NULL, > or we detect PF_EXITPIDONE, this doesn't really matter. > > What does matter (unless I missed something) is that -ESRCH is wrong > in this case. This mutex was unlocked. It is robust, so we should not > miss this unlock. Right,.. > So I think that in this case we either need to recheck that *uaddr is still > the > same (and turn -ESRCH into -EAGAIN otherwise), or change handle_futex_death() > to > serialize with X so that it can proceed and attach pi_state. > > No? I _think_ you're right, doing -ESRCH is wrong without first looking to see if uval changed and gained an FUTEX_OWNER_DIED. I don't think making handle_futex_death() wait on hb lock works because of the -EAGAIN loop releasing that lock. Now, I think Darren actually had a futex test suite; Darren can you add a robust-pi test like the above to stress this? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
