Re: [PATCH v2 1/6] ARM: cmpxchg: always inline __arch_xchg() and __cmpxchg()

["Gary Guo" <gary@garyguo.net>]

On Thu Oct 8, 2026 at 6:37 AM BST, Karl Mehltretter wrote:
> __arch_xchg(), __cmpxchg() and __cmpxchg_local() end in a default case
> that calls an undefined function. That turns an unsupported size into a
> link error. It relies on the functions being inlined, so that the
> compiler can drop the default case for a constant size.
>
> They are only marked inline. gcc 8.1.0 with CC_OPTIMIZE_FOR_SIZE does
> not inline them in rust/helpers/helpers.c, which calls xchg() and
> cmpxchg() from many small helpers. The out-of-line copies keep the
> default case and the link fails.
>
>   helpers.c:(.text+0x424): undefined reference to `__bad_xchg'
>   helpers.c:(.text+0x4ac): undefined reference to `__bad_cmpxchg'
>
> Seen with bcm2835_defconfig and CONFIG_RUST=y on v7.3-rc1. The same
> config links with CONFIG_RUST=n. It also links with gcc 15.2.0 and
> with clang.
>
> v6.19 links. Commit ab717dd98bee ("rust: helpers: Add i8/i16 atomic
> xchg_acquire helpers") in v7.0 triggers __bad_xchg. Commit
> ac8f06ade38a ("rust: sync: atomic: Add Atomic<*{mut,const} T> support")
> in v7.1 also triggers __bad_cmpxchg.

The change is okay, but the commit message is typical Claude style verbosity.
Especially this paragraph. Just describe what is the issue and what is the fix.
The mention of what other okay config you tried can be stripped. Mentions of
v6.19, v7.0 and v7.1 are completely unnecessary information.

Consider something like this:

    ...

    They are only marked inline. gcc 8.1.0 with CC_OPTIMIZE_FOR_SIZE decides to
    not inline them in rust/helpers/helpers.c, which calls xchg() and
    cmpxchg() from many small helpers. With bcm2835_defconfig and CONFIG_RUST=y,
    linking fails with:

        helpers.c:(.text+0x424): undefined reference to `__bad_xchg'
        helpers.c:(.text+0x4ac): undefined reference to `__bad_cmpxchg'

    Since the absence of inlining always produce a linker error, mark these
    functions as __always_inline instead.

With a better commit message:

Reviewed-by: Gary Guo <gary@garyguo.net>

>
> Mark the functions __always_inline.
>
> Fixes: ab717dd98bee ("rust: helpers: Add i8/i16 atomic xchg_acquire helpers")
> Fixes: ac8f06ade38a ("rust: sync: atomic: Add Atomic<*{mut,const} T> support")
> Cc: stable@vger.kernel.org

Arguably the commit in the fixed tags are not buggy themselves. They just add a
ok user that triggers the symptom of lack of inlining on an archaic GCC.

Best,
Gary

> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  arch/arm/include/asm/cmpxchg.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
> index 9beb64d30586..0ce5225af442 100644
> --- a/arch/arm/include/asm/cmpxchg.h
> +++ b/arch/arm/include/asm/cmpxchg.h
> @@ -26,7 +26,7 @@
>  #define swp_is_buggy
>  #endif
>  
> -static inline unsigned long
> +static __always_inline unsigned long
>  __arch_xchg(unsigned long x, volatile void *ptr, int size)
>  {
>  	extern void __bad_xchg(volatile void *, int);
> @@ -155,8 +155,8 @@ extern void __bad_cmpxchg(volatile void *ptr, int size);
>   * cmpxchg only support 32-bits operands on ARMv6.
>   */
>  
> -static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
> -				      unsigned long new, int size)
> +static __always_inline unsigned long
> +__cmpxchg(volatile void *ptr, unsigned long old, unsigned long new, int size)
>  {
>  	unsigned long oldval, res;
>  
> @@ -220,9 +220,9 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
>  				      sizeof(*(ptr)));			\
>  })
>  
> -static inline unsigned long __cmpxchg_local(volatile void *ptr,
> -					    unsigned long old,
> -					    unsigned long new, int size)
> +static __always_inline unsigned long
> +__cmpxchg_local(volatile void *ptr, unsigned long old, unsigned long new,
> +		int size)
>  {
>  	unsigned long ret;
>