Skip to content

Commit 1a149d7

Browse files
committed
ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
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) <[email protected]>
1 parent 12ecef0 commit 1a149d7

File tree

1 file changed

+17
-47
lines changed

1 file changed

+17
-47
lines changed

kernel/trace/ring_buffer.c

Lines changed: 17 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2538,69 +2538,39 @@ rb_wakeups(struct ring_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
25382538
* The lock and unlock are done within a preempt disable section.
25392539
* The current_context per_cpu variable can only be modified
25402540
* by the current task between lock and unlock. But it can
2541-
* be modified more than once via an interrupt. To pass this
2542-
* information from the lock to the unlock without having to
2543-
* access the 'in_interrupt()' functions again (which do show
2544-
* a bit of overhead in something as critical as function tracing,
2545-
* we use a bitmask trick.
2541+
* be modified more than once via an interrupt. There are four
2542+
* different contexts that we need to consider.
25462543
*
2547-
* bit 0 = NMI context
2548-
* bit 1 = IRQ context
2549-
* bit 2 = SoftIRQ context
2550-
* bit 3 = normal context.
2544+
* Normal context.
2545+
* SoftIRQ context
2546+
* IRQ context
2547+
* NMI context
25512548
*
2552-
* This works because this is the order of contexts that can
2553-
* preempt other contexts. A SoftIRQ never preempts an IRQ
2554-
* context.
2555-
*
2556-
* When the context is determined, the corresponding bit is
2557-
* checked and set (if it was set, then a recursion of that context
2558-
* happened).
2559-
*
2560-
* On unlock, we need to clear this bit. To do so, just subtract
2561-
* 1 from the current_context and AND it to itself.
2562-
*
2563-
* (binary)
2564-
* 101 - 1 = 100
2565-
* 101 & 100 = 100 (clearing bit zero)
2566-
*
2567-
* 1010 - 1 = 1001
2568-
* 1010 & 1001 = 1000 (clearing bit 1)
2569-
*
2570-
* The least significant bit can be cleared this way, and it
2571-
* just so happens that it is the same bit corresponding to
2572-
* the current context.
2549+
* If for some reason the ring buffer starts to recurse, we
2550+
* only allow that to happen at most 4 times (one for each
2551+
* context). If it happens 5 times, then we consider this a
2552+
* recusive loop and do not let it go further.
25732553
*/
25742554

25752555
static __always_inline int
25762556
trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer)
25772557
{
2578-
unsigned int val = cpu_buffer->current_context;
2579-
int bit;
2580-
2581-
if (in_interrupt()) {
2582-
if (in_nmi())
2583-
bit = RB_CTX_NMI;
2584-
else if (in_irq())
2585-
bit = RB_CTX_IRQ;
2586-
else
2587-
bit = RB_CTX_SOFTIRQ;
2588-
} else
2589-
bit = RB_CTX_NORMAL;
2590-
2591-
if (unlikely(val & (1 << bit)))
2558+
if (cpu_buffer->current_context >= 4)
25922559
return 1;
25932560

2594-
val |= (1 << bit);
2595-
cpu_buffer->current_context = val;
2561+
cpu_buffer->current_context++;
2562+
/* Interrupts must see this update */
2563+
barrier();
25962564

25972565
return 0;
25982566
}
25992567

26002568
static __always_inline void
26012569
trace_recursive_unlock(struct ring_buffer_per_cpu *cpu_buffer)
26022570
{
2603-
cpu_buffer->current_context &= cpu_buffer->current_context - 1;
2571+
/* Don't let the dec leak out */
2572+
barrier();
2573+
cpu_buffer->current_context--;
26042574
}
26052575

26062576
/**

0 commit comments

Comments
 (0)