Messages in this thread | | | Date | Tue, 28 Apr 2015 18:52:26 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3] locking/rwsem: reduce spinlock contention in wakeup after up_read/up_write |
| |
On Fri, Apr 24, 2015 at 01:54:29PM -0400, Waiman Long wrote: > +static inline bool rwsem_has_active_writer(struct rw_semaphore *sem) > +{ > + return READ_ONCE(sem->owner) != NULL; > +}
> +static inline bool rwsem_has_spinner(struct rw_semaphore *sem) > +{ > + return osq_is_locked(&sem->osq); > +}
> + /* > + * If a spinner is present, it is not necessary to do the wakeup. > + * Try to do wakeup only if the trylock succeeds to minimize > + * spinlock contention which may introduce too much delay in the > + * unlock operation. > + * > + * spinning writer up_write/up_read caller > + * --------------- ----------------------- > + * [S] osq_unlock() [L] osq > + * MB MB > + * [RmW] rwsem_try_write_lock() [RmW] spin_trylock(wait_lock) > + * > + * Here, it is important to make sure that there won't be a missed > + * wakeup while the rwsem is free and the only spinning writer goes > + * to sleep without taking the rwsem. In case the spinning writer is > + * just going to break out of the waiting loop, it will still do a > + * trylock in rwsem_down_write_failed() before sleeping. IOW, if > + * rwsem_has_spinner() is true, it will guarantee at least one > + * trylock attempt on the rwsem. > + */ > + if (!rwsem_has_spinner(sem)) { > + raw_spin_lock_irqsave(&sem->wait_lock, flags); > + } else { > + /* > + * rwsem_has_spinner() is an atomic read while spin_trylock > + * does not guarantee a full memory barrier. Insert a memory > + * barrier here to make sure that wait_lock isn't read until > + * after osq. > + * Note: smp_rmb__after_atomic() should be used if available. > + */ > + smp_mb__after_atomic();
Sorry, that's wrong. the smp_mb__{before,after}_atomic() are for all atomic (RmW) ops that do not return a value.
They end up as whatever barrier is required to make real atomic (RmW) ops (LOCK on x86, LL/SC on risc etc) ordered. And all atomic (RmW) ops that return a value are already guaranteed to imply full ordering semantics.
Note, the (RmW) part is important here, atomic_{set,read}() are _NOT_ read-modify-write ops.
> + if (!raw_spin_trylock_irqsave(&sem->wait_lock, flags)) > + return sem; > + } >
| |