Messages in this thread | | | Date | Mon, 11 May 2015 12:50:29 -0400 | From | Waiman Long <> | Subject | Re: [tip:locking/core] locking/pvqspinlock: Replace xchg() by the more descriptive set_mb() |
| |
On 05/11/2015 10:54 AM, Peter Zijlstra wrote: > On Mon, May 11, 2015 at 05:43:55AM -0700, tip-bot for Waiman Long wrote: >> - (void)xchg(&pn->state, vcpu_halted); >> + set_mb(pn->state, vcpu_halted); > Hmm, so I looked at the set_mb() definitions and I figure we want to do > something like the below, right?
Yes, I think we should do that just to be safe from unexpected compiler optimization.
> --- > arch/arm/include/asm/barrier.h | 2 +- > arch/arm64/include/asm/barrier.h | 2 +- > arch/ia64/include/asm/barrier.h | 2 +- > arch/metag/include/asm/barrier.h | 2 +- > arch/mips/include/asm/barrier.h | 2 +- > arch/powerpc/include/asm/barrier.h | 2 +- > arch/s390/include/asm/barrier.h | 2 +- > arch/sparc/include/asm/barrier_64.h | 2 +- > arch/x86/include/asm/barrier.h | 2 +- > include/asm-generic/barrier.h | 2 +- > 10 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h > index d2f81e6b8c1c..993150aea681 100644 > --- a/arch/arm/include/asm/barrier.h > +++ b/arch/arm/include/asm/barrier.h > @@ -81,7 +81,7 @@ do { \ > #define read_barrier_depends() do { } while(0) > #define smp_read_barrier_depends() do { } while(0) > > -#define set_mb(var, value) do { var = value; smp_mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); smp_mb(); } while (0) > > #define smp_mb__before_atomic() smp_mb() > #define smp_mb__after_atomic() smp_mb() > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h > index 71f19c4dc0de..ff7de78d01b8 100644 > --- a/arch/arm64/include/asm/barrier.h > +++ b/arch/arm64/include/asm/barrier.h > @@ -114,7 +114,7 @@ do { \ > #define read_barrier_depends() do { } while(0) > #define smp_read_barrier_depends() do { } while(0) > > -#define set_mb(var, value) do { var = value; smp_mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); smp_mb(); } while (0) > #define nop() asm volatile("nop"); > > #define smp_mb__before_atomic() smp_mb() > diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h > index f6769eb2bbf9..03117e7b2ab8 100644 > --- a/arch/ia64/include/asm/barrier.h > +++ b/arch/ia64/include/asm/barrier.h > @@ -82,7 +82,7 @@ do { \ > * acquire vs release semantics but we can't discuss this stuff with > * Linus just yet. Grrr... > */ > -#define set_mb(var, value) do { (var) = (value); mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); mb(); } while (0) > > /* > * The group barrier in front of the rsm& ssm are necessary to ensure > diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h > index d703d8e26a65..97eb018a2933 100644 > --- a/arch/metag/include/asm/barrier.h > +++ b/arch/metag/include/asm/barrier.h > @@ -84,7 +84,7 @@ static inline void fence(void) > #define read_barrier_depends() do { } while (0) > #define smp_read_barrier_depends() do { } while (0) > > -#define set_mb(var, value) do { var = value; smp_mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); smp_mb(); } while (0) > > #define smp_store_release(p, v) \ > do { \ > diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h > index 2b8bbbcb9be0..cff1bbdaa74a 100644 > --- a/arch/mips/include/asm/barrier.h > +++ b/arch/mips/include/asm/barrier.h > @@ -113,7 +113,7 @@ > #endif > > #define set_mb(var, value) \ > - do { var = value; smp_mb(); } while (0) > + do { WRITE_ONCE(var, value); smp_mb(); } while (0) > > #define smp_llsc_mb() __asm__ __volatile__(__WEAK_LLSC_MB : : :"memory") > > diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h > index a3bf5be111ff..2a072e48780d 100644 > --- a/arch/powerpc/include/asm/barrier.h > +++ b/arch/powerpc/include/asm/barrier.h > @@ -34,7 +34,7 @@ > #define rmb() __asm__ __volatile__ ("sync" : : : "memory") > #define wmb() __asm__ __volatile__ ("sync" : : : "memory") > > -#define set_mb(var, value) do { var = value; mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); mb(); } while (0) > > #ifdef __SUBARCH_HAS_LWSYNC > # define SMPWMB LWSYNC > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > index 8d724718ec21..b66cd53d35fc 100644 > --- a/arch/s390/include/asm/barrier.h > +++ b/arch/s390/include/asm/barrier.h > @@ -36,7 +36,7 @@ > #define smp_mb__before_atomic() smp_mb() > #define smp_mb__after_atomic() smp_mb() > > -#define set_mb(var, value) do { var = value; mb(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); mb(); } while (0) > > #define smp_store_release(p, v) \ > do { \ > diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h > index 76648941fea7..125fec7512f4 100644 > --- a/arch/sparc/include/asm/barrier_64.h > +++ b/arch/sparc/include/asm/barrier_64.h > @@ -41,7 +41,7 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \ > #define dma_wmb() wmb() > > #define set_mb(__var, __value) \ > - do { __var = __value; membar_safe("#StoreLoad"); } while(0) > + do { WRITE_ONCE(__var, __value); membar_safe("#StoreLoad"); } while(0) > > #ifdef CONFIG_SMP > #define smp_mb() mb() > diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h > index 959e45b81fe2..9de5cde133a1 100644 > --- a/arch/x86/include/asm/barrier.h > +++ b/arch/x86/include/asm/barrier.h > @@ -40,7 +40,7 @@ > #define smp_mb() barrier() > #define smp_rmb() barrier() > #define smp_wmb() barrier() > -#define set_mb(var, value) do { var = value; barrier(); } while (0) > +#define set_mb(var, value) do { WRITE_ONCE(var, value); barrier(); } while (0)
That part is in the !CONFIG_SMP portion. I don't think we need to change it.
Cheers, Longman
| |