Skip to contentSkip to conversation

sparc32: add the CPU to mm_cpumask in switch_mm

PATCH In review Imre Kaloz 2 participants 2 messages Sep 27

Subsystems (2)
lore
Share & tools
lore

Imre Kaloz

author
#0 lore raw

PATCH 1/1 sparc32: add the CPU to mm_cpumask in switch_mm

The SMP TLB and cache flushes in srmmu.c cross-call only the CPUs in mm_cpumask(), which only switch_to() updates. exec reaches switch_mm() through activate_mm(), so the exec'ing CPU is missing from the new mm's mask until its next context switch.

If a thread created meanwhile unmaps memory on another CPU, only that CPU's TLB is flushed, and stores through the first CPU's stale translations land in freed pages, as when a detached thread frees its own stack. Only sun4m cross-calls its TLB flushes.

Set the bit in switch_mm() as well. Tested on a dual-SuperSPARC SPARCstation 20.

This has been broken since 2.3.35.

---

Notes:
No Fixes: tag: this has been broken since 2.3.35, before the git history.

 arch/sparc/mm/srmmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/sparc/mm/srmmu.c +7 -0
index 9a74902ad181..7d423e5fdc94 100644
@@ -466,6 +466,13 @@ void switch_mm(struct mm_struct *old_mm, struct mm_struct *mm,
 {
 	unsigned long flags;
 
+	/*
+	 * The SMP TLB flushes cross-call only the CPUs in mm_cpumask().
+	 * switch_to() sets this CPU there, but activate_mm() at exec comes
+	 * straight here, and a CPU left out keeps stale user translations.
+	 */
+	cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
+
 	if (mm->context == NO_CONTEXT) {
 		spin_lock_irqsave(&srmmu_context_spinlock, flags);
 		alloc_context(old_mm, mm);

base-commit: 62f4c998b297cf233997a2b4cd6fc2d2df0319c9

Signature · 1 line
2.47.3
#1 lore raw

Hi Imre,

Quoted 46 lines · Imre Kaloz

The SMP TLB and cache flushes in srmmu.c cross-call only the CPUs in mm_cpumask(), which only switch_to() updates. exec reaches switch_mm() through activate_mm(), so the exec'ing CPU is missing from the new mm's mask until its next context switch.

If a thread created meanwhile unmaps memory on another CPU, only that CPU's TLB is flushed, and stores through the first CPU's stale translations land in freed pages, as when a detached thread frees its own stack. Only sun4m cross-calls its TLB flushes.

Set the bit in switch_mm() as well. Tested on a dual-SuperSPARC SPARCstation 20.

This has been broken since 2.3.35.

---

Notes:
No Fixes: tag: this has been broken since 2.3.35, before the git history.

 arch/sparc/mm/srmmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/sparc/mm/srmmu.c +7 -0
index 9a74902ad181..7d423e5fdc94 100644
@@ -466,6 +466,13 @@ void switch_mm(struct mm_struct *old_mm, struct mm_struct *mm,
 {
        unsigned long flags;
 
+       /*
+        * The SMP TLB flushes cross-call only the CPUs in mm_cpumask().
+        * switch_to() sets this CPU there, but activate_mm() at exec comes
+        * straight here, and a CPU left out keeps stale user translations.
+        */
+       cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
+
        if (mm->context == NO_CONTEXT) {
                spin_lock_irqsave(&srmmu_context_spinlock, flags);
                alloc_context(old_mm, mm);
Quoted 5 lines

base-commit: 62f4c998b297cf233997a2b4cd6fc2d2df0319c9 --
2.47.3

Nice catch! This looks correct, and it matches what I have been seeing here. I think
this is one of three mm_cpumask problems on sparc32, the other two are in a HyperSPARC series I am preparing: smp_flush_tlb_mm() narrows the mask under active_mm and can drop a CPU that is still running the mm, and the cache cross-calls are gated on the mask as well.

The last one does not go away once the mask is accurate. mm_cpumask records TLB residency, but on a virtually indexed write-back cache a dirty line outlives the TLB entry, so the mask cannot gate a cache flush at all. Broadcasting it unconditionally took a fork-heavy reproducer from 70 faults in 6258 forks to 0 in 5798 for about 7% fork throughput.

I'll build and test this on my SS-20.

Magnus