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

[Waiman Long <waiman.long@hpe.com>]

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@infradead.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