Messages in this thread Patch in this message | | | Date | Tue, 23 Aug 2016 23:03:54 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC][PATCH 1/3] locking/mutex: Rework mutex::owner |
| |
On Tue, Aug 23, 2016 at 01:52:46PM -0700, Tim Chen wrote: > On Tue, 2016-08-23 at 15:55 -0400, Waiman Long wrote: > > On 08/23/2016 08:46 AM, Peter Zijlstra wrote: > > > > I have 2 more comments about the code. > > 1) There are a couple of places where you only use 0x3 in mutex.c. They > > should be replaced by the symbolic name instead. > > May be easier to read if (owner & 0x3) and > (owner & ~0x3) are changed to something like > _owner_flag(owner) and _owner_task(owner).
Yes that would work. Something like the below..
Note the ';' in the last "-" line that currently ensures we always take the slow path.
----
--- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -55,7 +55,17 @@ EXPORT_SYMBOL(__mutex_init); #define MUTEX_FLAG_WAITERS 0x01 #define MUTEX_FLAG_HANDOFF 0x02 -#define MUTEX_FLAG_ALL 0x03 +#define MUTEX_FLAGS 0x03 + +static inline struct task_struct *__owner_task(unsigned long owner) +{ + return (struct task_struct *)(owner & ~MUTEX_FLAGS); +} + +static inline unsigned long __owner_flags(unsigned long owner) +{ + return owner & MUTEX_FLAGS; +} /* * Atomically try to take the lock when it is available @@ -65,7 +75,7 @@ static inline bool __mutex_trylock(struc unsigned long owner, new_owner; owner = atomic_long_read(&lock->owner); - if (owner & ~0x03) + if (__owner_task(owner)) return false; new_owner = owner | (unsigned long)current; @@ -466,14 +476,14 @@ void __sched mutex_unlock(struct mutex * if (owner & MUTEX_FLAG_HANDOFF) break; - old = atomic_long_cmpxchg_release(&lock->owner, owner, owner & 0x03); + old = atomic_long_cmpxchg_release(&lock->owner, owner, __owner_flags(owner)); if (old == owner) break; owner = old; } - if (owner & 0x03); + if (__owner_flags(owner)) __mutex_unlock_slowpath(lock, owner); } EXPORT_SYMBOL(mutex_unlock);
| |