On 2019/10/8 18:25, Will Deacon wrote:
> On Tue, Oct 08, 2019 at 10:33:17AM +0800, Yunfeng Ye wrote:
>> On 2019/10/7 23:37, Will Deacon wrote:
>>> On Mon, Oct 07, 2019 at 06:06:35PM +0800, Yunfeng Ye wrote:
>>>> @@ -617,25 +624,47 @@ static int t16_setend_handler(struct pt_regs *regs,
>>>> u32 instr)
>>>> */
>>>> static int __init armv8_deprecated_init(void)
>>>> {
>>>> - if (IS_ENABLED(CONFIG_SWP_EMULATION))
>>>> - register_insn_emulation(&swp_ops);
>>>> + int ret = 0;
>>>> + int err = 0;
>>>> +
>>>> + if (IS_ENABLED(CONFIG_SWP_EMULATION)) {
>>>> + ret = register_insn_emulation(&swp_ops);
>>>> + if (ret) {
>>>> + pr_err("register insn emulation swp: fail\n");
>>>> + err = ret;
>>>> + }
>>>> + }
>>>
>>> Is there much point in continuing here? May as well just return ret, I
>>> think. I also don't think you need to print anything, since kmalloc
>>> should already have shouted.
>>>
>> The registration of each instruction simulation is independent. I think
>> that one failure does not affect the registration of other instructions.
>
> Dunno, I think that if kmalloc() starts failing then it's time to give up!
>
>> In addition, if return directly, is it need to unregister? Of course,
>> the first instruction registration can be directly returned, If the
>> following instruction registration fails, is it need unregister operation?
>> currently the unregistration of instruction simulation is not be implemented
>> yet.
>
> That's an interesting one -- currently there isn't a way to unregister
> an emulation hook afaict. We could add unregister_insn_emulation() to
> remove the emulation hook from the insn_emulation list and free it, but
> I'm actually now starting to prefer your initial patch after all. The only
> way these failures will happen are either because the system is doomed
> or kmalloc fault injection is being used; so keeping things simple rather
> than add rarely executed complexity is probably best.
>
>> The purpose of printing information is to replace the direct return, which
>> can distinguish which instruction failed to register. There is no need to
>> print
>> information if it returns directly.
>
> What do you expect people to do with that information?
>
> Are you ok with me applying your original patch?
>
I agree, it is simple. thanks.
> Will
>
> .
>
