lkml.org 
[lkml]   [2015]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/4] x86/fpu: don't abuse drop_init_fpu() in flush_thread()
On Sun, Mar 15, 2015 at 07:16:43PM +0100, Oleg Nesterov wrote:
> Of course, drop_init_fpu() is fine if restore_fpu_checking() fails.
>
> Did you mean this from the very beginning? In this case I agree of course.
>
> Because I misinterpreted your initial comment:
>
> One example where drop_init_fpu() seems to make sense is
> __kernel_fpu_end(): kernel is done with FPU and current was using the
> FPU prior so let's restore it for the eagerfpu case.
>
> as if you suggest to use it _instead_ of restore_fpu_checking().

Nah, not "instead" - I didn't express myself precisely enough. I was
trying to think out loud and look for an example where drop_init_fpu()
would make sense.

In most of the places it is used, it is in the error path of restoring
the FPU state, i.e. we were unable to restore for some reason, let's
reinit instead of just drop only, in the eager case.

And your patch correctly removed it from flush_thread() where it didn't
make any sense except to cause CPUs to get needlessly warmer.

Anyway, we're on the same page and that was a good exercise :-)

Thanks Oleg!

Btw, we probably should start documenting stuff like that so that we
don't have to re-fault all that info 6 months/a year from now when we
have to touch that code again. Hmm, how about something like this:

---
diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 2d4adff428ac..996f20a31f0a 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -406,6 +406,17 @@ static inline void restore_init_xstate(void)
fxrstor_checking(&init_xstate_buf->i387);
}

+/*
+ * In addition to "forgetting" FPU state for @tsk, we restore the
+ * default FPU state in the eager case. Note, this is not needed in the
+ * non-eager case because there we will set CR0.TS and fault and setup
+ * an FPU state lazily.
+ *
+ * We restore the default FPU state in the eager case here as a means of
+ * addressing the failure of restoring the FPU state which @tsk points
+ * to and we still need some state to use so we use the default, clean
+ * one.
+ */
static inline void drop_init_fpu(struct task_struct *tsk)
{
if (!use_eager_fpu())
---
?

--
Regards/Gruss,
Boris.

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


\
 
 \ /
  Last update: 2015-03-15 20:01    [W:0.109 / U:0.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site