All of lore.kernel.org
 help / color / mirror / Atom feed
From: Claus-Justus Heine <ch@dot-heine.de>
To: linux-kernel@vger.kernel.org
Subject: 2.6.0-test4: bug WRT lazy FPU switching and CONFIG_PREEMPT
Date: Sun, 31 Aug 2003 17:42:11 +0200	[thread overview]
Message-ID: <3F521753.2030705@dot-heine.de> (raw)

Hi!

There seems to be a bug with CONFIG_PREEMPT=y and lazy FPU switching. The following small program just unmasks math exceptions 
and then continously triggers a divide by zero. When CONFIG_PREEMPT is in effect this eventually leads to other processes 
inheriting the math-status of this brain-damaged but legal user level program. Especially, their math-exceptions are unmasked 
and so they might catch an unwanted SIGFPE.

######################################### snip ####################################
#include <signal.h>
#include <stdio.h>
#include <fenv.h>

double volatile foobar = 0.0;

static void sigfpe(int sig)
{
	static int cnt;

	if (++cnt % 100000 == 0) {
		fprintf(stderr, ".");
	}
}

int main(int argc, char *argv[])
{
	signal(SIGFPE, sigfpe);
	feenableexcept(FE_ALL_EXCEPT);
	foobar = 1.0/foobar;
}
######################################## snap ######################################

The following patch seems to fix the problem. However, I must admit that I didn't fully understand why this happens. The problem 
seems to be a preemption occuring between the __save_init_fpu() (i.e. fsave/fxsave) and the stts(). But why should this be a 
problem?

The patch touches two files:

include/asm-i386/i387.h:
   here save_init_fpu() is defined and this seems to be the location of the problem

arch/i386/kernel/traps.c:
   by checking for TS_USEDFPU we spare an additional "device not available" fault (with CONFIG_PREEMPT=y) and thereby a 
superflous math_state_restore(). There is not point in restoring the math-state just to save it again (but this didn't cause the 
shooting with wild SIGFPEs).

Regards

Claus

######################################### snip ##########################################################################
diff -u --recursive --new-file linux-2.6.0-test4/arch/i386/kernel/traps.c linux-2.6.0-test4-mine/arch/i386/kernel/traps.c
--- linux-2.6.0-test4/arch/i386/kernel/traps.c	2003-08-31 15:22:42.000000000 +0200
+++ linux-2.6.0-test4-mine/arch/i386/kernel/traps.c	2003-08-31 15:35:49.000000000 +0200
@@ -605,7 +605,10 @@
  	 * Save the info for the exception handler and clear the error.
  	 */
  	task = current;
-	save_init_fpu(task);
+	/* don't trigger an unnecessary math_state_restore() */
+	if (task->thread_info->status & TS_USEDFPU) {
+		save_init_fpu(task);
+	}
  	task->thread.trap_no = 16;
  	task->thread.error_code = 0;
  	info.si_signo = SIGFPE;
@@ -667,7 +670,10 @@
  	 * Save the info for the exception handler and clear the error.
  	 */
  	task = current;
-	save_init_fpu(task);
+	/* don't trigger an unnecessary math_state_restore() */
+	if (task->thread_info->status & TS_USEDFPU) {
+		save_init_fpu(task);
+	}
  	task->thread.trap_no = 19;
  	task->thread.error_code = 0;
  	info.si_signo = SIGFPE;
diff -u --recursive --new-file linux-2.6.0-test4/include/asm-i386/i387.h linux-2.6.0-test4-mine/include/asm-i386/i387.h
--- linux-2.6.0-test4/include/asm-i386/i387.h	2003-07-27 19:06:54.000000000 +0200
+++ linux-2.6.0-test4-mine/include/asm-i386/i387.h	2003-08-31 15:39:04.000000000 +0200
@@ -30,7 +30,7 @@
  static inline void __save_init_fpu( struct task_struct *tsk )
  {
  	if ( cpu_has_fxsr ) {
-		asm volatile( "fxsave %0 ; fnclex"
+		asm volatile( "fxsave %0 ; fclex"
  			      : "=m" (tsk->thread.i387.fxsave) );
  	} else {
  		asm volatile( "fnsave %0 ; fwait"
@@ -41,8 +41,10 @@

  static inline void save_init_fpu( struct task_struct *tsk )
  {
+	preempt_disable();
  	__save_init_fpu(tsk);
  	stts();
+	preempt_enable_no_resched();
  }


@@ -53,11 +55,13 @@

  #define clear_fpu( tsk )					\
  do {								\
+	preempt_disable();					\
  	if ((tsk)->thread_info->status & TS_USEDFPU) {		\
  		asm volatile("fwait");				\
  		(tsk)->thread_info->status &= ~TS_USEDFPU;	\
  		stts();						\
  	}							\
+	preempt_enable_no_resched();				\
  } while (0)

  /*




             reply	other threads:[~2003-08-31 15:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-08-31 15:42 Claus-Justus Heine [this message]
     [not found] <qCCS.70v.9@gated-at.bofh.it>
2003-09-01 13:15 ` 2.6.0-test4: bug WRT lazy FPU switching and CONFIG_PREEMPT Claus-Justus Heine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3F521753.2030705@dot-heine.de \
    --to=ch@dot-heine.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.