tracing: Fixes for v6.19

tracing fixes for v6.19:

- Remove useless assignment of soft_mode variable

The function __ftrace_event_enable_disable() sets "soft_mode" in one of the branch paths but doesn't use it after that. Remove the setting of that variable.

- Add a cond_resched() in ring_buffer_resize()

The resize function that allocates all the pages for the ring buffer was causing a soft lockup on PREEMPT_NONE configs when allocating large buffers on machines with many CPUs. Hopefully this is the last cond_resched() needed to be added as PREEMPT_LAZY becomes the norm in the future.

- Make ftrace_graph_ent depth field signed

The "depth" field of struct ftrace_graph_ent was converted from "int" to "unsigned long" for alignment reasons to work with being embedded in other structures. The conversion from a signed to unsigned caused integrity checks to always pass as they were comparing "depth" to less than zero. Make the field signed long.

- Add recursion protection to stack trace events

A infinite recursion was triggered by a stack trace event calling RCU which internally called rcu_read_unlock_special(), which triggered an event that was also doing stacktraces which cause it to trigger the same RCU lock that called rcu_read_unlock_special() again.

Update the trace_test_and_set_recursion() to add a set of context checks for events to use, and have the stack trace event use that for recursion protection.

- Make the variable ftrace_dump_on_oops static

The cleanup of sysctl that moved all the updates to the files that use them moved the reference of ftrace_dump_on_oops to where it is used. It is no longer used outside of the trace.c file. Make it static.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: 1e2ed4bfd50ace3c4272cfab7e9aa90956fb7ae0

Ben Dooks (1):

      trace: ftrace_dump_on_oops[] is not exported, make it static

Julia Lawall (1):
tracing: Drop unneeded assignment to soft_mode

Steven Rostedt (2):
ftrace: Make ftrace_graph_ent depth field signed
tracing: Add recursion protection in kernel stack trace recording

Wupeng Ma (1):
ring-buffer: Avoid softlockup in ring_buffer_resize() during memory free

----

 include/linux/ftrace.h          | 2 +-
 include/linux/trace_recursion.h | 9 +++++++++
 kernel/trace/ring_buffer.c      | 2 ++
 kernel/trace/trace.c            | 8 +++++++-
 kernel/trace/trace_events.c     | 7 +++----
 5 files changed, 22 insertions(+), 6 deletions(-)

From: Julia Lawall <[email protected]>

soft_mode is not read in the enable case, so drop the assignment. Drop also the comment text that refers to the assignment and realign the comment.

---
 kernel/trace/trace_events.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

index 76067529db61..137b4d9bb116 100644
@@ -826,16 +826,15 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
 		 * When soft_disable is set and enable is set, we want to
 		 * register the tracepoint for the event, but leave the event
 		 * as is. That means, if the event was already enabled, we do
-		 * nothing (but set soft_mode). If the event is disabled, we
-		 * set SOFT_DISABLED before enabling the event tracepoint, so
-		 * it still seems to be disabled.
+		 * nothing. If the event is disabled, we set SOFT_DISABLED
+		 * before enabling the event tracepoint, so it still seems
+		 * to be disabled.
 		 */
 		if (!soft_disable)
 			clear_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
 		else {
 			if (atomic_inc_return(&file->sm_ref) > 1)
 				break;
-			soft_mode = true;
 			/* Enable use of trace_buffered_event */
 			trace_buffered_event_enable();
 		}

2.51.0

From: Wupeng Ma <[email protected]>

When user resize all trace ring buffer through file 'buffer_size_kb', then in ring_buffer_resize(), kernel allocates buffer pages for each cpu in a loop.

If the kernel preemption model is PREEMPT_NONE and there are many cpus and there are many buffer pages to be freed, it may not give up cpu for a long time and finally cause a softlockup.

To avoid it, call cond_resched() after each cpu buffer free as Commit f6bd2c92488c ("ring-buffer: Avoid softlockup in ring_buffer_resize()") does.

Detailed call trace as follow:

  rcu: INFO: rcu_sched self-detected stall on CPU
  rcu: 	24-....: (14837 ticks this GP) idle=521c/1/0x4000000000000000 softirq=230597/230597 fqs=5329
  rcu: 	(t=15004 jiffies g=26003221 q=211022 ncpus=96)
  CPU: 24 UID: 0 PID: 11253 Comm: bash Kdump: loaded Tainted: G            EL      6.18.2+ #278 NONE
  pc : arch_local_irq_restore+0x8/0x20
   arch_local_irq_restore+0x8/0x20 (P)
   free_frozen_page_commit+0x28c/0x3b0
   __free_frozen_pages+0x1c0/0x678
   ___free_pages+0xc0/0xe0
   free_pages+0x3c/0x50
   ring_buffer_resize.part.0+0x6a8/0x880
   ring_buffer_resize+0x3c/0x58
   __tracing_resize_ring_buffer.part.0+0x34/0xd8
   tracing_resize_ring_buffer+0x8c/0xd0
   tracing_entries_write+0x74/0xd8
   vfs_write+0xcc/0x288
   ksys_write+0x74/0x118
   __arm64_sys_write+0x24/0x38

---
 kernel/trace/ring_buffer.c | 2 ++
 1 file changed, 2 insertions(+)

index 41c9f5d079be..630221b00838 100644
@@ -3137,6 +3137,8 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 					list) {
 			list_del_init(&bpage->list);
 			free_buffer_page(bpage);
+
+			cond_resched();
 		}
 	}
  out_err_unlock:

2.51.0

From: Steven Rostedt <[email protected]>

The code has integrity checks to make sure that depth never goes below zero. But the depth field has recently been converted to unsigned long from "int" (for alignment reasons). As unsigned long can never be less than zero, the integrity checks no longer work.

Convert depth to long from unsigned long to allow the integrity checks to work again.

---
 include/linux/ftrace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 770f0dc993cc..a3a8989e3268 100644
@@ -1167,7 +1167,7 @@ static inline void ftrace_init(void) { }
  */
 struct ftrace_graph_ent {
 	unsigned long func; /* Current function */
-	unsigned long depth;
+	long depth; /* signed to check for less than zero */
 } __packed;
 
 /*

2.51.0

From: Steven Rostedt <[email protected]>

A bug was reported about an infinite recursion caused by tracing the rcu events with the kernel stack trace trigger enabled. The stack trace code called back into RCU which then called the stack trace again.

Expand the ftrace recursion protection to add a set of bits to protect events from recursion. Each bit represents the context that the event is in (normal, softirq, interrupt and NMI).

Have the stack trace code use the interrupt context to protect against recursion.

Note, the bug showed an issue in both the RCU code as well as the tracing stacktrace code. This only handles the tracing stack trace side of the bug. The RCU fix will be handled separately.

---
 include/linux/trace_recursion.h | 9 +++++++++
 kernel/trace/trace.c            | 6 ++++++
 2 files changed, 15 insertions(+)

index ae04054a1be3..e6ca052b2a85 100644
@@ -34,6 +34,13 @@ enum {
 	TRACE_INTERNAL_SIRQ_BIT,
 	TRACE_INTERNAL_TRANSITION_BIT,
 
+	/* Internal event use recursion bits */
+	TRACE_INTERNAL_EVENT_BIT,
+	TRACE_INTERNAL_EVENT_NMI_BIT,
+	TRACE_INTERNAL_EVENT_IRQ_BIT,
+	TRACE_INTERNAL_EVENT_SIRQ_BIT,
+	TRACE_INTERNAL_EVENT_TRANSITION_BIT,
+
 	TRACE_BRANCH_BIT,
 /*
  * Abuse of the trace_recursion.

@@ -58,6 +65,8 @@ enum {
 
 #define TRACE_LIST_START	TRACE_INTERNAL_BIT
 
+#define TRACE_EVENT_START	TRACE_INTERNAL_EVENT_BIT
+
 #define TRACE_CONTEXT_MASK	((1 << (TRACE_LIST_START + TRACE_CONTEXT_BITS)) - 1)
 
 /*

index 6f2148df14d9..aef9058537d5 100644
@@ -3012,6 +3012,11 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 	struct ftrace_stack *fstack;
 	struct stack_entry *entry;
 	int stackidx;
+	int bit;
+
+	bit = trace_test_and_set_recursion(_THIS_IP_, _RET_IP_, TRACE_EVENT_START);
+	if (bit < 0)
+		return;
 
 	/*
 	 * Add one, for this function and the call to save_stack_trace()

@@ -3080,6 +3085,7 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 	/* Again, don't let gcc optimize things here */
 	barrier();
 	__this_cpu_dec(ftrace_stack_reserve);
+	trace_clear_recursion(bit);
 }
 
 static inline void ftrace_trace_stack(struct trace_array *tr,

2.51.0

From: Ben Dooks <[email protected]>

The ftrace_dump_on_oops string is not used outside of trace.c so make it static to avoid the export warning from sparse:

kernel/trace/trace.c:141:6: warning: symbol 'ftrace_dump_on_oops' was not declared. Should it be static?

---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index aef9058537d5..baec63134ab6 100644
@@ -138,7 +138,7 @@ cpumask_var_t __read_mostly	tracing_buffer_mask;
  * by commas.
  */
 /* Set to string format zero to disable by default */
-char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
+static char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
 
 /* When set, tracing will stop when a WARN*() is hit */
 static int __disable_trace_on_warning;

2.51.0