x86, fpu: make use_eager_fpu() more eager
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest5 patches 8 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/5 | x86, fpu: change sanitize_restored_xstate() and convert_to_fxsr() to accept thread_xstatearch/ arch/ arch/ | +10 | 0 replies | |
| 2/5 | x86, fpu: don't drop_fpu() in __restore_xstate_sig() if use_eager_fpu()arch/ | +22 | 8 replies | |
| 3/5 | x86, fpu: don't drop_fpu() in exit_thread() if use_eager_fpu()arch/ | +2 | 0 replies | |
| 4/5 | x86, fpu: shift init_fpu() from eager_fpu_init() to eager_fpu_init_bp()arch/ | +3 | 0 replies | |
| 5/5 | x86, fpu: sanitize the usage of use_eager_fpu() in switch_fpu_prepare()arch/ | +4 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 0/5] x86, fpu: make use_eager_fpu() more eager
Quoted 11 lines · Oleg Nesterov
Al, Linus, could you take a look?
Looks simple, but I have to admit that every time I read FPU code I feel that I never read it before. And I never really understood it in details.
See the changelog, but in short
drop_init_fpu()insave_xstate_sig()looks wrong. This assumes that we are going to call the handler and thus we need the new FPU state. But this is only true ifsetup_frame()won't fail after that. If it fails, we simply lose the FPU state.
Add more cc's. Suresh, could you please review?
While at it, can't we cleanup the use_eager_fpu() logic? Just look at switch_fpu_prepare(),
else if (!use_eager_fpu()) stts();
looks "obviously buggy" if use_eager_fpu() && !tsk_used_math(new). In this case the "new" task will use the unitialized fpu state if it starts to use fpu. Yes, in fact this is not a bug, this is only possible when we know that the task can't return to usermode (__restore_xstate_sig(), exit_thread()).
But still this doesn't look clean, see 5/5. And I think we can do a lot more cleanups on top of this change.
But! I know absolutely nothing about i387, and I do not know how can I test these changes. In any case these patches should be ignored without authoritative acks. However the kernel seems to boot/run fine with the additional debugging patch below.
Oleg.
arch/x86/include/asm/fpu-internal.h | 10 +++--- arch/x86/kernel/i387.c | 6 ++-- arch/x86/kernel/process.c | 3 +- arch/x86/kernel/xsave.c | 55 +++++++++++++++++++--------------- 4 files changed, 41 insertions(+), 33 deletions(-)
-------------------------------------------------------------------------------
arch/x86/kernel/process_64.c +10 -0
@@ -283,8 +283,18 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)unsigned fsindex, gsindex;fpu_switch_t fpu; +if (use_eager_fpu ()) { +BUG_ON (!__thread_has_fpu (prev_p)); +BUG_ON (!tsk_used_math (prev_p)); +BUG_ON (!tsk_used_math (next_p)); + } + fpu =switch_fpu_prepare (prev_p, next_p, cpu); +if (use_eager_fpu ()) { +BUG_ON (!__thread_has_fpu (next_p)); + } +/* * Reload esp0, LDT and the page table pointer: */
include/linux/sched.h +1 -1
@@ -1923,7 +1923,7 @@ extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut,* child is not running and in turn not changing child->flags * at the same time the parent does it. */ -#define clear_stopped_child_used_math (child) do { (child)->flags &= ~PF_USED_MATH; } while (0) +#define clear_stopped_child_used_math (child) do { BUG_ON(use_eager_fpu()); (child)->flags &= ~PF_USED_MATH; } while (0)#define set_stopped_child_used_math (child) do { (child)->flags |= PF_USED_MATH; } while (0)#define clear_used_math () clear_stopped_child_used_math(current)#define set_used_math () set_stopped_child_used_math(current)