On Wed, 13 Oct 2021 09:50:17 +0800 王贇 <yun.wang@xxxxxxxxxxxxxxxxx> wrote: > >> - 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). Right that part is to be fixed by what you are adding here. The point that Miroslav is complaining about is that the preemption disabling is special in this case, and not just from the recursion point of view, which is why the comment is still required. -- Steve > > Seems like this lock pair was supposed to take care the preemtion itself?