On Fri, Mar 7, 2014 at 3:18 PM, H. Peter Anvin wrote:
>
> Hi Suresh,
>
> Any thoughts on this?
hi Peter,
Can you please pickup the second short patch
(https://lkml.org/lkml/2014/2/3/21) which actually fixes the reported
problem at hand. And tested and acked by all the problem reporters.
I will
Hi Suresh,
Any thoughts on this?
-hpa
On 02/27/2014 03:44 PM, H. Peter Anvin wrote:
> So, picking up this thread which got dropped on the floor...
>
> On 02/01/2014 11:19 PM, Suresh Siddha wrote:
>>
>> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
>> index e8368c6..4e5f77
So, picking up this thread which got dropped on the floor...
On 02/01/2014 11:19 PM, Suresh Siddha wrote:
>
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index e8368c6..4e5f770 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -5,6 +5,7 @@
> * General
On 03/02/14 07:56, Suresh Siddha wrote:
> 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.
Maarten reminded me that I should officially say it's worked for
On 03/02/14 07:56, Suresh Siddha wrote:
> 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.
I can confirm that your patch fixes the bug I reported (core dump
> 3.13 plus this patch: boots and fixes the testcase I reported (core dump
> on ecrypt).
>
> Tested-by: Nate Eldredge
It's looking good for me so far, too. I'm slower to reproduce, so I'd
like a couple more days before signing off on it, but no complaints yet.
--
To unsubscribe from this list:
On Sun, 2 Feb 2014, Suresh Siddha wrote:
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.
3.13 plus this patc
On Mon, 2014-02-03 at 10:20 -0800, Linus Torvalds wrote:
> Thinking about it some more, this patch is *almost* not needed at all.
>
> I'm wondering if you should just change the first patch to just always
> initialize the fpu when it is allocated, and at execve() time (ie in
> flush_thread()).
>
On Sun, Feb 2, 2014 at 10:56 PM, Suresh Siddha wrote:
>
> 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 h
On Sun, 2014-02-02 at 11:15 -0800, Linus Torvalds wrote:
> On Sat, Feb 1, 2014 at 11:19 PM, Suresh Siddha 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
On Sat, Feb 1, 2014 at 11:19 PM, Suresh Siddha 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
On Sat, 1 Feb 2014, Linus Torvalds wrote:
We could do that with the whole "task_work" thing (or perhaps just
do_notify_resume(), especially after merging the "don't necessarily
return with iret" patch I sent out earlier), with additionally making
sure that scheduling does the right thing wrt a "
On Sat, 2014-02-01 at 17:06 -0800, Suresh Siddha wrote:
> Meanwhile I have the patch removing the delayed dynamic allocation for
> non-eager fpu. will post it after some testing.
Appended the patch for this. Tested for last 4-5 hours on my laptop.
The real fix for Nate's problem will be coming fr
Yes, that is exactly the "eageronly" features - currently LWP and MPX.
On February 1, 2014 6:05:05 PM PST, Linus Torvalds
wrote:
>On Sat, Feb 1, 2014 at 5:57 PM, H. Peter Anvin wrote:
>>
>> Twiddling CR0.TS is pretty slow if we're not taking advantage of it.
>
>Immaterial.
>
>We *already* avoid
On Sat, Feb 1, 2014 at 5:57 PM, H. Peter Anvin wrote:
>
> Twiddling CR0.TS is pretty slow if we're not taking advantage of it.
Immaterial.
We *already* avoid twiddling TS if it's not needed.
It is true that we used to twiddle it at every context switch (and
then twiddle it *again* if we decided
On 02/01/2014 05:51 PM, Linus Torvalds wrote:
> On Sat, Feb 1, 2014 at 5:47 PM, Suresh Siddha wrote:
>>
>> So if the restore failed, we should do something like drop_init_fpu(),
>> which will restore init-state to the registers.
>>
>> for eager-fpu() paths we don't use clts() stts() etc.
>
> Uhh
On Sat, Feb 1, 2014 at 5:51 PM, Linus Torvalds
wrote:
> On Sat, Feb 1, 2014 at 5:47 PM, Suresh Siddha wrote:
>>
>> So if the restore failed, we should do something like drop_init_fpu(),
>> which will restore init-state to the registers.
>>
>> for eager-fpu() paths we don't use clts() stts() etc.
On Sat, Feb 1, 2014 at 5:47 PM, Suresh Siddha wrote:
>
> So if the restore failed, we should do something like drop_init_fpu(),
> which will restore init-state to the registers.
>
> for eager-fpu() paths we don't use clts() stts() etc.
Uhhuh. Ok.
Why do we do that, btw? I think it would make mu
On Sat, Feb 1, 2014 at 5:38 PM, Linus Torvalds
wrote:
> It definitely does not want an else, I think.
>
> If tsk_used_math() is false, or if the FPU restore failed, we
> *definitely* need that stts(). Otherwise we'd return to user mode with
> random contents in the FP state, and let user mode muck
On Sat, Feb 1, 2014 at 5:43 PM, H. Peter Anvin wrote:
> What does the inner if clause do? It looks like it returns either way...
Suresh broke it with his suggested version.
The inner if-statement is supposed to avoid the stts *if* we had used
math *and* the FPU restore worked.
But with the extr
What does the inner if clause do? It looks like it returns either way...
On February 1, 2014 5:35:13 PM PST, Suresh Siddha wrote:
>On Sat, Feb 1, 2014 at 5:26 PM, H. Peter Anvin wrote:
>> Even "b" does that, no?
>
>oh right. It needs an else. only for non-eager fpu case we should do
>stts()
>
>
On Sat, Feb 1, 2014 at 5:35 PM, Suresh Siddha wrote:
> On Sat, Feb 1, 2014 at 5:26 PM, H. Peter Anvin wrote:
>> Even "b" does that, no?
>
> oh right. It needs an else. only for non-eager fpu case we should do stts()
It definitely does not want an else, I think.
If tsk_used_math() is false, or i
On Sat, Feb 1, 2014 at 5:26 PM, H. Peter Anvin wrote:
> Even "b" does that, no?
oh right. It needs an else. only for non-eager fpu case we should do stts()
void __kernel_fpu_end(void)
{
if (use_eager_fpu()) {
struct task_struct *me = current;
if
On 02/01/2014 05:06 PM, Suresh Siddha wrote:
>
> so I will Ack for option "b", as option "a" breaks the features which
> don't take into account cr0.TS.
>
Even "b" does that, no? "a" should be fine as long as we don't ever use
those features in the kernel, even under kernel_fpu_begin/end().
>
On 02/01/2014 05:19 PM, George Spelvin wrote:
>> OK, let's circle back for a bit. We have an active bug, and we clearly
>> have a lot of restructuring that could/should be done. We need to fix
>> the bug first; if we're going to a bunch of restructuring then that
>> ought to be separate. The fir
> OK, let's circle back for a bit. We have an active bug, and we clearly
> have a lot of restructuring that could/should be done. We need to fix
> the bug first; if we're going to a bunch of restructuring then that
> ought to be separate. The first bit is how we fix the immediate bug.
Well, tha
On Sat, Feb 1, 2014 at 11:27 AM, Linus Torvalds
wrote:
> That said, regardless of the allocation issue, I do think that it's
> stupid for kernel_fpu_{begin,end} to save the math state if
> "used_math" was not set. So I do think__kernel_fpu_end() as-s is
> buggy and stupid.
For eager_fpu case, as
On Sat, Feb 1, 2014 at 3:40 PM, H. Peter Anvin wrote:
>
> OK, let's circle back for a bit. We have an active bug, and we clearly
> have a lot of restructuring that could/should be done. We need to fix
> the bug first; if we're going to a bunch of restructuring then that
> ought to be separate.
On 02/01/2014 01:17 PM, George Spelvin wrote:
>> .. which *does* actually bring up something that might work, and might
>> be a good idea: remove the "restore math state or set TS" from the
>> normal kernel paths *entirely*, and move it to the "return to user
>> space" phase.
>
> This definitely s
It would be good to know how often #3 happens on a real workload.
On February 1, 2014 1:17:10 PM PST, George Spelvin wrote:
>> .. which *does* actually bring up something that might work, and
>might
>> be a good idea: remove the "restore math state or set TS" from the
>> normal kernel paths *enti
> .. which *does* actually bring up something that might work, and might
> be a good idea: remove the "restore math state or set TS" from the
> normal kernel paths *entirely*, and move it to the "return to user
> space" phase.
This definitely seems much more sensible. For processors without
optim
On 02/01/2014 11:46 AM, Linus Torvalds wrote:
>
> .. which *does* actually bring up something that might work, and might
> be a good idea: remove the "restore math state or set TS" from the
> normal kernel paths *entirely*, and move it to the "return to user
> space" phase.
>
The task switch cod
On Sat, Feb 1, 2014 at 12:00 PM, H. Peter Anvin wrote:
> Of course, if we are really doing all eager fpu, we could just leave cr0.ts
> always clear and not touch it at all...
That's what the eager fpu code tries to do now (apart from process
initialization, I think). But the problem is that even
Of course, if we are really doing all eager fpu, we could just leave cr0.ts
always clear and not touch it at all...
On February 1, 2014 11:46:15 AM PST, Linus Torvalds
wrote:
>On Sat, Feb 1, 2014 at 11:35 AM, H. Peter Anvin wrote:
>>
>> This will obviously not protect eageronly features (MPX,
On Sat, Feb 1, 2014 at 11:35 AM, H. Peter Anvin wrote:
>
> This will obviously not protect eageronly features (MPX, LWP, ...) so this
> means those features are permanently unavailable to the kernel, even inside
> kernel_fpu_begin/end. Now, currently I don't think we have any plans to use
> those
On 02/01/2014 11:27 AM, Linus Torvalds wrote:
(a) "we don't want to restore at all, because once the kernel starts
using math, it might do so a lot, and saving/restoring is a bad idea":
void __kernel_fpu_end(void)
{
stts();
}
*or*
Quite frankly, I'd almost lean towards (
On Thu, Jan 30, 2014 at 11:33 PM, Suresh Siddha wrote:
>
> a. delayed dynamic allocation of FPU state area was not a good idea
> (from me). Given most of the future cases will be anyway using eager
> FPU (because of processor features like xsaveopt etc, applications
> implicitly using FPU because
hi,
On Thu, Jan 30, 2014 at 2:24 PM, Linus Torvalds
wrote:
> I'm adding in some people here, because I think in the end this bug
> was introduced by commit 304bceda6a18 ("x86, fpu: use non-lazy fpu
> restore for processors supporting xsave") that introduced that
> math_state_restore() in kernel_f
On Thu, Jan 30, 2014 at 2:01 PM, Nate Eldredge
wrote:
>
> If math_state_restore() is called in a task that has not used math, it
> needs to allocate some memory (via init_fpu()). Since this can sleep,
> it enables interrupts first. Currently, it always disables them
> afterwards, regardless of w
39 matches
Mail list logo
