* [PATCH 0/2] powerpc/64s: fix for CPU hotplug vs mm_cpumask bug @ 2020-11-20 2:57 Nicholas Piggin 2020-11-20 2:57 ` [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling Nicholas Piggin 2020-11-20 2:57 ` [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks Nicholas Piggin 0 siblings, 2 replies; 9+ messages in thread From: Nicholas Piggin @ 2020-11-20 2:57 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Thomas Gleixner, linux-kernel This fixes a race in powerpc mm_cpumask code, I hope the core kernel patch looks okay and we could take it through the powerpc tree with an ack from someone (Peter or Thomas, perhaps?) Thanks, Nick Nicholas Piggin (2): kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling powerpc/64s: Trim offlined CPUs from mm_cpumasks arch/powerpc/include/asm/book3s/64/mmu.h | 12 ++++++++++++ arch/powerpc/mm/book3s64/mmu_context.c | 20 ++++++++++++++++++++ arch/powerpc/platforms/powermac/smp.c | 2 ++ arch/powerpc/platforms/powernv/smp.c | 3 +++ arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 +++ kernel/cpu.c | 6 +++++- 6 files changed, 45 insertions(+), 1 deletion(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling 2020-11-20 2:57 [PATCH 0/2] powerpc/64s: fix for CPU hotplug vs mm_cpumask bug Nicholas Piggin @ 2020-11-20 2:57 ` Nicholas Piggin 2020-11-20 10:06 ` Peter Zijlstra 2020-11-20 2:57 ` [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks Nicholas Piggin 1 sibling, 1 reply; 9+ messages in thread From: Nicholas Piggin @ 2020-11-20 2:57 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Thomas Gleixner, linux-kernel powerpc keeps a counter in the mm which counts bits set in mm_cpumask as well as other things. This means it can't use generic code to clear bits out of the mask and doesn't adjust the arch specific counter. Add an arch override allowing powerpc to use clear_tasks_mm_cpumask(). Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- kernel/cpu.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/cpu.c b/kernel/cpu.c index 6ff2578ecf17..2b8d7a5db383 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -815,6 +815,10 @@ void __init cpuhp_threads_init(void) } #ifdef CONFIG_HOTPLUG_CPU +#ifndef arch_clear_mm_cpumask_cpu +#define arch_clear_mm_cpumask_cpu(cpu, mm) cpumask_clear_cpu(cpu, mm_cpumask(mm)) +#endif + /** * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU * @cpu: a CPU id @@ -850,7 +854,7 @@ void clear_tasks_mm_cpumask(int cpu) t = find_lock_task_mm(p); if (!t) continue; - cpumask_clear_cpu(cpu, mm_cpumask(t->mm)); + arch_clear_mm_cpumask_cpu(cpu, t->mm); task_unlock(t); } rcu_read_unlock(); -- 2.23.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling 2020-11-20 2:57 ` [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling Nicholas Piggin @ 2020-11-20 10:06 ` Peter Zijlstra 0 siblings, 0 replies; 9+ messages in thread From: Peter Zijlstra @ 2020-11-20 10:06 UTC (permalink / raw) To: Nicholas Piggin Cc: linuxppc-dev, Aneesh Kumar K.V, Anton Vorontsov, Thomas Gleixner, linux-kernel On Fri, Nov 20, 2020 at 12:57:56PM +1000, Nicholas Piggin wrote: > powerpc keeps a counter in the mm which counts bits set in mm_cpumask as > well as other things. This means it can't use generic code to clear bits > out of the mask and doesn't adjust the arch specific counter. > > Add an arch override allowing powerpc to use clear_tasks_mm_cpumask(). > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Seems reasonable enough.. Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/cpu.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/cpu.c b/kernel/cpu.c > index 6ff2578ecf17..2b8d7a5db383 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -815,6 +815,10 @@ void __init cpuhp_threads_init(void) > } > > #ifdef CONFIG_HOTPLUG_CPU > +#ifndef arch_clear_mm_cpumask_cpu > +#define arch_clear_mm_cpumask_cpu(cpu, mm) cpumask_clear_cpu(cpu, mm_cpumask(mm)) > +#endif > + > /** > * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU > * @cpu: a CPU id > @@ -850,7 +854,7 @@ void clear_tasks_mm_cpumask(int cpu) > t = find_lock_task_mm(p); > if (!t) > continue; > - cpumask_clear_cpu(cpu, mm_cpumask(t->mm)); > + arch_clear_mm_cpumask_cpu(cpu, t->mm); > task_unlock(t); > } > rcu_read_unlock(); > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-11-20 2:57 [PATCH 0/2] powerpc/64s: fix for CPU hotplug vs mm_cpumask bug Nicholas Piggin 2020-11-20 2:57 ` [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling Nicholas Piggin @ 2020-11-20 2:57 ` Nicholas Piggin 2020-12-10 9:06 ` Geert Uytterhoeven 1 sibling, 1 reply; 9+ messages in thread From: Nicholas Piggin @ 2020-11-20 2:57 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Thomas Gleixner, linux-kernel When offlining a CPU, powerpc/64s does not flush TLBs, rather it just leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs to manage its TLBs. However the exit_flush_lazy_tlbs() function expects that after returning, all CPUs (except self) have flushed TLBs for that mm, in which case TLBIEL can be used for this flush. This breaks for offline CPUs because they don't get the IPI to flush their TLB. This can lead to stale translations. Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs before going offline. These offlined CPU bits stuck in the cpumask also prevents the cpumask from being trimmed back to local mode, which means continual broadcast IPIs or TLBIEs are needed for TLB flushing. This patch prevents that situation too. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/book3s/64/mmu.h | 12 ++++++++++++ arch/powerpc/mm/book3s64/mmu_context.c | 20 ++++++++++++++++++++ arch/powerpc/platforms/powermac/smp.c | 2 ++ arch/powerpc/platforms/powernv/smp.c | 3 +++ arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 +++ 5 files changed, 40 insertions(+) diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index e0b52940e43c..750918451dd2 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.h @@ -242,6 +242,18 @@ extern void radix_init_pseries(void); static inline void radix_init_pseries(void) { }; #endif +#ifdef CONFIG_HOTPLUG_CPU +#define arch_clear_mm_cpumask_cpu(cpu, mm) \ + do { \ + if (cpumask_test_cpu(cpu, mm_cpumask(mm))) { \ + atomic_dec(&(mm)->context.active_cpus); \ + cpumask_clear_cpu(cpu, mm_cpumask(mm)); \ + } \ + } while (0) + +void cleanup_cpu_mmu_context(void); +#endif + static inline int get_user_context(mm_context_t *ctx, unsigned long ea) { int index = ea >> MAX_EA_BITS_PER_CONTEXT; diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c index 1c54821de7bf..0c8557220ae2 100644 --- a/arch/powerpc/mm/book3s64/mmu_context.c +++ b/arch/powerpc/mm/book3s64/mmu_context.c @@ -17,6 +17,7 @@ #include <linux/export.h> #include <linux/gfp.h> #include <linux/slab.h> +#include <linux/cpu.h> #include <asm/mmu_context.h> #include <asm/pgalloc.h> @@ -307,3 +308,22 @@ void radix__switch_mmu_context(struct mm_struct *prev, struct mm_struct *next) isync(); } #endif + +/** + * cleanup_cpu_mmu_context - Clean up MMU details for this CPU (newly offlined) + * + * This clears the CPU from mm_cpumask for all processes, and then flushes the + * local TLB to ensure TLB coherency in case the CPU is onlined again. + * + * KVM guest translations are not necessarily flushed here. If KVM started + * using mm_cpumask or the Linux APIs which do, this would have to be resolved. + */ +#ifdef CONFIG_HOTPLUG_CPU +void cleanup_cpu_mmu_context(void) +{ + int cpu = smp_processor_id(); + + clear_tasks_mm_cpumask(cpu); + tlbiel_all(); +} +#endif diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c index 74ebe664b016..adae2a6712e1 100644 --- a/arch/powerpc/platforms/powermac/smp.c +++ b/arch/powerpc/platforms/powermac/smp.c @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) mpic_cpu_set_priority(0xf); + cleanup_cpu_mmu_context(); + return 0; } diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c index 54c4ba45c7ce..cbb67813cd5d 100644 --- a/arch/powerpc/platforms/powernv/smp.c +++ b/arch/powerpc/platforms/powernv/smp.c @@ -143,6 +143,9 @@ static int pnv_smp_cpu_disable(void) xive_smp_disable_cpu(); else xics_migrate_irqs_away(); + + cleanup_cpu_mmu_context(); + return 0; } diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c index f2837e33bf5d..a02012f1b04a 100644 --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c @@ -90,6 +90,9 @@ static int pseries_cpu_disable(void) xive_smp_disable_cpu(); else xics_migrate_irqs_away(); + + cleanup_cpu_mmu_context(); + return 0; } -- 2.23.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-11-20 2:57 ` [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks Nicholas Piggin @ 2020-12-10 9:06 ` Geert Uytterhoeven 2020-12-14 4:15 ` Nicholas Piggin 0 siblings, 1 reply; 9+ messages in thread From: Geert Uytterhoeven @ 2020-12-10 9:06 UTC (permalink / raw) To: Nicholas Piggin Cc: linuxppc-dev, Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Thomas Gleixner, Linux Kernel Mailing List Hi Nicholas, On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin <npiggin@gmail.com> wrote: > > When offlining a CPU, powerpc/64s does not flush TLBs, rather it just > leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs > to manage its TLBs. > > However the exit_flush_lazy_tlbs() function expects that after > returning, all CPUs (except self) have flushed TLBs for that mm, in > which case TLBIEL can be used for this flush. This breaks for offline > CPUs because they don't get the IPI to flush their TLB. This can lead > to stale translations. > > Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs > before going offline. > > These offlined CPU bits stuck in the cpumask also prevents the cpumask > from being trimmed back to local mode, which means continual broadcast > IPIs or TLBIEs are needed for TLB flushing. This patch prevents that > situation too. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Thanks for your patch! > --- a/arch/powerpc/platforms/powermac/smp.c > +++ b/arch/powerpc/platforms/powermac/smp.c > @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) > > mpic_cpu_set_priority(0xf); > > + cleanup_cpu_mmu_context(); > + I guess this change broke pmac32_defconfig+SMP in v5.10-rc7? arch/powerpc/platforms/powermac/smp.c: error: implicit declaration of function 'cleanup_cpu_mmu_context' [-Werror=implicit-function-declaration]: => 914:2 http://kisskb.ellerman.id.au/kisskb/buildresult/14423174/ > return 0; > } > > diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c > index 54c4ba45c7ce..cbb67813cd5d 100644 > --- a/arch/powerpc/platforms/powernv/smp.c > +++ b/arch/powerpc/platforms/powernv/smp.c > @@ -143,6 +143,9 @@ static int pnv_smp_cpu_disable(void) > xive_smp_disable_cpu(); > else > xics_migrate_irqs_away(); > + > + cleanup_cpu_mmu_context(); > + > return 0; > } > > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c > index f2837e33bf5d..a02012f1b04a 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c > @@ -90,6 +90,9 @@ static int pseries_cpu_disable(void) > xive_smp_disable_cpu(); > else > xics_migrate_irqs_away(); > + > + cleanup_cpu_mmu_context(); > + > return 0; > } > > -- > 2.23.0 > -- Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-12-10 9:06 ` Geert Uytterhoeven @ 2020-12-14 4:15 ` Nicholas Piggin 2020-12-14 10:43 ` Michael Ellerman 0 siblings, 1 reply; 9+ messages in thread From: Nicholas Piggin @ 2020-12-14 4:15 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner Excerpts from Geert Uytterhoeven's message of December 10, 2020 7:06 pm: > Hi Nicholas, > > On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin <npiggin@gmail.com> wrote: >> >> When offlining a CPU, powerpc/64s does not flush TLBs, rather it just >> leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs >> to manage its TLBs. >> >> However the exit_flush_lazy_tlbs() function expects that after >> returning, all CPUs (except self) have flushed TLBs for that mm, in >> which case TLBIEL can be used for this flush. This breaks for offline >> CPUs because they don't get the IPI to flush their TLB. This can lead >> to stale translations. >> >> Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs >> before going offline. >> >> These offlined CPU bits stuck in the cpumask also prevents the cpumask >> from being trimmed back to local mode, which means continual broadcast >> IPIs or TLBIEs are needed for TLB flushing. This patch prevents that >> situation too. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Thanks for your patch! > >> --- a/arch/powerpc/platforms/powermac/smp.c >> +++ b/arch/powerpc/platforms/powermac/smp.c >> @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) >> >> mpic_cpu_set_priority(0xf); >> >> + cleanup_cpu_mmu_context(); >> + > > I guess this change broke pmac32_defconfig+SMP in v5.10-rc7? > > arch/powerpc/platforms/powermac/smp.c: error: implicit > declaration of function 'cleanup_cpu_mmu_context' > [-Werror=implicit-function-declaration]: => 914:2 > > http://kisskb.ellerman.id.au/kisskb/buildresult/14423174/ Hey, yeah it does thanks for catching it. This patch fixes it for me --- From a9b5ec92ffac975e81c6d7db6ff2b1486b2723f7 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin <npiggin@gmail.com> Date: Mon, 14 Dec 2020 13:52:39 +1000 Subject: [PATCH] powerpc/32s: Fix cleanup_cpu_mmu_context() compile bug 32s has no tlbiel_all() defined, so just disable the cleanup with a comment. Fixes: 01b0f0eae081 ("powerpc/64s: Trim offlined CPUs from mm_cpumasks") Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/platforms/powermac/smp.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c index adae2a6712e1..66ef5f8f4445 100644 --- a/arch/powerpc/platforms/powermac/smp.c +++ b/arch/powerpc/platforms/powermac/smp.c @@ -911,7 +911,16 @@ static int smp_core99_cpu_disable(void) mpic_cpu_set_priority(0xf); + /* + * Would be nice for consistency if all platforms clear mm_cpumask and + * flush TLBs on unplug, but the TLB invalidation bug described in + * commit 01b0f0eae081 ("powerpc/64s: Trim offlined CPUs from + * mm_cpumasks") only applies to 64s and for now we only have the TLB + * flush code for that platform. + */ +#ifdef CONFIG_PPC64 cleanup_cpu_mmu_context(); +#endif return 0; } -- 2.23.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-12-14 4:15 ` Nicholas Piggin @ 2020-12-14 10:43 ` Michael Ellerman 2020-12-14 11:04 ` Nicholas Piggin 0 siblings, 1 reply; 9+ messages in thread From: Michael Ellerman @ 2020-12-14 10:43 UTC (permalink / raw) To: Nicholas Piggin, Geert Uytterhoeven Cc: Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner Nicholas Piggin <npiggin@gmail.com> writes: > Excerpts from Geert Uytterhoeven's message of December 10, 2020 7:06 pm: >> Hi Nicholas, >> >> On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin <npiggin@gmail.com> wrote: >>> >>> When offlining a CPU, powerpc/64s does not flush TLBs, rather it just >>> leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs >>> to manage its TLBs. >>> >>> However the exit_flush_lazy_tlbs() function expects that after >>> returning, all CPUs (except self) have flushed TLBs for that mm, in >>> which case TLBIEL can be used for this flush. This breaks for offline >>> CPUs because they don't get the IPI to flush their TLB. This can lead >>> to stale translations. >>> >>> Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs >>> before going offline. >>> >>> These offlined CPU bits stuck in the cpumask also prevents the cpumask >>> from being trimmed back to local mode, which means continual broadcast >>> IPIs or TLBIEs are needed for TLB flushing. This patch prevents that >>> situation too. >>> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> >> Thanks for your patch! >> >>> --- a/arch/powerpc/platforms/powermac/smp.c >>> +++ b/arch/powerpc/platforms/powermac/smp.c >>> @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) >>> >>> mpic_cpu_set_priority(0xf); >>> >>> + cleanup_cpu_mmu_context(); >>> + >> >> I guess this change broke pmac32_defconfig+SMP in v5.10-rc7? >> >> arch/powerpc/platforms/powermac/smp.c: error: implicit >> declaration of function 'cleanup_cpu_mmu_context' >> [-Werror=implicit-function-declaration]: => 914:2 >> >> http://kisskb.ellerman.id.au/kisskb/buildresult/14423174/ > > Hey, yeah it does thanks for catching it. This patch fixes it for me > > --- > From a9b5ec92ffac975e81c6d7db6ff2b1486b2723f7 Mon Sep 17 00:00:00 2001 > From: Nicholas Piggin <npiggin@gmail.com> > Date: Mon, 14 Dec 2020 13:52:39 +1000 > Subject: [PATCH] powerpc/32s: Fix cleanup_cpu_mmu_context() compile bug > > 32s has no tlbiel_all() defined, so just disable the cleanup with a > comment. Or what about just: diff --git a/arch/powerpc/include/asm/book3s/32/mmu-hash.h b/arch/powerpc/include/asm/book3s/32/mmu-hash.h index 331187661236..685c589e723f 100644 --- a/arch/powerpc/include/asm/book3s/32/mmu-hash.h +++ b/arch/powerpc/include/asm/book3s/32/mmu-hash.h @@ -94,6 +94,7 @@ typedef struct { } mm_context_t; void update_bats(void); +static inline void cleanup_cpu_mmu_context(void) { }; /* patch sites */ extern s32 patch__hash_page_A0, patch__hash_page_A1, patch__hash_page_A2; cheers ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-12-14 10:43 ` Michael Ellerman @ 2020-12-14 11:04 ` Nicholas Piggin 2020-12-15 10:33 ` Michael Ellerman 0 siblings, 1 reply; 9+ messages in thread From: Nicholas Piggin @ 2020-12-14 11:04 UTC (permalink / raw) To: Geert Uytterhoeven, Michael Ellerman Cc: Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner Excerpts from Michael Ellerman's message of December 14, 2020 8:43 pm: > Nicholas Piggin <npiggin@gmail.com> writes: >> Excerpts from Geert Uytterhoeven's message of December 10, 2020 7:06 pm: >>> Hi Nicholas, >>> >>> On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin <npiggin@gmail.com> wrote: >>>> >>>> When offlining a CPU, powerpc/64s does not flush TLBs, rather it just >>>> leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs >>>> to manage its TLBs. >>>> >>>> However the exit_flush_lazy_tlbs() function expects that after >>>> returning, all CPUs (except self) have flushed TLBs for that mm, in >>>> which case TLBIEL can be used for this flush. This breaks for offline >>>> CPUs because they don't get the IPI to flush their TLB. This can lead >>>> to stale translations. >>>> >>>> Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs >>>> before going offline. >>>> >>>> These offlined CPU bits stuck in the cpumask also prevents the cpumask >>>> from being trimmed back to local mode, which means continual broadcast >>>> IPIs or TLBIEs are needed for TLB flushing. This patch prevents that >>>> situation too. >>>> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> >>> Thanks for your patch! >>> >>>> --- a/arch/powerpc/platforms/powermac/smp.c >>>> +++ b/arch/powerpc/platforms/powermac/smp.c >>>> @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) >>>> >>>> mpic_cpu_set_priority(0xf); >>>> >>>> + cleanup_cpu_mmu_context(); >>>> + >>> >>> I guess this change broke pmac32_defconfig+SMP in v5.10-rc7? >>> >>> arch/powerpc/platforms/powermac/smp.c: error: implicit >>> declaration of function 'cleanup_cpu_mmu_context' >>> [-Werror=implicit-function-declaration]: => 914:2 >>> >>> http://kisskb.ellerman.id.au/kisskb/buildresult/14423174/ >> >> Hey, yeah it does thanks for catching it. This patch fixes it for me >> >> --- >> From a9b5ec92ffac975e81c6d7db6ff2b1486b2723f7 Mon Sep 17 00:00:00 2001 >> From: Nicholas Piggin <npiggin@gmail.com> >> Date: Mon, 14 Dec 2020 13:52:39 +1000 >> Subject: [PATCH] powerpc/32s: Fix cleanup_cpu_mmu_context() compile bug >> >> 32s has no tlbiel_all() defined, so just disable the cleanup with a >> comment. > > Or what about just: That works, I kind of wanted it in there explicit that we don't clean up on 32s. I don't mind if you prefer this though. Thanks, Nick > > diff --git a/arch/powerpc/include/asm/book3s/32/mmu-hash.h b/arch/powerpc/include/asm/book3s/32/mmu-hash.h > index 331187661236..685c589e723f 100644 > --- a/arch/powerpc/include/asm/book3s/32/mmu-hash.h > +++ b/arch/powerpc/include/asm/book3s/32/mmu-hash.h > @@ -94,6 +94,7 @@ typedef struct { > } mm_context_t; > > void update_bats(void); > +static inline void cleanup_cpu_mmu_context(void) { }; > > /* patch sites */ > extern s32 patch__hash_page_A0, patch__hash_page_A1, patch__hash_page_A2; > > > cheers > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks 2020-12-14 11:04 ` Nicholas Piggin @ 2020-12-15 10:33 ` Michael Ellerman 0 siblings, 0 replies; 9+ messages in thread From: Michael Ellerman @ 2020-12-15 10:33 UTC (permalink / raw) To: Nicholas Piggin, Geert Uytterhoeven Cc: Aneesh Kumar K.V, Anton Vorontsov, Peter Zijlstra, Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner Nicholas Piggin <npiggin@gmail.com> writes: > Excerpts from Michael Ellerman's message of December 14, 2020 8:43 pm: >> Nicholas Piggin <npiggin@gmail.com> writes: >>> Excerpts from Geert Uytterhoeven's message of December 10, 2020 7:06 pm: >>>> Hi Nicholas, >>>> >>>> On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin <npiggin@gmail.com> wrote: >>>>> >>>>> When offlining a CPU, powerpc/64s does not flush TLBs, rather it just >>>>> leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs >>>>> to manage its TLBs. >>>>> >>>>> However the exit_flush_lazy_tlbs() function expects that after >>>>> returning, all CPUs (except self) have flushed TLBs for that mm, in >>>>> which case TLBIEL can be used for this flush. This breaks for offline >>>>> CPUs because they don't get the IPI to flush their TLB. This can lead >>>>> to stale translations. >>>>> >>>>> Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs >>>>> before going offline. >>>>> >>>>> These offlined CPU bits stuck in the cpumask also prevents the cpumask >>>>> from being trimmed back to local mode, which means continual broadcast >>>>> IPIs or TLBIEs are needed for TLB flushing. This patch prevents that >>>>> situation too. >>>>> >>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>>> >>>> Thanks for your patch! >>>> >>>>> --- a/arch/powerpc/platforms/powermac/smp.c >>>>> +++ b/arch/powerpc/platforms/powermac/smp.c >>>>> @@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void) >>>>> >>>>> mpic_cpu_set_priority(0xf); >>>>> >>>>> + cleanup_cpu_mmu_context(); >>>>> + >>>> >>>> I guess this change broke pmac32_defconfig+SMP in v5.10-rc7? >>>> >>>> arch/powerpc/platforms/powermac/smp.c: error: implicit >>>> declaration of function 'cleanup_cpu_mmu_context' >>>> [-Werror=implicit-function-declaration]: => 914:2 >>>> >>>> http://kisskb.ellerman.id.au/kisskb/buildresult/14423174/ >>> >>> Hey, yeah it does thanks for catching it. This patch fixes it for me >>> >>> --- >>> From a9b5ec92ffac975e81c6d7db6ff2b1486b2723f7 Mon Sep 17 00:00:00 2001 >>> From: Nicholas Piggin <npiggin@gmail.com> >>> Date: Mon, 14 Dec 2020 13:52:39 +1000 >>> Subject: [PATCH] powerpc/32s: Fix cleanup_cpu_mmu_context() compile bug >>> >>> 32s has no tlbiel_all() defined, so just disable the cleanup with a >>> comment. >> >> Or what about just: > > That works, I kind of wanted it in there explicit that we don't > clean up on 32s. I don't mind if you prefer this though. OK. I'll merge my version because I can do that without needing to merge master in order to get the broken commit into my tree. cheers ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-12-15 10:34 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-20 2:57 [PATCH 0/2] powerpc/64s: fix for CPU hotplug vs mm_cpumask bug Nicholas Piggin 2020-11-20 2:57 ` [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling Nicholas Piggin 2020-11-20 10:06 ` Peter Zijlstra 2020-11-20 2:57 ` [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks Nicholas Piggin 2020-12-10 9:06 ` Geert Uytterhoeven 2020-12-14 4:15 ` Nicholas Piggin 2020-12-14 10:43 ` Michael Ellerman 2020-12-14 11:04 ` Nicholas Piggin 2020-12-15 10:33 ` Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).