All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Nicholas Piggin <npiggin@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Anton Vorontsov <anton.vorontsov@linaro.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks
Date: Mon, 14 Dec 2020 21:43:21 +1100	[thread overview]
Message-ID: <87h7oozn06.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <1607919238.kj439g85v5.astroid@bobo.none>

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


WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Nicholas Piggin <npiggin@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Anton Vorontsov <anton.vorontsov@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks
Date: Mon, 14 Dec 2020 21:43:21 +1100	[thread overview]
Message-ID: <87h7oozn06.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <1607919238.kj439g85v5.astroid@bobo.none>

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


  reply	other threads:[~2020-12-14 10:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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  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-11-20 10:06   ` Peter Zijlstra
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-11-20  2:57   ` Nicholas Piggin
2020-12-10  9:06   ` Geert Uytterhoeven
2020-12-10  9:06     ` Geert Uytterhoeven
2020-12-14  4:15     ` Nicholas Piggin
2020-12-14  4:15       ` Nicholas Piggin
2020-12-14 10:43       ` Michael Ellerman [this message]
2020-12-14 10:43         ` Michael Ellerman
2020-12-14 11:04         ` Nicholas Piggin
2020-12-14 11:04           ` Nicholas Piggin
2020-12-15 10:33           ` Michael Ellerman
2020-12-15 10:33             ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h7oozn06.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=a.p.zijlstra@chello.nl \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=anton.vorontsov@linaro.org \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.