| Message ID | 20210426214113.27372-2-gpiccoli@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [B:linux,B:linux-aws,1/2] locking/barriers: Introduce smp_cond_load_relaxed() and atomic_cond_read_relaxed() | expand |
On 26.04.21 23:41, Guilherme G. Piccoli wrote: > From: Will Deacon <will.deacon@arm.com> > > BugLink: https://bugs.launchpad.net/bugs/1926184 > > Whilst we currently provide smp_cond_load_acquire() and > atomic_cond_read_acquire(), there are cases where the ACQUIRE semantics are > not required because of a subsequent fence or release operation once the > conditional loop has exited. > > This patch adds relaxed versions of the conditional spinning primitives > to avoid unnecessary barrier overhead on architectures such as arm64. > > Signed-off-by: Will Deacon <will.deacon@arm.com> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Acked-by: Waiman Long <longman@redhat.com> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: boqun.feng@gmail.com > Cc: linux-arm-kernel@lists.infradead.org > Cc: paulmck@linux.vnet.ibm.com > Link: http://lkml.kernel.org/r/1524738868-31318-2-git-send-email-will.deacon@arm.com > Signed-off-by: Ingo Molnar <mingo@kernel.org> > (cherry picked from commit fcfdfe30e324725007e9dc5814b62a4c430ea909) > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> > --- Target is bionic:linux AND bionic:linux-aws. But bionic:linux-aws is a derivative of bionic:linux. Why mention aws individually? -Stefan > include/asm-generic/atomic-long.h | 2 ++ > include/asm-generic/barrier.h | 27 +++++++++++++++++++++------ > include/linux/atomic.h | 2 ++ > 3 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h > index 34a028a7bcc5..5b2b0b5ea06d 100644 > --- a/include/asm-generic/atomic-long.h > +++ b/include/asm-generic/atomic-long.h > @@ -244,6 +244,8 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) > #define atomic_long_inc_not_zero(l) \ > ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) > > +#define atomic_long_cond_read_relaxed(v, c) \ > + ATOMIC_LONG_PFX(_cond_read_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (c)) > #define atomic_long_cond_read_acquire(v, c) \ > ATOMIC_LONG_PFX(_cond_read_acquire)((ATOMIC_LONG_PFX(_t) *)(v), (c)) > > diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h > index fe297b599b0a..305e03b19a26 100644 > --- a/include/asm-generic/barrier.h > +++ b/include/asm-generic/barrier.h > @@ -221,18 +221,17 @@ do { \ > #endif > > /** > - * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering > + * smp_cond_load_relaxed() - (Spin) wait for cond with no ordering guarantees > * @ptr: pointer to the variable to wait on > * @cond: boolean expression to wait for > * > - * Equivalent to using smp_load_acquire() on the condition variable but employs > - * the control dependency of the wait to reduce the barrier on many platforms. > + * Equivalent to using READ_ONCE() on the condition variable. > * > * Due to C lacking lambda expressions we load the value of *ptr into a > * pre-named variable @VAL to be used in @cond. > */ > -#ifndef smp_cond_load_acquire > -#define smp_cond_load_acquire(ptr, cond_expr) ({ \ > +#ifndef smp_cond_load_relaxed > +#define smp_cond_load_relaxed(ptr, cond_expr) ({ \ > typeof(ptr) __PTR = (ptr); \ > typeof(*ptr) VAL; \ > for (;;) { \ > @@ -241,10 +240,26 @@ do { \ > break; \ > cpu_relax(); \ > } \ > - smp_acquire__after_ctrl_dep(); \ > VAL; \ > }) > #endif > > +/** > + * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering > + * @ptr: pointer to the variable to wait on > + * @cond: boolean expression to wait for > + * > + * Equivalent to using smp_load_acquire() on the condition variable but employs > + * the control dependency of the wait to reduce the barrier on many platforms. > + */ > +#ifndef smp_cond_load_acquire > +#define smp_cond_load_acquire(ptr, cond_expr) ({ \ > + typeof(*ptr) _val; \ > + _val = smp_cond_load_relaxed(ptr, cond_expr); \ > + smp_acquire__after_ctrl_dep(); \ > + _val; \ > +}) > +#endif > + > #endif /* !__ASSEMBLY__ */ > #endif /* __ASM_GENERIC_BARRIER_H */ > diff --git a/include/linux/atomic.h b/include/linux/atomic.h > index 8b276fd9a127..01ce3997cb42 100644 > --- a/include/linux/atomic.h > +++ b/include/linux/atomic.h > @@ -654,6 +654,7 @@ static inline int atomic_dec_if_positive(atomic_t *v) > } > #endif > > +#define atomic_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c)) > #define atomic_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c)) > > #ifdef CONFIG_GENERIC_ATOMIC64 > @@ -1075,6 +1076,7 @@ static inline long long atomic64_fetch_andnot_release(long long i, atomic64_t *v > } > #endif > > +#define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c)) > #define atomic64_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c)) > > #include <asm-generic/atomic-long.h> >
On Tue, Apr 27, 2021 at 9:27 AM Stefan Bader <stefan.bader@canonical.com> wrote: > > > Target is bionic:linux AND bionic:linux-aws. But bionic:linux-aws is a > derivative of bionic:linux. Why mention aws individually? > > -Stefan > Hi Stefan! Because of the priority and testing - although the target is B/linux and B/linux-aws, this was tested (and it is a priority) on AWS. The message I wanted to pass is: "ok, this is for all kernels, but it is more urgent in AWS so I specifically tested with -aws kernels and would be nice to have the patches in their kernels rather sooner than later, generic kernels _might_ wait for next cycle". I couldn't encode this information better in the subject tags ... hehe I said something along these lines though, in the cover-letter [NOTE] section. Cheers, Guilherme
diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h index 34a028a7bcc5..5b2b0b5ea06d 100644 --- a/include/asm-generic/atomic-long.h +++ b/include/asm-generic/atomic-long.h @@ -244,6 +244,8 @@ static inline long atomic_long_add_unless(atomic_long_t *l, long a, long u) #define atomic_long_inc_not_zero(l) \ ATOMIC_LONG_PFX(_inc_not_zero)((ATOMIC_LONG_PFX(_t) *)(l)) +#define atomic_long_cond_read_relaxed(v, c) \ + ATOMIC_LONG_PFX(_cond_read_relaxed)((ATOMIC_LONG_PFX(_t) *)(v), (c)) #define atomic_long_cond_read_acquire(v, c) \ ATOMIC_LONG_PFX(_cond_read_acquire)((ATOMIC_LONG_PFX(_t) *)(v), (c)) diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h index fe297b599b0a..305e03b19a26 100644 --- a/include/asm-generic/barrier.h +++ b/include/asm-generic/barrier.h @@ -221,18 +221,17 @@ do { \ #endif /** - * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering + * smp_cond_load_relaxed() - (Spin) wait for cond with no ordering guarantees * @ptr: pointer to the variable to wait on * @cond: boolean expression to wait for * - * Equivalent to using smp_load_acquire() on the condition variable but employs - * the control dependency of the wait to reduce the barrier on many platforms. + * Equivalent to using READ_ONCE() on the condition variable. * * Due to C lacking lambda expressions we load the value of *ptr into a * pre-named variable @VAL to be used in @cond. */ -#ifndef smp_cond_load_acquire -#define smp_cond_load_acquire(ptr, cond_expr) ({ \ +#ifndef smp_cond_load_relaxed +#define smp_cond_load_relaxed(ptr, cond_expr) ({ \ typeof(ptr) __PTR = (ptr); \ typeof(*ptr) VAL; \ for (;;) { \ @@ -241,10 +240,26 @@ do { \ break; \ cpu_relax(); \ } \ - smp_acquire__after_ctrl_dep(); \ VAL; \ }) #endif +/** + * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering + * @ptr: pointer to the variable to wait on + * @cond: boolean expression to wait for + * + * Equivalent to using smp_load_acquire() on the condition variable but employs + * the control dependency of the wait to reduce the barrier on many platforms. + */ +#ifndef smp_cond_load_acquire +#define smp_cond_load_acquire(ptr, cond_expr) ({ \ + typeof(*ptr) _val; \ + _val = smp_cond_load_relaxed(ptr, cond_expr); \ + smp_acquire__after_ctrl_dep(); \ + _val; \ +}) +#endif + #endif /* !__ASSEMBLY__ */ #endif /* __ASM_GENERIC_BARRIER_H */ diff --git a/include/linux/atomic.h b/include/linux/atomic.h index 8b276fd9a127..01ce3997cb42 100644 --- a/include/linux/atomic.h +++ b/include/linux/atomic.h @@ -654,6 +654,7 @@ static inline int atomic_dec_if_positive(atomic_t *v) } #endif +#define atomic_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c)) #define atomic_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c)) #ifdef CONFIG_GENERIC_ATOMIC64 @@ -1075,6 +1076,7 @@ static inline long long atomic64_fetch_andnot_release(long long i, atomic64_t *v } #endif +#define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c)) #define atomic64_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c)) #include <asm-generic/atomic-long.h>