The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Uros Bizjak <ubizjak@gmail.com>, Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	kernel test robot <lkp@intel.com>
Subject: [PATCH] x86-32: fix cmpxchg8b_emu build error with clang
Date: Tue, 25 Jun 2024 18:09:25 -0700	[thread overview]
Message-ID: <20240626010924.478611-2-torvalds@linux-foundation.org> (raw)
In-Reply-To: <CAHk-=wiWEgSo2Tb_bih7mnS27zAPL+RGg_7yX4qK1f710-j-Ng@mail.gmail.com>

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>
---

Added commit message, and updated the asm to use '%a[ptr]' instead of
writing out the addressing by hand. 

Still doing the 'oldp' writeback unconmditionally.  The code generation
for the case I checked were the same for both clang and gcc, but until
Uros hits me with the big clue-hammer, I think it's the simpler code
that leaves room for potentially better optimizations too. 

This falls solidly in the "looks ok to me, but still untested" category
for me.  It fixes the clang build issue in my build testing, but I no
longer have a 32-bit test environment, so no actual runtime testing.

 arch/x86/include/asm/cmpxchg_32.h | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/cmpxchg_32.h b/arch/x86/include/asm/cmpxchg_32.h
index ed2797f132ce..4444a8292c7a 100644
--- a/arch/x86/include/asm/cmpxchg_32.h
+++ b/arch/x86/include/asm/cmpxchg_32.h
@@ -88,18 +88,17 @@ static __always_inline bool __try_cmpxchg64_local(volatile u64 *ptr, u64 *oldp,
 
 #define __arch_cmpxchg64_emu(_ptr, _old, _new, _lock_loc, _lock)	\
 ({									\
-	union __u64_halves o = { .full = (_old), },			\
-			   n = { .full = (_new), };			\
+	__u64 o = (_old);						\
+	union __u64_halves n = { .full = (_new), };			\
 									\
 	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)						\
+		     : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr)	\
 		     : "memory");					\
 									\
-	o.full;								\
+	o;								\
 })
 
 static __always_inline u64 arch_cmpxchg64(volatile u64 *ptr, u64 old, u64 new)
@@ -116,22 +115,19 @@ static __always_inline u64 arch_cmpxchg64_local(volatile u64 *ptr, u64 old, u64
 
 #define __arch_try_cmpxchg64_emu(_ptr, _oldp, _new, _lock_loc, _lock)	\
 ({									\
-	union __u64_halves o = { .full = *(_oldp), },			\
-			   n = { .full = (_new), };			\
+	__u64 o = *(_oldp);						\
+	union __u64_halves n = { .full = (_new), };			\
 	bool ret;							\
 									\
 	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)		\
+		     : CC_OUT(e) (ret), "+A" (o)			\
+		     : "b" (n.low), "c" (n.high), [ptr] "S" (_ptr)	\
 		     : "memory");					\
 									\
-	if (unlikely(!ret))						\
-		*(_oldp) = o.full;					\
+	*(_oldp) = o;							\
 									\
 	likely(ret);							\
 })
-- 
2.45.1.209.gc6f12300df


  reply	other threads:[~2024-06-26  1:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-23  2:01 arch/x86/include/asm/cmpxchg_32.h:149:9: error: inline assembly requires more registers than available kernel test robot
2024-06-23 10:14 ` Uros Bizjak
2024-06-23 15:42   ` Linus Torvalds
2024-06-23 17:42     ` Linus Torvalds
2024-06-23 17:43     ` Uros Bizjak
2024-06-23 17:48       ` Linus Torvalds
2024-06-23 18:07         ` Linus Torvalds
2024-06-24  7:36           ` Uros Bizjak
2024-06-24 13:59             ` Linus Torvalds
2024-06-24 14:17               ` Linus Torvalds
2024-06-24 15:42                 ` Uros Bizjak
2024-06-25 18:31                   ` Linus Torvalds
2024-06-26  1:09                     ` Linus Torvalds [this message]
2024-06-26  7:39                       ` [PATCH] x86-32: fix cmpxchg8b_emu build error with clang Uros Bizjak
2024-06-26  9:07                         ` Uros Bizjak
2024-06-26 15:11                         ` Linus Torvalds
2024-06-26  7:54                     ` arch/x86/include/asm/cmpxchg_32.h:149:9: error: inline assembly requires more registers than available Uros Bizjak
2024-06-26  9:28                     ` Peter Zijlstra
2024-06-26  9:33                       ` Uros Bizjak
2024-06-26  9:42                         ` Peter Zijlstra
2024-06-26 15:11                     ` [PATCH v2] x86-32: fix cmpxchg8b_emu build error with clang Linus Torvalds
2024-06-26 18:59                       ` Uros Bizjak
2024-06-30 23:12                         ` Linus Torvalds
2024-06-23 18:14         ` arch/x86/include/asm/cmpxchg_32.h:149:9: error: inline assembly requires more registers than available Uros Bizjak
2024-06-23 18:25           ` Linus Torvalds
2024-06-23 18:41             ` Uros Bizjak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240626010924.478611-2-torvalds@linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox