[PATCH v2 0/4] Clean up access to trace_event_file from a file struct

[PATCH v2 0/4] Clean up access to trace_event_file from a file struct

[Petr Pavlu]

This series includes several patches related to accessing
trace_event_file from a file struct. The first patch is a fix for an
edge case, the remaining patches are minor cleanups.

Changes since v1 [1]:
* Fix a compilation error when CONFIG_HIST_TRIGGERS is not set.
* Drop a patch that references the trace_event_file data in
  event_file_data() and keep the simpler implementation of storing the
  id in i_private.
* Inline event_file_data() into event_id_read() to enable adding
  additional checks to the former.

Petr Pavlu (4):
  tracing: Fix checking of freed trace_event_file for hist files
  tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
  tracing: Clean up access to trace_event_file from a file pointer
  tracing: Free up file->private_data for use by individual events

 include/linux/trace_events.h     |  5 +++++
 kernel/trace/trace.c             |  2 --
 kernel/trace/trace.h             | 17 +++++++++++------
 kernel/trace/trace_events.c      | 17 ++++++++---------
 kernel/trace/trace_events_hist.c |  8 ++------
 5 files changed, 26 insertions(+), 23 deletions(-)


base-commit: cee73b1e840c154f64ace682cb477c1ae2e29cc4
-- 
2.52.0

[PATCH v2 1/4] tracing: Fix checking of freed trace_event_file for hist files

[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.

A related issue is that although event_hist_poll() attempts to verify
whether an event file is being removed, 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 these issues by changing the access method to event_file_file() and
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 +++
 kernel/trace/trace_events_hist.c | 4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 3690221ba3d8..f925034e402d 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 137b4d9bb116..e8ed6ba155cf 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1295,6 +1295,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();
 }
 
 /*
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c97bb2fda5c0..744c2aa3d668 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5778,7 +5778,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;
 
@@ -5816,7 +5816,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.52.0

[PATCH v2 2/4] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED

[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 e8ed6ba155cf..2ca76b048638 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2153,12 +2153,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.52.0

[PATCH v2 3/4] tracing: Clean up access to trace_event_file from a file pointer

[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 8428c437cb9d..27fb3fe6a7e0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1739,11 +1739,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;
 
@@ -1764,12 +1759,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 2ca76b048638..d23e7a9d07e6 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2090,12 +2090,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.52.0

[PATCH v2 4/4] tracing: Free up file->private_data for use by individual events

[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 b1cb30a7b83d..01af8d88a468 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4804,8 +4804,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 744c2aa3d668..1ca3e14d7531 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5831,8 +5831,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);
@@ -6114,8 +6112,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.52.0

Re: [PATCH v2 1/4] tracing: Fix checking of freed trace_event_file for hist files

[Steven Rostedt]

On Mon, 16 Feb 2026 14:41:57 +0100
Petr Pavlu <petr.pavlu@suse.com> wrote:

> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 3690221ba3d8..f925034e402d 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 137b4d9bb116..e8ed6ba155cf 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1295,6 +1295,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();
>  }
>  
>  /*
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index c97bb2fda5c0..744c2aa3d668 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -5778,7 +5778,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;
>  
> @@ -5816,7 +5816,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;
> -- 

This should be broken into two different patches. One for the
hist_poll_wakeup() fix, the other to use event_file_file().

-- Steve

Re: [PATCH v2 1/4] tracing: Fix checking of freed trace_event_file for hist files

[Steven Rostedt]

On Tue, 17 Feb 2026 10:29:57 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 16 Feb 2026 14:41:57 +0100
> Petr Pavlu <petr.pavlu@suse.com> wrote:
> 
> > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> > index 3690221ba3d8..f925034e402d 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 137b4d9bb116..e8ed6ba155cf 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c
> > @@ -1295,6 +1295,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();
> >  }
> >  
> >  /*
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index c97bb2fda5c0..744c2aa3d668 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -5778,7 +5778,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;
> >  
> > @@ -5816,7 +5816,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;
> > --   
> 
> This should be broken into two different patches. One for the
> hist_poll_wakeup() fix, the other to use event_file_file().
> 
> -- Steve

Replying with my kernel.org account in case this email ended up in your spam folder.

-- Steve