[PATCH 0/8] x86,fpu: various small FPU cleanups and optimizations

[PATCH 0/8] x86,fpu: various small FPU cleanups and optimizations

[riel]

This includes the three patches by Oleg that are not in -tip yet,
and five more by myself.

I believe the changes to my patches address all the comments by
reviewers on the previous version.

[PATCH 1/8] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter

[riel]

From: Oleg Nesterov <oleg@redhat.com>

It is not clear why the "else" branch clears ->fpu_counter, this makes
no sense.

If use_eager_fpu() then this has no effect. Otherwise, if we actually
wanted to prevent fpu preload after the context switch we would need to
reset it unconditionally, even if __thread_has_fpu().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/kernel/i387.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 47348653503a..c3b92c0975cd 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -122,8 +122,7 @@ void unlazy_fpu(struct task_struct *tsk)
 	if (__thread_has_fpu(tsk)) {
 		__save_init_fpu(tsk);
 		__thread_fpu_end(tsk);
-	} else
-		tsk->thread.fpu_counter = 0;
+	}
 	preempt_enable();
 }
 EXPORT_SYMBOL(unlazy_fpu);
-- 
1.9.3

[PATCH 2/8] x86, fpu: unlazy_fpu: don't do __thread_fpu_end() if use_eager_fpu()

[riel]

From: Oleg Nesterov <oleg@redhat.com>

unlazy_fpu()->__thread_fpu_end() doesn't look right if use_eager_fpu().
Unconditional __thread_fpu_end() is only correct if we know that this
thread can't return to user-mode and use FPU.

Fortunately it has only 2 callers. fpu_copy() checks use_eager_fpu(),
and init_fpu(current) can be only called by the coredumping thread via
regset->get(). But it is exported to modules, and imo this should be
fixed anyway.

And if we check use_eager_fpu() we can use __save_fpu() like fpu_copy()
and save_init_fpu() do.

- It seems that even !use_eager_fpu() case doesn't need the unconditional
  __thread_fpu_end(), we only need it if __save_init_fpu() returns 0.

- It is still not clear to me if __save_init_fpu() can safely nest with
  another save + restore from __kernel_fpu_begin(). If not, we can use
  kernel_fpu_disable() to fix the race.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/kernel/i387.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index c3b92c0975cd..8e070a6c30e5 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -120,8 +120,12 @@ void unlazy_fpu(struct task_struct *tsk)
 {
 	preempt_disable();
 	if (__thread_has_fpu(tsk)) {
-		__save_init_fpu(tsk);
-		__thread_fpu_end(tsk);
+		if (use_eager_fpu()) {
+			__save_fpu(tsk);
+		} else {
+			__save_init_fpu(tsk);
+			__thread_fpu_end(tsk);
+		}
 	}
 	preempt_enable();
 }
-- 
1.9.3

[PATCH 3/8] x86, fpu: kill save_init_fpu(), change math_error() to use unlazy_fpu()

[riel]

From: Oleg Nesterov <oleg@redhat.com>

math_error() calls save_init_fpu() after conditional_sti(), this means
that the caller can be preempted. If !use_eager_fpu() we can hit the
WARN_ON_ONCE(!__thread_has_fpu(tsk)) and/or save the wrong FPU state.

Change math_error() to use unlazy_fpu() and kill save_init_fpu().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 18 ------------------
 arch/x86/kernel/traps.c             |  2 +-
 2 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 0dbc08282291..27d00e04f911 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -520,24 +520,6 @@ static inline void __save_fpu(struct task_struct *tsk)
 }
 
 /*
- * These disable preemption on their own and are safe
- */
-static inline void save_init_fpu(struct task_struct *tsk)
-{
-	WARN_ON_ONCE(!__thread_has_fpu(tsk));
-
-	if (use_eager_fpu()) {
-		__save_fpu(tsk);
-		return;
-	}
-
-	preempt_disable();
-	__save_init_fpu(tsk);
-	__thread_fpu_end(tsk);
-	preempt_enable();
-}
-
-/*
  * i387 state interaction
  */
 static inline unsigned short get_fpu_cwd(struct task_struct *tsk)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index fb4cb6adf225..51c465846f06 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -663,7 +663,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	/*
 	 * Save the info for the exception handler and clear the error.
 	 */
-	save_init_fpu(task);
+	unlazy_fpu(task);
 	task->thread.trap_nr = trapnr;
 	task->thread.error_code = error_code;
 	info.si_signo = SIGFPE;
-- 
1.9.3

[PATCH 4/8] x86,fpu: move lazy restore functions up a few lines

[riel]

From: Rik van Riel <riel@redhat.com>

We need another lazy restore related function, that will be called
from a function that is above where the lazy restore functions are
now. It would be nice to keep all three functions grouped together.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 27d00e04f911..439ac3921a1e 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -67,6 +67,24 @@ extern void finit_soft_fpu(struct i387_soft_struct *soft);
 static inline void finit_soft_fpu(struct i387_soft_struct *soft) {}
 #endif
 
+/*
+ * Must be run with preemption disabled: this clears the fpu_owner_task,
+ * on this CPU.
+ *
+ * This will disable any lazy FPU state restore of the current FPU state,
+ * but if the current thread owns the FPU, it will still be saved by.
+ */
+static inline void __cpu_disable_lazy_restore(unsigned int cpu)
+{
+	per_cpu(fpu_owner_task, cpu) = NULL;
+}
+
+static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
+{
+	return new == this_cpu_read_stable(fpu_owner_task) &&
+		cpu == new->thread.fpu.last_cpu;
+}
+
 static inline int is_ia32_compat_frame(void)
 {
 	return config_enabled(CONFIG_IA32_EMULATION) &&
@@ -400,24 +418,6 @@ static inline void drop_init_fpu(struct task_struct *tsk)
  */
 typedef struct { int preload; } fpu_switch_t;
 
-/*
- * Must be run with preemption disabled: this clears the fpu_owner_task,
- * on this CPU.
- *
- * This will disable any lazy FPU state restore of the current FPU state,
- * but if the current thread owns the FPU, it will still be saved by.
- */
-static inline void __cpu_disable_lazy_restore(unsigned int cpu)
-{
-	per_cpu(fpu_owner_task, cpu) = NULL;
-}
-
-static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
-{
-	return new == this_cpu_read_stable(fpu_owner_task) &&
-		cpu == new->thread.fpu.last_cpu;
-}
-
 static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct task_struct *new, int cpu)
 {
 	fpu_switch_t fpu;
-- 
1.9.3

[PATCH 5/8] x86,fpu: introduce task_disable_lazy_fpu_restore helper

[riel]

From: Rik van Riel <riel@redhat.com>

Currently there are a few magic assignments sprinkled through the
code that disable lazy FPU state restoring, some more effective than
others, and all equally mystifying.

It would be easier to have a helper to explicitly disable lazy
FPU state restoring for a task.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 439ac3921a1e..c1f66261ad12 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -79,6 +79,16 @@ static inline void __cpu_disable_lazy_restore(unsigned int cpu)
 	per_cpu(fpu_owner_task, cpu) = NULL;
 }
 
