Messages in this thread | | | Date | Sun, 19 Jan 2014 18:33:22 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v7 4/6] MCS Lock: Barrier corrections |
| |
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@hp.com> > Suggested-by: Michel Lespinasse <walken@google.com> > Signed-off-by: Waiman Long <Waiman.Long@hp.com> > Signed-off-by: Jason Low <jason.low2@hp.com> > Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
And this fixes my gripes in the first patch in this series, good!
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> --- > 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 > > >
| |