From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: [for-next][PATCH 04/16] ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
Date: Fri, 06 Oct 2017 14:06:59 -0400 [thread overview]
Message-ID: <20171006180728.854555994@goodmis.org> (raw)
In-Reply-To: 20171006180655.935479946@goodmis.org
[-- Attachment #1: 0004-ring-buffer-Rewrite-trace_recursive_-un-lock-to-be-s.patch --]
[-- Type: text/plain, Size: 4091 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The current method to prevent the ring buffer from entering into a recursize
loop is to use a bitmask and set the bit that maps to the current context
(normal, softirq, irq or NMI), and if that bit was already set, it is
considered a recursive loop.
New code is being added that may require the ring buffer to be entered a
second time in the current context. The recursive locking prevents that from
happening. Instead of mapping a bitmask to the current context, just allow 4
levels of nesting in the ring buffer. This matches the 4 context levels that
it can already nest. It is highly unlikely to have more than two levels,
thus it should be fine when we add the second entry into the ring buffer. If
that proves to be a problem, we can always up the number to 8.
An added benefit is that reading preempt_count() to get the current level
adds a very slight but noticeable overhead. This removes that need.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 64 ++++++++++++----------------------------------
1 file changed, 17 insertions(+), 47 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 81279c6602ff..f6ee9b1ef62a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2538,61 +2538,29 @@ rb_wakeups(struct ring_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
* The lock and unlock are done within a preempt disable section.
* The current_context per_cpu variable can only be modified
* by the current task between lock and unlock. But it can
- * be modified more than once via an interrupt. To pass this
- * information from the lock to the unlock without having to
- * access the 'in_interrupt()' functions again (which do show
- * a bit of overhead in something as critical as function tracing,
- * we use a bitmask trick.
+ * be modified more than once via an interrupt. There are four
+ * different contexts that we need to consider.
*
- * bit 0 = NMI context
- * bit 1 = IRQ context
- * bit 2 = SoftIRQ context
- * bit 3 = normal context.
+ * Normal context.
+ * SoftIRQ context
+ * IRQ context
+ * NMI context
*
- * This works because this is the order of contexts that can
- * preempt other contexts. A SoftIRQ never preempts an IRQ
- * context.
- *
- * When the context is determined, the corresponding bit is
- * checked and set (if it was set, then a recursion of that context
- * happened).
- *
- * On unlock, we need to clear this bit. To do so, just subtract
- * 1 from the current_context and AND it to itself.
- *
- * (binary)
- * 101 - 1 = 100
- * 101 & 100 = 100 (clearing bit zero)
- *
- * 1010 - 1 = 1001
- * 1010 & 1001 = 1000 (clearing bit 1)
- *
- * The least significant bit can be cleared this way, and it
- * just so happens that it is the same bit corresponding to
- * the current context.
+ * If for some reason the ring buffer starts to recurse, we
+ * only allow that to happen at most 4 times (one for each
+ * context). If it happens 5 times, then we consider this a
+ * recusive loop and do not let it go further.
*/
static __always_inline int
trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer)
{
- unsigned int val = cpu_buffer->current_context;
- int bit;
-
- if (in_interrupt()) {
- if (in_nmi())
- bit = RB_CTX_NMI;
- else if (in_irq())
- bit = RB_CTX_IRQ;
- else
- bit = RB_CTX_SOFTIRQ;
- } else
- bit = RB_CTX_NORMAL;
-
- if (unlikely(val & (1 << bit)))
+ if (cpu_buffer->current_context >= 4)
return 1;
- val |= (1 << bit);
- cpu_buffer->current_context = val;
+ cpu_buffer->current_context++;
+ /* Interrupts must see this update */
+ barrier();
return 0;
}
@@ -2600,7 +2568,9 @@ trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer)
static __always_inline void
trace_recursive_unlock(struct ring_buffer_per_cpu *cpu_buffer)
{
- cpu_buffer->current_context &= cpu_buffer->current_context - 1;
+ /* Don't let the dec leak out */
+ barrier();
+ cpu_buffer->current_context--;
}
/**
--
2.13.2
next prev parent reply other threads:[~2017-10-06 18:11 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 02/16] tracing: Remove redundant unread variable ret Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 03/16] tracing: Reverse the order of trace_types_lock and event_mutex Steven Rostedt
2017-10-06 18:06 ` Steven Rostedt [this message]
2017-10-06 18:07 ` [for-next][PATCH 05/16] tracing: Exclude generic fields from histograms Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 06/16] tracing: Remove lookups from tracing_map hitcount Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 07/16] tracing: Increase tracing map KEYS_MAX size Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 08/16] tracing: Make traceprobe parsing code reusable Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 09/16] tracing: Clean up hist_field_flags enum Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 10/16] tracing: Add hist_field_name() accessor Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 11/16] tracing: Reimplement log2 Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 12/16] ftrace: Add a ftrace_free_mem() function for modules to use Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 13/16] ftrace: Allow module init functions to be traced Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 14/16] ftrace: Save module init functions kallsyms symbols for tracing Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps Steven Rostedt
2017-10-07 6:41 ` Joel Fernandes (Google)
2017-10-07 13:32 ` Steven Rostedt
2017-10-08 4:52 ` Joel Fernandes (Google)
2017-10-08 8:42 ` Joel Fernandes
2017-10-08 18:42 ` Steven Rostedt
2017-10-08 18:56 ` Joel Fernandes
2017-10-09 6:07 ` Joel Fernandes
2017-10-09 6:02 ` Joel Fernandes
2017-10-06 18:07 ` [for-next][PATCH 16/16] ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171006180728.854555994@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.