Messages in this thread | | | From | Andy Lutomirski <> | Subject | Re: [patch 4/8] x86/entry: Move irq tracing on syscall entry to C-code | Date | Sat, 29 Feb 2020 11:25:24 -0800 |
| |
> On Feb 29, 2020, at 6:44 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > Thomas Gleixner <tglx@linutronix.de> writes: >> Andy Lutomirski <luto@amacapital.net> writes: >>>>> On Feb 26, 2020, at 12:17 AM, Peter Zijlstra <peterz@infradead.org> wrote: >>>>> On Tue, Feb 25, 2020 at 09:43:46PM -0800, Andy Lutomirski wrote: >>>>>> Your earlier patches suggest quite strongly that tracing isn't safe >>>>>> until enter_from_user_mode(). But trace_hardirqs_off() calls >>>>>> trace_irq_disable_rcuidle(), which looks [0] like a tracepoint. >>>>>> >>>>>> Did you perhaps mean to do this *after* enter_from_user_mode()? >>>>> >>>>> aside from the fact that enter_from_user_mode() itself also has a >>>>> tracepoint, the crucial detail is that we must not trace/kprobe the >>>>> function calling this. >>>>> >>>>> Specifically for #PF, because we need read_cr2() before this. See later >>>>> patches. >>> >>> Indeed. I’m fine with this patch, but I still don’t understand what >>> the changelog is about. >> >> Yeah, the changelog is not really helpful. Let me fix that. >> >>> And I’m still rather baffled by most of the notrace annotations in the >>> series. >> >> As discussed on IRC, this might be too broad, but then I rather have the >> actual C-entry points neither traceable nor probable in general and >> relax this by calling functions which can be traced and probed. >> >> My rationale for this decision was that enter_from_user_mode() is marked >> notrace/noprobe as well, so I kept the protection scope the same as we >> had in the ASM maze which is marked noprobe already. > > I have second thoughts vs. tracing in this context. > > While the tracer itself seems to handle this correctly, what about > things like BPF programs which can be attached to tracepoints and > function trace entries?
I think that everything using the tracing code, including BPF, should either do its own rcuidle stuff or explicitly not execute if we’re not in CONTEXT_KERNEL. That is, we probably need to patch BPF.
> > Is that really safe _before_ context tracking has updated RCU state? > > Thanks, > > tglx > >
| |