On Thu, Jan 16, 2014 at 04:08:28PM -0800, Tim Chen wrote: > This patch corrects the way memory barriers are used in the MCS lock > with smp_load_acquire and smp_store_release fucnction. > It removes ones that are not needed. > > Note that using the smp_load_acquire/smp_store_release pair is not > sufficient to form a full memory barrier across > cpus for many architectures (except x86) for mcs_unlock and mcs_lock. > For applications that absolutely need a full barrier across multiple cpus > with mcs_unlock and mcs_lock pair, smp_mb__after_unlock_lock() should be > used after mcs_lock if a full memory barrier needs to be guaranteed. > > From: Waiman Long <Waiman.Long@xxxxxx> > Suggested-by: Michel Lespinasse <walken@xxxxxxxxxx> > Signed-off-by: Waiman Long <Waiman.Long@xxxxxx> > Signed-off-by: Jason Low <jason.low2@xxxxxx> > Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> And this fixes my gripes in the first patch in this series, good! Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> > --- > kernel/locking/mcs_spinlock.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/kernel/locking/mcs_spinlock.c b/kernel/locking/mcs_spinlock.c > index 44fb092..6cdc730 100644 > --- a/kernel/locking/mcs_spinlock.c > +++ b/kernel/locking/mcs_spinlock.c > @@ -43,9 +43,12 @@ void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node) > return; > } > ACCESS_ONCE(prev->next) = node; > - smp_wmb(); > - /* Wait until the lock holder passes the lock down */ > - while (!ACCESS_ONCE(node->locked)) > + /* > + * Wait until the lock holder passes the lock down. > + * Using smp_load_acquire() provides a memory barrier that > + * ensures subsequent operations happen after the lock is acquired. > + */ > + while (!(smp_load_acquire(&node->locked))) > arch_mutex_cpu_relax(); > } > EXPORT_SYMBOL_GPL(mcs_spin_lock); > @@ -68,7 +71,12 @@ void mcs_spin_unlock(struct mcs_spinlock **lock, struct mcs_spinlock *node) > while (!(next = ACCESS_ONCE(node->next))) > arch_mutex_cpu_relax(); > } > - ACCESS_ONCE(next->locked) = 1; > - smp_wmb(); > + /* > + * Pass lock to next waiter. > + * smp_store_release() provides a memory barrier to ensure > + * all operations in the critical section has been completed > + * before unlocking. > + */ > + smp_store_release(&next->locked, 1); > } > EXPORT_SYMBOL_GPL(mcs_spin_unlock); > -- > 1.7.11.7 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
