[PATCH] x86: improve algorithm in clflush_cache_range

[PATCH] x86: improve algorithm in clflush_cache_range

[Ross Zwisler]

The current algorithm used in clflush_cache_range() can cause the last
cache line of the buffer to be flushed twice.

Fix that algorithm so that each cache line will only be flushed once,
and remove arithmetic on void pointers.  Void pointer arithmetic is
allowed by GCC extensions, but isn't part of the base C standards.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: H. Peter Anvin <hpa@zytor.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Borislav Petkov <bp@suse.de>
---
 arch/x86/mm/pageattr.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index ae242a7c11c7..b75ecac859f2 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -131,16 +131,15 @@ within(unsigned long addr, unsigned long start, unsigned long end)
  */
 void clflush_cache_range(void *vaddr, unsigned int size)
 {
-	void *vend = vaddr + size - 1;
+	unsigned long clflush_mask = boot_cpu_data.x86_clflush_size - 1;
+	char *vend = (char *)vaddr + size;
+	char *p;
 
 	mb();
 
-	for (; vaddr < vend; vaddr += boot_cpu_data.x86_clflush_size)
-		clflushopt(vaddr);
-	/*
-	 * Flush any possible final partial cacheline:
-	 */
-	clflushopt(vend);
+	for (p = (char *)((unsigned long)vaddr & ~clflush_mask);
+	     p < vend; p += boot_cpu_data.x86_clflush_size)
+		clflushopt(p);
 
 	mb();
 }
-- 
1.9.3

Re: [PATCH] x86: improve algorithm in clflush_cache_range

[Ingo Molnar]

* Ross Zwisler <ross.zwisler@linux.intel.com> wrote:

> The current algorithm used in clflush_cache_range() can cause the last
> cache line of the buffer to be flushed twice.
> 
> Fix that algorithm so that each cache line will only be flushed once,
> and remove arithmetic on void pointers.  Void pointer arithmetic is
> allowed by GCC extensions, but isn't part of the base C standards.

The optimization itself is fine, but that last argument is bogus: the 
Linux kernel very much relies on 'void *' arithmetics in a gazillion 
places.


Thanks,

	Ingo

Re: [PATCH] x86: improve algorithm in clflush_cache_range

[Ross Zwisler]

On Thu, 2015-03-12 at 12:09 +0100, Ingo Molnar wrote:
> * Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
> 
> > The current algorithm used in clflush_cache_range() can cause the last
> > cache line of the buffer to be flushed twice.
> > 
> > Fix that algorithm so that each cache line will only be flushed once,
> > and remove arithmetic on void pointers.  Void pointer arithmetic is
> > allowed by GCC extensions, but isn't part of the base C standards.
> 
> The optimization itself is fine, but that last argument is bogus: the 
> Linux kernel very much relies on 'void *' arithmetics in a gazillion 
> places.

Okay.  Are you happy with the patch as is or would you like me to
resubmit with that bit omitted from the change log?

- Ross

Re: [PATCH] x86: improve algorithm in clflush_cache_range

[Borislav Petkov]

On Tue, Apr 28, 2015 at 04:13:12PM -0600, Ross Zwisler wrote:
> The current algorithm used in clflush_cache_range() can cause the last
> cache line of the buffer to be flushed twice.  Fix that algorithm so
> that each cache line will only be flushed once.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: H. Peter Anvin <hpa@zytor.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: x86@kernel.org
> Cc: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/mm/pageattr.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

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

[PATCH] x86: improve algorithm in clflush_cache_range

[Ross Zwisler]

The current algorithm used in clflush_cache_range() can cause the last
cache line of the buffer to be flushed twice.  Fix that algorithm so
that each cache line will only be flushed once.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: H. Peter Anvin <hpa@zytor.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: Borislav Petkov <bp@suse.de>
---
 arch/x86/mm/pageattr.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 89af288ec674..338e507f95b8 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -129,16 +129,15 @@ within(unsigned long addr, unsigned long start, unsigned long end)
  */
 void clflush_cache_range(void *vaddr, unsigned int size)
 {
-	void *vend = vaddr + size - 1;
+	unsigned long clflush_mask = boot_cpu_data.x86_clflush_size - 1;
+	char *vend = (char *)vaddr + size;
+	char *p;
 
 	mb();
 
-	for (; vaddr < vend; vaddr += boot_cpu_data.x86_clflush_size)
-		clflushopt(vaddr);
-	/*
-	 * Flush any possible final partial cacheline:
-	 */
-	clflushopt(vend);
+	for (p = (char *)((unsigned long)vaddr & ~clflush_mask);
+	     p < vend; p += boot_cpu_data.x86_clflush_size)
+		clflushopt(p);
 
 	mb();
 }
-- 
1.9.3