+/*
+ * Used to indicate that the FPU state in memory is newer than the FPU
+ * state in registers, and the FPU state should be reloaded next time the
+ * task is run. Only safe on the current task, or non-running tasks.
+ */
+static inline void task_disable_lazy_fpu_restore(struct task_struct *tsk)
+{
+	tsk->thread.fpu.last_cpu = ~0;
+}
+
 static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
 {
 	return new == this_cpu_read_stable(fpu_owner_task) &&
-- 
1.9.3

[PATCH 6/8] x86,fpu: use an explicit if/else in switch_fpu_prepare

[riel]

From: Rik van Riel <riel@redhat.com>

Use an explicit if/else branch after __save_init_fpu(old) in
switch_fpu_prepare.  This makes substituting the assignment
with a call to task_disable_lazy_fpu() in the next patch easier
to review.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index c1f66261ad12..04063751ac80 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -440,8 +440,9 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 					     new->thread.fpu_counter > 5);
 	if (__thread_has_fpu(old)) {
 		if (!__save_init_fpu(old))
-			cpu = ~0;
-		old->thread.fpu.last_cpu = cpu;
+			old->thread.fpu.last_cpu = ~0;
+		else
+			old->thread.fpu.last_cpu = cpu;
 		old->thread.fpu.has_fpu = 0;	/* But leave fpu_owner_task! */
 
 		/* Don't change CR0.TS if we just switch! */
-- 
1.9.3

[PATCH 7/8] x86,fpu: use disable_task_lazy_fpu_restore helper

[riel]

From: Rik van Riel <riel@redhat.com>

Replace magic assignments of fpu.last_cpu = ~0 with more explicit
disable_task_lazy_fpu_restore calls.

This also fixes the lazy FPU restore disabling in drop_fpu, which
only really works when !use_eager_fpu().  This is fine for now,
because fpu_lazy_restore() is only used when !use_eager_fpu()
currently, but we may want to expand that.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 4 ++--
 arch/x86/kernel/i387.c              | 2 +-
 arch/x86/kernel/process.c           | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 04063751ac80..06af286593d7 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -440,7 +440,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 					     new->thread.fpu_counter > 5);
 	if (__thread_has_fpu(old)) {
 		if (!__save_init_fpu(old))
-			old->thread.fpu.last_cpu = ~0;
+			task_disable_lazy_fpu_restore(old);
 		else
 			old->thread.fpu.last_cpu = cpu;
 		old->thread.fpu.has_fpu = 0;	/* But leave fpu_owner_task! */
@@ -454,7 +454,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 			stts();
 	} else {
 		old->thread.fpu_counter = 0;
-		old->thread.fpu.last_cpu = ~0;
+		task_disable_lazy_fpu_restore(old);
 		if (fpu.preload) {
 			new->thread.fpu_counter++;
 			if (!use_eager_fpu() && fpu_lazy_restore(new, cpu))
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 8e070a6c30e5..8416b5f85806 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -250,7 +250,7 @@ int init_fpu(struct task_struct *tsk)
 	if (tsk_used_math(tsk)) {
 		if (cpu_has_fpu && tsk == current)
 			unlazy_fpu(tsk);
-		tsk->thread.fpu.last_cpu = ~0;
+		task_disable_lazy_fpu_restore(tsk);
 		return 0;
 	}
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index dd9a069a5ec5..83480373a642 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -68,8 +68,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 
 	dst->thread.fpu_counter = 0;
 	dst->thread.fpu.has_fpu = 0;
-	dst->thread.fpu.last_cpu = ~0;
 	dst->thread.fpu.state = NULL;
+	task_disable_lazy_fpu_restore(dst);
 	if (tsk_used_math(src)) {
 		int err = fpu_alloc(&dst->thread.fpu);
 		if (err)
-- 
1.9.3

[PATCH 8/8] x86,fpu: also check fpu_lazy_restore when use_eager_fpu

[riel]

From: Rik van Riel <riel@redhat.com>

With Oleg's patch "x86, fpu: don't abuse FPU in kernel threads if
use_eager_fpu()", kernel threads no longer have an FPU state even
on systems with use_eager_fpu()

That in turn means that a task may still have its FPU state
loaded in the FPU registers, if the task only got interrupted by
kernel threads from when it went to sleep, to when it woke up
again.

In that case, there is no need to restore the FPU state for
this task, since it is still in the registers.

The kernel can simply use the same logic to determine this as
is used for !use_eager_fpu() systems.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 06af286593d7..723b74da0685 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -457,7 +457,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 		task_disable_lazy_fpu_restore(old);
 		if (fpu.preload) {
 			new->thread.fpu_counter++;
-			if (!use_eager_fpu() && fpu_lazy_restore(new, cpu))
+			if (fpu_lazy_restore(new, cpu))
 				fpu.preload = 0;
 			else
 				prefetch(new->thread.fpu.state);
-- 
1.9.3

Re: [PATCH 0/8] x86,fpu: various small FPU cleanups and optimizations

[Rik van Riel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/06/2015 03:01 PM, riel@redhat.com wrote:
> This includes the three patches by Oleg that are not in -tip yet, 
> and five more by myself.
> 
> I believe the changes to my patches address all the comments by 
> reviewers on the previous version.

Ingo, Borislav, Peter,

what can I do to get these patches merged?

kind regards,

Rik
- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJU4gwbAAoJEM553pKExN6DeccH/1W8AqviPWFRd0GTwU2BuswK
YFoF6F/Hn/TGtAVUEkdB+7m3jJGDwPVj0UNC4osaP7mHtb8KY2snlzrUmPmreuw1
Y6lxCcx7y3CkD6UQJ4v7CJSAqdRwaQq+YB2llx/fprQkRmZIMuQQHTGGbYgg0uoG
pOxEYn6LvuFQzZlnvbpJNf0xKjLQzGNUukzXTiSpvp1q2HG3QCJj3IbULhzxEB4g
r+y9ePxej1ijiuqvaK/3rV3M7clUV5axZH+V0PH3Fk4mPgWk/8Zy5x4l1q8Al6YK
Tk/hIkWySCHpAgQZvZATtVwc6ilZ79rzmZlenGUJ5sY7XjUz0cWeaCUxgh0jgrU=
=yljP
-----END PGP SIGNATURE-----

Re: [PATCH 0/8] x86,fpu: various small FPU cleanups and optimizations

[Borislav Petkov]

On Mon, Feb 16, 2015 at 10:26:19AM -0500, Rik van Riel wrote:
> Ingo, Borislav, Peter,
> 
> what can I do to get these patches merged?

Let me take a look, as far as I can. I'll collect them if all is fine
and unless someone else chimes in with a desire to collect them :-)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 1/8] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter

[Borislav Petkov]

On Fri, Feb 06, 2015 at 03:01:58PM -0500, riel@redhat.com wrote:
> From: Oleg Nesterov <oleg@redhat.com>
> 
> It is not clear why the "else" branch clears ->fpu_counter, this makes
> no sense.

See below for why.

> If use_eager_fpu() then this has no effect. Otherwise, if we actually
> wanted to prevent fpu preload after the context switch we would need to
> reset it unconditionally, even if __thread_has_fpu().

and AFAICT, switch_fpu_prepare() already does that so I'd go ahead and
venture a guess that this has survived all the cleanups (I had to go
surf down memory lane to find Arjan's patch) over the years and is still
there for no apparent reason.

> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Rik van Riel <riel@redhat.com>
> ---
>  arch/x86/kernel/i387.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index 47348653503a..c3b92c0975cd 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -122,8 +122,7 @@ void unlazy_fpu(struct task_struct *tsk)
>  	if (__thread_has_fpu(tsk)) {
>  		__save_init_fpu(tsk);
>  		__thread_fpu_end(tsk);
> -	} else
> -		tsk->thread.fpu_counter = 0;
> +	}

... and so by looking at the unlazy_fpu() call sites, I think this makes
sense.

So how's that for a commit message instead:

---
x86, fpu, unlazy_fpu: Don't reset thread.fpu_counter

The "else" branch clears ->fpu_counter as a remnant of the lazy FPU
usage counting:

e07e23e1fd30 ("[PATCH] non lazy "sleazy" fpu implementation")

However, switch_fpu_prepare() does this now so that else branch is
superfluous.

If we do use_eager_fpu(), then this has no effect. Otherwise, if we
actually wanted to prevent fpu preload after the context switch we would
need to reset it unconditionally, even if __thread_has_fpu().
---

?

Thanks.

---
e07e23e1fd300:

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Tue, 26 Sep 2006 10:52:36 +0200
Subject: [PATCH] [PATCH] non lazy "sleazy" fpu implementation

Right now the kernel on x86-64 has a 100% lazy fpu behavior: after *every*
context switch a trap is taken for the first FPU use to restore the FPU
context lazily.  This is of course great for applications that have very
sporadic or no FPU use (since then you avoid doing the expensive
save/restore all the time).  However for very frequent FPU users...  you
take an extra trap every context switch.

The patch below adds a simple heuristic to this code: After 5 consecutive
context switches of FPU use, the lazy behavior is disabled and the context
gets restored every context switch.  If the app indeed uses the FPU, the
trap is avoided.  (the chance of the 6th time slice using FPU after the
previous 5 having done so are quite high obviously).

After 256 switches, this is reset and lazy behavior is returned (until
there are 5 consecutive ones again).  The reason for this is to give apps
that do longer bursts of FPU use still the lazy behavior back after some
time.

[akpm@osdl.org: place new task_struct field next to jit_keyring to save space]
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Andi Kleen <ak@suse.de>
Cc: Andi Kleen <ak@muc.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
 arch/x86_64/kernel/process.c | 10 ++++++++++
 arch/x86_64/kernel/traps.c   |  1 +
 include/asm-x86_64/i387.h    |  5 ++++-
 include/linux/sched.h        |  9 +++++++++
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/x86_64/kernel/process.c b/arch/x86_64/kernel/process.c
index 6fbd19564e4e..9e9a70e50c72 100644
--- a/arch/x86_64/kernel/process.c
+++ b/arch/x86_64/kernel/process.c
@@ -552,6 +552,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	int cpu = smp_processor_id();  
 	struct tss_struct *tss = &per_cpu(init_tss, cpu);
 
+	/* we're going to use this soon, after a few expensive things */
+	if (next_p->fpu_counter>5)
+		prefetch(&next->i387.fxsave);
+
 	/*
 	 * Reload esp0, LDT and the page table pointer:
 	 */
@@ -629,6 +633,12 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	    || test_tsk_thread_flag(prev_p, TIF_IO_BITMAP))
 		__switch_to_xtra(prev_p, next_p, tss);
 
+	/* If the task has used fpu the last 5 timeslices, just do a full
+	 * restore of the math state immediately to avoid the trap; the
+	 * chances of needing FPU soon are obviously high now
+	 */
+	if (next_p->fpu_counter>5)
+		math_state_restore();
 	return prev_p;
 }
 
diff --git a/arch/x86_64/kernel/traps.c b/arch/x86_64/kernel/traps.c
index 28e53342f294..ffc40cff1e07 100644
--- a/arch/x86_64/kernel/traps.c
+++ b/arch/x86_64/kernel/traps.c
@@ -1136,6 +1136,7 @@ asmlinkage void math_state_restore(void)
 		init_fpu(me);
 	restore_fpu_checking(&me->thread.i387.fxsave);
 	task_thread_info(me)->status |= TS_USEDFPU;
+	me->fpu_counter++;
 }
 
 void __init trap_init(void)
diff --git a/include/asm-x86_64/i387.h b/include/asm-x86_64/i387.h
index cba8a3b0cded..60c0f4853fdb 100644
--- a/include/asm-x86_64/i387.h
+++ b/include/asm-x86_64/i387.h
@@ -24,6 +24,7 @@ extern unsigned int mxcsr_feature_mask;
 extern void mxcsr_feature_mask_init(void);
 extern void init_fpu(struct task_struct *child);
 extern int save_i387(struct _fpstate __user *buf);
+extern asmlinkage void math_state_restore(void);
 
 /*
  * FPU lazy state save handling...
@@ -31,7 +32,9 @@ extern int save_i387(struct _fpstate __user *buf);
 
 #define unlazy_fpu(tsk) do { \
 	if (task_thread_info(tsk)->status & TS_USEDFPU) \
-		save_init_fpu(tsk); \
+		save_init_fpu(tsk); 			\
+	else						\
+		tsk->fpu_counter = 0;			\
 } while (0)
 
 /* Ignore delayed exceptions from user space */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 34ed0d99b1bd..807556c5bcd2 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -865,6 +865,15 @@ struct task_struct {
 	struct key *thread_keyring;	/* keyring private to this thread */
 	unsigned char jit_keyring;	/* default keyring to attach requested keys to */
 #endif
+	/*
+	 * fpu_counter contains the number of consecutive context switches
+	 * that the FPU is used. If this is over a threshold, the lazy fpu
+	 * saving becomes unlazy to save the trap. This is an unsigned char
+	 * so that after 256 times the counter wraps and the behavior turns
+	 * lazy again; this to deal with bursty apps that only use FPU for
+	 * a short time
+	 */
+	unsigned char fpu_counter;
 	int oomkilladj; /* OOM kill score adjustment (bit shift). */
 	char comm[TASK_COMM_LEN]; /* executable name excluding path
 				     - access with [gs]et_task_comm (which lock
-- 
2.2.0.33.gc18b867

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 1/8] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter

[Rik van Riel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/16/2015 12:04 PM, Borislav Petkov wrote:

> ... and so by looking at the unlazy_fpu() call sites, I think this
> makes sense.
> 
> So how's that for a commit message instead:
> 
> --- x86, fpu, unlazy_fpu: Don't reset thread.fpu_counter
> 
> The "else" branch clears ->fpu_counter as a remnant of the lazy
> FPU usage counting:
> 
> e07e23e1fd30 ("[PATCH] non lazy "sleazy" fpu implementation")
> 
> However, switch_fpu_prepare() does this now so that else branch is 
> superfluous.
> 
> If we do use_eager_fpu(), then this has no effect. Otherwise, if
> we actually wanted to prevent fpu preload after the context switch
> we would need to reset it unconditionally, even if
> __thread_has_fpu(). ---
> 
> ?

Good detective work.

Your changelog makes sense to me.

- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJU4i/NAAoJEM553pKExN6DWn4H/ArcSaAwA5ClOmxNACOTS644
m+8d/+3XZd+nWKBxLY2iyigLQ3FSek2dPSilDMSxtN97oFsG9mFw14gvP1/oR9OU
6vVhfuBWPRQ+Ay7CAL40v1acvzbYSTQJ6i9IJq20EFhuAS1CFwksOr9gQrhRHed1
FxFUpSnss0+p6Jp+RXY+UG2gNtSUOwgLzZFb1cmgeD5APC+RShGqHgDPTlNUESGK
f9g3QzOD3KAz8fVNzTwumN1nJzPNx6yBXuazu3U8HAxzcMskVvWSOEyEBlo48CdR
XxjLpI95qgzwUUQGFj6yax8ciq6nQGk6TsgqbdqHlpHSQ2zWU1F8XefGAS/r0vE=
=CXAX
-----END PGP SIGNATURE-----

Re: [PATCH 1/8] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter

[Oleg Nesterov]

On 02/16, Rik van Riel wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/16/2015 12:04 PM, Borislav Petkov wrote:
>
> > ... and so by looking at the unlazy_fpu() call sites, I think this
> > makes sense.
> >
> > So how's that for a commit message instead:
> >
> > --- x86, fpu, unlazy_fpu: Don't reset thread.fpu_counter
> >
> > The "else" branch clears ->fpu_counter as a remnant of the lazy
> > FPU usage counting:
> >
> > e07e23e1fd30 ("[PATCH] non lazy "sleazy" fpu implementation")
> >
> > However, switch_fpu_prepare() does this now so that else branch is
> > superfluous.
> >
> > If we do use_eager_fpu(), then this has no effect. Otherwise, if
> > we actually wanted to prevent fpu preload after the context switch
> > we would need to reset it unconditionally, even if
> > __thread_has_fpu(). ---
> >
> > ?
>
> Good detective work.
>
> Your changelog makes sense to me.

And to me, thanks.

Should I resend?

Oleg.

Re: [PATCH 1/8] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter

[Borislav Petkov]

On Mon, Feb 16, 2015 at 07:14:17PM +0100, Oleg Nesterov wrote:
> Should I resend?

No need, I've changed it before applying. :-)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/8] x86, fpu: unlazy_fpu: don't do __thread_fpu_end() if use_eager_fpu()

[Borislav Petkov]

On Fri, Feb 06, 2015 at 03:01:59PM -0500, riel@redhat.com wrote:
> From: Oleg Nesterov <oleg@redhat.com>
> 
> unlazy_fpu()->__thread_fpu_end() doesn't look right if use_eager_fpu().
> Unconditional __thread_fpu_end() is only correct if we know that this
> thread can't return to user-mode and use FPU.
> 
> Fortunately it has only 2 callers. fpu_copy() checks use_eager_fpu(),
> and init_fpu(current) can be only called by the coredumping thread via
> regset->get(). But it is exported to modules, and imo this should be
> fixed anyway.
> 
> And if we check use_eager_fpu() we can use __save_fpu() like fpu_copy()
> and save_init_fpu() do.
> 
> - It seems that even !use_eager_fpu() case doesn't need the unconditional
>   __thread_fpu_end(), we only need it if __save_init_fpu() returns 0.

I can follow so far.

> - It is still not clear to me if __save_init_fpu() can safely nest with
>   another save + restore from __kernel_fpu_begin(). If not, we can use
>   kernel_fpu_disable() to fix the race.

Well, my primitive understanding would say no, not safely, for the
simple reason that we have only one XSAVE state area per thread.
However, __kernel_fpu_begin() is called with preemption disabled so ...
I guess I'm still not seeing the race.

Btw, what is kernel_fpu_disable()? Can't find it here.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 3/8] x86, fpu: kill save_init_fpu(), change math_error() to use unlazy_fpu()

[Borislav Petkov]

On Fri, Feb 06, 2015 at 03:02:00PM -0500, riel@redhat.com wrote:
> From: Oleg Nesterov <oleg@redhat.com>
> 
> math_error() calls save_init_fpu() after conditional_sti(), this means
> that the caller can be preempted. If !use_eager_fpu() we can hit the
> WARN_ON_ONCE(!__thread_has_fpu(tsk)) and/or save the wrong FPU state.
> 
> Change math_error() to use unlazy_fpu() and kill save_init_fpu().
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Rik van Riel <riel@redhat.com>
> ---
>  arch/x86/include/asm/fpu-internal.h | 18 ------------------
>  arch/x86/kernel/traps.c             |  2 +-
>  2 files changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
> index 0dbc08282291..27d00e04f911 100644
> --- a/arch/x86/include/asm/fpu-internal.h
> +++ b/arch/x86/include/asm/fpu-internal.h
> @@ -520,24 +520,6 @@ static inline void __save_fpu(struct task_struct *tsk)
>  }
>  
>  /*
> - * These disable preemption on their own and are safe
> - */
> -static inline void save_init_fpu(struct task_struct *tsk)
> -{
> -	WARN_ON_ONCE(!__thread_has_fpu(tsk));
> -
> -	if (use_eager_fpu()) {
> -		__save_fpu(tsk);
> -		return;
> -	}
> -
> -	preempt_disable();
> -	__save_init_fpu(tsk);
> -	__thread_fpu_end(tsk);
> -	preempt_enable();
> -}
> -
> -/*
>   * i387 state interaction
>   */
>  static inline unsigned short get_fpu_cwd(struct task_struct *tsk)
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index fb4cb6adf225..51c465846f06 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -663,7 +663,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  	/*
>  	 * Save the info for the exception handler and clear the error.
>  	 */
> -	save_init_fpu(task);
> +	unlazy_fpu(task);

Do I see it correctly that even with this there's a not-so-small hole
*after* conditional_sti() and *before* unlazy_fpu() where caller can
still get preempted?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 3/8] x86, fpu: kill save_init_fpu(), change math_error() to use unlazy_fpu()

[Rik van Riel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/16/2015 04:09 PM, Borislav Petkov wrote:
> On Fri, Feb 06, 2015 at 03:02:00PM -0500, riel@redhat.com wrote:
>> From: Oleg Nesterov <oleg@redhat.com>

>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c 
>> index fb4cb6adf225..51c465846f06 100644 ---
>> a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -663,7
>> +663,7 @@ static void math_error(struct pt_regs *regs, int
>> error_code, int trapnr) /* * Save the info for the exception
>> handler and clear the error. */ -	save_init_fpu(task); +
>> unlazy_fpu(task);
> 
> Do I see it correctly that even with this there's a not-so-small
> hole *after* conditional_sti() and *before* unlazy_fpu() where
> caller can still get preempted?

That's ok, the context switch will save the register contents
to memory in that case. At that point unlazy_fpu will potentially
do nothing, and the task will process the FPU context that was
saved to memory previously.

- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJU4mGKAAoJEM553pKExN6DHoAH/1R3Pak5bZrvnCEBd3lp9/uD
bqtms5VjiWNNhlG0Hpob0eS71p8fO9La8+hPwFQJdK9yZ2lwvrDEXMCMGOG1P8by
HHbtxS74tUIm49qlIIUNxoiI+HMDnOQNhYv9dA4VdTMcY//2Asr6g4LM3wFH+f1q
tZJ5CU5/f0g2SCtTEzXxZDQdlqgpGXpvcGVymENe45uzn5ZYgcbCAiix+Dy5SiCw
+GjURoRW07iRSvK48w8vTVstTwl2WKgHpWEGVGhL1RyJY6aMIft3b4FH1jUrhVVJ
w3ncQAptA2ry/anU/6A2aTrZ646ebJuYejhF99gwTucolJ3netdX7xohOaFEpSc=
=/rU9
-----END PGP SIGNATURE-----

Re: [PATCH 6/8] x86,fpu: use an explicit if/else in switch_fpu_prepare

[Borislav Petkov]

On Fri, Feb 06, 2015 at 03:02:03PM -0500, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>
> 
> Use an explicit if/else branch after __save_init_fpu(old) in
> switch_fpu_prepare.  This makes substituting the assignment
> with a call to task_disable_lazy_fpu() in the next patch easier
> to review.
> 
> Signed-off-by: Rik van Riel <riel@redhat.com>

I did space-out stuff a bit ontop, it looks much more readable to me
this way and that fpu.preload assignment is actually parseable at a
quick glance now :-)

---
From: Rik van Riel <riel@redhat.com>
Subject: [PATCH] x86, fpu: Use an explicit if/else in switch_fpu_prepare

Use an explicit if/else branch after __save_init_fpu(old) in
switch_fpu_prepare. This makes substituting the assignment with a call
in task_disable_lazy_fpu_restore() in the next patch easier to review.

Signed-off-by: Rik van Riel <riel@redhat.com>
Link: http://lkml.kernel.org/r/1423252925-14451-7-git-send-email-riel@redhat.com
[ Boris: space out stuff for more readability. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 9c27f44e1c5c..04c2807aab66 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -434,13 +434,17 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 	 * If the task has used the math, pre-load the FPU on xsave processors
 	 * or if the past 5 consecutive context-switches used math.
 	 */
-	fpu.preload = tsk_used_math(new) && (use_eager_fpu() ||
-					     new->thread.fpu_counter > 5);
+	fpu.preload = tsk_used_math(new) &&
+		      (use_eager_fpu() || new->thread.fpu_counter > 5);
+
 	if (__thread_has_fpu(old)) {
 		if (!__save_init_fpu(old))
-			cpu = ~0;
-		old->thread.fpu.last_cpu = cpu;
-		old->thread.fpu.has_fpu = 0;	/* But leave fpu_owner_task! */
+			old->thread.fpu.last_cpu = ~0;
+		else
+			old->thread.fpu.last_cpu = cpu;
+
+		/* But leave fpu_owner_task! */
+		old->thread.fpu.has_fpu = 0;
 
 		/* Don't change CR0.TS if we just switch! */
 		if (fpu.preload) {
-- 
2.2.0.33.gc18b867

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 7/8] x86,fpu: use disable_task_lazy_fpu_restore helper

[Borislav Petkov]

On Fri, Feb 06, 2015 at 03:02:04PM -0500, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>
> 
> Replace magic assignments of fpu.last_cpu = ~0 with more explicit
> disable_task_lazy_fpu_restore calls.
> 
> This also fixes the lazy FPU restore disabling in drop_fpu, which
> only really works when !use_eager_fpu().  This is fine for now,
> because fpu_lazy_restore() is only used when !use_eager_fpu()
> currently, but we may want to expand that.

Sorry, I can't follow here. The only non-trivial change below is in
arch_dup_task_struct(). Please clarify.

[ leaving in the rest for context ]

> 
> Signed-off-by: Rik van Riel <riel@redhat.com>
> ---
>  arch/x86/include/asm/fpu-internal.h | 4 ++--
>  arch/x86/kernel/i387.c              | 2 +-
>  arch/x86/kernel/process.c           | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
> index 04063751ac80..06af286593d7 100644
> --- a/arch/x86/include/asm/fpu-internal.h
> +++ b/arch/x86/include/asm/fpu-internal.h
> @@ -440,7 +440,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
>  					     new->thread.fpu_counter > 5);
>  	if (__thread_has_fpu(old)) {
>  		if (!__save_init_fpu(old))
> -			old->thread.fpu.last_cpu = ~0;
> +			task_disable_lazy_fpu_restore(old);
>  		else
>  			old->thread.fpu.last_cpu = cpu;
>  		old->thread.fpu.has_fpu = 0;	/* But leave fpu_owner_task! */
> @@ -454,7 +454,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
>  			stts();
>  	} else {
>  		old->thread.fpu_counter = 0;
> -		old->thread.fpu.last_cpu = ~0;
> +		task_disable_lazy_fpu_restore(old);
>  		if (fpu.preload) {
>  			new->thread.fpu_counter++;
>  			if (!use_eager_fpu() && fpu_lazy_restore(new, cpu))
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index 8e070a6c30e5..8416b5f85806 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -250,7 +250,7 @@ int init_fpu(struct task_struct *tsk)
>  	if (tsk_used_math(tsk)) {
>  		if (cpu_has_fpu && tsk == current)
>  			unlazy_fpu(tsk);
> -		tsk->thread.fpu.last_cpu = ~0;
> +		task_disable_lazy_fpu_restore(tsk);
>  		return 0;
>  	}
>  
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index dd9a069a5ec5..83480373a642 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -68,8 +68,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>  
>  	dst->thread.fpu_counter = 0;
>  	dst->thread.fpu.has_fpu = 0;
> -	dst->thread.fpu.last_cpu = ~0;
>  	dst->thread.fpu.state = NULL;
> +	task_disable_lazy_fpu_restore(dst);
>  	if (tsk_used_math(src)) {
>  		int err = fpu_alloc(&dst->thread.fpu);
>  		if (err)
> -- 
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/8] x86, fpu: unlazy_fpu: don't do __thread_fpu_end() if use_eager_fpu()

[Oleg Nesterov]

On 02/16, Borislav Petkov wrote:
>
> On Fri, Feb 06, 2015 at 03:01:59PM -0500, riel@redhat.com wrote:
> > From: Oleg Nesterov <oleg@redhat.com>
> >
> > unlazy_fpu()->__thread_fpu_end() doesn't look right if use_eager_fpu().
> > Unconditional __thread_fpu_end() is only correct if we know that this
> > thread can't return to user-mode and use FPU.
> >
> > Fortunately it has only 2 callers. fpu_copy() checks use_eager_fpu(),
> > and init_fpu(current) can be only called by the coredumping thread via
> > regset->get(). But it is exported to modules, and imo this should be
> > fixed anyway.
> >
> > And if we check use_eager_fpu() we can use __save_fpu() like fpu_copy()
> > and save_init_fpu() do.
> >
> > - It seems that even !use_eager_fpu() case doesn't need the unconditional
> >   __thread_fpu_end(), we only need it if __save_init_fpu() returns 0.
>
> I can follow so far.
>
> > - It is still not clear to me if __save_init_fpu() can safely nest with
> >   another save + restore from __kernel_fpu_begin(). If not, we can use
> >   kernel_fpu_disable() to fix the race.
>
> Well, my primitive understanding would say no, not safely, for the
> simple reason that we have only one XSAVE state area per thread.
> However, __kernel_fpu_begin() is called with preemption disabled so ...
> I guess I'm still not seeing the race.

This is not about preemption. But let me first say that I do not know
how the FPU hardware actually works, and I do not understand the FPU
asm code at all.

Let's look at this code

	if (__thread_has_fpu(tsk)) {
		__save_init_fpu(tsk);	// interrupt -> kernel_fpu_begin()
		__thread_fpu_end(tsk);
	}

Suppose that kernel_fpu_begin() from interrupt races with __save_init_fpu()
in progress. Is this safe? I do not know.

My concern is that (I think) __save_init_fpu() can save the FPU state _and_
modify it (say, it can reset some register to default value). This means that
the nested __save_init_fpu() from __kernel_fpu_begin() can save the modified
register again to current->thread.fpu.

If my understanding is wrong, then why switch_fpu_prepare() clears .last_cpu
if __save_init_fpu() returns 0 (which iiuc means that CPU's state does not
match the saved state) ?

Plus I have other (more vague) concerns...

> Btw, what is kernel_fpu_disable()? Can't find it here.

It's already in Linus's tree, see

14e153ef75eecae8fd0738ffb42120f4962a00cd x86, fpu: Introduce per-cpu in_kernel_fpu state
33a3ebdc077fd85f1bf4d4586eea579b297461ae x86, fpu: Don't abuse has_fpu in __kernel_fpu_begin/end()
7575637ab293861a799f3bbafe0d8c597389f4e9 x86, fpu: Fix math_state_restore() race with kernel_fpu_begin()

And, Borislav, thanks for looking at this!

Oleg.

Re: [PATCH 3/8] x86, fpu: kill save_init_fpu(), change math_error() to use unlazy_fpu()

[Oleg Nesterov]

On 02/16, Rik van Riel wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/16/2015 04:09 PM, Borislav Petkov wrote:
> > On Fri, Feb 06, 2015 at 03:02:00PM -0500, riel@redhat.com wrote:
> >> From: Oleg Nesterov <oleg@redhat.com>
>
> >> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> >> index fb4cb6adf225..51c465846f06 100644 ---
> >> a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -663,7
> >> +663,7 @@ static void math_error(struct pt_regs *regs, int
> >> error_code, int trapnr) /* * Save the info for the exception
> >> handler and clear the error. */ -	save_init_fpu(task); +
> >> unlazy_fpu(task);
> >
> > Do I see it correctly that even with this there's a not-so-small
> > hole *after* conditional_sti() and *before* unlazy_fpu() where
> > caller can still get preempted?
>
> That's ok, the context switch will save the register contents
> to memory in that case.

Yes, thanks.

> At that point unlazy_fpu will potentially
> do nothing, and the task will process the FPU context that was
> saved to memory previously.

Or, if __thread_has_fpu() will be true again, it will save the registers
again.

And this equally applies to any other user of unlazy_fpu().

Oleg.

Re: [PATCH 7/8] x86,fpu: use disable_task_lazy_fpu_restore helper

[Oleg Nesterov]

On 02/17, Borislav Petkov wrote:
>
> On Fri, Feb 06, 2015 at 03:02:04PM -0500, riel@redhat.com wrote:
> > From: Rik van Riel <riel@redhat.com>
> >
> > Replace magic assignments of fpu.last_cpu = ~0 with more explicit
> > disable_task_lazy_fpu_restore calls.
> >
> > This also fixes the lazy FPU restore disabling in drop_fpu, which
> > only really works when !use_eager_fpu().  This is fine for now,
> > because fpu_lazy_restore() is only used when !use_eager_fpu()
> > currently, but we may want to expand that.
>
> Sorry, I can't follow here. The only non-trivial change below is in
> arch_dup_task_struct(). Please clarify.

I guess Rik forgot to update the changelog ;)

Initial version of this patch played with drop_fpu() as well, this part
was removed after discussion. So I guess the last sentence in the changelog
should go away too.

Oleg.

Re: [PATCH 2/8] x86, fpu: unlazy_fpu: don't do __thread_fpu_end() if use_eager_fpu()

[Borislav Petkov]

On Tue, Feb 17, 2015 at 11:47:30AM +0100, Oleg Nesterov wrote:
> On 02/16, Borislav Petkov wrote:
> >
> > On Fri, Feb 06, 2015 at 03:01:59PM -0500, riel@redhat.com wrote:
> > > From: Oleg Nesterov <oleg@redhat.com>
> > >
> > > unlazy_fpu()->__thread_fpu_end() doesn't look right if use_eager_fpu().
> > > Unconditional __thread_fpu_end() is only correct if we know that this
> > > thread can't return to user-mode and use FPU.
> > >
> > > Fortunately it has only 2 callers. fpu_copy() checks use_eager_fpu(),
> > > and init_fpu(current) can be only called by the coredumping thread via
> > > regset->get(). But it is exported to modules, and imo this should be
> > > fixed anyway.
> > >
> > > And if we check use_eager_fpu() we can use __save_fpu() like fpu_copy()
> > > and save_init_fpu() do.
> > >
> > > - It seems that even !use_eager_fpu() case doesn't need the unconditional
> > >   __thread_fpu_end(), we only need it if __save_init_fpu() returns 0.
> >
> > I can follow so far.
> >
> > > - It is still not clear to me if __save_init_fpu() can safely nest with
> > >   another save + restore from __kernel_fpu_begin(). If not, we can use
> > >   kernel_fpu_disable() to fix the race.
> >
> > Well, my primitive understanding would say no, not safely, for the
> > simple reason that we have only one XSAVE state area per thread.
> > However, __kernel_fpu_begin() is called with preemption disabled so ...
> > I guess I'm still not seeing the race.
> 
> This is not about preemption. But let me first say that I do not know
> how the FPU hardware actually works, and I do not understand the FPU
> asm code at all.

Same here :-P

> Let's look at this code
> 
> 	if (__thread_has_fpu(tsk)) {
> 		__save_init_fpu(tsk);	// interrupt -> kernel_fpu_begin()
> 		__thread_fpu_end(tsk);
> 	}
> 
> Suppose that kernel_fpu_begin() from interrupt races with __save_init_fpu()
> in progress. Is this safe? I do not know.

I guess this was the reason for

	WARN_ON_ONCE(!irq_fpu_usable());

in kernel_fpu_begin(). And even that solution is not sufficient. Check
out this thread:

https://lkml.kernel.org/r/tip-baa916f39b50ad91661534652110df40396acda0@git.kernel.org

for similar issues with the current handling of FPU state.

> My concern is that (I think) __save_init_fpu() can save the FPU state _and_
> modify it (say, it can reset some register to default value). This means that
> the nested __save_init_fpu() from __kernel_fpu_begin() can save the modified
> register again to current->thread.fpu.

Oh ok, I see what you mean.

> If my understanding is wrong, then why switch_fpu_prepare() clears .last_cpu
> if __save_init_fpu() returns 0 (which iiuc means that CPU's state does not
> match the saved state) ?

Right, this should be the reason why fpu_save_init() signals intact FPU
state with its retval.

Although I don't know what that XSTATE_FP bit checking is supposed to
mean as manuals say XCR0[0] is always 1. And nothing says that XSAVE
would ever clear it...

But the FNSAVE case, for example and IMHO, says that after FWAIT which
would raise unmasked FPU exceptions and thus clear FPU control register
bits...

Long story short, yeah, we better prevent concurrent __save_init_fpu()
from happening...

> Plus I have other (more vague) concerns...

Same here. :-\

Our FPU code is nuts and misses a lot of comments.

> > Btw, what is kernel_fpu_disable()? Can't find it here.
> 
> It's already in Linus's tree, see
> 
> 14e153ef75eecae8fd0738ffb42120f4962a00cd x86, fpu: Introduce per-cpu in_kernel_fpu state
> 33a3ebdc077fd85f1bf4d4586eea579b297461ae x86, fpu: Don't abuse has_fpu in __kernel_fpu_begin/end()
> 7575637ab293861a799f3bbafe0d8c597389f4e9 x86, fpu: Fix math_state_restore() race with kernel_fpu_begin()

Ah, and I was working on a plain 3.19, that's why I didn't see it. I'll rebase
your patches then.

> And, Borislav, thanks for looking at this!

Sure. Thanks for the patience and explaining stuff to me! :)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 7/8] x86,fpu: use disable_task_lazy_fpu_restore helper

[Borislav Petkov]

On Tue, Feb 17, 2015 at 12:04:05PM +0100, Oleg Nesterov wrote:
> Initial version of this patch played with drop_fpu() as well, this
> part was removed after discussion. So I guess the last sentence in the
> changelog should go away too.

Ok, removed.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[tip:x86/fpu] x86/fpu: Don't reset thread.fpu_counter

[tip-bot for Oleg Nesterov]

Commit-ID:  a9241ea5fd709fc935dade130f4e3b2612bbe9e3
Gitweb:     http://git.kernel.org/tip/a9241ea5fd709fc935dade130f4e3b2612bbe9e3
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:01:58 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:12:40 +0100

x86/fpu: Don't reset thread.fpu_counter

The "else" branch clears ->fpu_counter as a remnant of the lazy FPU
usage counting:

  e07e23e1fd30 ("[PATCH] non lazy "sleazy" fpu implementation")

However, switch_fpu_prepare() does this now so that else branch is
superfluous.

If we do use_eager_fpu(), then this has no effect. Otherwise, if we
actually wanted to prevent fpu preload after the context switch we would
need to reset it unconditionally, even if __thread_has_fpu().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1423252925-14451-2-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/i387.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index a9a4229..4d0db9e 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -108,8 +108,7 @@ void unlazy_fpu(struct task_struct *tsk)
 	if (__thread_has_fpu(tsk)) {
 		__save_init_fpu(tsk);
 		__thread_fpu_end(tsk);
-	} else
-		tsk->thread.fpu_counter = 0;
+	}
 	preempt_enable();
 }
 EXPORT_SYMBOL(unlazy_fpu);

[tip:x86/fpu] x86/fpu: Don't do __thread_fpu_end() if use_eager_fpu()

[tip-bot for Oleg Nesterov]

Commit-ID:  1a2a7f4ec8e3a7ac582dac4d01fcc7e8acd3bb30
Gitweb:     http://git.kernel.org/tip/1a2a7f4ec8e3a7ac582dac4d01fcc7e8acd3bb30
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:01:59 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:12:46 +0100

x86/fpu: Don't do __thread_fpu_end() if use_eager_fpu()

unlazy_fpu()->__thread_fpu_end() doesn't look right if use_eager_fpu().
Unconditional __thread_fpu_end() is only correct if we know that this
thread can't return to user-mode and use FPU.

Fortunately it has only 2 callers. fpu_copy() checks use_eager_fpu(),
and init_fpu(current) can be only called by the coredumping thread via
regset->get(). But it is exported to modules, and imo this should be
fixed anyway.

And if we check use_eager_fpu() we can use __save_fpu() like fpu_copy()
and save_init_fpu() do.

- It seems that even !use_eager_fpu() case doesn't need the unconditional
  __thread_fpu_end(), we only need it if __save_init_fpu() returns 0.

- It is still not clear to me if __save_init_fpu() can safely nest with
  another save + restore from __kernel_fpu_begin(). If not, we can use
  kernel_fpu_disable() to fix the race.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1423252925-14451-3-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/i387.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 4d0db9e..f3ced6f 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -106,8 +106,12 @@ void unlazy_fpu(struct task_struct *tsk)
 {
 	preempt_disable();
 	if (__thread_has_fpu(tsk)) {
-		__save_init_fpu(tsk);
-		__thread_fpu_end(tsk);
+		if (use_eager_fpu()) {
+			__save_fpu(tsk);
+		} else {
+			__save_init_fpu(tsk);
+			__thread_fpu_end(tsk);
+		}
 	}
 	preempt_enable();
 }

[tip:x86/fpu] x86/fpu: Change math_error() to use unlazy_fpu(), kill (now) unused save_init_fpu()

[tip-bot for Oleg Nesterov]

Commit-ID:  08a744c6bfded3d5fa66f94263f81773226113d1
Gitweb:     http://git.kernel.org/tip/08a744c6bfded3d5fa66f94263f81773226113d1
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:00 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:03 +0100

x86/fpu: Change math_error() to use unlazy_fpu(), kill (now) unused save_init_fpu()

math_error() calls save_init_fpu() after conditional_sti(), this means
that the caller can be preempted. If !use_eager_fpu() we can hit the
WARN_ON_ONCE(!__thread_has_fpu(tsk)) and/or save the wrong FPU state.

Change math_error() to use unlazy_fpu() and kill save_init_fpu().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1423252925-14451-4-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 18 ------------------
 arch/x86/kernel/traps.c             |  2 +-
 2 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index e97622f..02f2e08 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -518,24 +518,6 @@ static inline void __save_fpu(struct task_struct *tsk)
 }
 
 /*
- * These disable preemption on their own and are safe
- */
-static inline void save_init_fpu(struct task_struct *tsk)
-{
-	WARN_ON_ONCE(!__thread_has_fpu(tsk));
-
-	if (use_eager_fpu()) {
-		__save_fpu(tsk);
-		return;
-	}
-
-	preempt_disable();
-	__save_init_fpu(tsk);
-	__thread_fpu_end(tsk);
-	preempt_enable();
-}
-
-/*
  * i387 state interaction
  */
 static inline unsigned short get_fpu_cwd(struct task_struct *tsk)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 88900e2..9d889f7 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -663,7 +663,7 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
 	/*
 	 * Save the info for the exception handler and clear the error.
 	 */
-	save_init_fpu(task);
+	unlazy_fpu(task);
 	task->thread.trap_nr = trapnr;
 	task->thread.error_code = error_code;
 	info.si_signo = SIGFPE;

[tip:x86/fpu] x86/fpu: Move lazy restore functions up a few lines

[tip-bot for Rik van Riel]

Commit-ID:  1c927eea4cad83c439cb51e9c96ad19cb005157d
Gitweb:     http://git.kernel.org/tip/1c927eea4cad83c439cb51e9c96ad19cb005157d
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:01 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:53 +0100

x86/fpu: Move lazy restore functions up a few lines

We need another lazy restore related function, that will be called
from a function that is above where the lazy restore functions are
now. It would be nice to keep all three functions grouped together.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: http://lkml.kernel.org/r/1423252925-14451-5-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 02f2e08..217d6d7 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -67,6 +67,24 @@ extern void finit_soft_fpu(struct i387_soft_struct *soft);
 static inline void finit_soft_fpu(struct i387_soft_struct *soft) {}
 #endif
 
+/*
+ * Must be run with preemption disabled: this clears the fpu_owner_task,
+ * on this CPU.
+ *
+ * This will disable any lazy FPU state restore of the current FPU state,
+ * but if the current thread owns the FPU, it will still be saved by.
+ */
+static inline void __cpu_disable_lazy_restore(unsigned int cpu)
+{
+	per_cpu(fpu_owner_task, cpu) = NULL;
+}
+
+static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
+{
+	return new == this_cpu_read_stable(fpu_owner_task) &&
+		cpu == new->thread.fpu.last_cpu;
+}
+
 static inline int is_ia32_compat_frame(void)
 {
 	return config_enabled(CONFIG_IA32_EMULATION) &&
@@ -398,24 +416,6 @@ static inline void drop_init_fpu(struct task_struct *tsk)
  */
 typedef struct { int preload; } fpu_switch_t;
 
-/*
- * Must be run with preemption disabled: this clears the fpu_owner_task,
- * on this CPU.
- *
- * This will disable any lazy FPU state restore of the current FPU state,
- * but if the current thread owns the FPU, it will still be saved by.
- */
-static inline void __cpu_disable_lazy_restore(unsigned int cpu)
-{
-	per_cpu(fpu_owner_task, cpu) = NULL;
-}
-
-static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
-{
-	return new == this_cpu_read_stable(fpu_owner_task) &&
-		cpu == new->thread.fpu.last_cpu;
-}
-
 static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct task_struct *new, int cpu)
 {
 	fpu_switch_t fpu;

[tip:x86/fpu] x86/fpu: Introduce task_disable_lazy_fpu_restore() helper

[tip-bot for Rik van Riel]

Commit-ID:  33e03dedd759cc9396252d9641b25d01909a26bb
Gitweb:     http://git.kernel.org/tip/33e03dedd759cc9396252d9641b25d01909a26bb
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:02 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:53 +0100

x86/fpu: Introduce task_disable_lazy_fpu_restore() helper

Currently there are a few magic assignments sprinkled through the
code that disable lazy FPU state restoring, some more effective than
others, and all equally mystifying.

It would be easier to have a helper to explicitly disable lazy
FPU state restoring for a task.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: http://lkml.kernel.org/r/1423252925-14451-6-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 217d6d7..9c27f44 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -79,6 +79,16 @@ static inline void __cpu_disable_lazy_restore(unsigned int cpu)
 	per_cpu(fpu_owner_task, cpu) = NULL;
 }
 
+/*
+ * Used to indicate that the FPU state in memory is newer than the FPU
+ * state in registers, and the FPU state should be reloaded next time the
+ * task is run. Only safe on the current task, or non-running tasks.
+ */
+static inline void task_disable_lazy_fpu_restore(struct task_struct *tsk)
+{
+	tsk->thread.fpu.last_cpu = ~0;
+}
+
 static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu)
 {
 	return new == this_cpu_read_stable(fpu_owner_task) &&

[tip:x86/fpu] x86/fpu: Use an explicit if/ else in switch_fpu_prepare()

[tip-bot for Rik van Riel]

Commit-ID:  1361ef29c7e49ae7cf37220c25fac1904b77f71a
Gitweb:     http://git.kernel.org/tip/1361ef29c7e49ae7cf37220c25fac1904b77f71a
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:03 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:54 +0100

x86/fpu: Use an explicit if/else in switch_fpu_prepare()

Use an explicit if/else branch after __save_init_fpu(old) in
switch_fpu_prepare(). This makes substituting the assignment with a call
in task_disable_lazy_fpu_restore() in the next patch easier to review.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: http://lkml.kernel.org/r/1423252925-14451-7-git-send-email-riel@redhat.com
[ Space out stuff for more readability. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 9c27f44..04c2807 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -434,13 +434,17 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 	 * If the task has used the math, pre-load the FPU on xsave processors
 	 * or if the past 5 consecutive context-switches used math.
 	 */
-	fpu.preload = tsk_used_math(new) && (use_eager_fpu() ||
-					     new->thread.fpu_counter > 5);
+	fpu.preload = tsk_used_math(new) &&
+		      (use_eager_fpu() || new->thread.fpu_counter > 5);
+
 	if (__thread_has_fpu(old)) {
 		if (!__save_init_fpu(old))
-			cpu = ~0;
-		old->thread.fpu.last_cpu = cpu;
-		old->thread.fpu.has_fpu = 0;	/* But leave fpu_owner_task! */
+			old->thread.fpu.last_cpu = ~0;
+		else
+			old->thread.fpu.last_cpu = cpu;
+
+		/* But leave fpu_owner_task! */
+		old->thread.fpu.has_fpu = 0;
 
 		/* Don't change CR0.TS if we just switch! */
 		if (fpu.preload) {

[tip:x86/fpu] x86/fpu: Use task_disable_lazy_fpu_restore() helper

[tip-bot for Rik van Riel]

Commit-ID:  6a5fe8952bd676baf382d14df21e7b32b5d8943e
Gitweb:     http://git.kernel.org/tip/6a5fe8952bd676baf382d14df21e7b32b5d8943e
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:04 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:55 +0100

x86/fpu: Use task_disable_lazy_fpu_restore() helper

Replace magic assignments of fpu.last_cpu = ~0 with more explicit
task_disable_lazy_fpu_restore() calls.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1423252925-14451-8-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 4 ++--
 arch/x86/kernel/i387.c              | 2 +-
 arch/x86/kernel/process.c           | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 04c2807..e5f8f8e 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -439,7 +439,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 
 	if (__thread_has_fpu(old)) {
 		if (!__save_init_fpu(old))
-			old->thread.fpu.last_cpu = ~0;
+			task_disable_lazy_fpu_restore(old);
 		else
 			old->thread.fpu.last_cpu = cpu;
 
@@ -455,7 +455,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 			stts();
 	} else {
 		old->thread.fpu_counter = 0;
-		old->thread.fpu.last_cpu = ~0;
+		task_disable_lazy_fpu_restore(old);
 		if (fpu.preload) {
 			new->thread.fpu_counter++;
 			if (!use_eager_fpu() && fpu_lazy_restore(new, cpu))
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index f3ced6f..5722ab6 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -236,7 +236,7 @@ int init_fpu(struct task_struct *tsk)
 	if (tsk_used_math(tsk)) {
 		if (cpu_has_fpu && tsk == current)
 			unlazy_fpu(tsk);
-		tsk->thread.fpu.last_cpu = ~0;
+		task_disable_lazy_fpu_restore(tsk);
 		return 0;
 	}
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index e127dda..ce8b103 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -68,8 +68,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 
 	dst->thread.fpu_counter = 0;
 	dst->thread.fpu.has_fpu = 0;
-	dst->thread.fpu.last_cpu = ~0;
 	dst->thread.fpu.state = NULL;
+	task_disable_lazy_fpu_restore(dst);
 	if (tsk_used_math(src)) {
 		int err = fpu_alloc(&dst->thread.fpu);
 		if (err)

[tip:x86/fpu] x86/fpu: Also check fpu_lazy_restore() when use_eager_fpu()

[tip-bot for Rik van Riel]

Commit-ID:  728e53fef429a0f3c9dda3587c3ccc57ad268b70
Gitweb:     http://git.kernel.org/tip/728e53fef429a0f3c9dda3587c3ccc57ad268b70
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 6 Feb 2015 15:02:05 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 19 Feb 2015 11:15:55 +0100

x86/fpu: Also check fpu_lazy_restore() when use_eager_fpu()

With Oleg's patch:

  33a3ebdc077f ("x86, fpu: Don't abuse has_fpu in __kernel_fpu_begin/end()")

kernel threads no longer have an FPU state even on systems with
use_eager_fpu().

That in turn means that a task may still have its FPU state
loaded in the FPU registers, if the task only got interrupted by
kernel threads from when it went to sleep, to when it woke up
again.

In that case, there is no need to restore the FPU state for
this task, since it is still in the registers.

The kernel can simply use the same logic to determine this as
is used for !use_eager_fpu() systems.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: http://lkml.kernel.org/r/1423252925-14451-9-git-send-email-riel@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index e5f8f8e..19fb41c 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -458,7 +458,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
 		task_disable_lazy_fpu_restore(old);
 		if (fpu.preload) {
 			new->thread.fpu_counter++;
-			if (!use_eager_fpu() && fpu_lazy_restore(new, cpu))
+			if (fpu_lazy_restore(new, cpu))
 				fpu.preload = 0;
 			else
 				prefetch(new->thread.fpu.state);