[PATCH v3 0/2] tracing: Override trace_instance parameter

[PATCH v3 0/2] tracing: Override trace_instance parameter

[Masami Hiramatsu (Google)]

Hi,

Here is the 3rd version of the patches to change the "trace_instance="
kernel parameter to be overridden by later parameters.
The previous version is here:

 https://lore.kernel.org/all/179091984881.537422.15308457677039592540.stgit@devnote2/

This version fixes some issues pointed by Sashiko and add more selftests.
(the selftest is optional.)

Currently if the user specifies the same name trace_instance at
boot time, the later one is skipped.
However, this does not allow user to override the trace_instance
setting via bootloader if user specifies the default trace_instance
in the bootconfig, because the bootconfig parameter is prepended to
the kernel cmdline. (IOW, the bootconfig expects kernel parameters
follow a LAST_WIN policy.)

To fix this issue, [01/02] splits the command line parsing loop
and trace instance initialization loop. The parser checks the
overlap of the trace instance name and overrides with the later one
if those have the same name.

This series also includes a test set as [02/02], which is based on
the ftrace boottime test patchset [1], currently on probes/core.

Detailed changes are here:

Changes in v3:
 [01/02]
  - Count the number of instances in enable_instances().
  - Check whether source instance exists before the backup instance.
  - Fix memmap_area leaking across loop iterations.
  - Add array index range check for boot_instances array.
 [02/02]
  - Add test cases for overriding memory-mapped instances (@trace).
  - Add test cases for overriding backup instances (=source).
  - Add test cases for multiple tabs and empty strings.

[1] https://lore.kernel.org/all/178649540853.438282.4271870161169100680.stgit@devnote2/

Thank you,

---
base-commit: b15df7805dc52555e7c867d33a496edf5be11289

Masami Hiramatsu (Google) (2):
      tracing: Override the same name trace_instance= by latter one
      selftests/ftrace: Add test case for overriding trace_instance parameter


 Documentation/admin-guide/kernel-parameters.txt    |    7 +
 Documentation/trace/debugging.rst                  |   15 ++
 kernel/trace/trace.c                               |  150 +++++++++++++++-----
 .../cmdline-07-trace-instance-override.bconf       |    4 +
 .../cmdline-07-trace-instance-override.cmdline     |    1 
 .../selftests/ftrace/boottime/run_boottime_test.sh |    2 
 .../tests/cmdline-07-trace-instance-override.sh    |  148 ++++++++++++++++++++
 7 files changed, 290 insertions(+), 37 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/boottime/bootconfigs/cmdline-07-trace-instance-override.bconf
 create mode 100644 tools/testing/selftests/ftrace/boottime/cmdlines/cmdline-07-trace-instance-override.cmdline
 create mode 100755 tools/testing/selftests/ftrace/boottime/tests/cmdline-07-trace-instance-override.sh

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH v3 1/2] tracing: Override the same name trace_instance= by latter one

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Override the same name boot instance by the latter parameter.
This allows user to override the trace instance in bootconfig by the
cmdline from bootloader.

For example, set default trace_instance parameter in bootconfig
e.g.:

  kernel {
     reserve_mem=12M:32M:trace
     trace_instance=boot_map@trace
  }

This just adds a persistent trace instance.

And when you need to record events, you can override the boot_map
trace instance from bootloader:

  trace_instance=boot_map@trace,sched:*

Then the boot_map instance starts tracing sched:* events.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v3:
  - Count the number of tabs in trace_instance= option as the number
    of trace instance.
  - Check whether source instance exists before the backup instance.
  - Fix memmap_area leaking in the next loop.
  - Count nr_instances in enable_instances().
  - Add index range check for boot_instances array.
---
 Documentation/admin-guide/kernel-parameters.txt |    7 +
 Documentation/trace/debugging.rst               |   15 ++
 kernel/trace/trace.c                            |  150 +++++++++++++++++------
 3 files changed, 135 insertions(+), 37 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e75344f4e0cd..0695ba0234c9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7806,6 +7806,13 @@ Kernel parameters
 
 			NB: Both the mapped address and size must be page aligned for the architecture.
 
+			Multiple trace_instance= options can be specified to
+			create multiple instances. If the same instance name is
+			specified more than once, the latter one will override
+			any earlier definitions for that instance. This allows
+			overriding an instance configuration that was defined
+			earlier on the command line or via bootconfig.
+
 			See also Documentation/trace/debugging.rst
 
 
diff --git a/Documentation/trace/debugging.rst b/Documentation/trace/debugging.rst
index bca1710d92bf..b66cc070ae79 100644
--- a/Documentation/trace/debugging.rst
+++ b/Documentation/trace/debugging.rst
@@ -178,3 +178,18 @@ instance without stopping the trace.
 Note that this "backup" instance is readonly, and will be removed automatically
 if you clear the trace data or read out all trace data from the "trace_pipe"
 or the "trace_pipe_raw" files.
+
+Overriding trace instances
+--------------------------
+
+Multiple ``trace_instance=`` options can be specified on the kernel command
+line to create multiple instances. If the same instance name is specified
+more than once, the latter definition will override any previous definitions
+for that instance.
+
+This is useful when an instance configuration is defined in bootconfig, but
+needs to be overridden or modified from the bootloader command line (for
+example, to change the enabled events or add flags like ``traceoff``)::
+
+  trace_instance=boot_map^traceoff@trace
+
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e4a490d3d08c..8cd8b8b045fe 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9705,33 +9705,44 @@ __init static int backup_instance_area(const char *backup,
 	return 0;
 }
 
