2014-07-11 23:56 GMT+08:00 Paul Burton <paul.burton@xxxxxxxxxx>: > 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. Yes, you're right. """ /* arch/mips/kernel/unaligned.c */ lose_fpu(1); /* Save FPU state for the emulator. */ res = fpu_emulator_cop1Handler(regs, ¤t->thread.fpu, 1, &fault_addr); own_fpu(1); /* Restore FPU state. */ """ Going deep into the code, I find lost_fpu(1) will save fpu context if owns fpu (otherwise, if preempted, the fpu context will be saved in process switch), then fpu_emulator_cop1Handler manipulates the saved fpu context, own_fpu(1) restores it. So, remove "BUG_ON(!is_fpu_owner())" is OK.
