On Tue, Sep 10, 2019 at 11:23 AM Andrew Murray <andrew.mur...@arm.com> 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
