Re: [PATCH] arm64: fix unreachable code issue with cmpxchg

[Arnd Bergmann]

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:
>
> arch/x86/kvm/cpuid.h
> mm/kasan/generic.c
>
> Though some are not:
>
> include/linux/signal.h
> arch/arm64/include/asm/arm_dsu/pmu.h
>
> 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.

      Arnd