All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Suresh Siddha <sbsiddha@gmail.com>,
	Bean Anderson <bean@azulsystems.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH 1/4] x86, fpu: introduce per-cpu "bool in_kernel_fpu"
Date: Fri, 29 Aug 2014 20:16:03 +0200	[thread overview]
Message-ID: <20140829181603.GB30659@redhat.com> (raw)
In-Reply-To: <20140829181533.GA30659@redhat.com>

interrupted_kernel_fpu_idle() tries to detect if kernel_fpu_begin()
is safe or not. In particulat it should obviously deny the nested
kernel_fpu_begin() and this logic doesn't look clean.

If use_eager_fpu() == T we rely on a) __thread_has_fpu() check in
interrupted_kernel_fpu_idle(), and b) on the fact that _begin() does
__thread_clear_has_fpu().

Otherwise we demand that the interrupted task has no FPU if it is in
kernel mode, this works becase __kernel_fpu_begin() does clts().

Add the per-cpu "bool in_kernel_fpu" variable, and change this code
to check/set/clear it. This allows to do some cleanups (see the next
changes) and fixes.

Note that the current code looks racy. Say, kernel_fpu_begin() right
after math_state_restore()->__thread_fpu_begin() will overwrite the
regs we are going to restore. This patch doesn't even try to fix this,
it just adds the comment, but "in_kernel_fpu" can also be used to
implement kernel_fpu_disable() / kernel_fpu_enable().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/include/asm/i387.h |    2 +-
 arch/x86/kernel/i387.c      |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index ed8089d..5e275d3 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -40,8 +40,8 @@ extern void __kernel_fpu_end(void);
 
 static inline void kernel_fpu_begin(void)
 {
-	WARN_ON_ONCE(!irq_fpu_usable());
 	preempt_disable();
+	WARN_ON_ONCE(!irq_fpu_usable());
 	__kernel_fpu_begin();
 }
 
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index d5dd808..8fb8868 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -19,6 +19,8 @@
 #include <asm/fpu-internal.h>
 #include <asm/user.h>
 
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
 /*
  * Were we in an interrupt that interrupted kernel mode?
  *
@@ -33,6 +35,9 @@
  */
 static inline bool interrupted_kernel_fpu_idle(void)
 {
+	if (this_cpu_read(in_kernel_fpu))
+		return false;
+
 	if (use_eager_fpu())
 		return __thread_has_fpu(current);
 
@@ -73,6 +78,9 @@ void __kernel_fpu_begin(void)
 {
 	struct task_struct *me = current;
 
+	this_cpu_write(in_kernel_fpu, true);
+
+	/* FIXME: race with math_state_restore()-like code */
 	if (__thread_has_fpu(me)) {
 		__thread_clear_has_fpu(me);
 		__save_init_fpu(me);
@@ -99,6 +107,8 @@ void __kernel_fpu_end(void)
 	} else {
 		stts();
 	}
+
+	this_cpu_write(in_kernel_fpu, false);
 }
 EXPORT_SYMBOL(__kernel_fpu_end);
 
-- 
1.5.5.1



  reply	other threads:[~2014-08-29 18:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-27 18:51 [PATCH 0/4] x86, fpu: copy_process's FPU paths cleanups Oleg Nesterov
2014-08-27 18:51 ` [PATCH 1/4] x86, fpu: change __thread_fpu_begin() to use use_eager_fpu() Oleg Nesterov
2014-08-27 18:51 ` [PATCH 2/4] x86, fpu: copy_process: avoid fpu_alloc/copy if !used_math() Oleg Nesterov
2014-08-27 18:52 ` [PATCH 3/4] x86, fpu: copy_process: sanitize fpu->last_cpu initialization Oleg Nesterov
2014-08-27 18:52 ` [PATCH 4/4] x86, fpu: shift "fpu_counter = 0" from copy_thread() to arch_dup_task_struct() Oleg Nesterov
2014-08-27 20:43 ` [PATCH 0/4] x86, fpu: copy_process's FPU paths cleanups H. Peter Anvin
2014-08-28  6:50   ` Ingo Molnar
2014-08-28 12:25     ` Oleg Nesterov
2014-08-28 10:38   ` Oleg Nesterov
2014-08-28  1:17 ` Linus Torvalds
2014-08-28 11:16   ` Oleg Nesterov
2014-08-29 18:15     ` [PATCH 0/4] x86, fpu: kernel_fpu_begin/end cleanups Oleg Nesterov
2014-08-29 18:16       ` Oleg Nesterov [this message]
2014-09-02  6:43         ` [PATCH 1/4] x86, fpu: introduce per-cpu "bool in_kernel_fpu" Suresh Siddha
2014-08-29 18:16       ` [PATCH 2/4] x86, fpu: don't abuse ->has_fpu in __kernel_fpu_begin/end Oleg Nesterov
2014-08-29 18:17       ` [PATCH 3/4] x86, fpu: irq_fpu_usable: always return true if use_eager_fpu() Oleg Nesterov
2014-08-29 18:17       ` [PATCH 4/4] x86, fpu: irq_fpu_usable: kill all checks except !in_kernel_fpu Oleg Nesterov
2014-09-02  7:04         ` Suresh Siddha
2014-09-02 12:58           ` Oleg Nesterov
2014-09-02 14:13             ` Oleg Nesterov
2014-09-02  5:04 ` [PATCH 0/4] x86, fpu: copy_process's FPU paths cleanups Suresh Siddha

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=20140829181603.GB30659@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bean@azulsystems.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sbsiddha@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@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.