[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sun, 19 Jan 2014 18:33:22 -0800
From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To: Tim Chen <tim.c.chen@...ux.intel.com>
Cc: Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Will Deacon <will.deacon@....com>,
linux-kernel@...r.kernel.org, linux-mm <linux-mm@...ck.org>,
linux-arch@...r.kernel.org,
Linus Torvalds <torvalds@...ux-foundation.org>,
Waiman Long <waiman.long@...com>,
Andrea Arcangeli <aarcange@...hat.com>,
Alex Shi <alex.shi@...aro.org>,
Andi Kleen <andi@...stfloor.org>,
Michel Lespinasse <walken@...gle.com>,
Davidlohr Bueso <davidlohr.bueso@...com>,
Matthew R Wilcox <matthew.r.wilcox@...el.com>,
Dave Hansen <dave.hansen@...el.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Rik van Riel <riel@...hat.com>,
Peter Hurley <peter@...leysoftware.com>,
Raghavendra K T <raghavendra.kt@...ux.vnet.ibm.com>,
George Spelvin <linux@...izon.com>,
"H. Peter Anvin" <hpa@...or.com>, Arnd Bergmann <arnd@...db.de>,
Aswin Chandramouleeswaran <aswin@...com>,
Scott J Norton <scott.norton@...com>,
"Figo.zhang" <figo1802@...il.com>
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@...com>
> Suggested-by: Michel Lespinasse <walken@...gle.com>
> Signed-off-by: Waiman Long <Waiman.Long@...com>
> Signed-off-by: Jason Low <jason.low2@...com>
> Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
And this fixes my gripes in the first patch in this series, good!
Reviewed-by: Paul E. McKenney <paulmck@...ux.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
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists