Skip to contentSkip to conversation

powerpc/64s/radix: Fix exit lazy tlb mm switch with irqs enabled

PATCH Applied Nicholas Piggin 3 participants 3 messages Jun 7, 2023 – Jul 3, 2023 PowerPC

lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/1 powerpc/64s/radix: Fix exit lazy tlb mm switch with irqs enabled

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.

Fixes a665eec0a22e1 ("powerpc/64s/radix: Fix mm_cpumask trimming race vs kthread_use_mm")

---
This sounds worse than it probably is, radix can likely tolerate an interrupt hitting in mm switch, and the active_mm update may not be racy in practice either. Still be good to backport it because I'm not 100% sure of that.

This path can be stressed by reducing tlb_mm_cpumask_trim_timer (e.g., to 3).

Thanks,
Nick

 arch/powerpc/mm/book3s64/radix_tlb.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
arch/powerpc/mm/book3s64/radix_tlb.c +9 -1
index ce804b7bf84e..0bd4866d9824 100644
@@ -795,12 +795,20 @@ void exit_lazy_flush_tlb(struct mm_struct *mm, bool always_flush)
 		goto out;
 
 	if (current->active_mm == mm) {
+		unsigned long flags;
+
 		WARN_ON_ONCE(current->mm != NULL);
-		/* Is a kernel thread and is using mm as the lazy tlb */
+		/*
+		 * 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);
 		mmgrab_lazy_tlb(&init_mm);
 		current->active_mm = &init_mm;
 		switch_mm_irqs_off(mm, &init_mm, current);
 		mmdrop_lazy_tlb(mm);
+		local_irq_restore(flags);
 	}
 
 	/*
Signature · 1 line
2.40.1
Tested-by on v1 1/1
#1 lore raw
Quoted 12 lines
Fixes a665eec0a22e1 ("powerpc/64s/radix: Fix mm_cpumask trimming race vs kthread_use_mm")

---
This sounds worse than it probably is, radix can likely tolerate an interrupt hitting in mm switch, and the active_mm update may not be racy in practice either. Still be good to backport it because I'm not 100% sure of that.

Quoted 9 lines

This path can be stressed by reducing tlb_mm_cpumask_trim_timer (e.g., to 3).

Thanks,
Nick

arch/powerpc/mm/book3s64/radix_tlb.c | 10 +++++++++-

1 file changed, 9 insertions(+), 1 deletion(-)

This patch fixes the reported warning.

Ran powerpc selftests (with default value for tlb_mm_cpumask_trim_timer as well as tlb_mm_cpumask_trim_timer=3 ). No new errors were observed.

- Sachin

#2 lore raw
In reply to Nicholas Piggin
Quoted 11 lines · Nicholas Piggin

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

[...]

Applied to powerpc/fixes.

[1/1] powerpc/64s/radix: Fix exit lazy tlb mm switch with irqs enabled

      https://git.kernel.org/powerpc/c/dfaed3e1fa7099de8de4e89cbe7eb9c1bca27dfe

cheers