Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1631647 > unrolled thread

[PATCH 1/5] x86_64: use REP MOVSB in copy_page()

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] x86_64: use REP MOVSB in copy_page() Alexey Dobriyan <adobriyan@gmail.com> - 2017-04-26 20:30 +0200
    [PATCH 2/5] x86_64: inline copy_page() at call site Alexey Dobriyan <adobriyan@gmail.com> - 2017-04-26 20:30 +0200
      [PATCH 3/5] x86_64: rename clear_page() and copy_user() variants Alexey Dobriyan <adobriyan@gmail.com> - 2017-04-26 20:40 +0200
        [PATCH 5/5] x86_64: garbage collect headers in clear_page.S Alexey Dobriyan <adobriyan@gmail.com> - 2017-04-26 20:40 +0200
        [PATCH 4/5] x86_64: clobber "cc" in inlined clear_page() Alexey Dobriyan <adobriyan@gmail.com> - 2017-04-26 20:40 +0200
        Re: [PATCH 3/5] x86_64: rename clear_page() and copy_user() variants Borislav Petkov <bp@alien8.de> - 2017-05-05 19:00 +0200
      Re: [PATCH 2/5] x86_64: inline copy_page() at call site Borislav Petkov <bp@alien8.de> - 2017-04-28 23:10 +0200
        Re: [PATCH 2/5] x86_64: inline copy_page() at call site Alexey Dobriyan <adobriyan@gmail.com> - 2017-05-02 13:50 +0200
          Re: [PATCH 2/5] x86_64: inline copy_page() at call site Borislav Petkov <bp@alien8.de> - 2017-05-02 14:10 +0200

#1631647 — [PATCH 1/5] x86_64: use REP MOVSB in copy_page()

On my Broadwell-era Xeon copying page with REP MOVSB is ~7.8% faster
than with REP MOVSQ. Choose REP MOVSB copy_page() at runtime
with alternatives.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/lib/copy_page_64.S |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

--- a/arch/x86/lib/copy_page_64.S
+++ b/arch/x86/lib/copy_page_64.S
@@ -13,13 +13,21 @@
  */
 	ALIGN
 ENTRY(copy_page)
-	ALTERNATIVE "jmp copy_page_regs", "", X86_FEATURE_REP_GOOD
+	ALTERNATIVE_2 "jmp copy_page_regs",	\
+		"", X86_FEATURE_REP_GOOD,	\
+		"jmp copy_page_rep_movsb", X86_FEATURE_ERMS
 	movl	$4096/8, %ecx
 	rep	movsq
 	ret
 ENDPROC(copy_page)
 EXPORT_SYMBOL(copy_page)
 
+ENTRY(copy_page_rep_movsb)
+	mov	$4096, %ecx
+	rep movsb
+	ret
+ENDPROC(copy_page_rep_movsb)
+
 ENTRY(copy_page_regs)
 	subq	$2*8,	%rsp
 	movq	%rbx,	(%rsp)

[toc] | [next] | [standalone]

#1631648 — [PATCH 2/5] x86_64: inline copy_page() at call site

Avoid unconditional branch at every copy_page() call by using
alternatives and calling optimal variant directly.

Rename individual versions to immediately show which one is used in
profiles, etc.

RBX and R12 aren't clobbered because generic version restores them
and both REP versions don't touch them.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/include/asm/page_64.h |   16 ++++++++++++++--
 arch/x86/lib/copy_page_64.S    |   17 +++++++----------
 2 files changed, 21 insertions(+), 12 deletions(-)

--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -49,8 +49,20 @@ static inline void clear_page(void *page)
 			   : "memory", "rax", "rcx");
 }
 
-void copy_page(void *to, void *from);
-
+void copy_page_mov(void *to, void *from);
+void copy_page_rep_movsq(void *to, void *from);
+void copy_page_rep_movsb(void *to, void *from);
+static __always_inline void copy_page(void *to, void *from)
+{
+	alternative_call_2(
+		copy_page_mov,
+		copy_page_rep_movsq, X86_FEATURE_REP_GOOD,
+		copy_page_rep_movsb, X86_FEATURE_ERMS,
+		ASM_OUTPUT2("=D" (to), "=S" (from)),
+		"0" (to), "1" (from)
+		: "rax", "rcx", "rdx", "r8", "r9", "r10", "r11", "cc", "memory"
+	);
+}
 #endif	/* !__ASSEMBLY__ */
 
 #ifdef CONFIG_X86_VSYSCALL_EMULATION
--- a/arch/x86/lib/copy_page_64.S
+++ b/arch/x86/lib/copy_page_64.S
@@ -1,8 +1,6 @@
 /* Written 2003 by Andi Kleen, based on a kernel by Evandro Menezes */
 
 #include <linux/linkage.h>
-#include <asm/cpufeatures.h>
-#include <asm/alternative-asm.h>
 #include <asm/export.h>
 
 /*
@@ -12,23 +10,21 @@
  * prefetch distance based on SMP/UP.
  */
 	ALIGN
-ENTRY(copy_page)
-	ALTERNATIVE_2 "jmp copy_page_regs",	\
-		"", X86_FEATURE_REP_GOOD,	\
-		"jmp copy_page_rep_movsb", X86_FEATURE_ERMS
+ENTRY(copy_page_rep_movsq)
 	movl	$4096/8, %ecx
 	rep	movsq
 	ret
-ENDPROC(copy_page)
-EXPORT_SYMBOL(copy_page)
+ENDPROC(copy_page_rep_movsq)
+EXPORT_SYMBOL(copy_page_rep_movsq)
 
 ENTRY(copy_page_rep_movsb)
 	mov	$4096, %ecx
 	rep movsb
 	ret
 ENDPROC(copy_page_rep_movsb)
+EXPORT_SYMBOL(copy_page_rep_movsb)
 
-ENTRY(copy_page_regs)
+ENTRY(copy_page_mov)
 	subq	$2*8,	%rsp
 	movq	%rbx,	(%rsp)
 	movq	%r12,	1*8(%rsp)
@@ -93,4 +89,5 @@ ENTRY(copy_page_regs)
 	movq	1*8(%rsp), %r12
 	addq	$2*8, %rsp
 	ret
-ENDPROC(copy_page_regs)
+ENDPROC(copy_page_mov)
+EXPORT_SYMBOL(copy_page_mov)

[toc] | [prev] | [next] | [standalone]

#1631653 — [PATCH 3/5] x86_64: rename clear_page() and copy_user() variants

Patch changes market-ish acronyms like ERMS and chatty names
to consistent and shorter versions:

	xxx_mov
	xxx_rep_stosq	xxx_rep_movsq
	xxx_rep_stosb	xxx_rep_movsb

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/include/asm/page_64.h    |   12 ++++++------
 arch/x86/include/asm/uaccess_64.h |   18 +++++++++---------
 arch/x86/lib/clear_page_64.S      |   18 +++++++++---------
 arch/x86/lib/copy_user_64.S       |   20 ++++++++++----------
 tools/perf/ui/browsers/annotate.c |    2 +-
 5 files changed, 35 insertions(+), 35 deletions(-)

--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -35,15 +35,15 @@ extern unsigned long __phys_addr_symbol(unsigned long);
 #define pfn_valid(pfn)          ((pfn) < max_pfn)
 #endif
 
