LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Suresh Siddha <sbsiddha@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nate Eldredge <nate@thatsmathematics.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	stable <stable@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Maarten Baert <maarten-baert@hotmail.com>,
	Jan Kara <jack@suse.cz>, George Spelvin <linux@horizon.com>,
	Pekka Riikonen <priikone@iki.fi>
Subject: Re: [PATCH] Make math_state_restore() save and restore the interrupt flag
Date: Sun, 02 Feb 2014 22:56:23 -0800
Message-ID: <1391410583.3801.6.camel@europa> (raw)
In-Reply-To: <CA+55aFzz8a7K+KSB_TkGb1ih1hAg09-0TQusQgE5vwMJGegHDw@mail.gmail.com>

On Sun, 2014-02-02 at 11:15 -0800, Linus Torvalds wrote:
> On Sat, Feb 1, 2014 at 11:19 PM, Suresh Siddha <sbsiddha@gmail.com> wrote:
> >
> > The real fix for Nate's problem will be coming from Linus, with a
> > slightly modified option-b that Linus proposed. Linus, please let me
> > know if you want me to spin it. I can do it sunday night.
> 
> Please do it, since clearly I wasn't aware enough about the whole
> non-TS-checking FPU state details.
> 
> Also, since this issue doesn't seem to be a recent regression, I'm not
> going to take this patch directly (even though I'm planning on doing
> -rc1 in a few hours), and expect that I'll get it through the normal
> channels (presumably together with the __kernel_fpu_end cleanups). Ok
> with everybody?

Here is the second patch, which should fix the issue reported in this
thread. Maarten, Nate, George, please give this patch a try as is and
see if it helps address the issue you ran into. And please ack/review
with your test results.

Other patch which cleans up the irq_enable/disable logic in
math_state_restore() has been sent yesterday. You can run your
experiments with both these patches if you want. But your issue should
get fixed with just the appended patch here.

Peter, Please push both these patches through normal channels depending
on the results.

thanks,
suresh
---
From: Suresh Siddha <sbsiddha@gmail.com>
Subject: x86, fpu: check tsk_used_math() in kernel_fpu_end() for eager fpu

For non-eager fpu mode, thread's fpu state is allocated during the first
fpu usage (in the context of device not available exception). This
(math_state_restore()) can be a blocking call and hence we enable
interrupts (which were originally disabled when the exception happened),
allocate memory and disable interrupts etc.

But the eager-fpu mode, call's the same math_state_restore() from
kernel_fpu_end(). The assumption being that tsk_used_math() is always
set for the eager-fpu mode and thus avoid the code path of enabling
interrupts, allocating fpu state using blocking call and disable
interrupts etc. 

But the below issue was noticed by Maarten Baert, Nate Eldredge and
few others:

If a user process dumps core on an ecrypt fs while aesni-intel is loaded,
we get a BUG() in __find_get_block() complaining that it was called with
interrupts disabled; then all further accesses to our ecrypt fs hang
and we have to reboot.

The aesni-intel code (encrypting the core file that we are writing) needs
the FPU and quite properly wraps its code in kernel_fpu_{begin,end}(),
the latter of which calls math_state_restore(). So after kernel_fpu_end(),
interrupts may be disabled, which nobody seems to expect, and they stay
that way until we eventually get to __find_get_block() which barfs.

For eager fpu, most the time, tsk_used_math() is true. At few instances
during thread exit, signal return handling etc, tsk_used_math() might
be false.

In kernel_fpu_end(), for eager-fpu, call math_state_restore()
only if tsk_used_math() is set. Otherwise, don't bother. Kernel code
path which cleared tsk_used_math() knows what needs to be done
with the fpu state.

Reported-by: Maarten Baert <maarten-baert@hotmail.com>
Reported-by: Nate Eldredge <nate@thatsmathematics.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Suresh Siddha <sbsiddha@gmail.com>
Cc: George Spelvin <linux@horizon.com>
---
 arch/x86/kernel/i387.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 4e5f770..670bba1 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -87,10 +87,19 @@ EXPORT_SYMBOL(__kernel_fpu_begin);
 
 void __kernel_fpu_end(void)
 {
-	if (use_eager_fpu())
-		math_state_restore();
-	else
+	if (use_eager_fpu()) {
+		/*
+		 * For eager fpu, most the time, tsk_used_math() is true.
+		 * Restore the user math as we are done with the kernel usage.
+		 * At few instances during thread exit, signal handling etc,
+		 * tsk_used_math() is false. Those few places will take proper
+		 * actions, so we don't need to restore the math here.
+		 */
+		if (likely(tsk_used_math(current)))
+			math_state_restore();
+	} else {
 		stts();
+	}
 }
 EXPORT_SYMBOL(__kernel_fpu_end);
 



  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-30 22:01 Nate Eldredge
2014-01-30 22:24 ` Linus Torvalds
2014-01-31  7:33   ` Suresh Siddha
2014-02-01 19:27     ` Linus Torvalds
2014-02-01 19:35       ` H. Peter Anvin
2014-02-01 19:46         ` Linus Torvalds
2014-02-01 20:00           ` H. Peter Anvin
2014-02-01 20:16             ` Linus Torvalds
2014-02-01 20:16           ` H. Peter Anvin
2014-02-01 21:17           ` George Spelvin
2014-02-01 21:36             ` H. Peter Anvin
2014-02-01 23:40             ` H. Peter Anvin
2014-02-02  0:17               ` Linus Torvalds
2014-02-02  1:19               ` George Spelvin
2014-02-02  1:25                 ` H. Peter Anvin
2014-02-02  8:45           ` Pekka Riikonen
2014-02-02  1:06       ` Suresh Siddha
2014-02-02  1:26         ` H. Peter Anvin
2014-02-02  1:35           ` Suresh Siddha
2014-02-02  1:38             ` Linus Torvalds
2014-02-02  1:47               ` Suresh Siddha
2014-02-02  1:51                 ` Linus Torvalds
2014-02-02  1:57                   ` H. Peter Anvin
2014-02-02  2:05                     ` Linus Torvalds
2014-02-02  2:12                       ` H. Peter Anvin
2014-02-02  1:59                   ` Suresh Siddha
2014-02-02  1:43             ` H. Peter Anvin
2014-02-02  1:47               ` Linus Torvalds
2014-02-02  7:19         ` Suresh Siddha
2014-02-02 19:15           ` Linus Torvalds
2014-02-03  6:56             ` Suresh Siddha [this message]
2014-02-03 18:20               ` Linus Torvalds
2014-02-04  6:03                 ` Suresh Siddha
2014-02-06  5:26               ` Nate Eldredge
2014-02-06  5:34                 ` George Spelvin
2014-02-13 15:45               ` Maarten Baert
2014-02-13 20:00                 ` George Spelvin
2014-03-11 19:36               ` [tip:x86/urgent] x86, fpu: Check tsk_used_math() in kernel_fpu_end() for eager FPU tip-bot for Suresh Siddha
2014-02-27 23:44           ` [PATCH] Make math_state_restore() save and restore the interrupt flag H. Peter Anvin
2014-03-07 23:18             ` H. Peter Anvin
2014-03-08  6:18               ` 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=1391410583.3801.6.camel@europa \
    --to=sbsiddha@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@horizon.com \
    --cc=maarten-baert@hotmail.com \
    --cc=mingo@kernel.org \
    --cc=nate@thatsmathematics.com \
    --cc=priikone@iki.fi \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git