* [PATCH v3 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-19 16:27 [PATCH v3 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
@ 2026-02-19 16:27 ` Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 2/5] tracing: Wake up poll waiters for hist files when removing an event Petr Pavlu
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-02-19 16:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The event_hist_open() and event_hist_poll() functions currently retrieve
a trace_event_file pointer from a file struct by invoking
event_file_data(), which simply returns file->f_inode->i_private. The
functions then check if the pointer is NULL to determine whether the event
is still valid. This approach is flawed because i_private is assigned when
an eventfs inode is allocated and remains set throughout its lifetime.
Instead, the code should call event_file_file(), which checks for
EVENT_FILE_FL_FREED. Using the incorrect access function may result in the
code potentially opening a hist file for an event that is being removed or
becoming stuck while polling on this file.
Correct the access method to event_file_file() in both functions.
Fixes: 1bd13edbbed6 ("tracing/hist: Add poll(POLLIN) support on hist file")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_events_hist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index e6f449f53afc..768df987419e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5784,7 +5784,7 @@ static __poll_t event_hist_poll(struct file *file, struct poll_table_struct *wai
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file)
return EPOLLERR;
@@ -5822,7 +5822,7 @@ static int event_hist_open(struct inode *inode, struct file *file)
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file) {
ret = -ENODEV;
goto err;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/5] tracing: Wake up poll waiters for hist files when removing an event
2026-02-19 16:27 [PATCH v3 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
@ 2026-02-19 16:27 ` Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED Petr Pavlu
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-02-19 16:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The event_hist_poll() function attempts to verify whether an event file is
being removed, but this check may not occur or could be unnecessarily
delayed. This happens because hist_poll_wakeup() is currently invoked only
from event_hist_trigger() when a hist command is triggered. If the event
file is being removed, no associated hist command will be triggered and a
waiter will be woken up only after an unrelated hist command is triggered.
Fix the issue by adding a call to hist_poll_wakeup() in
remove_event_file_dir() after setting the EVENT_FILE_FL_FREED flag. This
ensures that a task polling on a hist file is woken up and receives
EPOLLERR.
Fixes: 1bd13edbbed6 ("tracing/hist: Add poll(POLLIN) support on hist file")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
include/linux/trace_events.h | 5 +++++
kernel/trace/trace_events.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 0a2b8229b999..37eb2f0f3dd8 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -683,6 +683,11 @@ static inline void hist_poll_wakeup(void)
#define hist_poll_wait(file, wait) \
poll_wait(file, &hist_poll_wq, wait)
+
+#else
+static inline void hist_poll_wakeup(void)
+{
+}
#endif
#define __TRACE_EVENT_FLAGS(name, value) \
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 61fe01dce7a6..b659653dc03a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1311,6 +1311,9 @@ static void remove_event_file_dir(struct trace_event_file *file)
free_event_filter(file->filter);
file->flags |= EVENT_FILE_FL_FREED;
event_file_put(file);
+
+ /* Wake up hist poll waiters to notice the EVENT_FILE_FL_FREED flag. */
+ hist_poll_wakeup();
}
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
2026-02-19 16:27 [PATCH v3 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 2/5] tracing: Wake up poll waiters for hist files when removing an event Petr Pavlu
@ 2026-02-19 16:27 ` Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 4/5] tracing: Clean up access to trace_event_file from a file pointer Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 5/5] tracing: Free up file->private_data for use by individual events Petr Pavlu
4 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-02-19 16:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The event_filter_write() function calls event_file_file() to retrieve
a trace_event_file associated with a given file struct. If a non-NULL
pointer is returned, the function then checks whether the trace_event_file
instance has the EVENT_FILE_FL_FREED flag set. This check is redundant
because event_file_file() already performs this validation and returns NULL
if the flag is set. The err value is also already initialized to -ENODEV.
Remove the unnecessary check for EVENT_FILE_FL_FREED in
event_filter_write().
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace_events.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index b659653dc03a..90915e1286da 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2245,12 +2245,8 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt,
mutex_lock(&event_mutex);
file = event_file_file(filp);
- if (file) {
- if (file->flags & EVENT_FILE_FL_FREED)
- err = -ENODEV;
- else
- err = apply_event_filter(file, buf);
- }
+ if (file)
+ err = apply_event_filter(file, buf);
mutex_unlock(&event_mutex);
kfree(buf);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 4/5] tracing: Clean up access to trace_event_file from a file pointer
2026-02-19 16:27 [PATCH v3 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
` (2 preceding siblings ...)
2026-02-19 16:27 ` [PATCH v3 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED Petr Pavlu
@ 2026-02-19 16:27 ` Petr Pavlu
2026-02-19 16:27 ` [PATCH v3 5/5] tracing: Free up file->private_data for use by individual events Petr Pavlu
4 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-02-19 16:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The tracing code provides two functions event_file_file() and
event_file_data() to obtain a trace_event_file pointer from a file struct.
The primary method to use is event_file_file(), as it checks for the
EVENT_FILE_FL_FREED flag to determine whether the event is being removed.
The second function event_file_data() is an optimization for retrieving the
same data when the event_mutex is still held.
In the past, when removing an event directory in remove_event_file_dir(),
the code set i_private to NULL for all event files and readers were
expected to check for this state to recognize that the event is being
removed. In the case of event_id_read(), the value was read using
event_file_data() without acquiring the event_mutex. This required
event_file_data() to use READ_ONCE() when retrieving the i_private data.
With the introduction of eventfs, i_private is assigned when an eventfs
inode is allocated and remains set throughout its lifetime.
Remove the now unnecessary READ_ONCE() access to i_private in both
event_file_file() and event_file_data(). Inline the access to i_private in
remove_event_file_dir(), which allows event_file_data() to handle i_private
solely as a trace_event_file pointer. Add a check in event_file_data() to
ensure that the event_mutex is held and that file->flags doesn't have the
EVENT_FILE_FL_FREED flag set. Finally, move event_file_data() immediately
after event_file_code() since the latter provides a comment explaining how
both functions should be used together.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.h | 17 +++++++++++------
kernel/trace/trace_events.c | 6 +++---
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b8f3804586a0..7db78a62f786 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1802,11 +1802,6 @@ extern struct trace_event_file *find_event_file(struct trace_array *tr,
const char *system,
const char *event);
-static inline void *event_file_data(struct file *filp)
-{
- return READ_ONCE(file_inode(filp)->i_private);
-}
-
extern struct mutex event_mutex;
extern struct list_head ftrace_events;
@@ -1827,12 +1822,22 @@ static inline struct trace_event_file *event_file_file(struct file *filp)
struct trace_event_file *file;
lockdep_assert_held(&event_mutex);
- file = READ_ONCE(file_inode(filp)->i_private);
+ file = file_inode(filp)->i_private;
if (!file || file->flags & EVENT_FILE_FL_FREED)
return NULL;
return file;
}
+static inline void *event_file_data(struct file *filp)
+{
+ struct trace_event_file *file;
+
+ lockdep_assert_held(&event_mutex);
+ file = file_inode(filp)->i_private;
+ WARN_ON(!file || file->flags & EVENT_FILE_FL_FREED);
+ return file;
+}
+
extern const struct file_operations event_trigger_fops;
extern const struct file_operations event_hist_fops;
extern const struct file_operations event_hist_debug_fops;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 90915e1286da..c52135cd9265 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2182,12 +2182,12 @@ static int trace_format_open(struct inode *inode, struct file *file)
static ssize_t
event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
{
- int id = (long)event_file_data(filp);
+ /* id is directly in i_private and available for inode's lifetime. */
+ int id = (long)file_inode(filp)->i_private;
char buf[32];
int len;
- if (unlikely(!id))
- return -ENODEV;
+ WARN_ON(!id);
len = sprintf(buf, "%d\n", id);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 5/5] tracing: Free up file->private_data for use by individual events
2026-02-19 16:27 [PATCH v3 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
` (3 preceding siblings ...)
2026-02-19 16:27 ` [PATCH v3 4/5] tracing: Clean up access to trace_event_file from a file pointer Petr Pavlu
@ 2026-02-19 16:27 ` Petr Pavlu
4 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-02-19 16:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The tracing_open_file_tr() function currently copies the trace_event_file
pointer from inode->i_private to file->private_data when the file is
successfully opened. This duplication is not particularly useful, as all
event code should utilize event_file_file() or event_file_data() to
retrieve a trace_event_file pointer from a file struct and these access
functions read file->f_inode->i_private. Moreover, this setup requires the
code for opening hist files to explicitly clear file->private_data before
calling single_open(), since this function expects the private_data member
to be set to NULL and uses it to store a pointer to a seq_file.
Remove the unnecessary setting of file->private_data in
tracing_open_file_tr() and simplify the hist code.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.c | 2 --
kernel/trace/trace_events_hist.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2f6fbf9e7caf..a51c675be0f4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4051,8 +4051,6 @@ int tracing_open_file_tr(struct inode *inode, struct file *filp)
event_file_get(file);
}
- filp->private_data = inode->i_private;
-
return 0;
}
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 768df987419e..57c76d40be2d 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5837,8 +5837,6 @@ static int event_hist_open(struct inode *inode, struct file *file)
hist_file->file = file;
hist_file->last_act = get_hist_hit_count(event_file);
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_show, hist_file);
if (ret) {
kfree(hist_file);
@@ -6127,8 +6125,6 @@ static int event_hist_debug_open(struct inode *inode, struct file *file)
if (ret)
return ret;
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_debug_show, file);
if (ret)
tracing_release_file_tr(inode, file);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread