[RFC & PATCH] fixing tlb flush race problem on smp

[RFC & PATCH] fixing tlb flush race problem on smp

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]


Many of us are aware of a hole in current TLB flushing code that
could cause processes using the same ASID for a SMP machine.

Actually there are several problems:

1) get_new_mmu_context() and following set_entryhi, etc are
not called automically in switch_mm() and active_mm().  If
an IPI happens and request to flush local tlb, bad things happen.

2) if local_flush_tlb_range() and local_flush_tlb_mm() are 
called from an IPI, they may call get_new_mmu_context() which
can bump up the ASID generation number with current active_mm
totally not aware of it.  Bad things will happen later.

3) during the time window after schedule() calling switch_mm()
before switch_to(), current->active_mm may be valid but does
really mean "current->active_mm" anymore.  This is because
the "current" process will soon become "prev".  The real active_mm
is actually "next->active_mm".  Because of this, it is not
enough for those two IPI'ed flushing routines to just check
again current->active_mm.  Long story made short - bad
things will happen.

It turns out that other arches have similar problems and solved
it in various ways.  Unfortunely I like none of them.

Here is one I am pretty happy with.  It is very small and efficient.
And conceptually it is clean too.  We basically keep the semantics
of ->mm and ->active_mm unchanged and only introduce a new bit
to mark which mm is the true owner of mmu hardware on a cpu.

The only downside is that cpu_vm_mask variable does not really
mean "mask for blocking IPI" in this approach.  It actually 
indicates whether current->active_mm is really active or not.  

Tested and passed the notorious fork/malloc test.

Let me know what you think.

Jun


[-- Attachment #2: junk --]
[-- Type: text/plain, Size: 2335 bytes --]

diff -Nru link/arch/mips/mm/tlb-sb1.c.orig link/arch/mips/mm/tlb-sb1.c
--- link/arch/mips/mm/tlb-sb1.c.orig	Tue Jan 21 13:54:59 2003
+++ link/arch/mips/mm/tlb-sb1.c	Tue Jan 21 13:58:50 2003
@@ -172,9 +172,13 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
+			if (mm == current->active_mm) {
+				get_new_mmu_context(mm, cpu);
 				write_c0_entryhi(cpu_asid(cpu, mm));
+			} else {
+				/* drop the current context completely */
+				CPU_CONTEXT(cpu, mm) = 0;
+			}
 		}
 	}
 	__restore_flags(flags);
@@ -258,9 +262,12 @@
 	__save_and_cli(flags);
 	cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
 		if (mm == current->active_mm) {
+			get_new_mmu_context(mm, smp_processor_id());
 			write_c0_entryhi(cpu_asid(cpu, mm));
+		} else {
+			/* drop the current context completely */
+			CPU_CONTEXT(cpu, mm) = 0;
 		}
 	}
 	__restore_flags(flags);
diff -Nru link/include/asm-mips/mmu_context.h.orig link/include/asm-mips/mmu_context.h
--- link/include/asm-mips/mmu_context.h.orig	Tue Jan 21 13:55:43 2003
+++ link/include/asm-mips/mmu_context.h	Tue Jan 21 14:01:19 2003
@@ -89,12 +89,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	__save_and_cli(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	__restore_flags(flags);
 }
 
 /*
@@ -112,11 +125,17 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	__save_and_cli(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	__restore_flags(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Juan Quintela]

>>>>> "jun" == Jun Sun <jsun@mvista.com> writes:

Hi
        just nickpitting.


jun> +		} else {
jun> +			/* drop the current context completely */
jun> +			CPU_CONTEXT(cpu, mm) = 0;
jun> }
jun> }
jun> __restore_flags(flags);

Perhaps creating a inline function for this, as the code is identical
in both branches?


jun> diff -Nru link/include/asm-mips/mmu_context.h.orig link/include/asm-mips/mmu_context.h
jun> --- link/include/asm-mips/mmu_context.h.orig	Tue Jan 21 13:55:43 2003
jun> +++ link/include/asm-mips/mmu_context.h	Tue Jan 21 14:01:19 2003
jun> @@ -89,12 +89,25 @@
jun> static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
jun> struct task_struct *tsk, unsigned cpu)
jun> {
jun> +	unsigned long flags;
jun> +
jun> +	__save_and_cli(flags);

s/__save_and_cli()/local_irq_save()/

jun> +	__restore_flags(flags);

s/__restore_flags()/local_irq_restore()/

Same in the other occurence, please.

Later, Juan.

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]


Thanks, Juan.

I also find a stupid typo and a subtle hole in my original patch.
Here is an updated version, for 2.4/mips only.  If it looks ok, I 
will extend to other sub-arches/trees.

This new one is pretty nice in that all mmu related operations
are put into one file and it is much easier to ensure correctness
later.

Jun

[-- Attachment #2: 030127-fix-smp-tlb-flush-holes.patch --]
[-- Type: text/plain, Size: 2579 bytes --]

diff -Nru link/arch/mips/mm/tlb-sb1.c.orig link/arch/mips/mm/tlb-sb1.c
--- link/arch/mips/mm/tlb-sb1.c.orig	Tue Jan 21 13:54:59 2003
+++ link/arch/mips/mm/tlb-sb1.c	Mon Jan 27 16:58:54 2003
@@ -172,9 +172,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	__restore_flags(flags);
@@ -258,10 +256,7 @@
 	__save_and_cli(flags);
 	cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		}
+		drop_mmu_context(mm, cpu);
 	}
 	__restore_flags(flags);
 }
diff -Nru link/include/asm-mips/mmu_context.h.orig link/include/asm-mips/mmu_context.h
--- link/include/asm-mips/mmu_context.h.orig	Tue Jan 21 13:55:43 2003
+++ link/include/asm-mips/mmu_context.h	Mon Jan 27 16:56:44 2003
@@ -89,12 +89,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -112,11 +125,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Ralf Baechle]

On Wed, Jan 22, 2003 at 08:43:26AM +0100, Juan Quintela wrote:

> jun> +	__save_and_cli(flags);
> 
> s/__save_and_cli()/local_irq_save()/
> 
> jun> +	__restore_flags(flags);
> 
> s/__restore_flags()/local_irq_restore()/
> 
> Same in the other occurence, please.

I've already done this recently for large parts of arch/mips* and
include/asm-mips*.

  Ralf

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Ralf Baechle]

On Mon, Jan 27, 2003 at 05:03:46PM -0800, Jun Sun wrote:

> I also find a stupid typo and a subtle hole in my original patch.
> Here is an updated version, for 2.4/mips only.  If it looks ok, I 
> will extend to other sub-arches/trees.
> 
> This new one is pretty nice in that all mmu related operations
> are put into one file and it is much easier to ensure correctness
> later.

I like this one.

> +
> +	/*
> +	 * Mark current->active_mm as not "active" anymore.
> +	 * We don't want to mislead possible IPI tlb flush routines.
> +	 */
> +	clear_bit(cpu, &prev->cpu_vm_mask);
> +	set_bit(cpu, &next->cpu_vm_mask);
> +
> +	local_irq_restore(flags);

I don't think it's necessary to protect the clear_bit and set_bit operations
with local_irq_save ... local_irq_restore.

In addition because switch_mm is always called with interrupts enabled you
can simplify that to local_irq_disable ... local_irq_enable.

  Ralf

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]


Here is a complete patch for both mips/mips64, 2.4 and 
2.5.  Of course only 2.4/mips combo is tested.

The clear_bit/set_bit actually need to be protected.
The flag setting needs to be in sync with the actual
hardware setting (set_entry_hi/set current pgd).  Otherwise
an tlb flushing IPI may does the wrong thing.

Jun

On Wed, Jan 29, 2003 at 09:06:27AM +0100, Ralf Baechle wrote:
> On Mon, Jan 27, 2003 at 05:03:46PM -0800, Jun Sun wrote:
> 
> > I also find a stupid typo and a subtle hole in my original patch.
> > Here is an updated version, for 2.4/mips only.  If it looks ok, I 
> > will extend to other sub-arches/trees.
> > 
> > This new one is pretty nice in that all mmu related operations
> > are put into one file and it is much easier to ensure correctness
> > later.
> 
> I like this one.
> 
> > +
> > +	/*
> > +	 * Mark current->active_mm as not "active" anymore.
> > +	 * We don't want to mislead possible IPI tlb flush routines.
> > +	 */
> > +	clear_bit(cpu, &prev->cpu_vm_mask);
> > +	set_bit(cpu, &next->cpu_vm_mask);
> > +
> > +	local_irq_restore(flags);
> 
> I don't think it's necessary to protect the clear_bit and set_bit operations
> with local_irq_save ... local_irq_restore.
> 
> In addition because switch_mm is always called with interrupts enabled you
> can simplify that to local_irq_disable ... local_irq_enable.
> 
>   Ralf
> 

[-- Attachment #2: 030204-2.4-smp-tlb-flush.patch --]
[-- Type: text/plain, Size: 9209 bytes --]

diff -Nru linux/arch/mips/mm/tlb-sb1.c.orig linux/arch/mips/mm/tlb-sb1.c
--- linux/arch/mips/mm/tlb-sb1.c.orig	Tue Feb  4 13:50:55 2003
+++ linux/arch/mips/mm/tlb-sb1.c	Tue Feb  4 14:01:24 2003
@@ -172,9 +172,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -253,17 +251,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips/mm/tlb-r4k.c.orig linux/arch/mips/mm/tlb-r4k.c
--- linux/arch/mips/mm/tlb-r4k.c.orig	Mon Jan 27 17:13:31 2003
+++ linux/arch/mips/mm/tlb-r4k.c	Tue Feb  4 14:15:04 2003
@@ -76,16 +76,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -133,9 +127,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips/mm/tlb-r3k.c.orig linux/arch/mips/mm/tlb-r3k.c
--- linux/arch/mips/mm/tlb-r3k.c.orig	Mon Jan 27 17:13:31 2003
+++ linux/arch/mips/mm/tlb-r3k.c	Tue Feb  4 14:09:04 2003
@@ -69,16 +69,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%lu>]", (unsigned long)cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm, cpu);
 	}
 }
 
@@ -119,9 +113,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-sb1.c.orig linux/arch/mips64/mm/tlb-sb1.c
--- linux/arch/mips64/mm/tlb-sb1.c.orig	Mon Jan 27 17:13:34 2003
+++ linux/arch/mips64/mm/tlb-sb1.c	Tue Feb  4 14:45:58 2003
@@ -180,9 +180,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -231,17 +229,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips64/mm/tlb-r4k.c.orig linux/arch/mips64/mm/tlb-r4k.c
--- linux/arch/mips64/mm/tlb-r4k.c.orig	Mon Jan 27 17:13:34 2003
+++ linux/arch/mips64/mm/tlb-r4k.c	Tue Feb  4 14:47:52 2003
@@ -80,16 +80,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -137,9 +131,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-andes.c.orig linux/arch/mips64/mm/tlb-andes.c
--- linux/arch/mips64/mm/tlb-andes.c.orig	Tue Feb  4 13:13:43 2003
+++ linux/arch/mips64/mm/tlb-andes.c	Tue Feb  4 14:49:59 2003
@@ -53,18 +53,12 @@
 
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		unsigned long flags;
-
+	int cpu = smp_processor_id();
+	if (cpu_context(cpu, mm) != 0) {
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-				    & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -106,10 +100,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-					    & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/include/asm-mips/mmu_context.h.orig linux/include/asm-mips/mmu_context.h
--- linux/include/asm-mips/mmu_context.h.orig	Tue Feb  4 13:50:55 2003
+++ linux/include/asm-mips/mmu_context.h	Tue Feb  4 13:51:03 2003
@@ -89,12 +89,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -112,11 +125,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */
diff -Nru linux/include/asm-mips64/mmu_context.h.orig linux/include/asm-mips64/mmu_context.h
--- linux/include/asm-mips64/mmu_context.h.orig	Tue Jan 21 13:55:43 2003
+++ linux/include/asm-mips64/mmu_context.h	Tue Feb  4 14:46:00 2003
@@ -80,12 +80,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -103,11 +116,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

[-- Attachment #3: 030204-2.5-smp-tlb-flush.patch --]
[-- Type: text/plain, Size: 9558 bytes --]

diff -Nru linux/arch/mips/mm/tlb-sb1.c.orig linux/arch/mips/mm/tlb-sb1.c
--- linux/arch/mips/mm/tlb-sb1.c.orig	Wed Dec 11 17:04:17 2002
+++ linux/arch/mips/mm/tlb-sb1.c	Tue Feb  4 15:00:53 2003
@@ -172,15 +172,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm) {
-				write_c0_entryhi(cpu_asid(cpu, mm));
-			} else if (!(cpu_asid(cpu, mm)) &&
-				   cpu_context(cpu, current->active_mm)) {
-				/* Just wrapped ASIDs, bump the active one */
-				get_new_mmu_context(current->active_mm, cpu);
-				write_c0_entryhi(cpu_context(cpu, current->active_mm)& 0xff);
-			}
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -325,17 +317,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips/mm/tlb-r4k.c.orig linux/arch/mips/mm/tlb-r4k.c
--- linux/arch/mips/mm/tlb-r4k.c.orig	Mon Jan 27 18:03:21 2003
+++ linux/arch/mips/mm/tlb-r4k.c	Tue Feb  4 15:04:29 2003
@@ -76,16 +76,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -134,9 +128,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips/mm/tlb-r3k.c.orig linux/arch/mips/mm/tlb-r3k.c
--- linux/arch/mips/mm/tlb-r3k.c.orig	Mon Jan 27 18:03:21 2003
+++ linux/arch/mips/mm/tlb-r3k.c	Tue Feb  4 15:05:22 2003
@@ -69,16 +69,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%lu>]", (unsigned long)cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm, cpu);
 	}
 }
 
@@ -120,9 +114,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-sb1.c.orig linux/arch/mips64/mm/tlb-sb1.c
--- linux/arch/mips64/mm/tlb-sb1.c.orig	Wed Dec 11 17:04:19 2002
+++ linux/arch/mips64/mm/tlb-sb1.c	Tue Feb  4 15:07:42 2003
@@ -180,9 +180,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -268,17 +266,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips64/mm/tlb-r4k.c.orig linux/arch/mips64/mm/tlb-r4k.c
--- linux/arch/mips64/mm/tlb-r4k.c.orig	Mon Jan 27 18:03:22 2003
+++ linux/arch/mips64/mm/tlb-r4k.c	Tue Feb  4 15:08:21 2003
@@ -80,16 +80,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -138,9 +132,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-andes.c.orig linux/arch/mips64/mm/tlb-andes.c
--- linux/arch/mips64/mm/tlb-andes.c.orig	Tue Feb  4 13:14:43 2003
+++ linux/arch/mips64/mm/tlb-andes.c	Tue Feb  4 14:53:28 2003
@@ -53,18 +53,12 @@
 
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		unsigned long flags;
-
+	int cpu = smp_processor_id();
+	if (cpu_context(cpu, mm) != 0) {
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-				    & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -108,10 +102,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-					    & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/include/asm-mips/mmu_context.h.orig linux/include/asm-mips/mmu_context.h
--- linux/include/asm-mips/mmu_context.h.orig	Mon Jan 27 18:03:23 2003
+++ linux/include/asm-mips/mmu_context.h	Tue Feb  4 14:53:28 2003
@@ -92,12 +92,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -115,11 +128,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */
diff -Nru linux/include/asm-mips64/mmu_context.h.orig linux/include/asm-mips64/mmu_context.h
--- linux/include/asm-mips64/mmu_context.h.orig	Mon Jan 27 18:03:23 2003
+++ linux/include/asm-mips64/mmu_context.h	Tue Feb  4 14:53:28 2003
@@ -83,12 +83,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -106,11 +119,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]


Here is a complete patch for both mips/mips64, 2.4 and 
2.5.  Of course only 2.4/mips combo is tested.

The clear_bit/set_bit actually need to be protected.
The flag setting needs to be in sync with the actual
hardware setting (set_entry_hi/set current pgd).  Otherwise
an tlb flushing IPI may does the wrong thing.

Jun

On Wed, Jan 29, 2003 at 09:06:27AM +0100, Ralf Baechle wrote:
> On Mon, Jan 27, 2003 at 05:03:46PM -0800, Jun Sun wrote:
> 
> > I also find a stupid typo and a subtle hole in my original patch.
> > Here is an updated version, for 2.4/mips only.  If it looks ok, I 
> > will extend to other sub-arches/trees.
> > 
> > This new one is pretty nice in that all mmu related operations
> > are put into one file and it is much easier to ensure correctness
> > later.
> 
> I like this one.
> 
> > +
> > +	/*
> > +	 * Mark current->active_mm as not "active" anymore.
> > +	 * We don't want to mislead possible IPI tlb flush routines.
> > +	 */
> > +	clear_bit(cpu, &prev->cpu_vm_mask);
> > +	set_bit(cpu, &next->cpu_vm_mask);
> > +
> > +	local_irq_restore(flags);
> 
> I don't think it's necessary to protect the clear_bit and set_bit operations
> with local_irq_save ... local_irq_restore.
> 
> In addition because switch_mm is always called with interrupts enabled you
> can simplify that to local_irq_disable ... local_irq_enable.
> 
>   Ralf
> 

[-- Attachment #2: 030204-2.4-smp-tlb-flush.patch --]
[-- Type: text/plain, Size: 9209 bytes --]

diff -Nru linux/arch/mips/mm/tlb-sb1.c.orig linux/arch/mips/mm/tlb-sb1.c
--- linux/arch/mips/mm/tlb-sb1.c.orig	Tue Feb  4 13:50:55 2003
+++ linux/arch/mips/mm/tlb-sb1.c	Tue Feb  4 14:01:24 2003
@@ -172,9 +172,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -253,17 +251,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips/mm/tlb-r4k.c.orig linux/arch/mips/mm/tlb-r4k.c
--- linux/arch/mips/mm/tlb-r4k.c.orig	Mon Jan 27 17:13:31 2003
+++ linux/arch/mips/mm/tlb-r4k.c	Tue Feb  4 14:15:04 2003
@@ -76,16 +76,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -133,9 +127,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips/mm/tlb-r3k.c.orig linux/arch/mips/mm/tlb-r3k.c
--- linux/arch/mips/mm/tlb-r3k.c.orig	Mon Jan 27 17:13:31 2003
+++ linux/arch/mips/mm/tlb-r3k.c	Tue Feb  4 14:09:04 2003
@@ -69,16 +69,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%lu>]", (unsigned long)cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm, cpu);
 	}
 }
 
@@ -119,9 +113,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-sb1.c.orig linux/arch/mips64/mm/tlb-sb1.c
--- linux/arch/mips64/mm/tlb-sb1.c.orig	Mon Jan 27 17:13:34 2003
+++ linux/arch/mips64/mm/tlb-sb1.c	Tue Feb  4 14:45:58 2003
@@ -180,9 +180,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -231,17 +229,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips64/mm/tlb-r4k.c.orig linux/arch/mips64/mm/tlb-r4k.c
--- linux/arch/mips64/mm/tlb-r4k.c.orig	Mon Jan 27 17:13:34 2003
+++ linux/arch/mips64/mm/tlb-r4k.c	Tue Feb  4 14:47:52 2003
@@ -80,16 +80,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -137,9 +131,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-andes.c.orig linux/arch/mips64/mm/tlb-andes.c
--- linux/arch/mips64/mm/tlb-andes.c.orig	Tue Feb  4 13:13:43 2003
+++ linux/arch/mips64/mm/tlb-andes.c	Tue Feb  4 14:49:59 2003
@@ -53,18 +53,12 @@
 
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		unsigned long flags;
-
+	int cpu = smp_processor_id();
+	if (cpu_context(cpu, mm) != 0) {
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-				    & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -106,10 +100,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-					    & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/include/asm-mips/mmu_context.h.orig linux/include/asm-mips/mmu_context.h
--- linux/include/asm-mips/mmu_context.h.orig	Tue Feb  4 13:50:55 2003
+++ linux/include/asm-mips/mmu_context.h	Tue Feb  4 13:51:03 2003
@@ -89,12 +89,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -112,11 +125,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */
diff -Nru linux/include/asm-mips64/mmu_context.h.orig linux/include/asm-mips64/mmu_context.h
--- linux/include/asm-mips64/mmu_context.h.orig	Tue Jan 21 13:55:43 2003
+++ linux/include/asm-mips64/mmu_context.h	Tue Feb  4 14:46:00 2003
@@ -80,12 +80,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -103,11 +116,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

[-- Attachment #3: 030204-2.5-smp-tlb-flush.patch --]
[-- Type: text/plain, Size: 9558 bytes --]

diff -Nru linux/arch/mips/mm/tlb-sb1.c.orig linux/arch/mips/mm/tlb-sb1.c
--- linux/arch/mips/mm/tlb-sb1.c.orig	Wed Dec 11 17:04:17 2002
+++ linux/arch/mips/mm/tlb-sb1.c	Tue Feb  4 15:00:53 2003
@@ -172,15 +172,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm) {
-				write_c0_entryhi(cpu_asid(cpu, mm));
-			} else if (!(cpu_asid(cpu, mm)) &&
-				   cpu_context(cpu, current->active_mm)) {
-				/* Just wrapped ASIDs, bump the active one */
-				get_new_mmu_context(current->active_mm, cpu);
-				write_c0_entryhi(cpu_context(cpu, current->active_mm)& 0xff);
-			}
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -325,17 +317,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips/mm/tlb-r4k.c.orig linux/arch/mips/mm/tlb-r4k.c
--- linux/arch/mips/mm/tlb-r4k.c.orig	Mon Jan 27 18:03:21 2003
+++ linux/arch/mips/mm/tlb-r4k.c	Tue Feb  4 15:04:29 2003
@@ -76,16 +76,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -134,9 +128,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips/mm/tlb-r3k.c.orig linux/arch/mips/mm/tlb-r3k.c
--- linux/arch/mips/mm/tlb-r3k.c.orig	Mon Jan 27 18:03:21 2003
+++ linux/arch/mips/mm/tlb-r3k.c	Tue Feb  4 15:05:22 2003
@@ -69,16 +69,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%lu>]", (unsigned long)cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm, cpu);
 	}
 }
 
@@ -120,9 +114,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-sb1.c.orig linux/arch/mips64/mm/tlb-sb1.c
--- linux/arch/mips64/mm/tlb-sb1.c.orig	Wed Dec 11 17:04:19 2002
+++ linux/arch/mips64/mm/tlb-sb1.c	Tue Feb  4 15:07:42 2003
@@ -180,9 +180,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
+			drop_mmu_context(mm, cpu);
 		}
 	}
 	local_irq_restore(flags);
@@ -268,17 +266,10 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	unsigned long flags;
-	int cpu;
-	local_irq_save(flags);
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 	if (cpu_context(cpu, mm) != 0) {
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm) {
-			write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
-		}
+		drop_mmu_context(mm, cpu);
 	}
-	local_irq_restore(flags);
 }
 
 /* Stolen from mips32 routines */
diff -Nru linux/arch/mips64/mm/tlb-r4k.c.orig linux/arch/mips64/mm/tlb-r4k.c
--- linux/arch/mips64/mm/tlb-r4k.c.orig	Mon Jan 27 18:03:22 2003
+++ linux/arch/mips64/mm/tlb-r4k.c	Tue Feb  4 15:08:21 2003
@@ -80,16 +80,10 @@
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", cpu_context(cpu, mm));
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, cpu);
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_asid(cpu, mm));
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -138,9 +132,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, cpu);
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_asid(cpu, mm));
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/arch/mips64/mm/tlb-andes.c.orig linux/arch/mips64/mm/tlb-andes.c
--- linux/arch/mips64/mm/tlb-andes.c.orig	Tue Feb  4 13:14:43 2003
+++ linux/arch/mips64/mm/tlb-andes.c	Tue Feb  4 14:53:28 2003
@@ -53,18 +53,12 @@
 
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		unsigned long flags;
-
+	int cpu = smp_processor_id();
+	if (cpu_context(cpu, mm) != 0) {
 #ifdef DEBUG_TLB
 		printk("[tlbmm<%d>]", mm->context);
 #endif
-		local_irq_save(flags);
-		get_new_mmu_context(mm, smp_processor_id());
-		if (mm == current->active_mm)
-			write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-				    & ASID_MASK);
-		local_irq_restore(flags);
+		drop_mmu_context(mm,cpu);
 	}
 }
 
@@ -108,10 +102,7 @@
 			}
 			write_c0_entryhi(oldpid);
 		} else {
-			get_new_mmu_context(mm, smp_processor_id());
-			if (mm == current->active_mm)
-				write_c0_entryhi(cpu_context(smp_processor_id(), mm)
-					    & ASID_MASK);
+			drop_mmu_context(mm, cpu);
 		}
 		local_irq_restore(flags);
 	}
diff -Nru linux/include/asm-mips/mmu_context.h.orig linux/include/asm-mips/mmu_context.h
--- linux/include/asm-mips/mmu_context.h.orig	Mon Jan 27 18:03:23 2003
+++ linux/include/asm-mips/mmu_context.h	Tue Feb  4 14:53:28 2003
@@ -92,12 +92,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -115,11 +128,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */
diff -Nru linux/include/asm-mips64/mmu_context.h.orig linux/include/asm-mips64/mmu_context.h
--- linux/include/asm-mips64/mmu_context.h.orig	Mon Jan 27 18:03:23 2003
+++ linux/include/asm-mips64/mmu_context.h	Tue Feb  4 14:53:28 2003
@@ -83,12 +83,25 @@
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                              struct task_struct *tsk, unsigned cpu)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
 
 	write_c0_entryhi(cpu_context(cpu, next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+
+	/*
+	 * Mark current->active_mm as not "active" anymore.
+	 * We don't want to mislead possible IPI tlb flush routines.
+	 */
+	clear_bit(cpu, &prev->cpu_vm_mask);
+	set_bit(cpu, &next->cpu_vm_mask);
+
+	local_irq_restore(flags);
 }
 
 /*
@@ -106,11 +119,39 @@
 static inline void
 activate_mm(struct mm_struct *prev, struct mm_struct *next)
 {
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, smp_processor_id());
 
 	write_c0_entryhi(cpu_context(smp_processor_id(), next));
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
+	
+	local_irq_restore(flags);
+}
+
+/*
+ * If mm is currently active_mm, we can't really drop it.  Instead,
+ * we will get a new one for it.
+ */
+static inline void
+drop_mmu_context(struct mm_struct *mm, unsigned cpu)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (test_bit(cpu, &mm->cpu_vm_mask))  {
+		get_new_mmu_context(mm, cpu);
+		set_entryhi(CPU_CONTEXT(cpu, mm) & 0xff);
+	} else {
+		/* will get a new context next time */
+		CPU_CONTEXT(cpu, mm) = 0;
+	}
+
+	local_irq_restore(flags);
 }
 
 #endif /* _ASM_MMU_CONTEXT_H */

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Atsushi Nemoto]

>>>>> On Tue, 4 Feb 2003 16:02:50 -0800, Jun Sun <jsun@mvista.com> said:
jsun> Here is a complete patch for both mips/mips64, 2.4 and 2.5.  Of
jsun> course only 2.4/mips combo is tested.

The attached patch seems to break r3k codes.  Here is a patch to fix
it (only for 2.4/mips).

diff -ur linux-mips-cvs/include/asm-mips/mmu_context.h linux.new/include/asm-mips/mmu_context.h
--- linux-mips-cvs/include/asm-mips/mmu_context.h	Fri Feb 14 09:41:31 2003
+++ linux.new/include/asm-mips/mmu_context.h	Fri Feb 14 13:40:24 2003
@@ -151,7 +151,7 @@
 
 	if (test_bit(cpu, &mm->cpu_vm_mask))  {
 		get_new_mmu_context(mm, cpu);
-		write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
+		write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
 	} else {
 		/* will get a new context next time */
 		cpu_context(cpu, mm) = 0;
---
Atsushi Nemoto

Re: [RFC & PATCH] fixing tlb flush race problem on smp

[Maciej W. Rozycki]

On Fri, 14 Feb 2003, Atsushi Nemoto wrote:

> The attached patch seems to break r3k codes.  Here is a patch to fix
> it (only for 2.4/mips).
> 
> diff -ur linux-mips-cvs/include/asm-mips/mmu_context.h linux.new/include/asm-mips/mmu_context.h
> --- linux-mips-cvs/include/asm-mips/mmu_context.h	Fri Feb 14 09:41:31 2003
> +++ linux.new/include/asm-mips/mmu_context.h	Fri Feb 14 13:40:24 2003
> @@ -151,7 +151,7 @@
>  
>  	if (test_bit(cpu, &mm->cpu_vm_mask))  {
>  		get_new_mmu_context(mm, cpu);
> -		write_c0_entryhi(cpu_context(cpu, mm) & 0xff);
> +		write_c0_entryhi(cpu_context(cpu, mm) & ASID_MASK);
>  	} else {
>  		/* will get a new context next time */
>  		cpu_context(cpu, mm) = 0;

 I've checked in a slightly different fix.  Thanks for spotting the
problem.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +