Re: [patch 0/3] futex/rtmutex: Fix issues exposed by trinity

["Carlos O'Donell" <carlos@redhat.com>]

On 05/14/2014 05:22 AM, Peter Zijlstra wrote:
>>> I believe the thinking goes that if we get to here, then the lock is in an
>>> inconsistent state (between kernel and userspace). I don't have an answer for
>>> why pausing forever would be preferable to returning an error however...
>>
>> What error would we return?
> 
> EDEADLK is a valid user return for pthread_mutex_lock() as per:
> 
>   http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html

How is that correct? It isn't a deadlock we've detected but inconsistent
state between glibc and the kernel. In this case glibc should assert.
Delaying indefinitely with pause() never seems correct (despite that being
what we do today).
 
>> This particular case is a serious error for which we have no good error code
>> to return to userspace. It's an implementation defect, a bug, we should probably
>> assert instead of pausing.
> 
> No, its perfectly fine to have a lock sequence abort with -EDEADLK.
> Userspace should release its locks and re-attempt.

I agree. If I can prove that it's actually a deadlock, and
that unlock/relock will work to fix it, then we can arrange for glibc
to return EDEADLK.

> You can implement usable locking schemes using this error, like
> wound/wait locking.

Agreed.

>> We can't cancel the stuck thread because pthread_mutex_lock is not a cancellation
>> point.
>>
>> In practice the rest of the application can make forward progress with a single
>> thread stuck. You can attach the debugger and inspect state, so it's useful
>> from that perspective.
> 
> That's just totally braindead. Return EDEADLK to userspace already, let
> the user deal with it.

Not all cases where EDEADLK returns is it such a case that the user
can make forward progress, it might be a corrupt state, in which case
if we detect the corrupt state I would assert. Otherwise, yes, we can
return EDEADLK and let the user figure it out.

Does that make sense?

Cheers,
Carlos.