-__init static void enable_instances(void)
+struct boot_instance {
+	char *name;
+	char *flag_delim;
+	char *addr_delim;
+	char *backup;
+	char *events;
+};
+
+__init static int parse_boot_instance_info(struct boot_instance *boot_instances,
+					   int nr_instances)
 {
-	struct trace_array *tr;
-	bool memmap_area = false;
-	char *curr_str;
+	char *flag_delim;
+	char *addr_delim;
+	char *backup;
+	char *events;
 	char *name;
-	char *str;
 	char *tok;
+	char *str;
+	int idx;
+	int nr = 0;
+	int i;
 
 	/* A tab is always appended */
 	boot_instance_info[boot_instance_index - 1] = '\0';
 	str = boot_instance_info;
 
-	while ((curr_str = strsep(&str, "\t"))) {
-		phys_addr_t start = 0;
-		phys_addr_t size = 0;
-		unsigned long addr = 0;
-		bool traceprintk = false;
-		bool traceoff = false;
-		char *flag_delim;
-		char *addr_delim;
-		char *rname __free(kfree) = NULL;
-		char *backup;
+	while ((events = strsep(&str, "\t"))) {
+		if (!*events)
+			continue;
 
-		tok = strsep(&curr_str, ",");
+		tok = strsep(&events, ",");
 
 		name = strsep(&tok, "=");
+		if (!*name) {
+			pr_warn("Tracing: Empty boot instance name\n");
+			continue;
+		}
+
 		backup = tok;
 
 		flag_delim = strchr(name, '^');
@@ -9743,20 +9754,84 @@ __init static void enable_instances(void)
 		if (flag_delim)
 			*flag_delim++ = '\0';
 
-		if (trace_array_find(name)) {
-			pr_warn("Tracing: Instance %s already exists\n", name);
-			continue;
+		for (idx = 0; idx < nr; idx++) {
+			/* Override by the last defined instance */
+			if (!strcmp(name, boot_instances[idx].name))
+				break;
 		}
 
 		if (backup) {
-			if (backup_instance_area(backup, &addr, &size) < 0)
+			/* Check if the backup source instance exists */
+			for (i = 0; i < idx; i++) {
+				if (!strcmp(backup, boot_instances[i].name))
+					break;
+			}
+			if (i == idx) {
+				pr_warn("Tracing: Backup source instance %s is not found for %s\n",
+					backup, name);
 				continue;
+			}
 		}
 
-		if (flag_delim) {
+		if (WARN_ON(idx >= nr_instances))
+			break;
+
+		boot_instances[idx].name = name;
+		boot_instances[idx].flag_delim = flag_delim;
+		boot_instances[idx].addr_delim = addr_delim;
+		boot_instances[idx].backup = backup;
+		boot_instances[idx].events = events;
+		if (idx == nr)
+			nr++;
+	}
+
+	return nr;
+}
+
+__init static void enable_instances(void)
+{
+	struct boot_instance *boot_instances __free(kfree) = NULL;
+	struct boot_instance *bi;
+	struct trace_array *tr;
+	int nr_instances = 1;
+	char *tok;
+
+	if (!boot_instance_index)
+		return;
+
+	for (char *p = boot_instance_info; *p; p++) {
+		if (*p == '\t')
+			nr_instances++;
+	}
+
+	/* Parse the instance name to check the overlap */
+	boot_instances = kmalloc_array(nr_instances,
+				       sizeof(*boot_instances), GFP_KERNEL);
+	if (!boot_instances)
+		return;
+
+	nr_instances = parse_boot_instance_info(boot_instances, nr_instances);
+
+	for (int i = 0; i < nr_instances; i++) {
+		char *rname __free(kfree) = NULL;
+		phys_addr_t start = 0;
+		phys_addr_t size = 0;
+		unsigned long addr = 0;
+		bool traceprintk = false;
+		bool traceoff = false;
+		bool memmap_area = false;
+
+		bi = &boot_instances[i];
+
+		if (bi->backup) {
+			if (backup_instance_area(bi->backup, &addr, &size) < 0)
+				continue;
+		}
+
+		if (bi->flag_delim) {
 			char *flag;
 
-			while ((flag = strsep(&flag_delim, "^"))) {
+			while ((flag = strsep(&bi->flag_delim, "^"))) {
 				if (strcmp(flag, "traceoff") == 0) {
 					traceoff = true;
 				} else if ((strcmp(flag, "printk") == 0) ||
@@ -9765,35 +9840,35 @@ __init static void enable_instances(void)
 					traceprintk = true;
 				} else {
 					pr_info("Tracing: Invalid instance flag '%s' for %s\n",
-						flag, name);
+						flag, bi->name);
 				}
 			}
 		}
 
-		tok = addr_delim;
+		tok = bi->addr_delim;
 		if (tok && isdigit(*tok)) {
 			start = memparse(tok, &tok);
 			if (!start) {
 				pr_warn("Tracing: Invalid boot instance address for %s\n",
-					name);
+					bi->name);
 				continue;
 			}
 			if (*tok != ':') {
-				pr_warn("Tracing: No size specified for instance %s\n", name);
+				pr_warn("Tracing: No size specified for instance %s\n", bi->name);
 				continue;
 			}
 			tok++;
 			size = memparse(tok, &tok);
 			if (!size) {
 				pr_warn("Tracing: Invalid boot instance size for %s\n",
-					name);
+					bi->name);
 				continue;
 			}
 			memmap_area = true;
 		} else if (tok) {
 			if (!reserve_mem_find_by_name(tok, &start, &size)) {
 				start = 0;
-				pr_warn("Failed to map boot instance %s to %s\n", name, tok);
+				pr_warn("Failed to map boot instance %s to %s\n", bi->name, tok);
 				continue;
 			}
 			rname = kstrdup(tok, GFP_KERNEL);
@@ -9816,20 +9891,20 @@ __init static void enable_instances(void)
 				addr = (unsigned long)phys_to_virt(start);
 			if (addr) {
 				pr_info("Tracing: mapped boot instance %s at physical memory %pa of size 0x%lx\n",
-					name, &start, (unsigned long)size);
+					bi->name, &start, (unsigned long)size);
 			} else {
-				pr_warn("Tracing: Failed to map boot instance %s\n", name);
+				pr_warn("Tracing: Failed to map boot instance %s\n", bi->name);
 				continue;
 			}
 		} else {
 			/* Only non mapped buffers have snapshot buffers */
-			do_allocate_snapshot(name);
+			do_allocate_snapshot(bi->name);
 		}
 
-		tr = trace_array_create_systems(name, NULL, addr, size);
+		tr = trace_array_create_systems(bi->name, NULL, addr, size);
 		if (IS_ERR(tr)) {
-			pr_warn("Tracing: Failed to create instance buffer '%s' (%ld)\n", name,
-				PTR_ERR(tr));
+			pr_warn("Tracing: Failed to create instance buffer '%s' (%ld)\n",
+				bi->name, PTR_ERR(tr));
 			continue;
 		}
 
@@ -9850,12 +9925,12 @@ __init static void enable_instances(void)
 		/*
 		 * Backup buffers can be freed but need vfree().
 		 */
-		if (backup) {
+		if (bi->backup) {
 			tr->flags |= TRACE_ARRAY_FL_VMALLOC | TRACE_ARRAY_FL_RDONLY;
 			trace_array_start_autoremove();
 		}
 
-		if (start || backup) {
+		if (start || bi->backup) {
 			tr->flags |= TRACE_ARRAY_FL_BOOT | TRACE_ARRAY_FL_LAST_BOOT;
 			tr->range_name = no_free_ptr(rname);
 		}
@@ -9864,11 +9939,12 @@ __init static void enable_instances(void)
 		 * Save the events to start and enabled them after all boot instances
 		 * have been created.
 		 */
-		tr->boot_events = curr_str;
+		tr->boot_events = bi->events;
 	}
 
 	/* Enable the events after all boot instances have been created */
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
+		char *curr_str;
 
 		if (!tr->boot_events || !(*tr->boot_events)) {
 			tr->boot_events = NULL;

[PATCH v3 2/2] selftests/ftrace: Add test case for overriding trace_instance parameter

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Add a boot-time tracing test case to verify that specifying multiple
trace_instance= parameters with the same instance name overrides earlier
definitions:

   - Verifies that a kernel command-line trace_instance= parameter
     overrides an earlier trace instance defined via bootconfig
     (kernel.trace_instance).
   - Verifies that a later kernel command-line trace_instance=
     parameter overrides an earlier trace_instance= on the same
     command line.
   - Verifies overriding a memory-mapped trace instance (@trace).
   - Verifies overriding a backup trace instance (=source).
   - Verifies parsing parameters containing multiple tabs and
     gracefully handling empty strings.
   - Confirms that the overridden instance has its previous events
     disabled, new events enabled, and flags (such as traceoff)
     properly applied.

Assisted-by: LLM
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
 - Add test cases for overriding memory-mapped instances (@trace).
 - Add test cases for overriding backup instances.
 - Add test cases for multiple tabs and empty strings.
 - Add dmesg and basename to required busybox applets.
---
 .../cmdline-07-trace-instance-override.bconf       |    4 +
 .../cmdline-07-trace-instance-override.cmdline     |    1 
 .../selftests/ftrace/boottime/run_boottime_test.sh |    2 
 .../tests/cmdline-07-trace-instance-override.sh    |  148 ++++++++++++++++++++
 4 files changed, 155 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/boottime/bootconfigs/cmdline-07-trace-instance-override.bconf
 create mode 100644 tools/testing/selftests/ftrace/boottime/cmdlines/cmdline-07-trace-instance-override.cmdline
 create mode 100755 tools/testing/selftests/ftrace/boottime/tests/cmdline-07-trace-instance-override.sh

diff --git a/tools/testing/selftests/ftrace/boottime/bootconfigs/cmdline-07-trace-instance-override.bconf b/tools/testing/selftests/ftrace/boottime/bootconfigs/cmdline-07-trace-instance-override.bconf
new file mode 100644
index 000000000000..74d8be54a798
--- /dev/null
+++ b/tools/testing/selftests/ftrace/boottime/bootconfigs/cmdline-07-trace-instance-override.bconf
@@ -0,0 +1,4 @@
+kernel.trace_instance = "boot_map@trace,sched:sched_switch",
+			"bak_to_norm=boot_map",
+			"bak_from_norm,sched:sched_switch",
+			"foo,sched:sched_switch"
diff --git a/tools/testing/selftests/ftrace/boottime/cmdlines/cmdline-07-trace-instance-override.cmdline b/tools/testing/selftests/ftrace/boottime/cmdlines/cmdline-07-trace-instance-override.cmdline
new file mode 100644
index 000000000000..3c16bb0f3545
--- /dev/null
+++ b/tools/testing/selftests/ftrace/boottime/cmdlines/cmdline-07-trace-instance-override.cmdline
@@ -0,0 +1 @@
+reserve_mem=12M:32M:trace trace_instance=foo^traceoff,sched:sched_waking trace_instance=bar,sched:sched_switch trace_instance=bar^traceoff,irq:irq_handler_entry trace_instance=boot_map^traceoff@trace,sched:sched_waking trace_instance=bak_to_norm^traceoff,irq:irq_handler_entry trace_instance=bak_from_norm=boot_map trace_instance="tab1,sched:sched_switch		tab2,sched:sched_switch" trace_instance=tab1^traceoff,sched:sched_waking trace_instance= trace_instance="" trace_instance=,
diff --git a/tools/testing/selftests/ftrace/boottime/run_boottime_test.sh b/tools/testing/selftests/ftrace/boottime/run_boottime_test.sh
index 6e8593d5a2c5..1d30adc388a6 100755
--- a/tools/testing/selftests/ftrace/boottime/run_boottime_test.sh
+++ b/tools/testing/selftests/ftrace/boottime/run_boottime_test.sh
@@ -46,6 +46,8 @@ BUSYBOX_APPLETS=(
 	grep
 	sleep
 	sed
+	dmesg
+	basename
 )
 
 usage() {
diff --git a/tools/testing/selftests/ftrace/boottime/tests/cmdline-07-trace-instance-override.sh b/tools/testing/selftests/ftrace/boottime/tests/cmdline-07-trace-instance-override.sh
new file mode 100755
index 000000000000..960a3cf54d97
--- /dev/null
+++ b/tools/testing/selftests/ftrace/boottime/tests/cmdline-07-trace-instance-override.sh
@@ -0,0 +1,148 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2026, Google LLC.
+# Check trace_instance= override with bootconfig and kernel command-line
+TRACEDIR="/sys/kernel/tracing"
+
+if [ -f /proc/bootconfig ] && \
+   grep -q "dump_bconf" /proc/cmdline 2>/dev/null; then
+	echo "=== /proc/bootconfig ==="
+	cat /proc/bootconfig
+	echo "========================"
+fi
+
+check_instance() {
+	instance="$1"
+	old_event="$2"
+	new_event="$3"
+
+	if [ ! -d "$TRACEDIR/instances/$instance" ]; then
+		echo "FAIL: trace instance $instance does not exist"
+		exit 1
+	fi
+
+	if [ -n "$old_event" ] && \
+	   [ -f "$TRACEDIR/instances/$instance/events/$old_event/enable" ]; then
+		ENABLE=$(cat "$TRACEDIR/instances/$instance/events/$old_event/enable")
+		if [ "$ENABLE" = "1" ]; then
+			echo "FAIL: overridden event $old_event" \
+				"is still enabled in instance $instance"
+			exit 1
+		fi
+	fi
+
+	if [ -n "$new_event" ]; then
+		if [ ! -d "$TRACEDIR/instances/$instance/events/$new_event" ]; then
+			echo "FAIL: event $new_event does not exist in instance $instance"
+			exit 1
+		fi
+
+		ENABLE=$(cat "$TRACEDIR/instances/$instance/events/$new_event/enable")
+		if [ "$ENABLE" != "1" ]; then
+			echo "FAIL: event $new_event is not enabled in instance $instance ($ENABLE)"
+			exit 1
+		fi
+	fi
+
+	TRACING_ON=$(cat "$TRACEDIR/instances/$instance/tracing_on")
+	if [ "$TRACING_ON" != "0" ]; then
+		echo "FAIL: tracing_on is not 0 in instance $instance ($TRACING_ON)"
+		exit 1
+	fi
+}
+
+check_backup_instance() {
+	instance="$1"
+
+	if [ ! -d "$TRACEDIR/instances/$instance" ]; then
+		echo "FAIL: trace instance $instance does not exist"
+		exit 1
+	fi
+
+	# Backup instances have a trace buffer file
+	if [ ! -f "$TRACEDIR/instances/$instance/trace" ]; then
+		echo "FAIL: backup instance $instance has no trace file"
+		exit 1
+	fi
+
+	# Backup instances are read-only and do not have tracing_on
+	if [ -f "$TRACEDIR/instances/$instance/tracing_on" ]; then
+		echo "FAIL: backup instance $instance unexpectedly has tracing_on"
+		exit 1
+	fi
+
+	# Backup instances do not support enabling events
+	if [ -f \
+	   "$TRACEDIR/instances/$instance/events/sched/sched_switch/enable" ]; then
+		echo "FAIL: backup instance $instance unexpectedly has enabled events"
+		exit 1
+	fi
+}
+
+check_mapped_instance() {
+	instance="$1"
+	old_event="$2"
+	new_event="$3"
+
+	check_instance "$instance" "$old_event" "$new_event"
+
+	# Check dmesg to confirm physical mapping occurred
+	if ! dmesg | grep -q "mapped boot instance $instance"; then
+		echo "FAIL: memory-mapped instance $instance was not mapped at boot"
+		exit 1
+	fi
+}
+
+# Test 1: bootconfig trace_instance overridden by cmdline parameter
+check_instance "foo" "sched/sched_switch" "sched/sched_waking"
+
+# Test 2: cmdline trace_instance overridden by subsequent cmdline parameter
+check_instance "bar" "sched/sched_switch" "irq/irq_handler_entry"
+
+# Test 3: memory-mapped (@trace) instance in bootconfig overridden by cmdline
+check_mapped_instance "boot_map" "sched/sched_switch" "sched/sched_waking"
+
+# Test 4a: backup instance in bootconfig overridden by normal
+# instance in cmdline
+check_instance "bak_to_norm" "" "irq/irq_handler_entry"
+
+# Test 4b: normal instance in bootconfig overridden by backup
+# instance in cmdline
+check_backup_instance "bak_from_norm"
+
+# Test 5: instances separated by multiple tabs, where first is overridden
+check_instance "tab1" "sched/sched_switch" "sched/sched_waking"
+# tab2 was not overridden, so its original event should remain enabled
+if [ ! -d "$TRACEDIR/instances/tab2/events/sched/sched_switch" ]; then
+	echo "FAIL: instance tab2 event sched/sched_switch does not exist"
+	exit 1
+fi
+ENABLE=$(cat "$TRACEDIR/instances/tab2/events/sched/sched_switch/enable")
+if [ "$ENABLE" != "1" ]; then
+	echo "FAIL: instance tab2 event sched/sched_switch is not enabled ($ENABLE)"
+	exit 1
+fi
+
+# Test 6: Verify empty strings and malformed entries did not create
+# bogus directories
+for dir in "$TRACEDIR/instances"/*; do
+	[ -d "$dir" ] || continue
+	bname=$(basename "$dir")
+	case "$bname" in
+		foo|bar|boot_map|bak_to_norm|bak_from_norm|tab1|tab2)
+			;;
+		*)
+			echo "FAIL: unexpected trace instance directory found: $bname"
+			exit 1
+			;;
+	esac
+done
+
+# Test 7: Verify no kernel warnings or assertion failures were triggered
+if dmesg | grep -q "WARNING: CPU:"; then
+	echo "FAIL: kernel warning triggered during boot"
+	exit 1
+fi
+
+echo "PASS: cmdline-07-trace-instance-override"
+exit 0