On Fri, Jul 11, 2014 at 11:14:13AM +0800, chenj wrote: > do_ade may be invoked with preempt enabled. do_cpu will be invoked with > preempt enabled. When it's preempted(in do_ade/do_cpu), TIF_USEDFPU will be > cleared, when it returns to do_ade/do_cpu, the fpu is actually disabled. > > e.g. > In do_ade() > emulate_load_store_insn(): > BUG_ON(!is_fpu_owner()); <-- This assertion may be breaked. > > In do_cpu() > enable_restore_fp_context(): > was_fpu_owner = is_fpu_owner(); Preemption should indeed be disabled around the assignment & use of the was_fpu_owner variable, but note that you can only hit the problem if using MSA. One of the MSA fixes I just submitted also fixes this along with another instance of the problem: http://patchwork.linux-mips.org/patch/7307/ I prefer my patch to this since it disables preemption for less time, in addition to fixing the !used_math() case. In emulate_load_store_insn I believe the correct fix is simply to remove that BUG_ON. The code is about to give up FPU ownership anyway, so it's not like there is any requirement being violated if it was already lost. Thanks, Paul > This patch simply disables interrupts in related handlers, and > disable preempt/enable interrupts in do_ade/do_cpu. > --- > arch/mips/kernel/genex.S | 4 ++-- > arch/mips/kernel/traps.c | 4 ++++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > index ac35e12..a5c6931 100644 > --- a/arch/mips/kernel/genex.S > +++ b/arch/mips/kernel/genex.S > @@ -370,7 +370,7 @@ NESTED(nmi_handler, PT_SIZE, sp) > .macro __build_clear_ade > MFC0 t0, CP0_BADVADDR > PTR_S t0, PT_BVADDR(sp) > - KMODE > + CLI > .endm > > .macro __BUILD_silent exception > @@ -422,7 +422,7 @@ NESTED(nmi_handler, PT_SIZE, sp) > BUILD_HANDLER dbe be cli silent /* #7 */ > BUILD_HANDLER bp bp sti silent /* #9 */ > BUILD_HANDLER ri ri sti silent /* #10 */ > - BUILD_HANDLER cpu cpu sti silent /* #11 */ > + BUILD_HANDLER cpu cpu cli silent /* #11 */ > BUILD_HANDLER ov ov sti silent /* #12 */ > BUILD_HANDLER tr tr sti silent /* #13 */ > BUILD_HANDLER msa_fpe msa_fpe sti silent /* #14 */ > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c > index 51706d6..0e0f7de 100644 > --- a/arch/mips/kernel/traps.c > +++ b/arch/mips/kernel/traps.c > @@ -1166,6 +1166,9 @@ asmlinkage void do_cpu(struct pt_regs *regs) > int status, err; > unsigned long __maybe_unused flags; > > + preempt_disable(); > + local_irq_enable(); > + > prev_state = exception_enter(); > cpid = (regs->cp0_cause >> CAUSEB_CE) & 3; > > @@ -1258,6 +1261,7 @@ asmlinkage void do_cpu(struct pt_regs *regs) > > out: > exception_exit(prev_state); > + preempt_enable(); > } > > asmlinkage void do_msa_fpe(struct pt_regs *regs) > -- > 1.9.0 > >
