Messages in this thread | | | Date | Fri, 18 Sep 2015 08:56:07 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH] kernel: fix data race in put_pid |
| |
On Fri, Sep 18, 2015 at 10:57:32AM +0200, Peter Zijlstra wrote: > On Fri, Sep 18, 2015 at 10:51:56AM +0200, Peter Zijlstra wrote: > > That would upgrade the atomic_read() path to a full READ_ONCE_CTRL(), > > and thereby avoid any of the kmem_cache_free() stores from leaking out. > > That said, on my TODO list is an item to review all atomic_{read,set}() > implementation to ensure they're (at least) {READ,WRITE}_ONCE(). > > A quick scan left me with this. There were indeed a few archs lacking > READ_ONCE (or ACCESS_ONCE) for atomic_read() and hardly anybody used > WRITE_ONCE() (or ACCESS_ONCE casting) for atomic_set().
You beat me to it! ;-)
When you get a signed-off version, please feel free to add my Acked-by.
Thanx, Paul
> --- > arch/alpha/include/asm/atomic.h | 8 ++++---- > arch/arc/include/asm/atomic.h | 8 ++++---- > arch/arm/include/asm/atomic.h | 4 ++-- > arch/arm64/include/asm/atomic.h | 2 +- > arch/avr32/include/asm/atomic.h | 4 ++-- > arch/frv/include/asm/atomic.h | 4 ++-- > arch/h8300/include/asm/atomic.h | 4 ++-- > arch/hexagon/include/asm/atomic.h | 2 +- > arch/ia64/include/asm/atomic.h | 8 ++++---- > arch/m32r/include/asm/atomic.h | 4 ++-- > arch/m68k/include/asm/atomic.h | 4 ++-- > arch/metag/include/asm/atomic_lnkget.h | 3 ++- > arch/metag/include/asm/atomic_lock1.h | 2 +- > arch/mips/include/asm/atomic.h | 8 ++++---- > arch/mn10300/include/asm/atomic.h | 4 ++-- > arch/parisc/include/asm/atomic.h | 2 +- > arch/sh/include/asm/atomic.h | 4 ++-- > arch/sparc/include/asm/atomic_64.h | 8 ++++---- > arch/tile/include/asm/atomic.h | 2 +- > arch/tile/include/asm/atomic_64.h | 6 +++--- > arch/x86/include/asm/atomic.h | 4 ++-- > arch/x86/include/asm/atomic64_64.h | 4 ++-- > arch/xtensa/include/asm/atomic.h | 4 ++-- > include/asm-generic/atomic.h | 4 ++-- > 24 files changed, 54 insertions(+), 53 deletions(-) > > diff --git a/arch/alpha/include/asm/atomic.h b/arch/alpha/include/asm/atomic.h > index e8c956098424..572b228c44c7 100644 > --- a/arch/alpha/include/asm/atomic.h > +++ b/arch/alpha/include/asm/atomic.h > @@ -17,11 +17,11 @@ > #define ATOMIC_INIT(i) { (i) } > #define ATOMIC64_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic64_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic64_read(v) READ_ONCE((v)->counter) > > -#define atomic_set(v,i) ((v)->counter = (i)) > -#define atomic64_set(v,i) ((v)->counter = (i)) > +#define atomic_set(v,i) WRITE_ONCE((v)->counter, (i)) > +#define atomic64_set(v,i) WRITE_ONCE((v)->counter, (i)) > > /* > * To get proper branch prediction for the main line, we must branch > diff --git a/arch/arc/include/asm/atomic.h b/arch/arc/include/asm/atomic.h > index c3ecda023e3a..7730d302cadb 100644 > --- a/arch/arc/include/asm/atomic.h > +++ b/arch/arc/include/asm/atomic.h > @@ -17,11 +17,11 @@ > #include <asm/barrier.h> > #include <asm/smp.h> > > -#define atomic_read(v) ((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > > #ifdef CONFIG_ARC_HAS_LLSC > > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #ifdef CONFIG_ARC_STAR_9000923308 > > @@ -107,7 +107,7 @@ static inline int atomic_##op##_return(int i, atomic_t *v) \ > #ifndef CONFIG_SMP > > /* violating atomic_xxx API locking protocol in UP for optimization sake */ > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #else > > @@ -125,7 +125,7 @@ static inline void atomic_set(atomic_t *v, int i) > unsigned long flags; > > atomic_ops_lock(flags); > - v->counter = i; > + WRITE_ONCE(v->counter, i); > atomic_ops_unlock(flags); > } > > diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h > index fe3ef397f5a4..2bf80afb7841 100644 > --- a/arch/arm/include/asm/atomic.h > +++ b/arch/arm/include/asm/atomic.h > @@ -27,8 +27,8 @@ > * strex/ldrex monitor on some implementations. The reason we can use it for > * atomic_set() is the clrex or dummy strex done on every exception return. > */ > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v,i) (((v)->counter) = (i)) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v,i) WRITE_ONCE(((v)->counter), (i)) > > #if __LINUX_ARM_ARCH__ >= 6 > > diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h > index 35a67783cfa0..1e247ac2601a 100644 > --- a/arch/arm64/include/asm/atomic.h > +++ b/arch/arm64/include/asm/atomic.h > @@ -54,7 +54,7 @@ > #define ATOMIC_INIT(i) { (i) } > > #define atomic_read(v) READ_ONCE((v)->counter) > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > #define atomic_xchg(v, new) xchg(&((v)->counter), (new)) > #define atomic_cmpxchg(v, old, new) cmpxchg(&((v)->counter), (old), (new)) > > diff --git a/arch/avr32/include/asm/atomic.h b/arch/avr32/include/asm/atomic.h > index 97c9bdf83409..d74fd8ce980a 100644 > --- a/arch/avr32/include/asm/atomic.h > +++ b/arch/avr32/include/asm/atomic.h > @@ -19,8 +19,8 @@ > > #define ATOMIC_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v, i) (((v)->counter) = i) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #define ATOMIC_OP_RETURN(op, asm_op, asm_con) \ > static inline int __atomic_##op##_return(int i, atomic_t *v) \ > diff --git a/arch/frv/include/asm/atomic.h b/arch/frv/include/asm/atomic.h > index 0da689def4cc..64f02d451aa8 100644 > --- a/arch/frv/include/asm/atomic.h > +++ b/arch/frv/include/asm/atomic.h > @@ -32,8 +32,8 @@ > */ > > #define ATOMIC_INIT(i) { (i) } > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > static inline int atomic_inc_return(atomic_t *v) > { > diff --git a/arch/h8300/include/asm/atomic.h b/arch/h8300/include/asm/atomic.h > index 702ee539f87d..4435a445ae7e 100644 > --- a/arch/h8300/include/asm/atomic.h > +++ b/arch/h8300/include/asm/atomic.h > @@ -11,8 +11,8 @@ > > #define ATOMIC_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v, i) (((v)->counter) = i) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #include <linux/kernel.h> > > diff --git a/arch/hexagon/include/asm/atomic.h b/arch/hexagon/include/asm/atomic.h > index 811d61f6422d..55696c4100d4 100644 > --- a/arch/hexagon/include/asm/atomic.h > +++ b/arch/hexagon/include/asm/atomic.h > @@ -48,7 +48,7 @@ static inline void atomic_set(atomic_t *v, int new) > * > * Assumes all word reads on our architecture are atomic. > */ > -#define atomic_read(v) ((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > > /** > * atomic_xchg - atomic > diff --git a/arch/ia64/include/asm/atomic.h b/arch/ia64/include/asm/atomic.h > index be4beeb77d57..8dfb5f6f6c35 100644 > --- a/arch/ia64/include/asm/atomic.h > +++ b/arch/ia64/include/asm/atomic.h > @@ -21,11 +21,11 @@ > #define ATOMIC_INIT(i) { (i) } > #define ATOMIC64_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic64_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic64_read(v) READ_ONCE((v)->counter) > > -#define atomic_set(v,i) (((v)->counter) = (i)) > -#define atomic64_set(v,i) (((v)->counter) = (i)) > +#define atomic_set(v,i) WRITE_ONCE(((v)->counter), (i)) > +#define atomic64_set(v,i) WRITE_ONCE(((v)->counter), (i)) > > #define ATOMIC_OP(op, c_op) \ > static __inline__ int \ > diff --git a/arch/m32r/include/asm/atomic.h b/arch/m32r/include/asm/atomic.h > index 025e2a170493..ea35160d632b 100644 > --- a/arch/m32r/include/asm/atomic.h > +++ b/arch/m32r/include/asm/atomic.h > @@ -28,7 +28,7 @@ > * > * Atomically reads the value of @v. > */ > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > > /** > * atomic_set - set atomic variable > @@ -37,7 +37,7 @@ > * > * Atomically sets the value of @v to @i. > */ > -#define atomic_set(v,i) (((v)->counter) = (i)) > +#define atomic_set(v,i) WRITE_ONCE(((v)->counter), (i)) > > #ifdef CONFIG_CHIP_M32700_TS1 > #define __ATOMIC_CLOBBER , "r4" > diff --git a/arch/m68k/include/asm/atomic.h b/arch/m68k/include/asm/atomic.h > index 039fac120cc0..4858178260f9 100644 > --- a/arch/m68k/include/asm/atomic.h > +++ b/arch/m68k/include/asm/atomic.h > @@ -17,8 +17,8 @@ > > #define ATOMIC_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v, i) (((v)->counter) = i) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > /* > * The ColdFire parts cannot do some immediate to memory operations, > diff --git a/arch/metag/include/asm/atomic_lnkget.h b/arch/metag/include/asm/atomic_lnkget.h > index 21c4c268b86c..1bd21c933435 100644 > --- a/arch/metag/include/asm/atomic_lnkget.h > +++ b/arch/metag/include/asm/atomic_lnkget.h > @@ -3,7 +3,8 @@ > > #define ATOMIC_INIT(i) { (i) } > > -#define atomic_set(v, i) ((v)->counter = (i)) > +/* XXX: should be LNKSETD ? */ > +#define atomic_set(v, i) WRITE_ONCE((v)->counter, (i)) > > #include <linux/compiler.h> > > diff --git a/arch/metag/include/asm/atomic_lock1.h b/arch/metag/include/asm/atomic_lock1.h > index f8efe380fe8b..0295d9b8d5bf 100644 > --- a/arch/metag/include/asm/atomic_lock1.h > +++ b/arch/metag/include/asm/atomic_lock1.h > @@ -10,7 +10,7 @@ > > static inline int atomic_read(const atomic_t *v) > { > - return (v)->counter; > + return READ_ONCE((v)->counter); > } > > /* > diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h > index 4c42fd9af777..f82d3af07931 100644 > --- a/arch/mips/include/asm/atomic.h > +++ b/arch/mips/include/asm/atomic.h > @@ -30,7 +30,7 @@ > * > * Atomically reads the value of @v. > */ > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > > /* > * atomic_set - set atomic variable > @@ -39,7 +39,7 @@ > * > * Atomically sets the value of @v to @i. > */ > -#define atomic_set(v, i) ((v)->counter = (i)) > +#define atomic_set(v, i) WRITE_ONCE((v)->counter, (i)) > > #define ATOMIC_OP(op, c_op, asm_op) \ > static __inline__ void atomic_##op(int i, atomic_t * v) \ > @@ -315,14 +315,14 @@ static __inline__ int __atomic_add_unless(atomic_t *v, int a, int u) > * @v: pointer of type atomic64_t > * > */ > -#define atomic64_read(v) ACCESS_ONCE((v)->counter) > +#define atomic64_read(v) READ_ONCE((v)->counter) > > /* > * atomic64_set - set atomic variable > * @v: pointer of type atomic64_t > * @i: required value > */ > -#define atomic64_set(v, i) ((v)->counter = (i)) > +#define atomic64_set(v, i) WRITE_ONCE((v)->counter, (i)) > > #define ATOMIC64_OP(op, c_op, asm_op) \ > static __inline__ void atomic64_##op(long i, atomic64_t * v) \ > diff --git a/arch/mn10300/include/asm/atomic.h b/arch/mn10300/include/asm/atomic.h > index 375e59140c9c..ce318d5ab23b 100644 > --- a/arch/mn10300/include/asm/atomic.h > +++ b/arch/mn10300/include/asm/atomic.h > @@ -34,7 +34,7 @@ > * > * Atomically reads the value of @v. Note that the guaranteed > */ > -#define atomic_read(v) (ACCESS_ONCE((v)->counter)) > +#define atomic_read(v) READ_ONCE((v)->counter) > > /** > * atomic_set - set atomic variable > @@ -43,7 +43,7 @@ > * > * Atomically sets the value of @v to @i. Note that the guaranteed > */ > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #define ATOMIC_OP(op) \ > static inline void atomic_##op(int i, atomic_t *v) \ > diff --git a/arch/parisc/include/asm/atomic.h b/arch/parisc/include/asm/atomic.h > index 2536965d00ea..1d109990a022 100644 > --- a/arch/parisc/include/asm/atomic.h > +++ b/arch/parisc/include/asm/atomic.h > @@ -67,7 +67,7 @@ static __inline__ void atomic_set(atomic_t *v, int i) > > static __inline__ int atomic_read(const atomic_t *v) > { > - return ACCESS_ONCE((v)->counter); > + return READ_ONCE((v)->counter); > } > > /* exported interface */ > diff --git a/arch/sh/include/asm/atomic.h b/arch/sh/include/asm/atomic.h > index 05b9f74ce2d5..c399e1c55685 100644 > --- a/arch/sh/include/asm/atomic.h > +++ b/arch/sh/include/asm/atomic.h > @@ -14,8 +14,8 @@ > > #define ATOMIC_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic_set(v,i) ((v)->counter = (i)) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic_set(v,i) WRITE_ONCE((v)->counter, (i)) > > #if defined(CONFIG_GUSA_RB) > #include <asm/atomic-grb.h> > diff --git a/arch/sparc/include/asm/atomic_64.h b/arch/sparc/include/asm/atomic_64.h > index 917084ace49d..f2fbf9e16faf 100644 > --- a/arch/sparc/include/asm/atomic_64.h > +++ b/arch/sparc/include/asm/atomic_64.h > @@ -14,11 +14,11 @@ > #define ATOMIC_INIT(i) { (i) } > #define ATOMIC64_INIT(i) { (i) } > > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > -#define atomic64_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > +#define atomic64_read(v) READ_ONCE((v)->counter) > > -#define atomic_set(v, i) (((v)->counter) = i) > -#define atomic64_set(v, i) (((v)->counter) = i) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > +#define atomic64_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #define ATOMIC_OP(op) \ > void atomic_##op(int, atomic_t *); \ > diff --git a/arch/tile/include/asm/atomic.h b/arch/tile/include/asm/atomic.h > index 709798460763..9fc0107a9c5e 100644 > --- a/arch/tile/include/asm/atomic.h > +++ b/arch/tile/include/asm/atomic.h > @@ -34,7 +34,7 @@ > */ > static inline int atomic_read(const atomic_t *v) > { > - return ACCESS_ONCE(v->counter); > + return READ_ONCE(v->counter); > } > > /** > diff --git a/arch/tile/include/asm/atomic_64.h b/arch/tile/include/asm/atomic_64.h > index 096a56d6ead4..51cabc26e387 100644 > --- a/arch/tile/include/asm/atomic_64.h > +++ b/arch/tile/include/asm/atomic_64.h > @@ -24,7 +24,7 @@ > > /* First, the 32-bit atomic ops that are "real" on our 64-bit platform. */ > > -#define atomic_set(v, i) ((v)->counter = (i)) > +#define atomic_set(v, i) WRITE_ONCE((v)->counter, (i)) > > /* > * The smp_mb() operations throughout are to support the fact that > @@ -82,8 +82,8 @@ static inline void atomic_xor(int i, atomic_t *v) > > #define ATOMIC64_INIT(i) { (i) } > > -#define atomic64_read(v) ((v)->counter) > -#define atomic64_set(v, i) ((v)->counter = (i)) > +#define atomic64_read(v) READ_ONCE((v)->counter) > +#define atomic64_set(v, i) WRITE_ONCE((v)->counter, (i)) > > static inline void atomic64_add(long i, atomic64_t *v) > { > diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h > index fb52aa644aab..ae5fb83e6d91 100644 > --- a/arch/x86/include/asm/atomic.h > +++ b/arch/x86/include/asm/atomic.h > @@ -24,7 +24,7 @@ > */ > static __always_inline int atomic_read(const atomic_t *v) > { > - return ACCESS_ONCE((v)->counter); > + return READ_ONCE((v)->counter); > } > > /** > @@ -36,7 +36,7 @@ static __always_inline int atomic_read(const atomic_t *v) > */ > static __always_inline void atomic_set(atomic_t *v, int i) > { > - v->counter = i; > + WRITE_ONCE(v->counter, i); > } > > /** > diff --git a/arch/x86/include/asm/atomic64_64.h b/arch/x86/include/asm/atomic64_64.h > index 50e33eff58de..037351022f54 100644 > --- a/arch/x86/include/asm/atomic64_64.h > +++ b/arch/x86/include/asm/atomic64_64.h > @@ -18,7 +18,7 @@ > */ > static inline long atomic64_read(const atomic64_t *v) > { > - return ACCESS_ONCE((v)->counter); > + return READ_ONCE((v)->counter); > } > > /** > @@ -30,7 +30,7 @@ static inline long atomic64_read(const atomic64_t *v) > */ > static inline void atomic64_set(atomic64_t *v, long i) > { > - v->counter = i; > + WRITE_ONCE(v->counter, i); > } > > /** > diff --git a/arch/xtensa/include/asm/atomic.h b/arch/xtensa/include/asm/atomic.h > index 93795d047303..fd8017ce298a 100644 > --- a/arch/xtensa/include/asm/atomic.h > +++ b/arch/xtensa/include/asm/atomic.h > @@ -47,7 +47,7 @@ > * > * Atomically reads the value of @v. > */ > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > > /** > * atomic_set - set atomic variable > @@ -56,7 +56,7 @@ > * > * Atomically sets the value of @v to @i. > */ > -#define atomic_set(v,i) ((v)->counter = (i)) > +#define atomic_set(v,i) WRITE_ONCE((v)->counter, (i)) > > #if XCHAL_HAVE_S32C1I > #define ATOMIC_OP(op) \ > diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h > index d4d7e337fdcb..74f1a3704d7a 100644 > --- a/include/asm-generic/atomic.h > +++ b/include/asm-generic/atomic.h > @@ -127,7 +127,7 @@ ATOMIC_OP(xor, ^) > * Atomically reads the value of @v. > */ > #ifndef atomic_read > -#define atomic_read(v) ACCESS_ONCE((v)->counter) > +#define atomic_read(v) READ_ONCE((v)->counter) > #endif > > /** > @@ -137,7 +137,7 @@ ATOMIC_OP(xor, ^) > * > * Atomically sets the value of @v to @i. > */ > -#define atomic_set(v, i) (((v)->counter) = (i)) > +#define atomic_set(v, i) WRITE_ONCE(((v)->counter), (i)) > > #include <linux/irqflags.h> > >
| |