On Wed, Apr 17, 2019 at 01:34:01PM -0400, Waiman Long wrote:
> On 04/17/2019 09:56 AM, Peter Zijlstra wrote:
> > On Sat, Apr 13, 2019 at 01:22:54PM -0400, Waiman Long wrote:
> >> @@ -549,7 +582,7 @@ static noinline enum owner_state
> >> rwsem_spin_on_owner(struct rw_semaphore *sem)
> >> return !owner ? OWNER_NULL : OWNER_READER;
> >> }
> >>
> >> -static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
> >> +static bool rwsem_optimistic_spin(struct rw_semaphore *sem, bool wlock)
> >> {
> >> bool taken = false;
> >> bool is_rt_task = rt_task(current);
> >> @@ -558,9 +591,6 @@ static bool rwsem_optimistic_spin(struct rw_semaphore
> >> *sem)
> >> preempt_disable();
> >>
> >> /* sem->wait_lock should not be held when doing optimistic spinning */
> >> - if (!rwsem_can_spin_on_owner(sem))
> >> - goto done;
> >> -
> >> if (!osq_lock(&sem->osq))
> >> goto done;
> >>
> >> @@ -580,10 +610,11 @@ static bool rwsem_optimistic_spin(struct
> >> rw_semaphore *sem)
> >> /*
> >> * Try to acquire the lock
> >> */
> >> - if (rwsem_try_write_lock_unqueued(sem)) {
> >> - taken = true;
> >> + taken = wlock ? rwsem_try_write_lock_unqueued(sem)
> >> + : rwsem_try_read_lock_unqueued(sem);
> >> +
> >> + if (taken)
> >> break;
> >> - }
> >>
> >> /*
> >> * An RT task cannot do optimistic spinning if it cannot
> > Alternatively you pass the trylock function as an argument:
> >
> > static bool rwsem_optimistic_spin(struct rw_semaphore *sem,
> > bool (*trylock)(struct rw_semaphore *sem))
> > {
> > ...
> > if (trylock(sem)) {
> > taken = true;
> > goto unlock;
> > }
> > ...
> > }
> >
> With retpoline, an indirect function call will be slower.
With compiler optimization we can avoid that. Just mark the function as
__always_inline, there's only two call-sites, each with a different
trylock.
It might have already done that anyway, and used constant propagation
on your bool, but the function pointer one is far easier to read.
