Messages in this thread | | | From | Andy Lutomirski <> | Date | Sun, 28 Jul 2019 21:25:58 -0700 | Subject | Re: [PATCH] tracing: Prevent RCU EQS breakage in preemptirq events |
| |
On Sun, Jul 28, 2019 at 6:08 PM Eiichi Tsukata <devel@etsukata.com> wrote: > > If context tracking is enabled, causing page fault in preemptirq > irq_enable or irq_disable events triggers the following RCU EQS warning. >
Yuck.
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c > index be01a4d627c9..860eaf9780e5 100644 > --- a/kernel/context_tracking.c > +++ b/kernel/context_tracking.c > @@ -148,6 +148,11 @@ void __context_tracking_exit(enum ctx_state state) > return; > > if (__this_cpu_read(context_tracking.state) == state) { > + /* > + * Change state before executing codes which can trigger > + * page fault leading unnecessary re-entrance. > + */ > + __this_cpu_write(context_tracking.state, CONTEXT_KERNEL);
Seems reasonable.
> if (__this_cpu_read(context_tracking.active)) { > /* > * We are going to run code that may use RCU. Inform > @@ -159,7 +164,6 @@ void __context_tracking_exit(enum ctx_state state) > trace_user_exit(0); > } > } > - __this_cpu_write(context_tracking.state, CONTEXT_KERNEL); > } > context_tracking_recursion_exit(); > } > diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c > index 4d8e99fdbbbe..031b51cb94d0 100644 > --- a/kernel/trace/trace_preemptirq.c > +++ b/kernel/trace/trace_preemptirq.c > @@ -10,6 +10,7 @@ > #include <linux/module.h> > #include <linux/ftrace.h> > #include <linux/kprobes.h> > +#include <linux/context_tracking.h> > #include "trace.h" > > #define CREATE_TRACE_POINTS > @@ -49,9 +50,14 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); > > __visible void trace_hardirqs_on_caller(unsigned long caller_addr) > { > + enum ctx_state prev_state; > + > if (this_cpu_read(tracing_irq_cpu)) { > - if (!in_nmi()) > + if (!in_nmi()) { > + prev_state = exception_enter(); > trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr); > + exception_exit(prev_state); > + } > tracer_hardirqs_on(CALLER_ADDR0, caller_addr); > this_cpu_write(tracing_irq_cpu, 0); > }
This seems a bit distressing. Now we're going to do a whole bunch of context tracking transitions for each kernel entry. Would a better fix me to change trace_hardirqs_on_caller to skip the trace event if the previous state was already IRQs on and, more importantly, to skip tracing IRQs off if IRQs were already off? The x86 code is very careful to avoid ever having IRQs on and CONTEXT_USER at the same time.
--Andy
| |