LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Borislav Petkov <bp@suse.de>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>,
	Ingo Molnar <mingo@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Pekka Riikonen <priikone@iki.fi>, Rik van Riel <riel@redhat.com>,
	Suresh Siddha <sbsiddha@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Yu, Fenghua" <fenghua.yu@intel.com>,
	Quentin Casasnovas <quentin.casasnovas@oracle.com>
Subject: Re: [PATCH 4/4] x86/fpu: don't abuse drop_init_fpu() in flush_thread()
Date: Sun, 15 Mar 2015 19:50:48 +0100	[thread overview]
Message-ID: <20150315185048.GB29134@pd.tnic> (raw)
In-Reply-To: <20150315181643.GA488@redhat.com>

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.
--

  reply	other threads:[~2015-03-15 18:52 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-04 18:30 Oops with tip/x86/fpu Dave Hansen
2015-03-04 19:06 ` Oleg Nesterov
2015-03-04 19:12   ` Dave Hansen
2015-03-04 20:06   ` Borislav Petkov
2015-03-05 15:14     ` Oleg Nesterov
     [not found]       ` <20150305182203.GA4203@redhat.com>
2015-03-05 18:34         ` Dave Hansen
2015-03-05 18:46           ` Oleg Nesterov
2015-03-05 18:41         ` Dave Hansen
2015-03-26 22:37         ` Yu, Fenghua
2015-03-26 22:43           ` Dave Hansen
2015-03-26 22:48             ` Yu, Fenghua
2015-03-27  7:30               ` Quentin Casasnovas
2015-03-27 19:06           ` Oleg Nesterov
2015-03-05  8:38   ` Quentin Casasnovas
2015-03-05 15:13     ` Oleg Nesterov
2015-03-05 18:42       ` Borislav Petkov
2015-03-05 22:16         ` Dave Hansen
2015-03-05 19:51 ` [PATCH 0/1] x86/fpu: math_state_restore() should not blindly disable irqs Oleg Nesterov
2015-03-05 19:51   ` [PATCH 1/1] " Oleg Nesterov
2015-03-05 20:11     ` Ingo Molnar
2015-03-05 21:25       ` Oleg Nesterov
2015-03-06  7:58         ` Ingo Molnar
2015-03-06 13:26           ` Oleg Nesterov
2015-03-06 13:39             ` Oleg Nesterov
2015-03-06 13:46             ` Ingo Molnar
2015-03-06 14:01               ` Oleg Nesterov
2015-03-06 14:17                 ` Oleg Nesterov
2015-03-06 15:00                 ` David Vrabel
2015-03-06 15:36                   ` Oleg Nesterov
2015-03-06 16:15                     ` David Vrabel
2015-03-06 16:31                       ` Oleg Nesterov
2015-03-06 17:33           ` Linus Torvalds
2015-03-06 18:15             ` Oleg Nesterov
2015-03-06 19:23             ` Andy Lutomirski
2015-03-06 22:00               ` Linus Torvalds
2015-03-06 22:28                 ` Andy Lutomirski
2015-03-07 10:36                   ` Ingo Molnar
2015-03-07 20:11                     ` Linus Torvalds
2015-03-08  8:55                       ` Ingo Molnar
2015-03-08 11:38                         ` Ingo Molnar
2015-03-08 13:59                         ` Andy Lutomirski
2015-03-08 14:38                           ` Andy Lutomirski
2015-03-07 10:32             ` Ingo Molnar
2015-03-07 15:38   ` [PATCH 0/1] x86/fpu: x86/fpu: avoid math_state_restore() without used_math() in __restore_xstate_sig() Oleg Nesterov
2015-03-07 15:38     ` [PATCH 1/1] " Oleg Nesterov
2015-03-09 14:07       ` Borislav Petkov
2015-03-09 14:34         ` Oleg Nesterov
2015-03-09 15:18           ` Borislav Petkov
2015-03-09 16:24             ` Oleg Nesterov
2015-03-09 16:53               ` Borislav Petkov
2015-03-09 17:05                 ` Oleg Nesterov
2015-03-09 17:23                   ` Borislav Petkov
2015-03-16 12:07       ` [tip:x86/urgent] x86/fpu: Avoid " tip-bot for Oleg Nesterov
2015-03-05 20:35 ` [PATCH 0/1] x86/fpu: math_state_restore() should not blindly disable irqs Oleg Nesterov
2015-03-09 17:10 ` [PATCH] x86/fpu: drop_fpu() should not assume that tsk == current Oleg Nesterov
2015-03-09 17:36   ` Rik van Riel
2015-03-09 17:48   ` Borislav Petkov
2015-03-09 18:06     ` Oleg Nesterov
2015-03-09 18:10       ` Borislav Petkov
2015-03-16 12:07   ` [tip:x86/urgent] x86/fpu: Drop_fpu() should not assume that tsk equals current tip-bot for Oleg Nesterov
2015-03-11 17:33 ` [PATCH 0/4] x86/fpu: avoid math_state_restore() on kthread exec Oleg Nesterov
2015-03-11 17:34   ` [PATCH 1/4] x86/fpu: document user_fpu_begin() Oleg Nesterov
2015-03-13  9:47     ` Borislav Petkov
2015-03-13 14:34       ` Oleg Nesterov
2015-03-23 12:20     ` [tip:x86/fpu] x86/fpu: Document user_fpu_begin() tip-bot for Oleg Nesterov
2015-03-11 17:34   ` [PATCH 2/4] x86/fpu: introduce restore_init_xstate() Oleg Nesterov
2015-03-13 10:34     ` Borislav Petkov
2015-03-13 14:39       ` Oleg Nesterov
2015-03-13 15:20         ` Borislav Petkov
2015-03-16 19:05           ` Rik van Riel
2015-03-23 12:20     ` [tip:x86/fpu] x86/fpu: Introduce restore_init_xstate() tip-bot for Oleg Nesterov
2015-03-11 17:34   ` [PATCH 3/4] x86/fpu: use restore_init_xstate() instead of math_state_restore() on kthread exec Oleg Nesterov
2015-03-13 10:48     ` Borislav Petkov
2015-03-13 14:45       ` Oleg Nesterov
2015-03-13 15:51         ` Borislav Petkov
2015-03-23 12:21     ` [tip:x86/fpu] x86/fpu: Use " tip-bot for Oleg Nesterov
2015-03-11 17:35   ` [PATCH 4/4] x86/fpu: don't abuse drop_init_fpu() in flush_thread() Oleg Nesterov
2015-03-13 10:52     ` Borislav Petkov
2015-03-13 14:55       ` Oleg Nesterov
2015-03-13 16:19         ` Borislav Petkov
2015-03-13 16:26           ` Oleg Nesterov
2015-03-13 19:27             ` Borislav Petkov
2015-03-14 14:48               ` Oleg Nesterov
2015-03-15 17:36                 ` Borislav Petkov
2015-03-15 18:16                   ` Oleg Nesterov
2015-03-15 18:50                     ` Borislav Petkov [this message]
2015-03-15 20:04                       ` Oleg Nesterov
2015-03-15 20:38                         ` Borislav Petkov
2015-03-16  9:35                           ` Borislav Petkov
2015-03-16 10:28                             ` Ingo Molnar
2015-03-16 14:39                             ` Oleg Nesterov
2015-03-16 15:26                               ` Borislav Petkov
2015-03-16 15:34                             ` Andy Lutomirski
2015-03-16 15:35                               ` Borislav Petkov
2015-03-13 17:30     ` [PATCH v2 " Oleg Nesterov
2015-03-14 10:55       ` Borislav Petkov
2015-03-14 10:57         ` [PATCH] x86/fpu: Fold __drop_fpu() into its sole user Borislav Petkov
2015-03-14 15:15           ` Oleg Nesterov
2015-03-16 10:27           ` Ingo Molnar
2015-03-23 12:21       ` [tip:x86/fpu] x86/fpu: Don't abuse drop_init_fpu() in flush_thread() tip-bot for Oleg Nesterov
2015-03-13 18:26 ` [PATCH 0/1] x86/cpu: don't allocate fpu->state for swapper/0 Oleg Nesterov
2015-03-13 18:27   ` [PATCH 1/1] " Oleg Nesterov
2015-03-16 10:18     ` Borislav Petkov
2015-03-23 12:22     ` [tip:x86/fpu] x86/fpu: Don't " tip-bot for Oleg Nesterov
2015-03-14 11:16   ` [PATCH 0/1] x86/cpu: don't " Borislav Petkov
2015-03-14 15:13     ` [PATCH 0/1] x86/cpu: kill eager_fpu_init_bp() Oleg Nesterov
2015-03-14 15:13       ` [PATCH 1/1] " Oleg Nesterov
2015-03-16 12:44         ` Borislav Petkov
2015-03-23 12:22         ` [tip:x86/fpu] x86/fpu: Kill eager_fpu_init_bp() tip-bot for Oleg Nesterov
2015-03-15 16:49 ` [PATCH RFC 0/2] x86/fpu: avoid "xstate_fault" in xsave_user/xrestore_user Oleg Nesterov
2015-03-15 16:50   ` [PATCH RFC 1/2] x86: introduce __user_insn() and __check_insn() Oleg Nesterov
2015-03-15 16:50   ` [PATCH RFC 2/2] x86/fpu: change xsave_user() and xrestore_user() to use __user_insn() Oleg Nesterov
2015-03-16 22:43     ` Quentin Casasnovas
2015-03-17  9:35       ` Borislav Petkov
2015-03-16 14:36   ` [PATCH RFC 0/2] x86/fpu: avoid "xstate_fault" in xsave_user/xrestore_user Borislav Petkov
2015-03-16 14:57     ` Oleg Nesterov
2015-03-16 17:58       ` Borislav Petkov
2015-03-16 22:37   ` Quentin Casasnovas
2015-03-17  9:47     ` Borislav Petkov
2015-03-17 10:00       ` Quentin Casasnovas
2015-03-17 11:20         ` Borislav Petkov
2015-03-17 11:36           ` Quentin Casasnovas
2015-03-17 12:07             ` Borislav Petkov
2015-03-18  9:06               ` Quentin Casasnovas
2015-03-18  9:53                 ` Borislav Petkov
2015-03-17 10:07       ` Quentin Casasnovas

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=20150315185048.GB29134@pd.tnic \
    --to=bp@suse.de \
    --cc=dave.hansen@intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=priikone@iki.fi \
    --cc=quentin.casasnovas@oracle.com \
    --cc=riel@redhat.com \
    --cc=sbsiddha@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH 4/4] x86/fpu: don'\''t abuse drop_init_fpu() in flush_thread()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).