Re: [PATCH] Not preempt in CP1 exception handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Paul,

You means my patch (http://patchwork.linux-mips.org/patch/7297/) is
the correct way?

Another question: Your patch
(http://patchwork.linux-mips.org/patch/7307/) remove
preempt_disable()/preempt_enable() in init_fpu(). It will cause
problems if there is another function call init_fpu() because it is
previously preempt-safe. Maybe introduce a new function (e.g.
__init_fpu()) is a better way?

Huacai

On Sat, Jul 12, 2014 at 7:28 AM, Chen Jie <chenj@xxxxxxxxxx> wrote:
> 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, &current->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.
>


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux