Re: [PATCH] lazy tlb: consolidate lazy tlb mm switching fix

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Nicholas Piggin <npiggin@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Sachin Sant <sachinp@linux.ibm.com>
Subject: Re: [PATCH] lazy tlb: consolidate lazy tlb mm switching fix
Date: Fri, 13 Oct 2023 13:12:22 +0000	[thread overview]
Message-ID: <340b4d49-df81-05ad-3753-c1e60ec0df06@csgroup.eu> (raw)
In-Reply-To: <20230607005622.583318-1-npiggin@gmail.com>

Le 07/06/2023 à 02:56, Nicholas Piggin a écrit :
> Fix an upstream powerpc bug that was discovered with a WARN_ON added in
> "lazy tlb: consolidate lazy tlb mm switching":
> 
>     Switching mm and tinkering with current->active_mm should be done with
>      irqs disabled. There is a path where exit_lazy_flush_tlb can be called
>      with irqs enabled:
> 
>          exit_lazy_flush_tlb
>          flush_type_needed
>          __flush_all_mm
>          tlb_finish_mmu
>          exit_mmap
> 
>      Which results in the switching being incorrectly with irqs enabled.
> 
>      Link: https://lore.kernel.org/linuxppc-dev/87a5xcgopc.fsf@mail.lhotse/T/#m105488939d0cd9f980978ed2fdeeb89bf731e673
>      Fixes: a665eec0a22e1 ("powerpc/64s/radix: Fix mm_cpumask trimming race vs k>
> 
> I will send that fix upstream, which will end up conflicting with these.
> The resolution will be to take akpm's side of the merge (with this patch).
> Or drop the patch when it conflicts and I'll resend it rebased on the fix.

This seems to badly conflict at the moment, is it still applicable ?

Christophe

> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/mm/book3s64/radix_tlb.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 90953cf9f648..8d71fe8f5cdb 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -795,8 +795,16 @@ void exit_lazy_flush_tlb(struct mm_struct *mm, bool always_flush)
>   		goto out;
>   
>   	if (current->active_mm == mm) {
> -		/* Is a kernel thread and is using mm as the lazy tlb */
> +		unsigned long flags;
> +
> +		/*
> +		 * It is a kernel thread and is using mm as the lazy tlb, so
> +		 * switch it to init_mm. This is not always called from IPI
> +		 * (e.g., flush_type_needed), so must disable irqs.
> +		 */
> +		local_irq_save(flags);
>   		kthread_end_lazy_tlb_mm();
> +		local_irq_restore(flags);
>   	}
>   
>   	/*