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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Dec 2016 14:36:00 +0000
From:   Chris Wilson <chris@...is-wilson.co.uk>
To:     Nicolai Hähnle <nhaehnle@...il.com>
Cc:     linux-kernel@...r.kernel.org,
        Nicolai Hähnle <Nicolai.Haehnle@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Maarten Lankhorst <dev@...ankhorst.nl>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner
 optimistic spin loop

On Thu, Dec 01, 2016 at 03:06:45PM +0100, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <Nicolai.Haehnle@....com>
> 
> 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