Messages in this thread Patch in this message | | | Date | Fri, 20 Feb 2015 19:13:43 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH 1/3] x86, fpu: __kernel_fpu_begin() should clear fpu_owner_task even if use_eager_fpu() |
| |
On Mon, Jan 19, 2015 at 07:51:32PM +0100, Oleg Nesterov wrote: > __kernel_fpu_begin() does nothing if !__thread_has_fpu() && use_eager_fpu(), > perhaps it assumes that this case is simply impossible. This is certainly > not possible if in_interrupt() == T; interrupted_user_mode() should have > FPU, and interrupted_kernel_fpu_idle() should fail if !__thread_has_fpu(). > > However, even if use_eager_fpu() == T a task can do drop_fpu(), then switch > to another thread which becomes fpu_owner_task, then resume and call some > function which does kernel_fpu_begin(). Say, an exiting task does a lot of > things after exit_thread(), it is not safe to assume that it can't use FPU > in these paths.
Yap, that makes sense. Applied.
> Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > arch/x86/kernel/i387.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c > index 81049ff..26f0e80 100644 > --- a/arch/x86/kernel/i387.c > +++ b/arch/x86/kernel/i387.c > @@ -93,9 +93,10 @@ void __kernel_fpu_begin(void) > > if (__thread_has_fpu(me)) { > __save_init_fpu(me); > - } else if (!use_eager_fpu()) { > + } else { > this_cpu_write(fpu_owner_task, NULL); > - clts(); > + if (!use_eager_fpu()) > + clts(); > }
Some git archeology:
304bceda6a18 ("x86, fpu: use non-lazy fpu restore for processors supporting xsave")
added that different handling on use_eager_fpu() boxes. interrupted_kernel_fpu_idle() failed then on those machines though and when it was switched to
if (use_eager_fpu()) return __thread_has_fpu(current);
in
5187b28ff082 ("x86: Allow FPU to be used at interrupt time even with eagerfpu")
it forgot to correct the "else if" in __kernel_fpu_begin().
Here's the relevant hunk from 304bceda6a18:
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c index ab6a2e8028ae..528557470ddb 100644 --- a/arch/x86/kernel/i387.c +++ b/arch/x86/kernel/i387.c @@ -22,7 +22,15 @@ /* * Were we in an interrupt that interrupted kernel mode? * - * We can do a kernel_fpu_begin/end() pair *ONLY* if that + * For now, on xsave platforms we will return interrupted + * kernel FPU as not-idle. TBD: As we use non-lazy FPU restore + * for xsave platforms, ideally we can change the return value + * to something like __thread_has_fpu(current). But we need to + * be careful of doing __thread_clear_has_fpu() before saving + * the FPU etc for supporting nested uses etc. For now, take + * the simple route! + * + * On others, we can do a kernel_fpu_begin/end() pair *ONLY* if that * pair does nothing at all: the thread must not have fpu (so * that we don't try to save the FPU state), and TS must * be set (so that the clts/stts pair does nothing that is @@ -30,6 +38,9 @@ */ static inline bool interrupted_kernel_fpu_idle(void) { + if (use_xsave()) + return 0; + return !__thread_has_fpu(current) && (read_cr0() & X86_CR0_TS); } @@ -73,7 +84,7 @@ void kernel_fpu_begin(void) __save_init_fpu(me); __thread_clear_has_fpu(me); /* We do 'stts()' in kernel_fpu_end() */ - } else { + } else if (!use_xsave()) { this_cpu_write(fpu_owner_task, NULL); clts(); } -- Regards/Gruss, Boris.
ECO tip #101: Trim your mails when you reply. --
| |