FPU context switching fix for SMP

FPU context switching fix for SMP

[Krzysztof Helt]

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

Here is a patch for FPU context switching in SMP.

A copy_thread() function clears PF_USEDFPU flag for a new thread,
but it does not clear the PSR_EF bit. Thus, a first FPU exception
called from the child thread is not handled properly. I made the
patch after PPC and MIPS architectures - both clear a similar bit
in copy_thread.
This bug does not exist in the UP kernel, probably because it
does not rely on PF_USEDFPU flag.

The patch below should be applied for both 2.4 and 2.6 branches.

I tested it with a test-fenv program from  glibc testsuite. The
test fails the same test cases in both SMP and UP with  the patch
applied. Without the patch, the test on SMP kernel failes in
almost two times more testcases.

Regards,
Krzysztof

----------------------------------------------------
Reklama Twojej strony WWW już od 20 groszy? - Oczywiście!
To nie jest żadna ściema! - Sprawdź:
http://klik.wp.pl/?adr=http%3A%2F%2Fbiznes.szukaj.wp.pl%2Fboksy.html&sid=299

[-- Attachment #2: copy_thread-fpu.24.patch --]
[-- Type: application/octet-stream, Size: 655 bytes --]

--- linux-2.4.29/arch/sparc/kernel/process.c.orig	2005-01-26 08:38:01.000000000 +0100
+++ linux-2.4.29/arch/sparc/kernel/process.c	2005-01-26 08:38:54.000000000 +0100
@@ -3,6 +3,8 @@
  *
  *  Copyright (C) 1995 David S. Miller (davem@caip.rutgers.edu)
  *  Copyright (C) 1996 Eddie C. Dost   (ecd@skynet.be)
+ *
+ *	2005-01-26	Krzysztof Helt	-	cleared PSR_EF in copy_thread
  */
 
 /*
@@ -512,6 +514,10 @@
 		}
 	}
 
+  	/* FPU must be disabled in SMP. */
+  	/* It does not hurt in UP either. */
+  	childregs->psr &= ~PSR_EF;
+  
 	/* Set the return value for the child. */
 	childregs->u_regs[UREG_I0] = current->pid;
 	childregs->u_regs[UREG_I1] = 1;

Re: FPU context switching fix for SMP

[William Lee Irwin III]

On Wed, Jan 26, 2005 at 09:24:42AM +0100, Krzysztof Helt wrote:
> Here is a patch for FPU context switching in SMP.
> A copy_thread() function clears PF_USEDFPU flag for a new thread,
> but it does not clear the PSR_EF bit. Thus, a first FPU exception
> called from the child thread is not handled properly. I made the
> patch after PPC and MIPS architectures - both clear a similar bit
> in copy_thread.
> This bug does not exist in the UP kernel, probably because it
> does not rely on PF_USEDFPU flag.
> The patch below should be applied for both 2.4 and 2.6 branches.
> I tested it with a test-fenv program from  glibc testsuite. The
> test fails the same test cases in both SMP and UP with  the patch
> applied. Without the patch, the test on SMP kernel failes in
> almost two times more testcases.

Are you sure a bugfix merits a copyright attribution? It's atypical
elsewhere...


-- wli

Re: FPU context switching fix for SMP

[Krzysztof Helt]

Dnia 26-01-2005 o godz. 9:26 William Lee Irwin III napisa³(a):
> On Wed, Jan 26, 2005 at 09:24:42AM +0100, Krzysztof Helt wrote:
> > Here is a patch for FPU context switching in SMP.
> > A copy_thread() function clears PF_USEDFPU flag for a new thread,
> > but it does not clear the PSR_EF bit. Thus, a first FPU exception
> > called from the child thread is not handled properly. I made the
> > patch after PPC and MIPS architectures - both clear a similar bit
> > in copy_thread.
> > This bug does not exist in the UP kernel, probably because it
> > does not rely on PF_USEDFPU flag.
> > The patch below should be applied for both 2.4 and 2.6 branches.
> > I tested it with a test-fenv program from  glibc testsuite. The
> > test fails the same test cases in both SMP and UP with  the patch
> > applied. Without the patch, the test on SMP kernel failes in
> > almost two times more testcases.
> 
> Are you sure a bugfix merits a copyright attribution? It's atypical
> elsewhere...
> 
> 

So please remove it. I am quite new, so just don't know how to
measure it...

Krzysztof


----------------------------------------------------
W Stepford kobiety s± idealne, a mê¿czy¼ni szczê¶liwi, dlaczego?
Zobacz, Nicole Kidman w filmie "¯ony ze Stepford" 
Musisz je mieæ na DVD i Wideo!
http://klik.wp.pl/?adr=http%3A%2F%2Ffilm.wp.pl%2Fp%2Ffilm.html%3Fid%3D24449&sid05

Re: FPU context switching fix for SMP

[Baurjan Ismagulov]

On Wed, Jan 26, 2005 at 12:26:10AM -0800, William Lee Irwin III wrote:
> Are you sure a bugfix merits a copyright attribution? It's atypical
> elsewhere...

IANAL, but as I understand section 2 of GPL, it requires that any
changes are logged.

With kind regards,
Baurjan.

Re: FPU context switching fix for SMP

[David S. Miller]

On Wed, 26 Jan 2005 23:06:41 +0100
Baurjan Ismagulov <ibr@ata.cs.hun.edu.tr> wrote:

> On Wed, Jan 26, 2005 at 12:26:10AM -0800, William Lee Irwin III wrote:
> > Are you sure a bugfix merits a copyright attribution? It's atypical
> > elsewhere...
> 
> IANAL, but as I understand section 2 of GPL, it requires that any
> changes are logged.

It's only relevant if it is significant enough for copyright purposes.
Changes or bug fixes that are only 2 or 3 lines in nature, generally
aren't.

Also, two more points:

1) copyright law and licensing are two different things, don't confuse
   them

2) even if an author fails to put an explicit copyright statement
   into his work, many countries define their copyright law such that
   copyright of the work by that author is implicit at least to some
   extent

Re: FPU context switching fix for SMP

[Baurjan Ismagulov]

On Wed, Jan 26, 2005 at 03:09:51PM -0800, David S. Miller wrote:
> On Wed, 26 Jan 2005 23:06:41 +0100 Baurjan Ismagulov <ibr@ata.cs.hun.edu.tr> wrote:
> > On Wed, Jan 26, 2005 at 12:26:10AM -0800, William Lee Irwin III wrote:
> > > Are you sure a bugfix merits a copyright attribution? It's atypical
> > > elsewhere...
> > 
> > IANAL, but as I understand section 2 of GPL, it requires that any
> > changes are logged.
> 
> It's only relevant if it is significant enough for copyright purposes.
> Changes or bug fixes that are only 2 or 3 lines in nature, generally
> aren't.

Ok, then our legal dept. interprets the license too literally :) .


> 1) copyright law and licensing are two different things, don't confuse
>    them

Sure. The OP hasn't claimed copyright, he's just put a modification
notice. As he distributes his modification, however, he has to comply
with the GPL, which doesn't say that 2a applies to legally significant
changes only, that is why I wrote my previous e-mail.


Thanks for the explanations!


With kind regards,
Baurjan.

Re: FPU context switching fix for SMP

[David S. Miller]

On Wed, 26 Jan 2005 09:24:42 +0100
"Krzysztof Helt" <krzysztof.h1@wp.pl> wrote:

> Here is a patch for FPU context switching in SMP.
> 
> A copy_thread() function clears PF_USEDFPU flag for a new thread,
> but it does not clear the PSR_EF bit. Thus, a first FPU exception
> called from the child thread is not handled properly. I made the
> patch after PPC and MIPS architectures - both clear a similar bit
> in copy_thread.
> This bug does not exist in the UP kernel, probably because it
> does not rely on PF_USEDFPU flag.
> 
> The patch below should be applied for both 2.4 and 2.6 branches.

I've modified your patch to only do this in the SMP case.

Here is how the two mechanisms work:

UP:
	last_task_used_math keeps trash of the last thread to
	use the FPU.  As we context switch, we clear PSR_EF in
	the %psr register.  If we take a FPU disabled trap:
	1) if this is last_task_used_math, simply re-enable FPU
	2) else, save FPU state to last_task_used_math, restore
	   FPU state of current thread, and update last_task_used_math
	   value
	When we exec() or exit() and last_task_used_math is the
	current thread, we reset it to NULL.

SMP:
	last_task_used_math is not used at all.
	Instead, a state bit assosciated with each thread keeps
	track of whether that thread used the FPU or not.
	At schedule() time, if set we clear the bit and save
	the FPU state of that task and clear PSR_EF.
	At FPU disabled trap time, we set the state bit
	and load the FPU state for the process.

The SMP case works in that each thread, when scheduled(), has
it's PSR_EF flag cleared.

This fork() bug takes away that invariant, which is why the
problems arose.

Thanks a lot Krzysztof.

Re: FPU context switching fix for SMP

[William Lee Irwin III]

On Tue, Feb 15, 2005 at 09:40:57AM -0800, David S. Miller wrote:
> I've modified your patch to only do this in the SMP case.
> Here is how the two mechanisms work:
[...]
> The SMP case works in that each thread, when scheduled(), has
> it's PSR_EF flag cleared.
> This fork() bug takes away that invariant, which is why the
> problems arose.
> Thanks a lot Krzysztof.

Looks like you got this one wrapped up before I figured out what was
going on.

Thanks.


-- wli