On Sun, 21 May 2023 22:26:37 +0200 Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > On Sun, May 21, 2023 at 11:10:16PM +0800, Ze Gao wrote: > > > kprobe_multi/fprobe share the same set of attachments with fentry. > > > Currently, fentry does not filter with !rcu_is_watching, maybe > > > because this is an extreme corner case. Not sure whether it is > > > worthwhile or not. > > > > Agreed, it's rare, especially after Peter's patches which push narrow > > down rcu eqs regions > > in the idle path and reduce the chance of any traceable functions > > happening in between. > > > > However, from RCU's perspective, we ought to check if rcu_is_watching > > theoretically > > when there's a chance our code will run in the idle path and also we > > need rcu to be alive, > > And also we cannot simply make assumptions for any future changes in > > the idle path. > > You know, just like what was hit in the thread. > > > > > Maybe if you can give a concrete example (e.g., attachment point) > > > with current code base to show what the issue you encountered and > > > it will make it easier to judge whether adding !rcu_is_watching() > > > is necessary or not. > > > > I can reproduce likely warnings on v6.1.18 where arch_cpu_idle is > > traceable but not on the latest version > > so far. But as I state above, in theory we need it. So here is a > > gentle ping :) . > > hum, this change [1] added rcu_is_watching check to ftrace_test_recursion_trylock, > which we use in fprobe_handler and is coming to fprobe_exit_handler in [2] > > I might be missing something, but it seems like we don't need another > rcu_is_watching call on kprobe_multi level Good point! OK, then it seems we don't need it. The rethook continues to use the rcu_is_watching() because it is also used from kprobes, but the kprobe_multi doesn't need it. Thank you, > > jirka > > > [1] d099dbfd3306 cpuidle: tracing: Warn about !rcu_is_watching() > [2] https://lore.kernel.org/bpf/20230517034510.15639-4-zegao@xxxxxxxxxxx/ -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
