From: Alastair D'Silva <[email protected]>
This series addresses a few issues discovered in how we flush caches:
1. Flushes were truncated at 4GB, so larger flushes were incorrect.
2. Flushing the dcache in arch_add_memory was unnecessary
This series also converts much of the cache assembler to C, with the
aim of making it easier to maintain.
Alastair D'Silva (6):
powerpc: Allow flush_icache_range to work across ranges >4GB
powerpc: define helpers to get L1 icache sizes
powerpc: Convert flush_icache_range & friends to C
powerpc: Chunk calls to flush_dcache_range in arch_*_memory
powerpc: Remove 'extern' from func prototypes in cache headers
powerpc: Don't flush caches when adding memory
Changelog:
V3:
- factor out chunking loop
- Replace __asm__ __volatile__ with asm volatile
- Replace flush_coherent_icache_or_return macro with
flush_coherent_icache function
- factor our invalidate_icache_range
- Replace code duplicating clean_dcache_range() in
__flush_dcache_icache() with a call to clean_dcache_range()
- Remove redundant #ifdef CONFIG_44x
- Fix preprocessor logic:
#if !defined(CONFIG_PPC_8xx) & !defined(CONFIG_PPC64)
- Added loop(1|2) to earlyclobbers in flush_dcache_icache_phys
- Drop "Remove extern" patch
- Replace 32 bit shifts in 64 bit VDSO with 64 bit ones
V2:
- Replace C implementation of flush_dcache_icache_phys() with
inline assembler authored by Christophe Leroy
- Add memory clobbers for iccci implementation
- Give __flush_dcache_icache a real implementation, it can't
just be a wrapper around flush_icache_range()
- Remove PPC64_CACHES from misc_64.S
- Replace code duplicating clean_dcache_range() in
flush_icache_range() with a call to clean_dcache_range()
- Replace #ifdef CONFIG_44x with IS_ENABLED(...) in
flush_icache_cange()
- Use 1GB chunks instead of 16GB in arch_*_memory
Alastair D'Silva (5):
powerpc: Allow flush_icache_range to work across ranges >4GB
powerpc: define helpers to get L1 icache sizes
powerpc: Convert flush_icache_range & friends to C
powerpc: Chunk calls to flush_dcache_range in arch_*_memory
powerpc: Don't flush caches when adding memory
arch/powerpc/include/asm/cache.h | 55 +++++---
arch/powerpc/include/asm/cacheflush.h | 36 +++--
arch/powerpc/kernel/misc_32.S | 117 ----------------
arch/powerpc/kernel/misc_64.S | 102 --------------
arch/powerpc/kernel/vdso64/cacheflush.S | 4 +-
arch/powerpc/mm/mem.c | 176 +++++++++++++++++++++++-
6 files changed, 228 insertions(+), 262 deletions(-)
--
2.21.0
From: Alastair D'Silva <[email protected]>
When presented with large amounts of memory being hotplugged
(in my test case, ~890GB), the call to flush_dcache_range takes
a while (~50 seconds), triggering RCU stalls.
This patch breaks up the call into 1GB chunks, calling
cond_resched() inbetween to allow the scheduler to run.
Signed-off-by: Alastair D'Silva <[email protected]>
---
arch/powerpc/mm/mem.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 85b40bad90c0..4f892da59a6a 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -104,6 +104,27 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
return -ENODEV;
}
+#define FLUSH_CHUNK_SIZE SZ_1G
+/**
+ * flush_dcache_range_chunked(): Write any modified data cache blocks out to
+ * memory and invalidate them, in chunks of up to FLUSH_CHUNK_SIZE
+ * Does not invalidate the corresponding instruction cache blocks.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ * @chunk: the max size of the chunks
+ */
+static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
+ unsigned long chunk)
+{
+ unsigned long i;
+
+ for (i = start; i < stop; i += FLUSH_CHUNK_SIZE) {
+ flush_dcache_range(i, min(stop, start + FLUSH_CHUNK_SIZE));
+ cond_resched();
+ }
+}
+
int __ref arch_add_memory(int nid, u64 start, u64 size,
struct mhp_restrictions *restrictions)
{
@@ -120,7 +141,8 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
start, start + size, rc);
return -EFAULT;
}
- flush_dcache_range(start, start + size);
+
+ flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
return __add_pages(nid, start_pfn, nr_pages, restrictions);
}
@@ -137,7 +159,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
/* Remove htab bolted mappings for this section of memory */
start = (unsigned long)__va(start);
- flush_dcache_range(start, start + size);
+ flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
+
ret = remove_section_mapping(start, start + size);
WARN_ON_ONCE(ret);
--
2.21.0
From: Alastair D'Silva <[email protected]>
When calling flush_icache_range with a size >4GB, we were masking
off the upper 32 bits, so we would incorrectly flush a range smaller
than intended.
__kernel_sync_dicache in the 64 bit VDSO has the same bug.
This patch replaces the 32 bit shifts with 64 bit ones, so that
the full size is accounted for.
Signed-off-by: Alastair D'Silva <[email protected]>
Cc: [email protected]
---
arch/powerpc/kernel/misc_64.S | 4 ++--
arch/powerpc/kernel/vdso64/cacheflush.S | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index b55a7b4cb543..9bc0aa9aeb65 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -82,7 +82,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
subf r8,r6,r4 /* compute length */
add r8,r8,r5 /* ensure we get enough */
lwz r9,DCACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of cache block size */
- srw. r8,r8,r9 /* compute line count */
+ srd. r8,r8,r9 /* compute line count */
beqlr /* nothing to do? */
mtctr r8
1: dcbst 0,r6
@@ -98,7 +98,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
subf r8,r6,r4 /* compute length */
add r8,r8,r5
lwz r9,ICACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of Icache block size */
- srw. r8,r8,r9 /* compute line count */
+ srd. r8,r8,r9 /* compute line count */
beqlr /* nothing to do? */
mtctr r8
2: icbi 0,r6
diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
index 3f92561a64c4..526f5ba2593e 100644
--- a/arch/powerpc/kernel/vdso64/cacheflush.S
+++ b/arch/powerpc/kernel/vdso64/cacheflush.S
@@ -35,7 +35,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
subf r8,r6,r4 /* compute length */
add r8,r8,r5 /* ensure we get enough */
lwz r9,CFG_DCACHE_LOGBLOCKSZ(r10)
- srw. r8,r8,r9 /* compute line count */
+ srd. r8,r8,r9 /* compute line count */
crclr cr0*4+so
beqlr /* nothing to do? */
mtctr r8
@@ -52,7 +52,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
subf r8,r6,r4 /* compute length */
add r8,r8,r5
lwz r9,CFG_ICACHE_LOGBLOCKSZ(r10)
- srw. r8,r8,r9 /* compute line count */
+ srd. r8,r8,r9 /* compute line count */
crclr cr0*4+so
beqlr /* nothing to do? */
mtctr r8
--
2.21.0
From: Alastair D'Silva <[email protected]>
This operation takes a significant amount of time when hotplugging
large amounts of memory (~50 seconds with 890GB of persistent memory).
This was orignally in commit fb5924fddf9e
("powerpc/mm: Flush cache on memory hot(un)plug") to support memtrace,
but the flush on add is not needed as it is flushed on remove.
Signed-off-by: Alastair D'Silva <[email protected]>
---
arch/powerpc/mm/mem.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 4f892da59a6a..dd1aa80e854a 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -142,8 +142,6 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
return -EFAULT;
}
- flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
-
return __add_pages(nid, start_pfn, nr_pages, restrictions);
}
--
2.21.0
"Alastair D'Silva" <[email protected]> writes:
> From: Alastair D'Silva <[email protected]>
>
> When calling flush_icache_range with a size >4GB, we were masking
> off the upper 32 bits, so we would incorrectly flush a range smaller
> than intended.
>
> __kernel_sync_dicache in the 64 bit VDSO has the same bug.
Please fix that in a separate patch.
Your subject doesn't mention __kernel_sync_dicache(), and also the two
changes backport differently, so it's better if they're done as separate
patches.
cheers
> This patch replaces the 32 bit shifts with 64 bit ones, so that
> the full size is accounted for.
>
> Signed-off-by: Alastair D'Silva <[email protected]>
> Cc: [email protected]
> ---
> arch/powerpc/kernel/misc_64.S | 4 ++--
> arch/powerpc/kernel/vdso64/cacheflush.S | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index b55a7b4cb543..9bc0aa9aeb65 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -82,7 +82,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> subf r8,r6,r4 /* compute length */
> add r8,r8,r5 /* ensure we get enough */
> lwz r9,DCACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of cache block size */
> - srw. r8,r8,r9 /* compute line count */
> + srd. r8,r8,r9 /* compute line count */
> beqlr /* nothing to do? */
> mtctr r8
> 1: dcbst 0,r6
> @@ -98,7 +98,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> subf r8,r6,r4 /* compute length */
> add r8,r8,r5
> lwz r9,ICACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of Icache block size */
> - srw. r8,r8,r9 /* compute line count */
> + srd. r8,r8,r9 /* compute line count */
> beqlr /* nothing to do? */
> mtctr r8
> 2: icbi 0,r6
> diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
> index 3f92561a64c4..526f5ba2593e 100644
> --- a/arch/powerpc/kernel/vdso64/cacheflush.S
> +++ b/arch/powerpc/kernel/vdso64/cacheflush.S
> @@ -35,7 +35,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
> subf r8,r6,r4 /* compute length */
> add r8,r8,r5 /* ensure we get enough */
> lwz r9,CFG_DCACHE_LOGBLOCKSZ(r10)
> - srw. r8,r8,r9 /* compute line count */
> + srd. r8,r8,r9 /* compute line count */
> crclr cr0*4+so
> beqlr /* nothing to do? */
> mtctr r8
> @@ -52,7 +52,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
> subf r8,r6,r4 /* compute length */
> add r8,r8,r5
> lwz r9,CFG_ICACHE_LOGBLOCKSZ(r10)
> - srw. r8,r8,r9 /* compute line count */
> + srd. r8,r8,r9 /* compute line count */
> crclr cr0*4+so
> beqlr /* nothing to do? */
> mtctr r8
> --
> 2.21.0
On Thu, 2019-09-19 at 13:43 +1000, Michael Ellerman wrote:
> "Alastair D'Silva" <[email protected]> writes:
> > From: Alastair D'Silva <[email protected]>
> >
> > When calling flush_icache_range with a size >4GB, we were masking
> > off the upper 32 bits, so we would incorrectly flush a range
> > smaller
> > than intended.
> >
> > __kernel_sync_dicache in the 64 bit VDSO has the same bug.
>
> Please fix that in a separate patch.
>
> Your subject doesn't mention __kernel_sync_dicache(), and also the
> two
> changes backport differently, so it's better if they're done as
> separate
> patches.
>
Ok.
> cheers
>
> > This patch replaces the 32 bit shifts with 64 bit ones, so that
> > the full size is accounted for.
> >
> > Signed-off-by: Alastair D'Silva <[email protected]>
> > Cc: [email protected]
> > ---
> > arch/powerpc/kernel/misc_64.S | 4 ++--
> > arch/powerpc/kernel/vdso64/cacheflush.S | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/misc_64.S
> > b/arch/powerpc/kernel/misc_64.S
> > index b55a7b4cb543..9bc0aa9aeb65 100644
> > --- a/arch/powerpc/kernel/misc_64.S
> > +++ b/arch/powerpc/kernel/misc_64.S
> > @@ -82,7 +82,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> > subf r8,r6,r4 /* compute length */
> > add r8,r8,r5 /* ensure we get enough */
> > lwz r9,DCACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of cache block
> > size */
> > - srw. r8,r8,r9 /* compute line count */
> > + srd. r8,r8,r9 /* compute line count */
> > beqlr /* nothing to do? */
> > mtctr r8
> > 1: dcbst 0,r6
> > @@ -98,7 +98,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> > subf r8,r6,r4 /* compute length */
> > add r8,r8,r5
> > lwz r9,ICACHEL1LOGBLOCKSIZE(r10) /* Get log-2 of Icache
> > block size */
> > - srw. r8,r8,r9 /* compute line count */
> > + srd. r8,r8,r9 /* compute line count */
> > beqlr /* nothing to do? */
> > mtctr r8
> > 2: icbi 0,r6
> > diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S
> > b/arch/powerpc/kernel/vdso64/cacheflush.S
> > index 3f92561a64c4..526f5ba2593e 100644
> > --- a/arch/powerpc/kernel/vdso64/cacheflush.S
> > +++ b/arch/powerpc/kernel/vdso64/cacheflush.S
> > @@ -35,7 +35,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
> > subf r8,r6,r4 /* compute length */
> > add r8,r8,r5 /* ensure we get enough */
> > lwz r9,CFG_DCACHE_LOGBLOCKSZ(r10)
> > - srw. r8,r8,r9 /* compute line count */
> > + srd. r8,r8,r9 /* compute line count */
> > crclr cr0*4+so
> > beqlr /* nothing to do? */
> > mtctr r8
> > @@ -52,7 +52,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
> > subf r8,r6,r4 /* compute length */
> > add r8,r8,r5
> > lwz r9,CFG_ICACHE_LOGBLOCKSZ(r10)
> > - srw. r8,r8,r9 /* compute line count */
> > + srd. r8,r8,r9 /* compute line count */
> > crclr cr0*4+so
> > beqlr /* nothing to do? */
> > mtctr r8
> > --
> > 2.21.0
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819