Re: [PATCH 2/2] powerpc/64s: Trim offlined CPUs from mm_cpumasks

Excerpts from Michael Ellerman's message of December 14, 2020 8:43 pm:

Nicholas Piggin [off-list ref] writes:

quoted

Excerpts from Geert Uytterhoeven's message of December 10, 2020 7:06 pm:

quoted

Hi Nicholas,

On Fri, Nov 20, 2020 at 4:01 AM Nicholas Piggin [off-list ref] wrote:

quoted

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!

quoted

--- 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