arch/x86/include/asm/cmpxchg_32.h | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
The kernel test robot reported that clang no longer compiles the 32-bit
x86 kernel in some configurations due to commit 95ece48165c1
("locking/atomic/x86: Rewrite x86_32 arch_atomic64_{,fetch}_{and,or,xor}()
functions").
The build fails with
arch/x86/include/asm/cmpxchg_32.h:149:9: error: inline assembly requires more registers than available
and the reason seems to be that not only does the cmpxchg8b instruction
need four fixed registers (EDX:EAX and ECX:EBX), with the emulation
fallback the inline asm also wants a fifth fixed register for the
address (it uses %esi for that, but that's just a software convention
with cmpxchg8b_emu).
Avoiding using another pointer input to the asm (and just forcing it to
use the "0(%esi)" addressing that we end up requiring for the sw
fallback) seems to fix the issue.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202406230912.F6XFIyA6-lkp@intel.com/
Fixes: 95ece48165c1 ("locking/atomic/x86: Rewrite x86_32 arch_atomic64_{,fetch}_{and,or,xor}() functions")
Link: https://lore.kernel.org/all/202406230912.F6XFIyA6-lkp@intel.com/
Suggested-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
This is the minimal patch, with the other simplifications removed.
I still think that the case where somebody passes in a "oldp" that can
change during the cmpxchg is entirely broken, and is not actually valid.
If it was valid, we'd have to use READ_ONCE() and WRITE_ONCE() in the
"oldp" updates anyway just to make verification tools happy. We don't.
But hey, that simplification doesn't matter for fixing this build issue,
so let's keep the change minimal.
arch/x86/include/asm/cmpxchg_32.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/cmpxchg_32.h b/arch/x86/include/asm/cmpxchg_32.h
index ed2797f132ce..62cef2113ca7 100644
--- a/arch/x86/include/asm/cmpxchg_32.h
+++ b/arch/x86/include/asm/cmpxchg_32.h
@@ -93,10 +93,9 @@ static __always_inline bool __try_cmpxchg64_local(volatile u64 *ptr, u64 *oldp,
\
asm volatile(ALTERNATIVE(_lock_loc \
"call cmpxchg8b_emu", \
- _lock "cmpxchg8b %[ptr]", X86_FEATURE_CX8) \
- : [ptr] "+m" (*(_ptr)), \
- "+a" (o.low), "+d" (o.high) \
- : "b" (n.low), "c" (n.high), "S" (_ptr) \
+ _lock "cmpxchg8b %a[ptr]", X86_FEATURE_CX8) \
+ : "+a" (o.low), "+d" (o.high) \
+ : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr) \
: "memory"); \
\
o.full; \
@@ -122,12 +121,11 @@ static __always_inline u64 arch_cmpxchg64_local(volatile u64 *ptr, u64 old, u64
\
asm volatile(ALTERNATIVE(_lock_loc \
"call cmpxchg8b_emu", \
- _lock "cmpxchg8b %[ptr]", X86_FEATURE_CX8) \
+ _lock "cmpxchg8b %a[ptr]", X86_FEATURE_CX8) \
CC_SET(e) \
: CC_OUT(e) (ret), \
- [ptr] "+m" (*(_ptr)), \
"+a" (o.low), "+d" (o.high) \
- : "b" (n.low), "c" (n.high), "S" (_ptr) \
+ : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr) \
: "memory"); \
\
if (unlikely(!ret)) \
--
2.45.1.209.gc6f12300dfOn Wed, Jun 26, 2024 at 5:14 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The kernel test robot reported that clang no longer compiles the 32-bit
> x86 kernel in some configurations due to commit 95ece48165c1
> ("locking/atomic/x86: Rewrite x86_32 arch_atomic64_{,fetch}_{and,or,xor}()
> functions").
>
> The build fails with
>
> arch/x86/include/asm/cmpxchg_32.h:149:9: error: inline assembly requires more registers than available
>
> and the reason seems to be that not only does the cmpxchg8b instruction
> need four fixed registers (EDX:EAX and ECX:EBX), with the emulation
> fallback the inline asm also wants a fifth fixed register for the
> address (it uses %esi for that, but that's just a software convention
> with cmpxchg8b_emu).
>
> Avoiding using another pointer input to the asm (and just forcing it to
> use the "0(%esi)" addressing that we end up requiring for the sw
> fallback) seems to fix the issue.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202406230912.F6XFIyA6-lkp@intel.com/
> Fixes: 95ece48165c1 ("locking/atomic/x86: Rewrite x86_32 arch_atomic64_{,fetch}_{and,or,xor}() functions")
> Link: https://lore.kernel.org/all/202406230912.F6XFIyA6-lkp@intel.com/
> Suggested-by: Uros Bizjak <ubizjak@gmail.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-and-Tested-by: Uros Bizjak <ubizjak@gmail.com>
> ---
>
> This is the minimal patch, with the other simplifications removed.
>
> I still think that the case where somebody passes in a "oldp" that can
> change during the cmpxchg is entirely broken, and is not actually valid.
>
> If it was valid, we'd have to use READ_ONCE() and WRITE_ONCE() in the
> "oldp" updates anyway just to make verification tools happy. We don't.
>
> But hey, that simplification doesn't matter for fixing this build issue,
> so let's keep the change minimal.
>
> arch/x86/include/asm/cmpxchg_32.h | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/cmpxchg_32.h b/arch/x86/include/asm/cmpxchg_32.h
> index ed2797f132ce..62cef2113ca7 100644
> --- a/arch/x86/include/asm/cmpxchg_32.h
> +++ b/arch/x86/include/asm/cmpxchg_32.h
> @@ -93,10 +93,9 @@ static __always_inline bool __try_cmpxchg64_local(volatile u64 *ptr, u64 *oldp,
> \
> asm volatile(ALTERNATIVE(_lock_loc \
> "call cmpxchg8b_emu", \
> - _lock "cmpxchg8b %[ptr]", X86_FEATURE_CX8) \
> - : [ptr] "+m" (*(_ptr)), \
> - "+a" (o.low), "+d" (o.high) \
> - : "b" (n.low), "c" (n.high), "S" (_ptr) \
> + _lock "cmpxchg8b %a[ptr]", X86_FEATURE_CX8) \
> + : "+a" (o.low), "+d" (o.high) \
> + : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr) \
> : "memory"); \
> \
> o.full; \
> @@ -122,12 +121,11 @@ static __always_inline u64 arch_cmpxchg64_local(volatile u64 *ptr, u64 old, u64
> \
> asm volatile(ALTERNATIVE(_lock_loc \
> "call cmpxchg8b_emu", \
> - _lock "cmpxchg8b %[ptr]", X86_FEATURE_CX8) \
> + _lock "cmpxchg8b %a[ptr]", X86_FEATURE_CX8) \
> CC_SET(e) \
> : CC_OUT(e) (ret), \
> - [ptr] "+m" (*(_ptr)), \
> "+a" (o.low), "+d" (o.high) \
> - : "b" (n.low), "c" (n.high), "S" (_ptr) \
> + : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr) \
> : "memory"); \
> \
> if (unlikely(!ret)) \
> --
> 2.45.1.209.gc6f12300df
>
On Wed, 26 Jun 2024 at 12:00, Uros Bizjak <ubizjak@gmail.com> wrote:
>
> Reviewed-and-Tested-by: Uros Bizjak <ubizjak@gmail.com>
I ended up committing this directly for rc6 as commit 769327258a14
("x86-32: fix cmpxchg8b_emu build error with clang") since nobody else
seemed to react much and it looked obvious enough.
Linus
© 2016 - 2025 Red Hat, Inc.