Re: [PATCH] um/asm: Replace "REP; NOP" with PAUSE mnemonic

[Uros Bizjak <ubizjak@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

On Wed, Apr 16, 2025 at 7:53 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Tue, Apr 15, 2025 at 7:56 PM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Mon, 14 Apr 2025 16:55:57 +0200
> > Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > > Current minimum required version of binutils is 2.25,
> > > which supports PAUSE instruction mnemonic.
> > >
> > > Replace "REP; NOP" with this proper mnemonic.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > > Cc: Richard Weinberger <richard@nod.at>
> > > Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
> > > Cc: Johannes Berg <johannes@sipsolutions.net>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Borislav Petkov <bp@alien8.de>
> > > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > > ---
> > >  arch/x86/um/asm/processor.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/um/asm/processor.h b/arch/x86/um/asm/processor.h
> > > index 478710384b34..233a7a0d29c9 100644
> > > --- a/arch/x86/um/asm/processor.h
> > > +++ b/arch/x86/um/asm/processor.h
> > > @@ -24,7 +24,7 @@
> > >  /* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */
> > >  static __always_inline void rep_nop(void)
> > >  {
> > > -     __asm__ __volatile__("rep;nop": : :"memory");
> > > +     __asm__ __volatile__("pause": : :"memory");
> > >  }
> > >
> >
> > That only makes sense if you also change the function name.
>
> This function is used in several places, and is also defined for x86
> in arch/x86/include/asm/vdso/processor.h. The renaming should be
> coordinated with x86 and should definitely be a separate patch.

Something like the attached patch that also moves (duplicated)
function definition to a shared place.

Uros.

[-- Attachment #2: pause.diff.txt --]
[-- Type: text/plain, Size: 2646 bytes --]

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 6266d6b9e0b8..59bd25ac7292 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -115,6 +115,12 @@ static inline void wrpkru(u32 pkru)
 }
 #endif
 
+/* PAUSE is a good thing to insert into busy-wait loops. */
+static __always_inline void native_pause(void)
+{
+	asm volatile("pause": : :"memory");
+}
+
 static __always_inline void wbinvd(void)
 {
 	asm volatile("wbinvd": : :"memory");
diff --git a/arch/x86/include/asm/vdso/processor.h b/arch/x86/include/asm/vdso/processor.h
index c9b2ba7a9ec4..169219864d61 100644
--- a/arch/x86/include/asm/vdso/processor.h
+++ b/arch/x86/include/asm/vdso/processor.h
@@ -7,15 +7,11 @@
 
 #ifndef __ASSEMBLER__
 
-/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */
-static __always_inline void rep_nop(void)
-{
-	asm volatile("rep; nop" ::: "memory");
-}
+#include <asm/special_insns.h>
 
 static __always_inline void cpu_relax(void)
 {
-	rep_nop();
+	native_pause();
 }
 
 struct getcpu_cache;
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index eebc360ed1bb..ba5a4ccda37a 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1486,7 +1486,7 @@ static void __init delay_with_tsc(void)
 	 * 1 GHz == 40 jiffies
 	 */
 	do {
-		rep_nop();
+		native_pause();
 		now = rdtsc();
 	} while ((now - start) < 40000000000ULL / HZ &&	time_before_eq(jiffies, end));
 }
diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c
index e86eda2c0b04..eb2d2e1cbddd 100644
--- a/arch/x86/lib/delay.c
+++ b/arch/x86/lib/delay.c
@@ -75,7 +75,7 @@ static void delay_tsc(u64 cycles)
 
 		/* Allow RT tasks to run */
 		preempt_enable();
-		rep_nop();
+		native_pause();
 		preempt_disable();
 
 		/*
diff --git a/arch/x86/um/asm/processor.h b/arch/x86/um/asm/processor.h
index 478710384b34..31d71198778e 100644
--- a/arch/x86/um/asm/processor.h
+++ b/arch/x86/um/asm/processor.h
@@ -19,21 +19,16 @@
 #define ARCH_IS_STACKGROW(address) \
        (address + 65536 + 32 * sizeof(unsigned long) >= UPT_SP(&current->thread.regs.regs))
 
+#include <asm/special_insns.h>
 #include <asm/user.h>
 
-/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */
-static __always_inline void rep_nop(void)
-{
-	__asm__ __volatile__("rep;nop": : :"memory");
-}
-
 static __always_inline void cpu_relax(void)
 {
 	if (time_travel_mode == TT_MODE_INFCPU ||
 	    time_travel_mode == TT_MODE_EXTERNAL)
 		time_travel_ndelay(1);
 	else
-		rep_nop();
+		native_pause();
 }
 
 #define task_pt_regs(t) (&(t)->thread.regs)