On Sat, Apr 27, 2013 at 02:15:52PM -0700, Davidlohr Bueso wrote: > From: Davidlohr Bueso <davidlohr.bu...@hp.com> > Subject: [PATCH] rwsem: check counter to avoid cmpxchg calls > > This patch tries to reduce the amount of cmpxchg calls in the > writer failed path by checking the counter value first before > issuing the instruction. If ->count is not set to RWSEM_WAITING_BIAS > then there is no point wasting a cmpxchg call. > > Signed-off-by: Davidlohr Bueso <davidlohr.bu...@hp.com> > --- > lib/rwsem.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/lib/rwsem.c b/lib/rwsem.c > index 50fdd89..a79dc95 100644 > --- a/lib/rwsem.c > +++ b/lib/rwsem.c > @@ -222,9 +222,10 @@ struct rw_semaphore __sched > *rwsem_down_write_failed(struct rw_semaphore *sem) > count = RWSEM_ACTIVE_WRITE_BIAS; > if (!list_is_singular(&sem->wait_list)) > count += RWSEM_WAITING_BIAS; > - if (cmpxchg(&sem->count, RWSEM_WAITING_BIAS, count) == > - RWSEM_WAITING_BIAS) > - break; > + if ((*(volatile long *)&(sem)->count) == > RWSEM_WAITING_BIAS) > + if (cmpxchg(&sem->count, RWSEM_WAITING_BIAS, > count) == > + RWSEM_WAITING_BIAS) > + break; > } > > raw_spin_unlock_irq(&sem->wait_lock);
Quite impressed that this makes as much of a difference - this code block already checks that the active count was 0 (which implies the sem->count must be RWSEM_WAITING_BIAS, since the writer thread is known to be waiting) not long before. But I suppose it helps due to the case where someone else steals the lock while we're trying to acquire sem->wait_lock. Regarding style: I would prefer if the sem->count read didn't use the volatile cast. The compiler will already be forced to reload the value due to the barriers in set_task_state() and raw_spin_lock_irq(). And if you absolutely need to have that volatile, I would prefer you use ACCESS_ONCE() rather than the hand written equivalent. I'm going to try pushing this to Linus tomorrow; I feel I shouldn't change your patch without putting it through tests again; are you OK with sending this as a followup to the series rather than me including it ? -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
