[PATCH]Preemption patch for 2.6

[PATCH]Preemption patch for 2.6

[Manish Lachwani]

Hello !

The attached patch incorporates preemption enable/disable in some parts
of the kernel. I have tested this on the Broadcom Sibyte. Please review
...

Thanks
Manish Lachwani

Index: linux-2.6.8.1/arch/mips/kernel/signal32.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/kernel/signal32.c
+++ linux-2.6.8.1/arch/mips/kernel/signal32.c
@@ -295,6 +295,8 @@
 
 	err |= __get_user(current->used_math, &sc->sc_used_math);
 
+	preempt_disable();
+
 	if (current->used_math) {
 		/* restore fpu context if we have used it before */
 		own_fpu();
@@ -304,6 +306,8 @@
 		lose_fpu();
 	}
 
+	preempt_enable();
+
 	return err;
 }
 
@@ -489,12 +493,16 @@
 	 * Save FPU state to signal context.  Signal handler will "inherit"
 	 * current FPU state.
 	 */
+	preempt_disable();
+
 	if (!is_fpu_owner()) {
 		own_fpu();
 		restore_fp(current);
 	}
 	err |= save_fp_context32(sc);
 
+	preempt_enable();
+
 out:
 	return err;
 }
Index: linux-2.6.8.1/arch/mips/mm/c-sb1.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/mm/c-sb1.c
+++ linux-2.6.8.1/arch/mips/mm/c-sb1.c
@@ -197,10 +197,14 @@
 	if (!(vma->vm_flags & VM_EXEC))
 		return;
 
+	preempt_disable();
+
 	addr &= PAGE_MASK;
 	args.vma = vma;
 	args.addr = addr;
 	on_each_cpu(sb1_flush_cache_page_ipi, (void *) &args, 1, 1);
+
+	preempt_enable();
 }
 #else
 void sb1_flush_cache_page(struct vm_area_struct *vma, unsigned long
addr)
@@ -243,7 +247,9 @@
 
 static void sb1___flush_cache_all(void)
 {
+	preempt_disable();
 	on_each_cpu(sb1___flush_cache_all_ipi, 0, 1, 1);
+	preempt_enable();
 }
 #else
 void sb1___flush_cache_all(void)
@@ -291,9 +297,13 @@
 {
 	struct flush_icache_range_args args;
 
+	preempt_disable();
+
 	args.start = start;
 	args.end = end;
 	on_each_cpu(sb1_flush_icache_range_ipi, &args, 1, 1);
+
+	preempt_enable();
 }
 #else
 void sb1_flush_icache_range(unsigned long start, unsigned long end)
@@ -348,9 +358,14 @@
 
 	if (!(vma->vm_flags & VM_EXEC))
 		return;
+
+	preempt_disable();
+
 	args.vma = vma;
 	args.page = page;
 	on_each_cpu(sb1_flush_icache_page_ipi, (void *) &args, 1, 1);
+
+	preempt_enable();
 }
 #else
 void sb1_flush_icache_page(struct vm_area_struct *vma, struct page
*page)
@@ -377,7 +392,9 @@
 
 static void sb1_flush_cache_sigtramp(unsigned long addr)
 {
+	preempt_disable();
 	on_each_cpu(sb1_flush_cache_sigtramp_ipi, (void *) addr, 1, 1);
+	preempt_enable();
 }
 #else
 void sb1_flush_cache_sigtramp(unsigned long addr)
Index: linux-2.6.8.1/arch/mips/mm/tlb-sb1.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/mm/tlb-sb1.c
+++ linux-2.6.8.1/arch/mips/mm/tlb-sb1.c
@@ -286,10 +286,17 @@
    these entries, we just bump the asid. */
 void local_flush_tlb_mm(struct mm_struct *mm)
 {
-	int cpu = smp_processor_id();
+	int cpu;
+
+	preempt_disable();
+
+	cpu = smp_processor_id();
+
 	if (cpu_context(cpu, mm) != 0) {
 		drop_mmu_context(mm, cpu);
 	}
+
+	preempt_enable();
 }
 
 /* Stolen from mips32 routines */
Index: linux-2.6.8.1/arch/mips/kernel/traps.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/kernel/traps.c
+++ linux-2.6.8.1/arch/mips/kernel/traps.c
@@ -411,6 +411,8 @@
 		goto sig;
 	}
 
+	preempt_disable();
+
 	if (ll_task == NULL || ll_task == current) {
 		ll_bit = 1;
 	} else {
@@ -418,6 +420,8 @@
 	}
 	ll_task = current;
 
+	preempt_enable();
+
 	regs->regs[(opcode & RT) >> 16] = value;
 
 	compute_return_epc(regs);
@@ -450,12 +454,18 @@
 		signal = SIGBUS;
 		goto sig;
 	}
+
+	preempt_disable();
+
 	if (ll_bit == 0 || ll_task != current) {
 		regs->regs[reg] = 0;
+		preempt_enable();
 		compute_return_epc(regs);
 		return;
 	}
 
+	preempt_enable();
+
 	if (put_user(regs->regs[reg], vaddr)) {
 		signal = SIGSEGV;
 		goto sig;
@@ -515,6 +525,8 @@
 	if (fcr31 & FPU_CSR_UNI_X) {
 		int sig;
 
+		preempt_disable();
+
 		/*
 	 	 * Unimplemented operation exception.  If we've got the full
 		 * software emulator on-board, let's use it...
@@ -540,6 +552,8 @@
 		/* Restore the hardware register state */
 		restore_fp(current);
 
+		preempt_enable();
+
 		/* If something went wrong, signal */
 		if (sig)
 			force_sig(sig, current);
@@ -659,6 +673,8 @@
 		break;
 
 	case 1:
+		preempt_disable();
+
 		own_fpu();
 		if (current->used_math) {	/* Using the FPU again.  */
 			restore_fp(current);
@@ -674,6 +690,8 @@
 				force_sig(sig, current);
 		}
 
+		preempt_enable();
+
 		return;
 
 	case 2:
Index: linux-2.6.8.1/arch/mips/kernel/process.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/kernel/process.c
+++ linux-2.6.8.1/arch/mips/kernel/process.c
@@ -99,10 +99,14 @@
 
 	childksp = (unsigned long)ti + THREAD_SIZE - 32;
 
+	preempt_disable();
+
 	if (is_fpu_owner()) {
 		save_fp(p);
 	}
 
+	preempt_enable();
+
 	/* set up new TSS. */
 	childregs = (struct pt_regs *) childksp - 1;
 	*childregs = *regs;
Index: linux-2.6.8.1/arch/mips/kernel/signal.c
===================================================================
--- linux-2.6.8.1.orig/arch/mips/kernel/signal.c
+++ linux-2.6.8.1/arch/mips/kernel/signal.c
@@ -178,6 +178,8 @@
 
 	err |= __get_user(current->used_math, &sc->sc_used_math);
 
+	preempt_disable();
+
 	if (current->used_math) {
 		/* restore fpu context if we have used it before */
 		own_fpu();
@@ -187,6 +189,8 @@
 		lose_fpu();
 	}
 
+	preempt_enable();
+
 	return err;
 }
 
@@ -320,12 +324,16 @@
 	 * Save FPU state to signal context.  Signal handler will "inherit"
 	 * current FPU state.
 	 */
+	preempt_disable();
+
 	if (!is_fpu_owner()) {
 		own_fpu();
 		restore_fp(current);
 	}
 	err |= save_fp_context(sc);
 
+	preempt_enable();
+
 out:
 	return err;
 }

Re: [PATCH]Preemption patch for 2.6

[Atsushi Nemoto]

>>>>> On 22 Oct 2004 11:06:43 -0700, Manish Lachwani <mlachwani@mvista.com> said:

mlachwani> The attached patch incorporates preemption enable/disable
mlachwani> in some parts of the kernel. I have tested this on the
mlachwani> Broadcom Sibyte. Please review ...

1. You add preempt_disable/preempt_enable to c-sb1.c and tlb-sb1.c.
   Those are SB1 specific issue?  If not, please fix other c-*.c and
   tlb-*.c same way.

2. fpu_emulator_cop1Handler and save/restore_fp_context contain
   calling of get_user/put_user which is not allowed during preempt
   disabled.  (But it might be a kernel bug.  Please refer recent
   discussion on this ML)  I will post revised patch again.

Anyway, thanks for your fixes.

---
Atsushi Nemoto

Re: [PATCH]Preemption patch for 2.6

[Ralf Baechle]

On Mon, Oct 25, 2004 at 12:28:50AM +0900, Atsushi Nemoto wrote:

> mlachwani> The attached patch incorporates preemption enable/disable
> mlachwani> in some parts of the kernel. I have tested this on the
> mlachwani> Broadcom Sibyte. Please review ...
> 
> 1. You add preempt_disable/preempt_enable to c-sb1.c and tlb-sb1.c.
>    Those are SB1 specific issue?  If not, please fix other c-*.c and
>    tlb-*.c same way.

This an SMP issue and only affects the SB1 code.

The other CPU for which CVS supports SMP is the R10000 family; thanks to
having nice caches it's immune mostly immune to this kind of issue.

  Ralf

Preemption in do_cpu (Re: [PATCH]Preemption patch for 2.6)

[Pavel Kiryukhin]

Hi,
the preempt_disable/preempt_enable sequence in do_cpu() [traps.c]
exists quite long (patch submitted in Oct. 2004), so it should be nothing
wrong there.

Can somebody please comment why use of preempt_disable/enable in do_cpu
will not result in "scheduling while atomic" for fpu-less cpu (with enabled
preemption).

The sequence looks like

do_cpu()
| preempt_disable()
| fpu_emulator_cop1Handler()
| | cond_reshed()
| | | schedule()  <------ scheduling while atomic


The proposed patch was tested for Sibyte, but it has fpu (AFAIK) and has no
fpu_emulator_cop1Handler called.

--
Thank you,
Pavel Kiryukhin                   mailto:vksavl@cityline.ru



Friday, October 22, 2004, 10:06:43 PM, you wrote:

ML> Hello !

ML> The attached patch incorporates preemption enable/disable in some parts
ML> of the kernel. I have tested this on the Broadcom Sibyte. Please review
ML> ...

ML> Thanks
ML> Manish Lachwani


<skip>

ML> Index: linux-2.6.8.1/arch/mips/kernel/traps.c
ML> ===================================================================
ML> --- linux-2.6.8.1.orig/arch/mips/kernel/traps.c
ML> +++ linux-2.6.8.1/arch/mips/kernel/traps.c

<skip>

ML>  case 1:
ML> +preempt_disable();
ML> +
ML>  own_fpu();
ML>  if (current->used_math) {	/* Using the FPU again.  */
ML>  restore_fp(current);
ML> @@ -674,6 +690,8 @@
ML>  force_sig(sig, current);
ML>  }
 
ML> +preempt_enable();
ML> +
ML>  return;
 
ML>  case 2:

<skip>

Re: Preemption in do_cpu (Re: [PATCH]Preemption patch for 2.6)

[Jun Sun]

On Mon, Apr 18, 2005 at 01:32:46PM +0400, Pavel Kiryukhin wrote:
> Hi,
> the preempt_disable/preempt_enable sequence in do_cpu() [traps.c]
> exists quite long (patch submitted in Oct. 2004), so it should be nothing
> wrong there.
> 
> Can somebody please comment why use of preempt_disable/enable in do_cpu
> will not result in "scheduling while atomic" for fpu-less cpu (with enabled
> preemption).
> 
> The sequence looks like
> 
> do_cpu()
> | preempt_disable()
> | fpu_emulator_cop1Handler()
> | | cond_reshed()
> | | | schedule()  <------ scheduling while atomic
> 
> 
> The proposed patch was tested for Sibyte, but it has fpu (AFAIK) and has no
> fpu_emulator_cop1Handler called.
>

fpu_emulator maintains global variables and in general is dangerous
to be preempted in the middle of processing.

The quick fix for this problem is probably to move preemption disabling/
enabling inside fpu_emulator_cop1Handler().

Better fix is probably to modify fpu emulator so that it is preemption
safe overall.

Jun

Re: Preemption in do_cpu (Re: [PATCH]Preemption patch for 2.6)

[Manish Lachwani]

Jun Sun wrote:

>On Mon, Apr 18, 2005 at 01:32:46PM +0400, Pavel Kiryukhin wrote:
>  
>
>>Hi,
>>the preempt_disable/preempt_enable sequence in do_cpu() [traps.c]
>>exists quite long (patch submitted in Oct. 2004), so it should be nothing
>>wrong there.
>>
>>Can somebody please comment why use of preempt_disable/enable in do_cpu
>>will not result in "scheduling while atomic" for fpu-less cpu (with enabled
>>preemption).
>>
>>The sequence looks like
>>
>>do_cpu()
>>| preempt_disable()
>>| fpu_emulator_cop1Handler()
>>| | cond_reshed()
>>| | | schedule()  <------ scheduling while atomic
>>
>>
>>The proposed patch was tested for Sibyte, but it has fpu (AFAIK) and has no
>>fpu_emulator_cop1Handler called.
>>
>>    
>>
>
>fpu_emulator maintains global variables and in general is dangerous
>to be preempted in the middle of processing.
>
>The quick fix for this problem is probably to move preemption disabling/
>enabling inside fpu_emulator_cop1Handler().
>
>Better fix is probably to modify fpu emulator so that it is preemption
>safe overall.
>
>Jun
>  
>
Missed this one ! I had a patch that enables preemption before the 
cond_resched and disables right after it. I forgot to send it to 
linux-mips though. But, I needed it to work on fpu-less CPU. My bad.

Thanks
Manish Lachwani

Re: Preemption in do_cpu

[Atsushi Nemoto]

>>>>> On Mon, 18 Apr 2005 14:20:21 -0700, Jun Sun <jsun@junsun.net> said:
jsun> fpu_emulator maintains global variables and in general is
jsun> dangerous to be preempted in the middle of processing.

jsun> The quick fix for this problem is probably to move preemption
jsun> disabling/ enabling inside fpu_emulator_cop1Handler().

Also, get_user/put_user should not be used with preempt disabled.

Here is Quick and dirty workaround (including some other preemption fixes):

http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=20041025.003619.92586674.anemo%40mba.ocn.ne.jp

jsun> Better fix is probably to modify fpu emulator so that it is
jsun> preemption safe overall.

Sure.  It will make fpu emulator SMP safe also.

---
Atsushi Nemoto