linux-kernel - Re: [RFC][PATCH -v3 10/10] locking/mutex: Implement alternative HANDOFF

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:   Fri, 9 Sep 2016 18:32:04 -0400
From:   Waiman Long <waiman.long@....com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Jason Low <jason.low2@....com>,
        Ding Tianhong <dingtianhong@...wei.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <Will.Deacon@....com>,
        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>
Subject: Re: [RFC][PATCH -v3 10/10] locking/mutex: Implement alternative HANDOFF

On 09/05/2016 08:36 AM, Peter Zijlstra wrote:
> As mentioned in a previous patch, its possible to implement the
> handoff logic differently, avoiding the issue where we 'leak' a HADOFF
> flag.
>
> This patch does so, just to show what it looks like; I'm not at all
> convinced this is worth it.
>
> Signed-off-by: Peter Zijlstra (Intel)<peterz@...radead.org>
>

I have another way of catching the uncleared handoff flag. See the 
following code to see if you think that will work.

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 9492494..362ff83 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -85,7 +85,13 @@ static inline bool __mutex_trylock(struct mutex 
*lock, const

         owner = atomic_long_read(&lock->owner);
         for (;;) { /* must loop, can race against a flag */
-               unsigned long old;
+               unsigned long old, flags = __owner_flags(owner);
+
+               /*
+                * We don't need to keep the HANDOFF flag for the waiter.
+                */
+               if (handoff)
+                       flags &= ~MUTEX_FLAG_HANDOFF;

                 if (__owner_task(owner)) {
                         if (handoff && unlikely(__owner_task(owner) == 
current))
@@ -107,7 +113,7 @@ static inline bool __mutex_trylock(struct mutex 
*lock, const
                 }

                 old = atomic_long_cmpxchg_acquire(&lock->owner, owner,
-                                                 curr | 
__owner_flags(owner));
+                                                 curr | flags);
                 if (old == owner)
                         return true;

@@ -688,7 +694,7 @@ __mutex_lock_common(struct mutex *lock, long state, 
unsigned
                  * state back to RUNNING and fall through the next 
schedule(),
                  * or we must see its unlock and acquire.
                  */
-               if (__mutex_trylock(lock, true))
+               if (__mutex_trylock(lock, first))
                         break;

                 spin_lock_mutex(&lock->wait_lock, flags);
@@ -700,8 +706,6 @@ remove_waiter:
         mutex_remove_waiter(lock, &waiter, task);
         if (likely(list_empty(&lock->wait_list)))
                 __mutex_clear_flag(lock, MUTEX_FLAGS);
-       else if (first && (atomic_long_read(&lock->owner) & 
MUTEX_FLAG_HANDOFF))
-               __mutex_clear_flag(lock, MUTEX_FLAG_HANDOFF);

         debug_mutex_free_waiter(&waiter);


Cheers,
Longman