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 >
