Messages in this thread | | | Date | 28 Jan 2014 13:53:15 -0500 | From | "George Spelvin" <> | Subject | Re: math_state_restore and kernel_fpu_end disable interrupts? |
| |
> I'm trying it now. But it takes a while for me to reproduce, and even > longer to be sure the problem has gone away. So anything you hear from > me within a week will be bad news.
Well, it's been a week, and: good news!
I'd still wish for some review by someone who really understands this code; in particular it seems dangerous to just enable interrupts for a window without re-checking the condition afterward.
What if an interrupt hander wants to use the FPU and triggers the allocate itself? Shouldn't it be: * Enable interrupts * Allocate * Disable interrupts * Check that tsk->thread.xstate is still NULL * (If it has been filled in, free and return.) * Fill in tsk->thread.xstate
I don't feel I really understand the irq_fpu_usable() logic in arch/x86/i387.c.
But this patch clearly doesn't make these issues any *worse*, so these concerns are no reason to block it.
Would you like add an appropriate commit message and send in the patch?
Something like:
Subject: arch/x86/kernel/traps.c: make math_state_restore preserve IRQ status.
Commit aa283f4927 (in 2.6.26!) to add lazy FPU save are allocation did an local_irq_enable()/local_irq_disable() around the allocate. However, that assumes that it is only called with interrupts disabled.
math_state_restore() can also be called from kernel_fpu_end() with interrupts enabled. Very occasionally, this triggers an FPU state allocation. Disabling interrupts unconditionally is Bad.
Not-yet-Signed-off-by: Nate Eldredge <nate@thatsmathematics.com> Tested-by: George Spelvin <linux@horizon.com> Cc: <stable@vger.kernel.org> Fixes: aa283f4927
| |