Skip to contentSkip to conversation

tracing: More clean ups of triggers

PATCH In review Steven Rostedt 3 participants 9 messages Nov 25, 2025 – Dec 3, 2025 Tracing

lore
Share & tools
lore
#0 lore raw

v2 cover tracing: More clean ups of triggers

This is based on top of:

https://lore.kernel.org/linux-trace-kernel/[email protected]

Changes since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/

- Moved include of llist.h to trace.h as it is used there (Masami Hiramatsu)

Steven Rostedt (3):
tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()
tracing: Add bulk garbage collection of freeing event_trigger_data
tracing: Use strim() in trigger_process_regex() instead of skip_spaces()

----

 kernel/trace/trace.h                |  2 ++
 kernel/trace/trace_events_trigger.c | 64 +++++++++++++++++++++++++++++++------
 2 files changed, 56 insertions(+), 10 deletions(-)
#1 lore raw

PATCH v2 1/3 tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()

From: Steven Rostedt <[email protected]>

In event_trigger_regex_release(), the only code is:

	mutex_lock(&event_mutex);
	if (file->f_mode & FMODE_READ)
		seq_release(inode, file);
	mutex_unlock(&event_mutex);

	return 0;

There's nothing special about the file->f_mode or the seq_release() that requires any locking. Remove the unnecessary locks.

---
 kernel/trace/trace_events_trigger.c | 4 ----
 1 file changed, 4 deletions(-)
kernel/trace/trace_events_trigger.c +0 -4
index 7795af600466..e5dcfcbb2cd5 100644
@@ -314,13 +314,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
 
 static int event_trigger_regex_release(struct inode *inode, struct file *file)
 {
-	mutex_lock(&event_mutex);
-
 	if (file->f_mode & FMODE_READ)
 		seq_release(inode, file);
 
-	mutex_unlock(&event_mutex);
-
 	return 0;
 }
 
Signature · 1 line
2.51.0
Reviewed-by on v2 1/3
#2 lore raw

Re: [PATCH v2 1/3] tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()

Quoted 11 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

In event_trigger_regex_release(), the only code is:

	mutex_lock(&event_mutex);
	if (file->f_mode & FMODE_READ)
		seq_release(inode, file);
	mutex_unlock(&event_mutex);

	return 0;
Quoted 6 lines

There's nothing special about the file->f_mode or the seq_release() that
requires any locking. Remove the unnecessary locks.

Looks good to me.

Quoted 25 lines

---
 kernel/trace/trace_events_trigger.c | 4 ----

 1 file changed, 4 deletions(-)
kernel/trace/trace_events_trigger.c +0 -0
index 7795af600466..e5dcfcbb2cd5 100644
@@ -314,13 +314,9 @@ static ssize_t event_trigger_regex_write(struct
 file *file,


 static int event_trigger_regex_release(struct inode *inode, struct file *file)
 {

kernel/trace/trace_events_trigger.c +0 -2
-	mutex_lock(&event_mutex);
-

  if (file->f_mode & FMODE_READ)
  seq_release(inode, file);

kernel/trace/trace_events_trigger.c +0 -2
-	mutex_unlock(&event_mutex);
-

  return 0;
 }

#3 lore raw

PATCH v2 2/3 tracing: Add bulk garbage collection of freeing event_trigger_data

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The event trigger data requires a full tracepoint_synchronize_unregister() call before freeing. That call can take 100s of milliseconds to complete. In order to allow for bulk freeing of the trigger data, it can not call the tracepoint_synchronize_unregister() for every individual trigger data being free.

Create a kthread that gets created the first time a trigger data is freed, and have it use the lockless llist to get the list of data to free, run the tracepoint_synchronize_unregister() then free everything in the list.

By freeing hundreds of event_trigger_data elements together, it only requires two runs of the synchronization function, and not hundreds of runs. This speeds up the operation by orders of magnitude (milliseconds instead of several seconds).

---
Changes since v1: https://patch.msgid.link/[email protected]

- Moved include of llist.h to trace.h as it is used there (Masami Hiramatsu)

 kernel/trace/trace.h                |  2 ++
 kernel/trace/trace_events_trigger.c | 55 +++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 3 deletions(-)
kernel/trace/trace.h +2 -0
index 5863800b1ab3..911fc75dc6c4 100644
@@ -22,6 +22,7 @@
 #include <linux/ctype.h>
 #include <linux/once_lite.h>
 #include <linux/ftrace_regs.h>
+#include <linux/llist.h>
 
 #include "pid_list.h"
 
@@ -1808,6 +1809,7 @@ struct event_trigger_data {
 	char				*name;
 	struct list_head		named_list;
 	struct event_trigger_data	*named_data;
+	struct llist_node		llist;
 };
 
 /* Avoid typos */
kernel/trace/trace_events_trigger.c +52 -3

73 lines not on this page · raw message

Signature · 1 line
2.51.0
Reviewed-by on v2 2/3
#4 lore raw

Re: [PATCH v2 2/3] tracing: Add bulk garbage collection of freeing event_trigger_data

Quoted 12 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

The event trigger data requires a full tracepoint_synchronize_unregister() call before freeing. That call can take 100s of milliseconds to complete. In order to allow for bulk freeing of the trigger data, it can not call the tracepoint_synchronize_unregister() for every individual trigger data being free.

Create a kthread that gets created the first time a trigger data is freed, and have it use the lockless llist to get the list of data to free, run the tracepoint_synchronize_unregister() then free everything in the list.

Quoted 7 lines

By freeing hundreds of event_trigger_data elements together, it only requires two runs of the synchronization function, and not hundreds of runs. This speeds up the operation by orders of magnitude (milliseconds instead of several seconds).

Very nice!

Quoted 108 lines

---
Changes since v1: https://patch.msgid.link/[email protected]

- Moved include of llist.h to trace.h as it is used there (Masami Hiramatsu)

 kernel/trace/trace.h                |  2 ++ kernel/trace/trace_events_trigger.c | 55 +++++++++++++++++++++++++++--

 2 files changed, 54 insertions(+), 3 deletions(-)
kernel/trace/trace.h +1 -0
index 5863800b1ab3..911fc75dc6c4 100644
@@ -22,6 +22,7 @@
  #include <linux/ctype.h>
  #include <linux/once_lite.h>
  #include <linux/ftrace_regs.h>
+#include <linux/llist.h>


 #include "pid_list.h"

kernel/trace/trace.h +1 -0
@@ -1808,6 +1809,7 @@ struct event_trigger_data {
  	char				*name;
  	struct list_head		named_list;
  	struct event_trigger_data	*named_data;
+	struct llist_node		llist;
  };


 /* Avoid typos */

kernel/trace/trace_events_trigger.c +0 -0
index e5dcfcbb2cd5..3b97c242b795 100644
@@ -6,6 +6,7 @@
   */


 #include <linux/security.h>
+#include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/ctype.h>
 #include <linux/mutex.h>

kernel/trace/trace_events_trigger.c +0 -0
@@ -17,15 +18,63 @@
  static LIST_HEAD(trigger_commands);
  static DEFINE_MUTEX(trigger_cmd_mutex);

kernel/trace/trace_events_trigger.c +30 -0
+static struct task_struct *trigger_kthread;
+static struct llist_head trigger_data_free_list;
+static DEFINE_MUTEX(trigger_data_kthread_mutex);
+
+/* Bulk garbage collection of event_trigger_data elements */
+static int trigger_kthread_fn(void *ignore)
+{
+	struct event_trigger_data *data, *tmp;
+	struct llist_node *llnodes;
+
+	/* Once this task starts, it lives forever */
+	for (;;) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (llist_empty(&trigger_data_free_list))
+			schedule();
+
+		__set_current_state(TASK_RUNNING);
+
+		llnodes = llist_del_all(&trigger_data_free_list);
+
+		/* make sure current triggers exit before free */
+		tracepoint_synchronize_unregister();
+
+		llist_for_each_entry_safe(data, tmp, llnodes, llist)
+			kfree(data);
+	}
+
+	return 0;
+}
+

 void trigger_data_free(struct event_trigger_data *data) {
  if (data->cmd_ops->set_filter) data->cmd_ops->set_filter(NULL, data, NULL);

kernel/trace/trace_events_trigger.c +19 -2
-	/* make sure current triggers exit before free */
-	tracepoint_synchronize_unregister();
+	if (unlikely(!trigger_kthread)) {
+		guard(mutex)(&trigger_data_kthread_mutex);
+		/* 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);
+		}
+	}
+
+	if (!trigger_kthread) {
+		/* Do it the slow way */
+		tracepoint_synchronize_unregister();
+		kfree(data);
+		return;
+	}

kernel/trace/trace_events_trigger.c +2 -1
-	kfree(data);
+	llist_add(&data->llist, &trigger_data_free_list);
+	wake_up_process(trigger_kthread);

 }

 static inline void data_ops_trigger(struct event_trigger_data *data,

#5 lore raw

PATCH v2 3/3 tracing: Use strim() in trigger_process_regex() instead of skip_spaces()

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The function trigger_process_regex() is called by a few functions, where only one calls strim() on the buffer passed to it. That leaves the other functions not trimming the end of the buffer passed in and making it a little inconsistent.

Remove the strim() from event_trigger_regex_write() and have trigger_process_regex() use strim() instead of skip_spaces(). The buff variable is not passed in as const, so it can be modified.

---
 kernel/trace/trace_events_trigger.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/trace/trace_events_trigger.c +2 -3
index 3b97c242b795..96aad82b1628 100644
@@ -308,7 +308,8 @@ int trigger_process_regex(struct trace_event_file *file, char *buff)
 	char *command, *next;
 	struct event_command *p;
 
-	next = buff = skip_spaces(buff);
+	next = buff = strim(buff);
+
 	command = strsep(&next, ": \t");
 	if (next) {
 		next = skip_spaces(next);
@@ -345,8 +346,6 @@ static ssize_t event_trigger_regex_write(struct file *file,
 	if (IS_ERR(buf))
 		return PTR_ERR(buf);
 
-	strim(buf);
-
 	guard(mutex)(&event_mutex);
 
 	event_file = event_file_file(file);
Signature · 1 line
2.51.0
Reviewed-by on v2 3/3
#6 lore raw

Re: [PATCH v2 3/3] tracing: Use strim() in trigger_process_regex() instead of skip_spaces()

Quoted 7 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

The function trigger_process_regex() is called by a few functions, where only one calls strim() on the buffer passed to it. That leaves the other functions not trimming the end of the buffer passed in and making it a little inconsistent.

Quoted 6 lines

Remove the strim() from event_trigger_regex_write() and have trigger_process_regex() use strim() instead of skip_spaces(). The buff variable is not passed in as const, so it can be modified.

Looks good to me.

Quoted 27 lines

---
 kernel/trace/trace_events_trigger.c | 5 ++---

 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/trace/trace_events_trigger.c +0 -0
index 3b97c242b795..96aad82b1628 100644
@@ -308,7 +308,8 @@ int trigger_process_regex(struct trace_event_file *file, char *buff)
  	char *command, *next;
  	struct event_command *p;

kernel/trace/trace_events_trigger.c +2 -1
-	next = buff = skip_spaces(buff);
+	next = buff = strim(buff);
+

  command = strsep(&next, ": \t");
  if (next) {
  next = skip_spaces(next);

kernel/trace/trace_events_trigger.c +0 -0
@@ -345,8 +346,6 @@ static ssize_t event_trigger_regex_write(struct file *file,
  	if (IS_ERR(buf))
  		return PTR_ERR(buf);

kernel/trace/trace_events_trigger.c +0 -2
-	strim(buf);
-

  guard(mutex)(&event_mutex);

  event_file = event_file_file(file);

maintainer
#7 lore raw

Re: [PATCH v2 3/3] tracing: Use strim() in trigger_process_regex() instead of skip_spaces()

Quoted 10 lines · On Wed, 03 Dec 2025 16:40:04 -0600 Tom Zanussi <[email protected]> wrote:
Quoted 6 lines

Remove the strim() from event_trigger_regex_write() and have trigger_process_regex() use strim() instead of skip_spaces(). The buff variable is not passed in as const, so it can be modified.

Looks good to me.

Thanks for all the reviews, but I already did the pull request ;-)

Although it hasn't been accepted yet and there's always a chance Linus may have an issue with one of the commits (I had a lot of commits this time, affecting various aspects of the code, but couldn't use topic branches due to too many conflicts). If it is rejected, I'll add your reviewed-by tags.

-- Steve

#8 lore raw

Re: [PATCH v2 3/3] tracing: Use strim() in trigger_process_regex() instead of skip_spaces()

Quoted 14 lines · Steven Rostedt
Quoted 10 lines · On Wed, 03 Dec 2025 16:40:04 -0600 Tom Zanussi <[email protected]> wrote:
Quoted 6 lines

Remove the strim() from event_trigger_regex_write() and have trigger_process_regex() use strim() instead of skip_spaces(). The buff variable is not passed in as const, so it can be modified.

Looks good to me.

Quoted 8 lines

Thanks for all the reviews, but I already did the pull request ;-)

Although it hasn't been accepted yet and there's always a chance Linus may have an issue with one of the commits (I had a lot of commits this time, affecting various aspects of the code, but couldn't use topic branches due to too many conflicts). If it is rejected, I'll add your reviewed-by tags.

-- Steve

No problem, my fault for being so slow to get to these. :-/

Tom