public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] tracing: preserve repeated boot-time parameters and drain deferred trigger frees
@ 2026-03-10  6:47 Wesley Atwell
  2026-03-10  6:47 ` [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters Wesley Atwell
  2026-03-10  6:47 ` [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails Wesley Atwell
  0 siblings, 2 replies; 5+ messages in thread
From: Wesley Atwell @ 2026-03-10  6:47 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, corbet, skhan, linux-doc,
	linux-kernel, linux-trace-kernel, Wesley Atwell

Patch 1 updates the affected early tracing boot-parameter parsers to
preserve repeated instances in the format their existing parsers already
consume, and documents that repeated-parameter behavior.

Patch 2 fixes deferred trigger-data cleanup so boot-deferred frees are
drained even when the cleanup kthread never starts.

v3:
- Patch 1: use a shared trace_append_boot_param() helper
- Patch 1: document repeated-parameter behavior in kernel-parameters.txt
- Patch 1: reframe as an improvement and drop the Fixes tags
- Patch 2: no changes

v2:
- Patch 1: no changes
- Patch 2: restore the dropped mutex recheck comment
- Patch 2: clarify the synchronous fallback drain path

Wesley Atwell (2):
  tracing: preserve repeated boot-time tracing parameters
  tracing: drain deferred trigger frees if kthread startup fails

 .../admin-guide/kernel-parameters.txt         | 18 ++++-
 kernel/trace/ftrace.c                         | 12 ++-
 kernel/trace/trace.c                          |  3 +-
 kernel/trace/trace.h                          | 29 +++++++
 kernel/trace/trace_events.c                   | 26 +++++-
 kernel/trace/trace_events_trigger.c           | 79 ++++++++++++++++---
 kernel/trace/trace_kprobe.c                   |  3 +-
 7 files changed, 145 insertions(+), 25 deletions(-)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters
  2026-03-10  6:47 [PATCH v3 0/2] tracing: preserve repeated boot-time parameters and drain deferred trigger frees Wesley Atwell
@ 2026-03-10  6:47 ` Wesley Atwell
  2026-03-24 18:43   ` Steven Rostedt
  2026-03-10  6:47 ` [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails Wesley Atwell
  1 sibling, 1 reply; 5+ messages in thread
From: Wesley Atwell @ 2026-03-10  6:47 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, corbet, skhan, linux-doc,
	linux-kernel, linux-trace-kernel, Wesley Atwell

Some tracing boot parameters already accept delimited value lists, but
their __setup() handlers keep only the last instance seen at boot.
Make repeated instances append to the same boot-time buffer in the
format each parser already consumes, and document that behavior in
admin-guide/kernel-parameters.txt.

Use a shared trace_append_boot_param() helper for the ftrace filters,
trace_options, and kprobe_event boot parameters. trace_trigger=
tokenizes its backing storage in place, so keep a running offset and
only parse the newly appended chunk into bootup_triggers[].

This also lets Bootconfig array values work naturally when they expand
to repeated param=value entries.

Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
---
 .../admin-guide/kernel-parameters.txt         | 18 ++++++++++--
 kernel/trace/ftrace.c                         | 12 +++++---
 kernel/trace/trace.c                          |  3 +-
 kernel/trace/trace.h                          | 29 +++++++++++++++++++
 kernel/trace/trace_events.c                   | 26 +++++++++++++++--
 kernel/trace/trace_kprobe.c                   |  3 +-
 6 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 55ffc0f8858a..203863c1839b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1803,13 +1803,15 @@ Kernel parameters
 			tracer at boot up. function-list is a comma-separated
 			list of functions. This list can be changed at run
 			time by the set_ftrace_filter file in the debugfs
-			tracing directory.
+			tracing directory. Repeated instances append more
+			functions to the same list.
 
 	ftrace_notrace=[function-list]
 			[FTRACE] Do not trace the functions specified in
 			function-list. This list can be changed at run time
 			by the set_ftrace_notrace file in the debugfs
-			tracing directory.
+			tracing directory. Repeated instances append more
+			functions to the same list.
 
 	ftrace_graph_filter=[function-list]
 			[FTRACE] Limit the top level callers functions traced
@@ -1817,12 +1819,16 @@ Kernel parameters
 			function-list is a comma-separated list of functions
 			that can be changed at run time by the
 			set_graph_function file in the debugfs tracing directory.
+			Repeated instances append more functions to the same
+			list.
 
 	ftrace_graph_notrace=[function-list]
 			[FTRACE] Do not trace from the functions specified in
 			function-list.  This list is a comma-separated list of
 			functions that can be changed at run time by the
 			set_graph_notrace file in the debugfs tracing directory.
+			Repeated instances append more functions to the same
+			list.
 
 	ftrace_graph_max_depth=<uint>
 			[FTRACE] Used with the function graph tracer. This is
@@ -3053,6 +3059,8 @@ Kernel parameters
 			The probe-list is a semicolon delimited list of probe
 			definitions. Each definition is same as kprobe_events
 			interface, but the parameters are comma delimited.
+			Repeated instances append more probe definitions to
+			the same boot-time list.
 			For example, to add a kprobe event on vfs_read with
 			arg1 and arg2, add to the command line;
 
@@ -7820,6 +7828,9 @@ Kernel parameters
 
 			    /sys/kernel/tracing/trace_options
 
+			Repeated instances append more options to the same
+			boot-time list.
+
 			For example, to enable stacktrace option (to dump the
 			stack trace of each event), add to the command line:
 
@@ -7831,7 +7842,8 @@ Kernel parameters
 	trace_trigger=[trigger-list]
 			[FTRACE] Add an event trigger on specific events.
 			Set a trigger on top of a specific event, with an optional
-			filter.
+			filter. Repeated instances append more triggers to
+			the same boot-time list.
 
 			The format is "trace_trigger=<event>.<trigger>[ if <filter>],..."
 			Where more than one trigger may be specified that are comma delimited.
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 8df69e702706..d0a486b63ed6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6841,7 +6841,8 @@ bool ftrace_filter_param __initdata;
 static int __init set_ftrace_notrace(char *str)
 {
 	ftrace_filter_param = true;
-	strscpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
+	trace_append_boot_param(ftrace_notrace_buf, str, ',',
+				FTRACE_FILTER_SIZE);
 	return 1;
 }
 __setup("ftrace_notrace=", set_ftrace_notrace);
@@ -6849,7 +6850,8 @@ __setup("ftrace_notrace=", set_ftrace_notrace);
 static int __init set_ftrace_filter(char *str)
 {
 	ftrace_filter_param = true;
-	strscpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
+	trace_append_boot_param(ftrace_filter_buf, str, ',',
+				FTRACE_FILTER_SIZE);
 	return 1;
 }
 __setup("ftrace_filter=", set_ftrace_filter);
@@ -6861,14 +6863,16 @@ static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
 
 static int __init set_graph_function(char *str)
 {
-	strscpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
+	trace_append_boot_param(ftrace_graph_buf, str, ',',
+				FTRACE_FILTER_SIZE);
 	return 1;
 }
 __setup("ftrace_graph_filter=", set_graph_function);
 
 static int __init set_graph_notrace_function(char *str)
 {
-	strscpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
+	trace_append_boot_param(ftrace_graph_notrace_buf, str, ',',
+				FTRACE_FILTER_SIZE);
 	return 1;
 }
 __setup("ftrace_graph_notrace=", set_graph_notrace_function);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ebd996f8710e..5086239a75dc 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -329,7 +329,8 @@ static char trace_boot_options_buf[MAX_TRACER_SIZE] __initdata;
 
 static int __init set_trace_boot_options(char *str)
 {
-	strscpy(trace_boot_options_buf, str, MAX_TRACER_SIZE);
+	trace_append_boot_param(trace_boot_options_buf, str, ',',
+				MAX_TRACER_SIZE);
 	return 1;
 }
 __setup("trace_options=", set_trace_boot_options);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b8f3804586a0..4f5abac4bd19 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -23,6 +23,7 @@
 #include <linux/once_lite.h>
 #include <linux/ftrace_regs.h>
 #include <linux/llist.h>
+#include <linux/string.h>
 
 #include "pid_list.h"
 
@@ -262,6 +263,34 @@ static inline bool still_need_pid_events(int type, struct trace_pid_list *pid_li
 		(!(type & TRACE_NO_PIDS) && no_pid_list);
 }
 
+/*
+ * Repeated boot parameters, including Bootconfig array expansions, need
+ * to stay in the delimiter form that the existing parser consumes.
+ */
+static inline void __init trace_append_boot_param(char *buf, const char *str,
+						  char sep, size_t size)
+{
+	size_t len, str_len;
+
+	if (buf[0] == '\0') {
+		strscpy(buf, str, size);
+		return;
+	}
+
+	str_len = strlen(str);
+	if (!str_len)
+		return;
+
+	len = strlen(buf);
+	if (len >= size - 1)
+		return;
+	if (str_len >= size - len - 1)
+		return;
+
+	buf[len] = sep;
+	strscpy(buf + len + 1, str, size - len - 1);
+}
+
 typedef bool (*cond_update_fn_t)(struct trace_array *tr, void *cond_data);
 
 /**
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 249d1cba72c0..5f72be33f2d1 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3679,20 +3679,40 @@ static struct boot_triggers {
 } bootup_triggers[MAX_BOOT_TRIGGERS];
 
 static char bootup_trigger_buf[COMMAND_LINE_SIZE];
+static size_t bootup_trigger_buf_len;
 static int nr_boot_triggers;
 
 static __init int setup_trace_triggers(char *str)
 {
 	char *trigger;
 	char *buf;
+	size_t start, str_len;
 	int i;
 
-	strscpy(bootup_trigger_buf, str, COMMAND_LINE_SIZE);
+	if (bootup_trigger_buf_len >= COMMAND_LINE_SIZE)
+		return 1;
+
+	start = bootup_trigger_buf_len;
+	if (start && !*str)
+		return 1;
+
+	str_len = strlen(str);
+	if (start && str_len >= COMMAND_LINE_SIZE - start)
+		return 1;
+
+	/*
+	 * trace_trigger= parsing tokenizes the backing storage in place.
+	 * Copy each repeated parameter into fresh space and only parse that
+	 * newly copied chunk here.
+	 */
+	trace_append_boot_param(bootup_trigger_buf + start, str, '\0',
+				COMMAND_LINE_SIZE - start);
+	bootup_trigger_buf_len += strlen(bootup_trigger_buf + start) + 1;
 	trace_set_ring_buffer_expanded(NULL);
 	disable_tracing_selftest("running event triggers");
 
-	buf = bootup_trigger_buf;
-	for (i = 0; i < MAX_BOOT_TRIGGERS; i++) {
+	buf = bootup_trigger_buf + start;
+	for (i = nr_boot_triggers; i < MAX_BOOT_TRIGGERS; i++) {
 		trigger = strsep(&buf, ",");
 		if (!trigger)
 			break;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a5dbb72528e0..e9f1c55aea64 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -31,7 +31,8 @@ static char kprobe_boot_events_buf[COMMAND_LINE_SIZE] __initdata;
 
 static int __init set_kprobe_boot_events(char *str)
 {
-	strscpy(kprobe_boot_events_buf, str, COMMAND_LINE_SIZE);
+	trace_append_boot_param(kprobe_boot_events_buf, str, ';',
+				COMMAND_LINE_SIZE);
 	disable_tracing_selftest("running kprobe events");
 
 	return 1;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails
  2026-03-10  6:47 [PATCH v3 0/2] tracing: preserve repeated boot-time parameters and drain deferred trigger frees Wesley Atwell
  2026-03-10  6:47 ` [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters Wesley Atwell
@ 2026-03-10  6:47 ` Wesley Atwell
  2026-03-24 18:53   ` Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Wesley Atwell @ 2026-03-10  6:47 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: mark.rutland, mathieu.desnoyers, corbet, skhan, linux-doc,
	linux-kernel, linux-trace-kernel, Wesley Atwell

Boot-time trigger registration can fail before the trigger-data cleanup
kthread exists. Deferring those frees until late init is fine, but the
post-boot fallback must still drain the deferred list if kthread
creation never succeeds.

Otherwise, boot-deferred nodes can accumulate on
trigger_data_free_list, later frees fall back to synchronously freeing
only the current object, and the older queued entries are leaked
forever.

Keep the deferred boot-time behavior, but when kthread creation fails,
drain the whole queued list synchronously. Do the same in the late-init
drain path so queued entries are not stranded there either.

Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
---
 kernel/trace/trace_events_trigger.c | 79 ++++++++++++++++++++++++-----
 1 file changed, 66 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d5230b759a2d..428b46272ac8 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -22,6 +22,39 @@ static struct task_struct *trigger_kthread;
 static struct llist_head trigger_data_free_list;
 static DEFINE_MUTEX(trigger_data_kthread_mutex);
 
+static int trigger_kthread_fn(void *ignore);
+
+static void trigger_start_kthread_locked(void)
+{
+	lockdep_assert_held(&trigger_data_kthread_mutex);
+
+	if (!trigger_kthread) {
+		struct task_struct *kthread;
+
+		kthread = kthread_create(trigger_kthread_fn, NULL,
+					 "trigger_data_free");
+		if (!IS_ERR(kthread))
+			WRITE_ONCE(trigger_kthread, kthread);
+	}
+}
+
+static void trigger_data_free_queued_locked(void)
+{
+	struct event_trigger_data *data, *tmp;
+	struct llist_node *llnodes;
+
+	lockdep_assert_held(&trigger_data_kthread_mutex);
+
+	llnodes = llist_del_all(&trigger_data_free_list);
+	if (!llnodes)
+		return;
+
+	tracepoint_synchronize_unregister();
+
+	llist_for_each_entry_safe(data, tmp, llnodes, llist)
+		kfree(data);
+}
+
 /* Bulk garbage collection of event_trigger_data elements */
 static int trigger_kthread_fn(void *ignore)
 {
@@ -56,30 +89,50 @@ void trigger_data_free(struct event_trigger_data *data)
 	if (data->cmd_ops->set_filter)
 		data->cmd_ops->set_filter(NULL, data, NULL);
 
+	/*
+	 * Boot-time trigger registration can fail before kthread creation
+	 * works. Keep the deferred-free semantics during boot and let late
+	 * init start the kthread to drain the list.
+	 */
+	if (system_state == SYSTEM_BOOTING && !trigger_kthread) {
+		llist_add(&data->llist, &trigger_data_free_list);
+		return;
+	}
+
 	if (unlikely(!trigger_kthread)) {
 		guard(mutex)(&trigger_data_kthread_mutex);
+
+		trigger_start_kthread_locked();
 		/* Check again after taking mutex */
 		if (!trigger_kthread) {
-			struct task_struct *kthread;
-
-			kthread = kthread_create(trigger_kthread_fn, NULL,
-						 "trigger_data_free");
-			if (!IS_ERR(kthread))
-				WRITE_ONCE(trigger_kthread, kthread);
+			llist_add(&data->llist, &trigger_data_free_list);
+			/* Drain the queued frees synchronously if startup failed. */
+			trigger_data_free_queued_locked();
+			return;
 		}
 	}
 
-	if (!trigger_kthread) {
-		/* Do it the slow way */
-		tracepoint_synchronize_unregister();
-		kfree(data);
-		return;
-	}
-
 	llist_add(&data->llist, &trigger_data_free_list);
 	wake_up_process(trigger_kthread);
 }
 
+static int __init trigger_data_free_init(void)
+{
+	guard(mutex)(&trigger_data_kthread_mutex);
+
+	if (llist_empty(&trigger_data_free_list))
+		return 0;
+
+	trigger_start_kthread_locked();
+	if (trigger_kthread)
+		wake_up_process(trigger_kthread);
+	else
+		trigger_data_free_queued_locked();
+
+	return 0;
+}
+late_initcall(trigger_data_free_init);
+
 static inline void data_ops_trigger(struct event_trigger_data *data,
 				    struct trace_buffer *buffer,  void *rec,
 				    struct ring_buffer_event *event)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters
  2026-03-10  6:47 ` [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters Wesley Atwell
@ 2026-03-24 18:43   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-03-24 18:43 UTC (permalink / raw)
  To: Wesley Atwell
  Cc: mhiramat, mark.rutland, mathieu.desnoyers, corbet, skhan,
	linux-doc, linux-kernel, linux-trace-kernel

On Tue, 10 Mar 2026 00:47:14 -0600
Wesley Atwell <atwellwea@gmail.com> wrote:

Hi,

FYI, the tracing subsystem uses capital letters in subjects:

   tracing: Preserve repeated boot-time tracing parameters

> Some tracing boot parameters already accept delimited value lists, but
> their __setup() handlers keep only the last instance seen at boot.
> Make repeated instances append to the same boot-time buffer in the
> format each parser already consumes, and document that behavior in
> admin-guide/kernel-parameters.txt.
> 
> Use a shared trace_append_boot_param() helper for the ftrace filters,
> trace_options, and kprobe_event boot parameters. trace_trigger=
> tokenizes its backing storage in place, so keep a running offset and
> only parse the newly appended chunk into bootup_triggers[].
> 
> This also lets Bootconfig array values work naturally when they expand
> to repeated param=value entries.
> 
> Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 18 ++++++++++--
>  kernel/trace/ftrace.c                         | 12 +++++---
>  kernel/trace/trace.c                          |  3 +-
>  kernel/trace/trace.h                          | 29 +++++++++++++++++++
>  kernel/trace/trace_events.c                   | 26 +++++++++++++++--
>  kernel/trace/trace_kprobe.c                   |  3 +-
>  6 files changed, 79 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 55ffc0f8858a..203863c1839b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1803,13 +1803,15 @@ Kernel parameters
>  			tracer at boot up. function-list is a comma-separated
>  			list of functions. This list can be changed at run
>  			time by the set_ftrace_filter file in the debugfs
> -			tracing directory.
> +			tracing directory. Repeated instances append more
> +			functions to the same list.
>  
>  	ftrace_notrace=[function-list]
>  			[FTRACE] Do not trace the functions specified in
>  			function-list. This list can be changed at run time
>  			by the set_ftrace_notrace file in the debugfs
> -			tracing directory.
> +			tracing directory. Repeated instances append more
> +			functions to the same list.
>  
>  	ftrace_graph_filter=[function-list]
>  			[FTRACE] Limit the top level callers functions traced
> @@ -1817,12 +1819,16 @@ Kernel parameters
>  			function-list is a comma-separated list of functions
>  			that can be changed at run time by the
>  			set_graph_function file in the debugfs tracing directory.
> +			Repeated instances append more functions to the same
> +			list.
>  
>  	ftrace_graph_notrace=[function-list]
>  			[FTRACE] Do not trace from the functions specified in
>  			function-list.  This list is a comma-separated list of
>  			functions that can be changed at run time by the
>  			set_graph_notrace file in the debugfs tracing directory.
> +			Repeated instances append more functions to the same
> +			list.
>  
>  	ftrace_graph_max_depth=<uint>
>  			[FTRACE] Used with the function graph tracer. This is
> @@ -3053,6 +3059,8 @@ Kernel parameters
>  			The probe-list is a semicolon delimited list of probe
>  			definitions. Each definition is same as kprobe_events
>  			interface, but the parameters are comma delimited.
> +			Repeated instances append more probe definitions to
> +			the same boot-time list.
>  			For example, to add a kprobe event on vfs_read with
>  			arg1 and arg2, add to the command line;
>  
> @@ -7820,6 +7828,9 @@ Kernel parameters
>  
>  			    /sys/kernel/tracing/trace_options
>  
> +			Repeated instances append more options to the same
> +			boot-time list.
> +
>  			For example, to enable stacktrace option (to dump the
>  			stack trace of each event), add to the command line:
>  
> @@ -7831,7 +7842,8 @@ Kernel parameters
>  	trace_trigger=[trigger-list]
>  			[FTRACE] Add an event trigger on specific events.
>  			Set a trigger on top of a specific event, with an optional
> -			filter.
> +			filter. Repeated instances append more triggers to
> +			the same boot-time list.

I know Masami mentioned to document this, but honestly, I don't think this
update is needed. Please remove it.

>  
>  			The format is "trace_trigger=<event>.<trigger>[ if <filter>],..."
>  			Where more than one trigger may be specified that are comma delimited.
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 8df69e702706..d0a486b63ed6 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6841,7 +6841,8 @@ bool ftrace_filter_param __initdata;
>  static int __init set_ftrace_notrace(char *str)
>  {
>  	ftrace_filter_param = true;
> -	strscpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
> +	trace_append_boot_param(ftrace_notrace_buf, str, ',',
> +				FTRACE_FILTER_SIZE);
>  	return 1;
>  }
>  __setup("ftrace_notrace=", set_ftrace_notrace);
> @@ -6849,7 +6850,8 @@ __setup("ftrace_notrace=", set_ftrace_notrace);
>  static int __init set_ftrace_filter(char *str)
>  {
>  	ftrace_filter_param = true;
> -	strscpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
> +	trace_append_boot_param(ftrace_filter_buf, str, ',',
> +				FTRACE_FILTER_SIZE);
>  	return 1;
>  }
>  __setup("ftrace_filter=", set_ftrace_filter);
> @@ -6861,14 +6863,16 @@ static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
>  
>  static int __init set_graph_function(char *str)
>  {
> -	strscpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
> +	trace_append_boot_param(ftrace_graph_buf, str, ',',
> +				FTRACE_FILTER_SIZE);
>  	return 1;
>  }
>  __setup("ftrace_graph_filter=", set_graph_function);
>  
>  static int __init set_graph_notrace_function(char *str)
>  {
> -	strscpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
> +	trace_append_boot_param(ftrace_graph_notrace_buf, str, ',',
> +				FTRACE_FILTER_SIZE);
>  	return 1;
>  }
>  __setup("ftrace_graph_notrace=", set_graph_notrace_function);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ebd996f8710e..5086239a75dc 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -329,7 +329,8 @@ static char trace_boot_options_buf[MAX_TRACER_SIZE] __initdata;
>  
>  static int __init set_trace_boot_options(char *str)
>  {
> -	strscpy(trace_boot_options_buf, str, MAX_TRACER_SIZE);
> +	trace_append_boot_param(trace_boot_options_buf, str, ',',
> +				MAX_TRACER_SIZE);
>  	return 1;
>  }
>  __setup("trace_options=", set_trace_boot_options);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b8f3804586a0..4f5abac4bd19 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -23,6 +23,7 @@
>  #include <linux/once_lite.h>
>  #include <linux/ftrace_regs.h>
>  #include <linux/llist.h>
> +#include <linux/string.h>
>  
>  #include "pid_list.h"
>  
> @@ -262,6 +263,34 @@ static inline bool still_need_pid_events(int type, struct trace_pid_list *pid_li
>  		(!(type & TRACE_NO_PIDS) && no_pid_list);
>  }
>  
> +/*
> + * Repeated boot parameters, including Bootconfig array expansions, need
> + * to stay in the delimiter form that the existing parser consumes.
> + */
> +static inline void __init trace_append_boot_param(char *buf, const char *str,
> +						  char sep, size_t size)
> +{

Masami said:

  Please make a generic append function in kernel/trace/trace.h, e.g.

  void trace_append_boot_param(char *buf, const char *str, char sep, size_t ssize);

  and use it instead of strscpy.

He did not say to make a static inline in the header. Please make this a
normal function in trace.c and just add the prototype in the header.

-- Steve


> +	size_t len, str_len;
> +
> +	if (buf[0] == '\0') {
> +		strscpy(buf, str, size);
> +		return;
> +	}
> +
> +	str_len = strlen(str);
> +	if (!str_len)
> +		return;
> +
> +	len = strlen(buf);
> +	if (len >= size - 1)
> +		return;
> +	if (str_len >= size - len - 1)
> +		return;
> +
> +	buf[len] = sep;
> +	strscpy(buf + len + 1, str, size - len - 1);
> +}
> +
>  typedef bool (*cond_update_fn_t)(struct trace_array *tr, void *cond_data);
>  
>  /**
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 249d1cba72c0..5f72be33f2d1 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3679,20 +3679,40 @@ static struct boot_triggers {
>  } bootup_triggers[MAX_BOOT_TRIGGERS];
>  
>  static char bootup_trigger_buf[COMMAND_LINE_SIZE];
> +static size_t bootup_trigger_buf_len;
>  static int nr_boot_triggers;
>  
>  static __init int setup_trace_triggers(char *str)
>  {
>  	char *trigger;
>  	char *buf;
> +	size_t start, str_len;
>  	int i;
>  
> -	strscpy(bootup_trigger_buf, str, COMMAND_LINE_SIZE);
> +	if (bootup_trigger_buf_len >= COMMAND_LINE_SIZE)
> +		return 1;
> +
> +	start = bootup_trigger_buf_len;
> +	if (start && !*str)
> +		return 1;
> +
> +	str_len = strlen(str);
> +	if (start && str_len >= COMMAND_LINE_SIZE - start)
> +		return 1;
> +
> +	/*
> +	 * trace_trigger= parsing tokenizes the backing storage in place.
> +	 * Copy each repeated parameter into fresh space and only parse that
> +	 * newly copied chunk here.
> +	 */
> +	trace_append_boot_param(bootup_trigger_buf + start, str, '\0',
> +				COMMAND_LINE_SIZE - start);
> +	bootup_trigger_buf_len += strlen(bootup_trigger_buf + start) + 1;
>  	trace_set_ring_buffer_expanded(NULL);
>  	disable_tracing_selftest("running event triggers");
>  
> -	buf = bootup_trigger_buf;
> -	for (i = 0; i < MAX_BOOT_TRIGGERS; i++) {
> +	buf = bootup_trigger_buf + start;
> +	for (i = nr_boot_triggers; i < MAX_BOOT_TRIGGERS; i++) {
>  		trigger = strsep(&buf, ",");
>  		if (!trigger)
>  			break;
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index a5dbb72528e0..e9f1c55aea64 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -31,7 +31,8 @@ static char kprobe_boot_events_buf[COMMAND_LINE_SIZE] __initdata;
>  
>  static int __init set_kprobe_boot_events(char *str)
>  {
> -	strscpy(kprobe_boot_events_buf, str, COMMAND_LINE_SIZE);
> +	trace_append_boot_param(kprobe_boot_events_buf, str, ';',
> +				COMMAND_LINE_SIZE);
>  	disable_tracing_selftest("running kprobe events");
>  
>  	return 1;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails
  2026-03-10  6:47 ` [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails Wesley Atwell
@ 2026-03-24 18:53   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-03-24 18:53 UTC (permalink / raw)
  To: Wesley Atwell
  Cc: mhiramat, mark.rutland, mathieu.desnoyers, corbet, skhan,
	linux-doc, linux-kernel, linux-trace-kernel

On Tue, 10 Mar 2026 00:47:15 -0600
Wesley Atwell <atwellwea@gmail.com> wrote:

> Boot-time trigger registration can fail before the trigger-data cleanup
> kthread exists. Deferring those frees until late init is fine, but the
> post-boot fallback must still drain the deferred list if kthread
> creation never succeeds.
> 
> Otherwise, boot-deferred nodes can accumulate on
> trigger_data_free_list, later frees fall back to synchronously freeing
> only the current object, and the older queued entries are leaked
> forever.
> 
> Keep the deferred boot-time behavior, but when kthread creation fails,
> drain the whole queued list synchronously. Do the same in the late-init
> drain path so queued entries are not stranded there either.
> 
> Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
> Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
> ---
>  kernel/trace/trace_events_trigger.c | 79 ++++++++++++++++++++++++-----
>  1 file changed, 66 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index d5230b759a2d..428b46272ac8 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -22,6 +22,39 @@ static struct task_struct *trigger_kthread;
>  static struct llist_head trigger_data_free_list;
>  static DEFINE_MUTEX(trigger_data_kthread_mutex);
>  
> +static int trigger_kthread_fn(void *ignore);
> +
> +static void trigger_start_kthread_locked(void)
> +{
> +	lockdep_assert_held(&trigger_data_kthread_mutex);
> +
> +	if (!trigger_kthread) {
> +		struct task_struct *kthread;
> +
> +		kthread = kthread_create(trigger_kthread_fn, NULL,
> +					 "trigger_data_free");

This only creates the thread and doesn't start it. The function name is
confusing. Please change it to:

	trigger_create_kthread_locked()


> +		if (!IS_ERR(kthread))
> +			WRITE_ONCE(trigger_kthread, kthread);
> +	}
> +}
> +
> +static void trigger_data_free_queued_locked(void)
> +{
> +	struct event_trigger_data *data, *tmp;
> +	struct llist_node *llnodes;
> +
> +	lockdep_assert_held(&trigger_data_kthread_mutex);
> +
> +	llnodes = llist_del_all(&trigger_data_free_list);
> +	if (!llnodes)
> +		return;
> +
> +	tracepoint_synchronize_unregister();
> +
> +	llist_for_each_entry_safe(data, tmp, llnodes, llist)
> +		kfree(data);
> +}
> +
>  /* Bulk garbage collection of event_trigger_data elements */
>  static int trigger_kthread_fn(void *ignore)
>  {
> @@ -56,30 +89,50 @@ void trigger_data_free(struct event_trigger_data *data)
>  	if (data->cmd_ops->set_filter)
>  		data->cmd_ops->set_filter(NULL, data, NULL);
>  
> +	/*
> +	 * Boot-time trigger registration can fail before kthread creation
> +	 * works. Keep the deferred-free semantics during boot and let late
> +	 * init start the kthread to drain the list.
> +	 */
> +	if (system_state == SYSTEM_BOOTING && !trigger_kthread) {
> +		llist_add(&data->llist, &trigger_data_free_list);
> +		return;
> +	}
> +
>  	if (unlikely(!trigger_kthread)) {
>  		guard(mutex)(&trigger_data_kthread_mutex);
> +
> +		trigger_start_kthread_locked();
>  		/* Check again after taking mutex */
>  		if (!trigger_kthread) {
> -			struct task_struct *kthread;
> -
> -			kthread = kthread_create(trigger_kthread_fn, NULL,
> -						 "trigger_data_free");
> -			if (!IS_ERR(kthread))
> -				WRITE_ONCE(trigger_kthread, kthread);
> +			llist_add(&data->llist, &trigger_data_free_list);
> +			/* Drain the queued frees synchronously if startup failed. */

                                                       s/startup/creation/

> +			trigger_data_free_queued_locked();
> +			return;
>  		}
>  	}

-- Steve

>  
> -	if (!trigger_kthread) {
> -		/* Do it the slow way */
> -		tracepoint_synchronize_unregister();
> -		kfree(data);
> -		return;
> -	}
> -
>  	llist_add(&data->llist, &trigger_data_free_list);
>  	wake_up_process(trigger_kthread);
>  }
>  
> +static int __init trigger_data_free_init(void)
> +{
> +	guard(mutex)(&trigger_data_kthread_mutex);
> +
> +	if (llist_empty(&trigger_data_free_list))
> +		return 0;
> +
> +	trigger_start_kthread_locked();
> +	if (trigger_kthread)
> +		wake_up_process(trigger_kthread);
> +	else
> +		trigger_data_free_queued_locked();
> +
> +	return 0;
> +}
> +late_initcall(trigger_data_free_init);
> +
>  static inline void data_ops_trigger(struct event_trigger_data *data,
>  				    struct trace_buffer *buffer,  void *rec,
>  				    struct ring_buffer_event *event)


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-24 19:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-10  6:47 [PATCH v3 0/2] tracing: preserve repeated boot-time parameters and drain deferred trigger frees Wesley Atwell
2026-03-10  6:47 ` [PATCH v3 1/2] tracing: preserve repeated boot-time tracing parameters Wesley Atwell
2026-03-24 18:43   ` Steven Rostedt
2026-03-10  6:47 ` [PATCH v3 2/2] tracing: drain deferred trigger frees if kthread startup fails Wesley Atwell
2026-03-24 18:53   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox