linux-kernel - Re: [PATCH -v4 2/8] locking/mutex: Rework mutex::owner

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20161013151848.GC13138@arm.com>
Date:   Thu, 13 Oct 2016 16:18:48 +0100
From:   Will Deacon <will.deacon@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Waiman Long <waiman.long@....com>,
        Jason Low <jason.low2@....com>,
        Ding Tianhong <dingtianhong@...wei.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Imre Deak <imre.deak@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Terry Rudd <terry.rudd@....com>,
        "Paul E. McKenney" <paulmck@...ibm.com>,
        Jason Low <jason.low2@...com>,
        Chris Wilson <chris@...is-wilson.co.uk>,
        Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [PATCH -v4 2/8] locking/mutex: Rework mutex::owner

On Fri, Oct 07, 2016 at 04:52:45PM +0200, Peter Zijlstra wrote:
> The current mutex implementation has an atomic lock word and a
> non-atomic owner field.
> 
> This disparity leads to a number of issues with the current mutex code
> as it means that we can have a locked mutex without an explicit owner
> (because the owner field has not been set, or already cleared).
> 
> This leads to a number of weird corner cases, esp. between the
> optimistic spinning and debug code. Where the optimistic spinning
> code needs the owner field updated inside the lock region, the debug
> code is more relaxed because the whole lock is serialized by the
> wait_lock.
> 
> Also, the spinning code itself has a few corner cases where we need to
> deal with a held lock without an owner field.
> 
> Furthermore, it becomes even more of a problem when trying to fix
> starvation cases in the current code. We end up stacking special case
> on special case.
> 
> To solve this rework the basic mutex implementation to be a single
> atomic word that contains the owner and uses the low bits for extra
> state.
> 
> This matches how PI futexes and rt_mutex already work. By having the
> owner an integral part of the lock state a lot of the problems
> dissapear and we get a better option to deal with starvation cases,
> direct owner handoff.
> 
> Changing the basic mutex does however invalidate all the arch specific
> mutex code; this patch leaves that unused in-place, a later patch will
> remove that.
> 
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  include/linux/mutex-debug.h  |   24 --
>  include/linux/mutex.h        |   46 +++--
>  kernel/locking/mutex-debug.c |   13 -
>  kernel/locking/mutex-debug.h |   10 -
>  kernel/locking/mutex.c       |  371 ++++++++++++++++++-------------------------
>  kernel/locking/mutex.h       |   26 ---
>  kernel/sched/core.c          |    2 
>  7 files changed, 187 insertions(+), 305 deletions(-)

Looks good to me:

Reviewed-by: Will Deacon <will.deacon@....com>

Will