Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Andy Lutomirski
On Sun, Mar 8, 2015 at 6:59 AM, Andy Lutomirski wrote: > There's another problem, though: We don't have a real stack pointer > just after syscall and just before sysexit, and therefore we *must* > use IST for anything that can happen while we still have > user-controlled rsp. That includes #DB,

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Andy Lutomirski
On Mar 8, 2015 4:55 AM, "Ingo Molnar" wrote: > > > * Linus Torvalds wrote: > > > On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar wrote: > > > > > > We could save the same 10 cycles from page fault overhead as well, > > > AFAICS. > > > > Are trap gates actually noticeably faster? Or is it just he >

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Ingo Molnar
* Ingo Molnar wrote: > Doing that would give us four (theoretical) performance advantages: > > - No implicit irq disabling overhead when the syscall instruction is > executed: we could change MSR_SYSCALL_MASK from 0xc084 to > 0xc284, which removes the implicit CLI on syscall

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Ingo Molnar
* Linus Torvalds wrote: > On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar wrote: > > > > We could save the same 10 cycles from page fault overhead as well, > > AFAICS. > > Are trap gates actually noticeably faster? Or is it just he > "conditional_sti()" you're worried about? ( I'll talk about

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Ingo Molnar
* Linus Torvalds torva...@linux-foundation.org wrote: On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar mi...@kernel.org wrote: We could save the same 10 cycles from page fault overhead as well, AFAICS. Are trap gates actually noticeably faster? Or is it just he conditional_sti() you're

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Ingo Molnar
* Ingo Molnar mi...@kernel.org wrote: Doing that would give us four (theoretical) performance advantages: - No implicit irq disabling overhead when the syscall instruction is executed: we could change MSR_SYSCALL_MASK from 0xc084 to 0xc284, which removes the implicit CLI

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Andy Lutomirski
On Mar 8, 2015 4:55 AM, Ingo Molnar mi...@kernel.org wrote: * Linus Torvalds torva...@linux-foundation.org wrote: On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar mi...@kernel.org wrote: We could save the same 10 cycles from page fault overhead as well, AFAICS. Are trap gates

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-08 Thread Andy Lutomirski
On Sun, Mar 8, 2015 at 6:59 AM, Andy Lutomirski l...@amacapital.net wrote: There's another problem, though: We don't have a real stack pointer just after syscall and just before sysexit, and therefore we *must* use IST for anything that can happen while we still have user-controlled rsp.

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Linus Torvalds
On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar wrote: > > We could save the same 10 cycles from page fault overhead as well, > AFAICS. Are trap gates actually noticeably faster? Or is it just he "conditional_sti()" you're worried about? Anyway, for page faulting, we traditionally actually wanted

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Ingo Molnar
* Andy Lutomirski wrote: > On Fri, Mar 6, 2015 at 2:00 PM, Linus Torvalds > wrote: > > On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski > > wrote: > >> > >> Please don't. IMO it's really nice that we don't use trap gates at > >> all on x86_64, and I find the conditional_sti thing much nicer

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Ingo Molnar
* Linus Torvalds wrote: > On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar wrote: > > > > math_state_restore() was historically called with irqs disabled, > > because that's how the hardware generates the trap, and also because > > back in the days it was possible for it to be an asynchronous > >

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Ingo Molnar
* Andy Lutomirski l...@amacapital.net wrote: On Fri, Mar 6, 2015 at 2:00 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski l...@amacapital.net wrote: Please don't. IMO it's really nice that we don't use trap gates at all on

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Ingo Molnar
* Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar mi...@kernel.org wrote: math_state_restore() was historically called with irqs disabled, because that's how the hardware generates the trap, and also because back in the days it was

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-07 Thread Linus Torvalds
On Sat, Mar 7, 2015 at 2:36 AM, Ingo Molnar mi...@kernel.org wrote: We could save the same 10 cycles from page fault overhead as well, AFAICS. Are trap gates actually noticeably faster? Or is it just he conditional_sti() you're worried about? Anyway, for page faulting, we traditionally

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Andy Lutomirski
On Fri, Mar 6, 2015 at 2:00 PM, Linus Torvalds wrote: > On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski wrote: >> >> Please don't. IMO it's really nice that we don't use trap gates at >> all on x86_64, and I find the conditional_sti thing much nicer than >> having to audit all of the entry

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Linus Torvalds
On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski wrote: > > Please don't. IMO it's really nice that we don't use trap gates at > all on x86_64, and I find the conditional_sti thing much nicer than > having to audit all of the entry code to see whether it's safe to run > it with IRQs on. So I'm

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Andy Lutomirski
On Fri, Mar 6, 2015 at 9:33 AM, Linus Torvalds wrote: > On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar wrote: >> >> math_state_restore() was historically called with irqs disabled, >> because that's how the hardware generates the trap, and also because >> back in the days it was possible for it to

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Linus Torvalds wrote: > > On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar wrote: > > > > math_state_restore() was historically called with irqs disabled, > > because that's how the hardware generates the trap, and also because > > back in the days it was possible for it to be an

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Linus Torvalds
On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar wrote: > > math_state_restore() was historically called with irqs disabled, > because that's how the hardware generates the trap, and also because > back in the days it was possible for it to be an asynchronous > interrupt and interrupt handlers run

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, David Vrabel wrote: > > On 06/03/15 15:36, Oleg Nesterov wrote: > > > > This needs more discussion, but in short so far I think that fpu_alloc() > > from #NM exception is fine if user_mode(regs) == T. > > I think a memory allocation here, where the only behaviour on a failure > is to

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread David Vrabel
On 06/03/15 15:36, Oleg Nesterov wrote: > On 03/06, David Vrabel wrote: >> >> On 06/03/15 14:01, Oleg Nesterov wrote: >> >>> Not sure I understand it correctly after the first quick look, but >>> >>> 1. It conflicts with the recent changes in tip/x86/fpu >>> >>> 2. fpu_ini() initializes

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, David Vrabel wrote: > > On 06/03/15 14:01, Oleg Nesterov wrote: > > > Not sure I understand it correctly after the first quick look, but > > > > 1. It conflicts with the recent changes in tip/x86/fpu > > > > 2. fpu_ini() initializes current->thread.fpu.state. This looks unneeded, > >

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread David Vrabel
On 06/03/15 14:01, Oleg Nesterov wrote: > On 03/06, Ingo Molnar wrote: >> >> * Oleg Nesterov wrote: >> >>> On 03/06, Ingo Molnar wrote: * Oleg Nesterov wrote: > [...] The patch above looks "obviously safe", but perhaps I am > paranoid too much... IMHO your hack

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Oleg Nesterov wrote: > > On 03/06, Ingo Molnar wrote: > > > > How about the patch from David Vrabel? That seems to solve the > > irq-disable problem too, right? > > I wasn't cc'ed, I guess you mean > > [PATCHv4] x86, fpu: remove the logic of non-eager fpu mem allocation at > the

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Ingo Molnar wrote: > > * Oleg Nesterov wrote: > > > On 03/06, Ingo Molnar wrote: > > > > > > * Oleg Nesterov wrote: > > > > > > > [...] The patch above looks "obviously safe", but perhaps I am > > > > paranoid too much... > > > > > > IMHO your hack above isn't really acceptable, even

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Ingo Molnar
* Oleg Nesterov wrote: > On 03/06, Ingo Molnar wrote: > > > > * Oleg Nesterov wrote: > > > > > [...] The patch above looks "obviously safe", but perhaps I am > > > paranoid too much... > > > > IMHO your hack above isn't really acceptable, even for a backport. > > So lets test the patch below

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Oleg Nesterov wrote: > > OK, but please note that this patch is not beckportable. If you think > that -stable doesn't need this fix, then I agree. > > If the caller is do_device_not_available(), then we can not enable > irqs before __thread_fpu_begin() + restore_fpu_checking(). > > 1.

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Ingo Molnar wrote: > > * Oleg Nesterov wrote: > > > [...] The patch above looks "obviously safe", but perhaps I am > > paranoid too much... > > IMHO your hack above isn't really acceptable, even for a backport. > So lets test the patch below (assuming it's the right thing to do) > and

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Oleg Nesterov wrote: On 03/06, Ingo Molnar wrote: How about the patch from David Vrabel? That seems to solve the irq-disable problem too, right? I wasn't cc'ed, I guess you mean [PATCHv4] x86, fpu: remove the logic of non-eager fpu mem allocation at the first usage

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread David Vrabel
On 06/03/15 14:01, Oleg Nesterov wrote: On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: [...] The patch above looks obviously safe, but perhaps I am paranoid too much... IMHO your hack above isn't

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, David Vrabel wrote: On 06/03/15 14:01, Oleg Nesterov wrote: Not sure I understand it correctly after the first quick look, but 1. It conflicts with the recent changes in tip/x86/fpu 2. fpu_ini() initializes current-thread.fpu.state. This looks unneeded, the kernel

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: [...] The patch above looks obviously safe, but perhaps I am paranoid too much... IMHO your hack above isn't really acceptable, even

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Oleg Nesterov wrote: OK, but please note that this patch is not beckportable. If you think that -stable doesn't need this fix, then I agree. If the caller is do_device_not_available(), then we can not enable irqs before __thread_fpu_begin() + restore_fpu_checking(). 1. Preemption

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: [...] The patch above looks obviously safe, but perhaps I am paranoid too much... IMHO your hack above isn't really acceptable, even for a backport. So lets test the patch below (assuming it's the right thing to do) and

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Ingo Molnar
* Oleg Nesterov o...@redhat.com wrote: On 03/06, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: [...] The patch above looks obviously safe, but perhaps I am paranoid too much... IMHO your hack above isn't really acceptable, even for a backport. So lets test the

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread David Vrabel
On 06/03/15 15:36, Oleg Nesterov wrote: On 03/06, David Vrabel wrote: On 06/03/15 14:01, Oleg Nesterov wrote: Not sure I understand it correctly after the first quick look, but 1. It conflicts with the recent changes in tip/x86/fpu 2. fpu_ini() initializes current-thread.fpu.state. This

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, David Vrabel wrote: On 06/03/15 15:36, Oleg Nesterov wrote: This needs more discussion, but in short so far I think that fpu_alloc() from #NM exception is fine if user_mode(regs) == T. I think a memory allocation here, where the only behaviour on a failure is to kill the task,

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Linus Torvalds
On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar mi...@kernel.org wrote: math_state_restore() was historically called with irqs disabled, because that's how the hardware generates the trap, and also because back in the days it was possible for it to be an asynchronous interrupt and interrupt

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Oleg Nesterov
On 03/06, Linus Torvalds wrote: On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar mi...@kernel.org wrote: math_state_restore() was historically called with irqs disabled, because that's how the hardware generates the trap, and also because back in the days it was possible for it to be an

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Andy Lutomirski
On Fri, Mar 6, 2015 at 9:33 AM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Mar 5, 2015 at 11:58 PM, Ingo Molnar mi...@kernel.org wrote: math_state_restore() was historically called with irqs disabled, because that's how the hardware generates the trap, and also because back

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Linus Torvalds
On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski l...@amacapital.net wrote: Please don't. IMO it's really nice that we don't use trap gates at all on x86_64, and I find the conditional_sti thing much nicer than having to audit all of the entry code to see whether it's safe to run it with

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-06 Thread Andy Lutomirski
On Fri, Mar 6, 2015 at 2:00 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Fri, Mar 6, 2015 at 11:23 AM, Andy Lutomirski l...@amacapital.net wrote: Please don't. IMO it's really nice that we don't use trap gates at all on x86_64, and I find the conditional_sti thing much nicer

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Ingo Molnar
* Oleg Nesterov wrote: > On 03/05, Ingo Molnar wrote: > > > > * Oleg Nesterov wrote: > > > > > --- a/arch/x86/kernel/traps.c > > > +++ b/arch/x86/kernel/traps.c > > > @@ -774,7 +774,10 @@ void math_state_restore(void) > > > struct task_struct *tsk = current; > > > > > > if

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Oleg Nesterov
On 03/05, Ingo Molnar wrote: > > * Oleg Nesterov wrote: > > > --- a/arch/x86/kernel/traps.c > > +++ b/arch/x86/kernel/traps.c > > @@ -774,7 +774,10 @@ void math_state_restore(void) > > struct task_struct *tsk = current; > > > > if (!tsk_used_math(tsk)) { > > -

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Ingo Molnar
* Oleg Nesterov wrote: > math_state_restore() assumes it is called with irqs disabled, but > this is not true if the caller is __restore_xstate_sig(). > > This means that if ia32_fxstate == T and __copy_from_user() fails > __restore_xstate_sig() returns with irqs disabled too. This trgiggers >

[PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Oleg Nesterov
math_state_restore() assumes it is called with irqs disabled, but this is not true if the caller is __restore_xstate_sig(). This means that if ia32_fxstate == T and __copy_from_user() fails __restore_xstate_sig() returns with irqs disabled too. This trgiggers BUG: sleeping function

[PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Oleg Nesterov
math_state_restore() assumes it is called with irqs disabled, but this is not true if the caller is __restore_xstate_sig(). This means that if ia32_fxstate == T and __copy_from_user() fails __restore_xstate_sig() returns with irqs disabled too. This trgiggers BUG: sleeping function

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Ingo Molnar
* Oleg Nesterov o...@redhat.com wrote: math_state_restore() assumes it is called with irqs disabled, but this is not true if the caller is __restore_xstate_sig(). This means that if ia32_fxstate == T and __copy_from_user() fails __restore_xstate_sig() returns with irqs disabled too. This

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Oleg Nesterov
On 03/05, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -774,7 +774,10 @@ void math_state_restore(void) struct task_struct *tsk = current; if (!tsk_used_math(tsk)) { - local_irq_enable();

Re: [PATCH 1/1] x86/fpu: math_state_restore() should not blindly disable irqs

2015-03-05 Thread Ingo Molnar
* Oleg Nesterov o...@redhat.com wrote: On 03/05, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -774,7 +774,10 @@ void math_state_restore(void) struct task_struct *tsk = current; if