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

[Waiman Long]

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@xxxxxxxxxxxxx>

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