On 2021/10/12 下午8:24, Miroslav Benes wrote:
[snip]
>>
>> func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
>> stack_node);
>> @@ -120,7 +115,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>> klp_arch_set_pc(fregs, (unsigned long)func->new_func);
>>
>> unlock:
>> - preempt_enable_notrace();
>> ftrace_test_recursion_unlock(bit);
>> }
>
> I don't like this change much. We have preempt_disable there not because
> of ftrace_test_recursion, but because of RCU. ftrace_test_recursion was
> added later. Yes, it would work with the change, but it would also hide
> things which should not be hidden in my opinion.
Not very sure about the backgroup stories, but just found this in
'Documentation/trace/ftrace-uses.rst':
Note, on success,
ftrace_test_recursion_trylock() will disable preemption, and the
ftrace_test_recursion_unlock() will enable it again (if it was previously
enabled).
Seems like this lock pair was supposed to take care the preemtion itself?
Regards,
Michael Wang
>
> Miroslav
>
