Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 6 Apr 2023 10:19:33 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Ankur Arora <ankur.a.arora@oracle.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org,
        torvalds@linux-foundation.org, akpm@linux-foundation.org,
        luto@kernel.org, bp@alien8.de, dave.hansen@linux.intel.com,
        hpa@zytor.com, mingo@redhat.com, juri.lelli@redhat.com,
        willy@infradead.org, mgorman@suse.de, rostedt@goodmis.org,
        tglx@linutronix.de, vincent.guittot@linaro.org, jon.grimm@amd.com,
        bharata@amd.com, boris.ostrovsky@oracle.com, konrad.wilk@oracle.com
Subject: Re: [PATCH 4/9] x86/clear_page: parameterize clear_page*() to
 specify length
Message-ID: <20230406081933.GD386572@hirez.programming.kicks-ass.net>
References: <20230403052233.1880567-1-ankur.a.arora@oracle.com>
 <20230403052233.1880567-5-ankur.a.arora@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230403052233.1880567-5-ankur.a.arora@oracle.com>
Precedence: bulk

On Sun, Apr 02, 2023 at 10:22:28PM -0700, Ankur Arora wrote:
> Change clear_page*() to take a length parameter.
> Rename to clear_pages_*().
> 
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>

> diff --git a/arch/x86/lib/clear_page_64.S b/arch/x86/lib/clear_page_64.S
> index ecbfb4dd3b01..6069acf6072f 100644
> --- a/arch/x86/lib/clear_page_64.S
> +++ b/arch/x86/lib/clear_page_64.S
> @@ -2,6 +2,8 @@
>  #include <linux/linkage.h>
>  #include <asm/asm.h>
>  #include <asm/export.h>
> +#include <asm/cpufeatures.h>
> +#include <asm/alternative.h>
>  
>  /*
>   * Most CPUs support enhanced REP MOVSB/STOSB instructions. It is
> @@ -13,18 +15,30 @@
>  /*
>   * Zero a page.
>   * %rdi	- page
> + * %esi	- page-length
> + *
> + * Clobbers: %rax, %rcx
>   */
> -SYM_FUNC_START(clear_page_rep)
> -	movl $4096/8,%ecx
> +SYM_FUNC_START(clear_pages_rep)
> +	movl %esi, %ecx
>  	xorl %eax,%eax
> +	shrl $3,%ecx
>  	rep stosq
>  	RET
> -SYM_FUNC_END(clear_page_rep)
> -EXPORT_SYMBOL_GPL(clear_page_rep)
> +SYM_FUNC_END(clear_pages_rep)
> +EXPORT_SYMBOL_GPL(clear_pages_rep)
>  
> -SYM_FUNC_START(clear_page_orig)
> +/*
> + * Original page zeroing loop.
> + * %rdi	- page
> + * %esi	- page-length
> + *
> + * Clobbers: %rax, %rcx, %rflags
> + */
> +SYM_FUNC_START(clear_pages_orig)
> +	movl   %esi, %ecx
>  	xorl   %eax,%eax
> -	movl   $4096/64,%ecx
> +	shrl   $6,%ecx
>  	.p2align 4
>  .Lloop:
>  	decl	%ecx
> @@ -41,16 +55,23 @@ SYM_FUNC_START(clear_page_orig)
>  	jnz	.Lloop
>  	nop
>  	RET
> -SYM_FUNC_END(clear_page_orig)
> -EXPORT_SYMBOL_GPL(clear_page_orig)
> +SYM_FUNC_END(clear_pages_orig)
> +EXPORT_SYMBOL_GPL(clear_pages_orig)
>  
> -SYM_FUNC_START(clear_page_erms)
> -	movl $4096,%ecx
> +/*
> + * Zero a page.
> + * %rdi	- page
> + * %esi	- page-length
> + *
> + * Clobbers: %rax, %rcx
> + */
> +SYM_FUNC_START(clear_pages_erms)
> +	movl %esi, %ecx
>  	xorl %eax,%eax
>  	rep stosb
>  	RET
> -SYM_FUNC_END(clear_page_erms)
> -EXPORT_SYMBOL_GPL(clear_page_erms)
> +SYM_FUNC_END(clear_pages_erms)
> +EXPORT_SYMBOL_GPL(clear_pages_erms)

So it seems to me that clear_user_*() and clear_page_*() are now very
similar; is there really no way to de-duplicate all that?