ftrace: Handle tracing when switching between context
authorSteven Rostedt (VMware) <rostedt@goodmis.org>
Thu, 29 Oct 2020 23:35:08 +0000 (19:35 -0400)
committerSteven Rostedt (VMware) <rostedt@goodmis.org>
Mon, 2 Nov 2020 13:52:18 +0000 (08:52 -0500)
When an interrupt or NMI comes in and switches the context, there's a delay
from when the preempt_count() shows the update. As the preempt_count() is
used to detect recursion having each context have its own bit get set when
tracing starts, and if that bit is already set, it is considered a recursion
and the function exits. But if this happens in that section where context
has changed but preempt_count() has not been updated, this will be
incorrectly flagged as a recursion.

To handle this case, create another bit call TRANSITION and test it if the
current context bit is already set. Flag the call as a recursion if the
TRANSITION bit is already set, and if not, set it and continue. The
TRANSITION bit will be cleared normally on the return of the function that
set it, or if the current context bit is clear, set it and clear the
TRANSITION bit to allow for another transition between the current context
and an even higher one.

Cc: stable@vger.kernel.org
Fixes: edc15cafcbfa3 ("tracing: Avoid unnecessary multiple recursion checks")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
kernel/trace/trace.h
kernel/trace/trace_selftest.c

index fee535a8956059b3a4559494398e9b510cea3161..1dadef445cd1e860cc68142fe8a6e657eae81fff 100644 (file)
@@ -637,6 +637,12 @@ enum {
         * function is called to clear it.
         */
        TRACE_GRAPH_NOTRACE_BIT,
+
+       /*
+        * When transitioning between context, the preempt_count() may
+        * not be correct. Allow for a single recursion to cover this case.
+        */
+       TRACE_TRANSITION_BIT,
 };
 
 #define trace_recursion_set(bit)       do { (current)->trace_recursion |= (1<<(bit)); } while (0)
@@ -691,8 +697,21 @@ static __always_inline int trace_test_and_set_recursion(int start, int max)
                return 0;
 
        bit = trace_get_context_bit() + start;
-       if (unlikely(val & (1 << bit)))
-               return -1;
+       if (unlikely(val & (1 << bit))) {
+               /*
+                * It could be that preempt_count has not been updated during
+                * a switch between contexts. Allow for a single recursion.
+                */
+               bit = TRACE_TRANSITION_BIT;
+               if (trace_recursion_test(bit))
+                       return -1;
+               trace_recursion_set(bit);
+               barrier();
+               return bit + 1;
+       }
+
+       /* Normal check passed, clear the transition to allow it again */
+       trace_recursion_clear(TRACE_TRANSITION_BIT);
 
        val |= 1 << bit;
        current->trace_recursion = val;
index b5e3496cf8033df69a33307db09a8bd0f34bca2e..4738ad48a66740fb72d77872a34b58f0a41f98f3 100644 (file)
@@ -492,8 +492,13 @@ trace_selftest_function_recursion(void)
        unregister_ftrace_function(&test_rec_probe);
 
        ret = -1;
-       if (trace_selftest_recursion_cnt != 1) {
-               pr_cont("*callback not called once (%d)* ",
+       /*
+        * Recursion allows for transitions between context,
+        * and may call the callback twice.
+        */
+       if (trace_selftest_recursion_cnt != 1 &&
+           trace_selftest_recursion_cnt != 2) {
+               pr_cont("*callback not called once (or twice) (%d)* ",
                        trace_selftest_recursion_cnt);
                goto out;
        }