| Message ID | 20190926045419.22827-6-alastair@au1.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: convert cache asm to C | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch next (6edfc6487b474fe01857dc3f1a9cd701bb9b21c8) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
Comment below... On Thu, 26 Sep 2019 at 12:18, Alastair D'Silva <alastair@au1.ibm.com> wrote: > > From: Alastair D'Silva <alastair@d-silva.org> > > 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 <alastair@d-silva.org> > --- > 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 cff947cb2a84..a2758e473d58 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) { Here you ignore the function parameter "chunk" and use the define FLUSH_CHUNK_SIZE. You should do one or the other; use the parameter or remove it. > + 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 >
On Fri, 2019-09-27 at 08:37 +0200, Mark Marshall wrote: > Comment below... > > On Thu, 26 Sep 2019 at 12:18, Alastair D'Silva <alastair@au1.ibm.com> > wrote: > > From: Alastair D'Silva <alastair@d-silva.org> > > > > 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 <alastair@d-silva.org> > > --- > > 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 cff947cb2a84..a2758e473d58 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) { > Here you ignore the function parameter "chunk" and use the define > FLUSH_CHUNK_SIZE. > You should do one or the other; use the parameter or remove it. Good catch, thankyou :) > > + 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 > >
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index cff947cb2a84..a2758e473d58 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);