-void clear_page_orig(void *page);
-void clear_page_rep(void *page);
-void clear_page_erms(void *page);
+void clear_page_mov(void *page);
+void clear_page_rep_stosq(void *page);
+void clear_page_rep_stosb(void *page);
 
 static inline void clear_page(void *page)
 {
-	alternative_call_2(clear_page_orig,
-			   clear_page_rep, X86_FEATURE_REP_GOOD,
-			   clear_page_erms, X86_FEATURE_ERMS,
+	alternative_call_2(clear_page_mov,
+			   clear_page_rep_stosq, X86_FEATURE_REP_GOOD,
+			   clear_page_rep_stosb, X86_FEATURE_ERMS,
 			   "=D" (page),
 			   "0" (page)
 			   : "memory", "rax", "rcx");
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -18,11 +18,11 @@
 
 /* Handles exceptions in both to and from, but doesn't do access_ok */
 __must_check unsigned long
-copy_user_enhanced_fast_string(void *to, const void *from, unsigned len);
+copy_user_rep_movsb(void *to, const void *from, unsigned len);
 __must_check unsigned long
-copy_user_generic_string(void *to, const void *from, unsigned len);
+copy_user_rep_movsq(void *to, const void *from, unsigned len);
 __must_check unsigned long
-copy_user_generic_unrolled(void *to, const void *from, unsigned len);
+copy_user_mov(void *to, const void *from, unsigned len);
 
 static __always_inline __must_check unsigned long
 copy_user_generic(void *to, const void *from, unsigned len)
@@ -30,14 +30,14 @@ copy_user_generic(void *to, const void *from, unsigned len)
 	unsigned ret;
 
 	/*
-	 * If CPU has ERMS feature, use copy_user_enhanced_fast_string.
-	 * Otherwise, if CPU has rep_good feature, use copy_user_generic_string.
-	 * Otherwise, use copy_user_generic_unrolled.
+	 * If CPU has ERMS feature, use copy_user_rep_movsb.
+	 * Otherwise, if CPU has rep_good feature, use copy_user_rep_movsq.
+	 * Otherwise, use copy_user_mov.
 	 */
-	alternative_call_2(copy_user_generic_unrolled,
-			 copy_user_generic_string,
+	alternative_call_2(copy_user_mov,
+			 copy_user_rep_movsq,
 			 X86_FEATURE_REP_GOOD,
-			 copy_user_enhanced_fast_string,
+			 copy_user_rep_movsb,
 			 X86_FEATURE_ERMS,
 			 ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
 				     "=d" (len)),
--- a/arch/x86/lib/clear_page_64.S
+++ b/arch/x86/lib/clear_page_64.S
@@ -14,15 +14,15 @@
  * Zero a page.
  * %rdi	- page
  */
-ENTRY(clear_page_rep)
+ENTRY(clear_page_rep_stosq)
 	movl $4096/8,%ecx
 	xorl %eax,%eax
 	rep stosq
 	ret
-ENDPROC(clear_page_rep)
-EXPORT_SYMBOL_GPL(clear_page_rep)
+ENDPROC(clear_page_rep_stosq)
+EXPORT_SYMBOL_GPL(clear_page_rep_stosq)
 
-ENTRY(clear_page_orig)
+ENTRY(clear_page_mov)
 	xorl   %eax,%eax
 	movl   $4096/64,%ecx
 	.p2align 4
@@ -41,13 +41,13 @@ ENTRY(clear_page_orig)
 	jnz	.Lloop
 	nop
 	ret
-ENDPROC(clear_page_orig)
-EXPORT_SYMBOL_GPL(clear_page_orig)
+ENDPROC(clear_page_mov)
+EXPORT_SYMBOL_GPL(clear_page_mov)
 
-ENTRY(clear_page_erms)
+ENTRY(clear_page_rep_stosb)
 	movl $4096,%ecx
 	xorl %eax,%eax
 	rep stosb
 	ret
-ENDPROC(clear_page_erms)
-EXPORT_SYMBOL_GPL(clear_page_erms)
+ENDPROC(clear_page_rep_stosb)
+EXPORT_SYMBOL_GPL(clear_page_rep_stosb)
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -17,7 +17,7 @@
 #include <asm/export.h>
 
 /*
- * copy_user_generic_unrolled - memory copy with exception handling.
+ * copy_user_mov - memory copy with exception handling.
  * This version is for CPUs like P4 that don't have efficient micro
  * code for rep movsq
  *
@@ -29,7 +29,7 @@
  * Output:
  * eax uncopied bytes or 0 if successful.
  */
-ENTRY(copy_user_generic_unrolled)
+ENTRY(copy_user_mov)
 	ASM_STAC
 	cmpl $8,%edx
 	jb 20f		/* less then 8 bytes, go to byte copy loop */
@@ -111,8 +111,8 @@ ENTRY(copy_user_generic_unrolled)
 	_ASM_EXTABLE(19b,40b)
 	_ASM_EXTABLE(21b,50b)
 	_ASM_EXTABLE(22b,50b)
-ENDPROC(copy_user_generic_unrolled)
-EXPORT_SYMBOL(copy_user_generic_unrolled)
+ENDPROC(copy_user_mov)
+EXPORT_SYMBOL(copy_user_mov)
 
 /* Some CPUs run faster using the string copy instructions.
  * This is also a lot simpler. Use them when possible.
@@ -132,7 +132,7 @@ EXPORT_SYMBOL(copy_user_generic_unrolled)
  * Output:
  * eax uncopied bytes or 0 if successful.
  */
-ENTRY(copy_user_generic_string)
+ENTRY(copy_user_rep_movsq)
 	ASM_STAC
 	cmpl $8,%edx
 	jb 2f		/* less than 8 bytes, go to byte copy loop */
@@ -157,8 +157,8 @@ ENTRY(copy_user_generic_string)
 
 	_ASM_EXTABLE(1b,11b)
 	_ASM_EXTABLE(3b,12b)
-ENDPROC(copy_user_generic_string)
-EXPORT_SYMBOL(copy_user_generic_string)
+ENDPROC(copy_user_rep_movsq)
+EXPORT_SYMBOL(copy_user_rep_movsq)
 
 /*
  * Some CPUs are adding enhanced REP MOVSB/STOSB instructions.
@@ -172,7 +172,7 @@ EXPORT_SYMBOL(copy_user_generic_string)
  * Output:
  * eax uncopied bytes or 0 if successful.
  */
-ENTRY(copy_user_enhanced_fast_string)
+ENTRY(copy_user_rep_movsb)
 	ASM_STAC
 	movl %edx,%ecx
 1:	rep
@@ -187,8 +187,8 @@ ENTRY(copy_user_enhanced_fast_string)
 	.previous
 
 	_ASM_EXTABLE(1b,12b)
-ENDPROC(copy_user_enhanced_fast_string)
-EXPORT_SYMBOL(copy_user_enhanced_fast_string)
+ENDPROC(copy_user_rep_movsb)
+EXPORT_SYMBOL(copy_user_rep_movsb)
 
 /*
  * copy_user_nocache - Uncached memory copy with exception handling
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -1084,7 +1084,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 			 * routines that comes with labels in the same column
 			 * as the address in objdump, sigh.
 			 *
-			 * E.g. copy_user_generic_unrolled
+			 * E.g. copy_user_mov
  			 */
 			if (pos->offset < (s64)size)
 				browser.offsets[pos->offset] = pos;

[toc] | [prev] | [next] | [standalone]

#1631658 — [PATCH 5/5] x86_64: garbage collect headers in clear_page.S

Not necessary after inlining clear_page().

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/lib/clear_page_64.S |    2 --
 1 file changed, 2 deletions(-)

--- a/arch/x86/lib/clear_page_64.S
+++ b/arch/x86/lib/clear_page_64.S
@@ -1,6 +1,4 @@
 #include <linux/linkage.h>
-#include <asm/cpufeatures.h>
-#include <asm/alternative-asm.h>
 #include <asm/export.h>
 
 /*

[toc] | [prev] | [next] | [standalone]

#1631659 — [PATCH 4/5] x86_64: clobber "cc" in inlined clear_page()

Both REP variants clobber flags because of "xor eax, eax" instructions.
While they can be changed to "mov eax, 0", generic version probably
can not because it has to do comparison at some point.

And it is kind of not worth it to not clobber flags anyway.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/x86/include/asm/page_64.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -46,7 +46,7 @@ static inline void clear_page(void *page)
 			   clear_page_rep_stosb, X86_FEATURE_ERMS,
 			   "=D" (page),
 			   "0" (page)
-			   : "memory", "rax", "rcx");
+			   : "cc", "memory", "rax", "rcx");
 }
 
 void copy_page_mov(void *to, void *from);

[toc] | [prev] | [next] | [standalone]

#1636490 — Re: [PATCH 3/5] x86_64: rename clear_page() and copy_user() variants

On Wed, Apr 26, 2017 at 09:30:47PM +0300, Alexey Dobriyan wrote:
> Patch changes market-ish acronyms like ERMS and chatty names
> to consistent and shorter versions:
> 
> 	xxx_mov
> 	xxx_rep_stosq	xxx_rep_movsq
> 	xxx_rep_stosb	xxx_rep_movsb
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  arch/x86/include/asm/page_64.h    |   12 ++++++------
>  arch/x86/include/asm/uaccess_64.h |   18 +++++++++---------
>  arch/x86/lib/clear_page_64.S      |   18 +++++++++---------
>  arch/x86/lib/copy_user_64.S       |   20 ++++++++++----------
>  tools/perf/ui/browsers/annotate.c |    2 +-
>  5 files changed, 35 insertions(+), 35 deletions(-)
> 
> --- a/arch/x86/include/asm/page_64.h
> +++ b/arch/x86/include/asm/page_64.h
> @@ -35,15 +35,15 @@ extern unsigned long __phys_addr_symbol(unsigned long);
>  #define pfn_valid(pfn)          ((pfn) < max_pfn)
>  #endif
>  
> -void clear_page_orig(void *page);
> -void clear_page_rep(void *page);
> -void clear_page_erms(void *page);
> +void clear_page_mov(void *page);
> +void clear_page_rep_stosq(void *page);
> +void clear_page_rep_stosb(void *page);
>  
>  static inline void clear_page(void *page)
>  {
> -	alternative_call_2(clear_page_orig,
> -			   clear_page_rep, X86_FEATURE_REP_GOOD,
> -			   clear_page_erms, X86_FEATURE_ERMS,
> +	alternative_call_2(clear_page_mov,
> +			   clear_page_rep_stosq, X86_FEATURE_REP_GOOD,
> +			   clear_page_rep_stosb, X86_FEATURE_ERMS,
>  			   "=D" (page),
>  			   "0" (page)
>  			   : "memory", "rax", "rcx");
> --- a/arch/x86/include/asm/uaccess_64.h
> +++ b/arch/x86/include/asm/uaccess_64.h
> @@ -18,11 +18,11 @@
>  
>  /* Handles exceptions in both to and from, but doesn't do access_ok */
>  __must_check unsigned long
> -copy_user_enhanced_fast_string(void *to, const void *from, unsigned len);
> +copy_user_rep_movsb(void *to, const void *from, unsigned len);

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#62: FILE: arch/x86/include/asm/uaccess_64.h:21:
+copy_user_rep_movsb(void *to, const void *from, unsigned len);

Pls convert them while at it.

>  __must_check unsigned long
> -copy_user_generic_string(void *to, const void *from, unsigned len);
> +copy_user_rep_movsq(void *to, const void *from, unsigned len);
>  __must_check unsigned long
> -copy_user_generic_unrolled(void *to, const void *from, unsigned len);
> +copy_user_mov(void *to, const void *from, unsigned len);
>  
>  static __always_inline __must_check unsigned long
>  copy_user_generic(void *to, const void *from, unsigned len)
> @@ -30,14 +30,14 @@ copy_user_generic(void *to, const void *from, unsigned len)
>  	unsigned ret;
>  
>  	/*
> -	 * If CPU has ERMS feature, use copy_user_enhanced_fast_string.
> -	 * Otherwise, if CPU has rep_good feature, use copy_user_generic_string.
> -	 * Otherwise, use copy_user_generic_unrolled.
> +	 * If CPU has ERMS feature, use copy_user_rep_movsb.
> +	 * Otherwise, if CPU has rep_good feature, use copy_user_rep_movsq.

REP_GOOD, while you're at it. Also, end function names with ().

> +	 * Otherwise, use copy_user_mov.
>  	 */
> -	alternative_call_2(copy_user_generic_unrolled,
> -			 copy_user_generic_string,
> +	alternative_call_2(copy_user_mov,
> +			 copy_user_rep_movsq,
>  			 X86_FEATURE_REP_GOOD,
> -			 copy_user_enhanced_fast_string,
> +			 copy_user_rep_movsb,
>  			 X86_FEATURE_ERMS,
>  			 ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
>  				     "=d" (len)),
> --- a/arch/x86/lib/clear_page_64.S
> +++ b/arch/x86/lib/clear_page_64.S
> @@ -14,15 +14,15 @@
>   * Zero a page.
>   * %rdi	- page
>   */
> -ENTRY(clear_page_rep)
> +ENTRY(clear_page_rep_stosq)
>  	movl $4096/8,%ecx
>  	xorl %eax,%eax
>  	rep stosq
>  	ret
> -ENDPROC(clear_page_rep)
> -EXPORT_SYMBOL_GPL(clear_page_rep)
> +ENDPROC(clear_page_rep_stosq)
> +EXPORT_SYMBOL_GPL(clear_page_rep_stosq)
>  
> -ENTRY(clear_page_orig)
> +ENTRY(clear_page_mov)
>  	xorl   %eax,%eax
>  	movl   $4096/64,%ecx
>  	.p2align 4
> @@ -41,13 +41,13 @@ ENTRY(clear_page_orig)
>  	jnz	.Lloop
>  	nop
>  	ret
> -ENDPROC(clear_page_orig)
> -EXPORT_SYMBOL_GPL(clear_page_orig)
> +ENDPROC(clear_page_mov)
> +EXPORT_SYMBOL_GPL(clear_page_mov)

Same issue as with the previous patch: _orig was dumb but since you're
changing the names, pls change them to something more descriptive.

> -ENTRY(clear_page_erms)
> +ENTRY(clear_page_rep_stosb)
>  	movl $4096,%ecx
>  	xorl %eax,%eax
>  	rep stosb
>  	ret
> -ENDPROC(clear_page_erms)
> -EXPORT_SYMBOL_GPL(clear_page_erms)
> +ENDPROC(clear_page_rep_stosb)
> +EXPORT_SYMBOL_GPL(clear_page_rep_stosb)
> --- a/arch/x86/lib/copy_user_64.S
> +++ b/arch/x86/lib/copy_user_64.S
> @@ -17,7 +17,7 @@
>  #include <asm/export.h>
>  
>  /*
> - * copy_user_generic_unrolled - memory copy with exception handling.
> + * copy_user_mov - memory copy with exception handling.

This rename is actually losing information from the function name:
"generic_unrolled" explains exactly what the function does.

>   * This version is for CPUs like P4 that don't have efficient micro
>   * code for rep movsq
>   *

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1633159 — Re: [PATCH 2/5] x86_64: inline copy_page() at call site

On Wed, Apr 26, 2017 at 09:28:06PM +0300, Alexey Dobriyan wrote:
> Avoid unconditional branch at every copy_page() call by using
> alternatives and calling optimal variant directly.
> 
> Rename individual versions to immediately show which one is used in
> profiles, etc.
> 
> RBX and R12 aren't clobbered because generic version restores them
> and both REP versions don't touch them.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  arch/x86/include/asm/page_64.h |   16 ++++++++++++++--
>  arch/x86/lib/copy_page_64.S    |   17 +++++++----------
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> --- a/arch/x86/include/asm/page_64.h
> +++ b/arch/x86/include/asm/page_64.h
> @@ -49,8 +49,20 @@ static inline void clear_page(void *page)
>  			   : "memory", "rax", "rcx");
>  }
>  
> -void copy_page(void *to, void *from);
> -
> +void copy_page_mov(void *to, void *from);
> +void copy_page_rep_movsq(void *to, void *from);
> +void copy_page_rep_movsb(void *to, void *from);

<---- newline here.

> +static __always_inline void copy_page(void *to, void *from)
> +{
> +	alternative_call_2(

Please align at the opening brace, like clear_page() above it:

	alternative_call_2(copy_page_mov,
			   copy_page_rep_movsq, X86_FEATURE_REP_GOOD,
			   ...


> +		copy_page_rep_movsb, X86_FEATURE_ERMS,
> +		ASM_OUTPUT2("=D" (to), "=S" (from)),
> +		"0" (to), "1" (from)
> +		: "rax", "rcx", "rdx", "r8", "r9", "r10", "r11", "cc", "memory"
> +	);
> +}
>  #endif	/* !__ASSEMBLY__ */
>  
>  #ifdef CONFIG_X86_VSYSCALL_EMULATION

...

>  ENTRY(copy_page_rep_movsb)
>  	mov	$4096, %ecx
>  	rep movsb
>  	ret
>  ENDPROC(copy_page_rep_movsb)
> +EXPORT_SYMBOL(copy_page_rep_movsb)
>  
> -ENTRY(copy_page_regs)
> +ENTRY(copy_page_mov)

copy_page_regs() is a better name IMO. copy_page_mov() doesn't tell me
anything - all three use "mov". copy_page_unrolled() sounds ok too.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1634361 — Re: [PATCH 2/5] x86_64: inline copy_page() at call site

On Sat, Apr 29, 2017 at 12:04 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Apr 26, 2017 at 09:28:06PM +0300, Alexey Dobriyan wrote:

>> +static __always_inline void copy_page(void *to, void *from)
>> +{
>> +     alternative_call_2(
>
> Please align at the opening brace, like clear_page() above it:

Then I'd have to split clobber list and no lines will be saved.

>         alternative_call_2(copy_page_mov,
>                            copy_page_rep_movsq, X86_FEATURE_REP_GOOD,
>                            ...
>
>
>> +             copy_page_rep_movsb, X86_FEATURE_ERMS,
>> +             ASM_OUTPUT2("=D" (to), "=S" (from)),
>> +             "0" (to), "1" (from)
>> +             : "rax", "rcx", "rdx", "r8", "r9", "r10", "r11", "cc", "memory"
>> +     );
>> +}
>>  #endif       /* !__ASSEMBLY__ */
>>
>>  #ifdef CONFIG_X86_VSYSCALL_EMULATION
>
> ...
>
>>  ENTRY(copy_page_rep_movsb)
>>       mov     $4096, %ecx
>>       rep movsb
>>       ret
>>  ENDPROC(copy_page_rep_movsb)
>> +EXPORT_SYMBOL(copy_page_rep_movsb)
>>
>> -ENTRY(copy_page_regs)
>> +ENTRY(copy_page_mov)
>
> copy_page_regs() is a better name IMO. copy_page_mov() doesn't tell me
> anything - all three use "mov". copy_page_unrolled() sounds ok too.

It says unambiguously which instruction does the actual copying.

[toc] | [prev] | [next] | [standalone]

#1634366 — Re: [PATCH 2/5] x86_64: inline copy_page() at call site

On Tue, May 02, 2017 at 02:49:04PM +0300, Alexey Dobriyan wrote:
> It says unambiguously which instruction does the actual copying.

And that doesn't tell me that it is an unrolled moving using registers.
And that is much more useful info when I look at stack traces than just
"mov".

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web