Re: [PATCH] arm64: fix unreachable code issue with cmpxchg
From: Arnd Bergmann
Date: Tue Sep 10 2019 - 05:38:56 EST
On Tue, Sep 10, 2019 at 11:23 AM Andrew Murray <andrew.murray@xxxxxxx> wrote:
> > arch/arm64/include/asm/cmpxchg.h | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> > diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> > index a1398f2f9994..fd64dc8a235f 100644
> > --- a/arch/arm64/include/asm/cmpxchg.h
> > +++ b/arch/arm64/include/asm/cmpxchg.h
> > @@ -19,7 +19,7 @@
> > * acquire+release for the latter.
> > */
> > #define __XCHG_CASE(w, sfx, name, sz, mb, nop_lse, acq, acq_lse, rel, cl) \
> > -static inline u##sz __xchg_case_##name##sz(u##sz x, volatile void *ptr) \
> > +static __always_inline u##sz __xchg_case_##name##sz(u##sz x, volatile void *ptr)\
> This hunk isn't needed, there is no BUILD_BUG here.
Right, I noticed this, but it seemed like a good idea regardless given the small
size of the function compared with the overhead of a function call. We clearly
want these to be inlined all the time.
Same for the others.
> Alternatively is it possible to replace the BUILD_BUG's with something else?
> I think because we use BUILD_BUG at the end of a switch statement, we make
> the assumption that size is known at compile time, for this reason we should
> ensure the function containing the BUILD_BUG is __always_inline.
> Looking across the kernel where BUILD_BUG is used as a default in a switch
> statment ($ git grep -B 3 BUILD_BUG\( | grep default), most instances are
> within macros, but many are found in an __always_inline function:
> Though some are not:
> I wonder if there may be a latent mole ready to whack with pmu.h?
Right, it can't hurt to annotate those as well. I actually have another
fixup for linux/signal.h that I would have to revisit at some point.
See https://bugs.llvm.org/show_bug.cgi?id=38789, I think this is
fixed with clang-9 now, but maybe not with clang-8.