Re: [PATCH v2 3/9] rcu,tracing: Create trace_rcu_{enter,exit}()

Thomas Gleixner
2020-03-06T00:42:46Z

Steven Rostedt <rostedt@goodmis.org> writes:
> rcu_nmi_enter() was marked NOKPROBE or other reasons. See commit

Very well said: OR other reasons. I assume you meant 'for' but ...

> c13324a505c77 ("x86/kprobes: Prohibit probing on functions before
> kprobe_int3_handler()")
>
> The issue was that we must not allow anything in do_int3() call kprobe
> code before kprobe_int3_handler() is called. Because ist_enter() (in
> do_int3()) calls rcu_nmi_enter() it had to be marked NOKPROBE. It had
> nothing to do with it being RCU nor NMI, but because it was simply
> called in do_int3().
>
> Thus, there's no reason to make rcu_nmi_exit() NOKPROBE. But a commont
> to why rcu_nmi_enter() would probably be useful, like below:

... this is really wrong.

While the int3 issue was the reason why it was marked NOKPROBE, fact is
that aside of int3 problem (which is probably true for any other
architecture using breakpoints for patching) any probe _before_ RCU is
watching and _after_ RCU stopped watching is broken. Same applies for
BPF and tracepoints which call into BPF or other nonsense.

Can we please stop claiming that instrumentation can touch anything it
wants and just admit that anything outside RCU covered regions is
off-limits for instrumentation? Same applies for entry code and critical
exceptions/traps.

There is a reason why the tracer can't trace itself and there are very
valid reasons to limit the instrumentation ability in other places.

It's nice to be able to see into 'everything' but for 99.9999% of the
cases the coverage of these things is absolutely irrelevant.

Yes I know, "Correctness first" is this old school thing for grumpy old
greybeards who are still stuck in the 90's. "Features first" is the new
mantra. I deal with that every day by mopping up the mess it creates.

Thanks,

tglx