[v2,3/5] locking/arch: Wire up local_try_cmpxchg
Commit Message
Implement target specific support for local_try_cmpxchg
and local_cmpxchg using typed C wrappers that call their
_local counterpart and provide additional checking of
their input arguments.
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: WANG Xuerui <kernel@xen0n.name>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Jun Yi <yijun@loongson.cn>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
arch/alpha/include/asm/local.h | 12 ++++++++++--
arch/loongarch/include/asm/local.h | 13 +++++++++++--
arch/mips/include/asm/local.h | 13 +++++++++++--
arch/powerpc/include/asm/local.h | 11 +++++++++++
arch/x86/include/asm/local.h | 13 +++++++++++--
5 files changed, 54 insertions(+), 8 deletions(-)
Comments
On Wed, Apr 05, 2023 at 04:17:08PM +0200, Uros Bizjak wrote:
> diff --git a/arch/powerpc/include/asm/local.h b/arch/powerpc/include/asm/local.h
> index bc4bd19b7fc2..45492fb5bf22 100644
> --- a/arch/powerpc/include/asm/local.h
> +++ b/arch/powerpc/include/asm/local.h
> @@ -90,6 +90,17 @@ static __inline__ long local_cmpxchg(local_t *l, long o, long n)
> return t;
> }
>
> +static __inline__ bool local_try_cmpxchg(local_t *l, long *po, long n)
> +{
> + long o = *po, r;
> +
> + r = local_cmpxchg(l, o, n);
> + if (unlikely(r != o))
> + *po = r;
> +
> + return likely(r == o);
> +}
> +
Why is the ppc one different from the rest? Why can't it use the
try_cmpxchg_local() fallback and needs to have it open-coded?
On Wed, Apr 12, 2023 at 1:33 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Apr 05, 2023 at 04:17:08PM +0200, Uros Bizjak wrote:
> > diff --git a/arch/powerpc/include/asm/local.h b/arch/powerpc/include/asm/local.h
> > index bc4bd19b7fc2..45492fb5bf22 100644
> > --- a/arch/powerpc/include/asm/local.h
> > +++ b/arch/powerpc/include/asm/local.h
> > @@ -90,6 +90,17 @@ static __inline__ long local_cmpxchg(local_t *l, long o, long n)
> > return t;
> > }
> >
> > +static __inline__ bool local_try_cmpxchg(local_t *l, long *po, long n)
> > +{
> > + long o = *po, r;
> > +
> > + r = local_cmpxchg(l, o, n);
> > + if (unlikely(r != o))
> > + *po = r;
> > +
> > + return likely(r == o);
> > +}
> > +
>
> Why is the ppc one different from the rest? Why can't it use the
> try_cmpxchg_local() fallback and needs to have it open-coded?
Please note that ppc directly defines local_cmpxchg that bypasses
cmpxchg_local/arch_cmpxchg_local machinery. The patch takes the same
approach for local_try_cmpxchg, because fallbacks are using
arch_cmpxchg_local definitions.
PPC should be converted to use arch_cmpxchg_local (to also enable
instrumentation), but this is not the scope of the proposed patchset.
Uros.
On Wed, Apr 12, 2023 at 03:37:50PM +0200, Uros Bizjak wrote:
> On Wed, Apr 12, 2023 at 1:33 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Apr 05, 2023 at 04:17:08PM +0200, Uros Bizjak wrote:
> > > diff --git a/arch/powerpc/include/asm/local.h b/arch/powerpc/include/asm/local.h
> > > index bc4bd19b7fc2..45492fb5bf22 100644
> > > --- a/arch/powerpc/include/asm/local.h
> > > +++ b/arch/powerpc/include/asm/local.h
> > > @@ -90,6 +90,17 @@ static __inline__ long local_cmpxchg(local_t *l, long o, long n)
> > > return t;
> > > }
> > >
> > > +static __inline__ bool local_try_cmpxchg(local_t *l, long *po, long n)
> > > +{
> > > + long o = *po, r;
> > > +
> > > + r = local_cmpxchg(l, o, n);
> > > + if (unlikely(r != o))
> > > + *po = r;
> > > +
> > > + return likely(r == o);
> > > +}
> > > +
> >
> > Why is the ppc one different from the rest? Why can't it use the
> > try_cmpxchg_local() fallback and needs to have it open-coded?
>
> Please note that ppc directly defines local_cmpxchg that bypasses
> cmpxchg_local/arch_cmpxchg_local machinery. The patch takes the same
> approach for local_try_cmpxchg, because fallbacks are using
> arch_cmpxchg_local definitions.
>
> PPC should be converted to use arch_cmpxchg_local (to also enable
> instrumentation), but this is not the scope of the proposed patchset.
Ah indeed. Thanks!
> +static __inline__ bool local_try_cmpxchg(local_t *l, long *old, long new)
> +{
> + typeof(l->a.counter) *__old = (typeof(l->a.counter) *) old;
> + return try_cmpxchg_local(&l->a.counter, __old, new);
> +}
> +
This patch then causes following sparse errors:
./arch/x86/include/asm/local.h:131:16: warning: symbol '__old'
shadows an earlier one
./arch/x86/include/asm/local.h:130:30: originally declared here
This is then visible in all kinds of builds - which makes it hard to
find out actual problems with sparse.
@@ -52,8 +52,16 @@ static __inline__ long local_sub_return(long i, local_t * l)
return result;
}
-#define local_cmpxchg(l, o, n) \
- (cmpxchg_local(&((l)->a.counter), (o), (n)))
+static __inline__ long local_cmpxchg(local_t *l, long old, long new)
+{
+ return cmpxchg_local(&l->a.counter, old, new);
+}
+
+static __inline__ bool local_try_cmpxchg(local_t *l, long *old, long new)
+{
+ return try_cmpxchg_local(&l->a.counter, (s64 *)old, new);
+}
+
#define local_xchg(l, n) (xchg_local(&((l)->a.counter), (n)))
/**
@@ -56,8 +56,17 @@ static inline long local_sub_return(long i, local_t *l)
return result;
}
-#define local_cmpxchg(l, o, n) \
- ((long)cmpxchg_local(&((l)->a.counter), (o), (n)))
+static inline long local_cmpxchg(local_t *l, long old, long new)
+{
+ return cmpxchg_local(&l->a.counter, old, new);
+}
+
+static inline bool local_try_cmpxchg(local_t *l, long *old, long new)
+{
+ typeof(l->a.counter) *__old = (typeof(l->a.counter) *) old;
+ return try_cmpxchg_local(&l->a.counter, __old, new);
+}
+
#define local_xchg(l, n) (atomic_long_xchg((&(l)->a), (n)))
/**
@@ -94,8 +94,17 @@ static __inline__ long local_sub_return(long i, local_t * l)
return result;
}
-#define local_cmpxchg(l, o, n) \
- ((long)cmpxchg_local(&((l)->a.counter), (o), (n)))
+static __inline__ long local_cmpxchg(local_t *l, long old, long new)
+{
+ return cmpxchg_local(&l->a.counter, old, new);
+}
+
+static __inline__ bool local_try_cmpxchg(local_t *l, long *old, long new)
+{
+ typeof(l->a.counter) *__old = (typeof(l->a.counter) *) old;
+ return try_cmpxchg_local(&l->a.counter, __old, new);
+}
+
#define local_xchg(l, n) (atomic_long_xchg((&(l)->a), (n)))
/**
@@ -90,6 +90,17 @@ static __inline__ long local_cmpxchg(local_t *l, long o, long n)
return t;
}
+static __inline__ bool local_try_cmpxchg(local_t *l, long *po, long n)
+{
+ long o = *po, r;
+
+ r = local_cmpxchg(l, o, n);
+ if (unlikely(r != o))
+ *po = r;
+
+ return likely(r == o);
+}
+
static __inline__ long local_xchg(local_t *l, long n)
{
long t;
@@ -120,8 +120,17 @@ static inline long local_sub_return(long i, local_t *l)
#define local_inc_return(l) (local_add_return(1, l))
#define local_dec_return(l) (local_sub_return(1, l))
-#define local_cmpxchg(l, o, n) \
- (cmpxchg_local(&((l)->a.counter), (o), (n)))
+static inline long local_cmpxchg(local_t *l, long old, long new)
+{
+ return cmpxchg_local(&l->a.counter, old, new);
+}
+
+static inline bool local_try_cmpxchg(local_t *l, long *old, long new)
+{
+ typeof(l->a.counter) *__old = (typeof(l->a.counter) *) old;
+ return try_cmpxchg_local(&l->a.counter, __old, new);
+}
+
/* Always has a lock prefix */
#define local_xchg(l, n) (xchg(&((l)->a.counter), (n)))