lkml.org 
[lkml]   [2019]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2019-07-29 06:27    [W:0.135 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site