Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop

[Chris Wilson]

On Thu, Dec 01, 2016 at 03:06:45PM +0100, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <Nicolai.Haehnle@xxxxxxx>
> 
> In the following scenario, thread #1 should back off its attempt to lock
> ww1 and unlock ww2 (assuming the acquire context stamps are ordered
> accordingly).
> 
>     Thread #0               Thread #1
>     ---------               ---------
>                             successfully lock ww2
>     set ww1->base.owner
>                             attempt to lock ww1
>                             confirm ww1->ctx == NULL
>                             enter mutex_spin_on_owner
>     set ww1->ctx
> 
> What was likely to happen previously is:
> 
>     attempt to lock ww2
>     refuse to spin because
>       ww2->ctx != NULL
>     schedule()
>                             detect thread #0 is off CPU
>                             stop optimistic spin
>                             return -EDEADLK
>                             unlock ww2
>                             wakeup thread #0
>     lock ww2
> 
> Now, we are more likely to see:
> 
>                             detect ww1->ctx != NULL
>                             stop optimistic spin
>                             return -EDEADLK
>                             unlock ww2
>     successfully lock ww2
> 
> ... because thread #1 will stop its optimistic spin as soon as possible.
> 
> The whole scenario is quite unlikely, since it requires thread #1 to get
> between thread #0 setting the owner and setting the ctx. But since we're
> idling here anyway, the additional check is basically free.
> 
> Found by inspection.

Similar question can be raised for can_spin_on_owner() as well. Is it
worth for a contending ww_mutex to enter the osq queue if we expect a
EDEADLK? It seems to boil down to how likely is the EDEADLK going to
evaporate if we wait for the owner to finish and unlock.

The patch looks reasonable, just a question of desirability.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre