* [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-10 11:28 [PATCH 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
@ 2026-02-10 11:28 ` Petr Pavlu
2026-02-10 15:59 ` Masami Hiramatsu
` (2 more replies)
2026-02-10 11:28 ` [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files Petr Pavlu
` (3 subsequent siblings)
4 siblings, 3 replies; 13+ messages in thread
From: Petr Pavlu @ 2026-02-10 11:28 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.
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>
---
kernel/trace/trace_events.c | 3 +++
kernel/trace/trace_events_hist.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-10 11:28 ` [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
@ 2026-02-10 15:59 ` Masami Hiramatsu
2026-02-10 17:40 ` kernel test robot
2026-02-10 18:57 ` kernel test robot
2 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2026-02-10 15:59 UTC (permalink / raw)
To: Petr Pavlu
Cc: Steven Rostedt, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel
On Tue, 10 Feb 2026 12:28:16 +0100
Petr Pavlu <petr.pavlu@suse.com> wrote:
> 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.
>
Thanks for fixing!
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Fixes: 1bd13edbbed6 ("tracing/hist: Add poll(POLLIN) support on hist file")
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/trace/trace_events.c | 3 +++
> kernel/trace/trace_events_hist.c | 4 ++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> 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
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-10 11:28 ` [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
2026-02-10 15:59 ` Masami Hiramatsu
@ 2026-02-10 17:40 ` kernel test robot
2026-02-11 16:28 ` Steven Rostedt
2026-02-10 18:57 ` kernel test robot
2 siblings, 1 reply; 13+ messages in thread
From: kernel test robot @ 2026-02-10 17:40 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Masami Hiramatsu
Cc: oe-kbuild-all, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel, Petr Pavlu
Hi Petr,
kernel test robot noticed the following build errors:
[auto build test ERROR on 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b]
url: https://github.com/intel-lab-lkp/linux/commits/Petr-Pavlu/tracing-Fix-checking-of-freed-trace_event_file-for-hist-files/20260210-194023
base: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
patch link: https://lore.kernel.org/r/20260210113427.1068932-2-petr.pavlu%40suse.com
patch subject: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
config: x86_64-buildonly-randconfig-004-20260210 (https://download.01.org/0day-ci/archive/20260211/202602110125.71oIfNm0-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260211/202602110125.71oIfNm0-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602110125.71oIfNm0-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/trace/trace_events.c: In function 'remove_event_file_dir':
>> kernel/trace/trace_events.c:1300:9: error: implicit declaration of function 'hist_poll_wakeup' [-Wimplicit-function-declaration]
1300 | hist_poll_wakeup();
| ^~~~~~~~~~~~~~~~
vim +/hist_poll_wakeup +1300 kernel/trace/trace_events.c
1289
1290 static void remove_event_file_dir(struct trace_event_file *file)
1291 {
1292 eventfs_remove_dir(file->ei);
1293 list_del(&file->list);
1294 remove_subsystem(file->system);
1295 free_event_filter(file->filter);
1296 file->flags |= EVENT_FILE_FL_FREED;
1297 event_file_put(file);
1298
1299 /* Wake up hist poll waiters to notice the EVENT_FILE_FL_FREED flag. */
> 1300 hist_poll_wakeup();
1301 }
1302
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-10 17:40 ` kernel test robot
@ 2026-02-11 16:28 ` Steven Rostedt
0 siblings, 0 replies; 13+ messages in thread
From: Steven Rostedt @ 2026-02-11 16:28 UTC (permalink / raw)
To: kernel test robot
Cc: Petr Pavlu, Masami Hiramatsu, oe-kbuild-all, Mathieu Desnoyers,
Tom Zanussi, linux-kernel, linux-trace-kernel
On Wed, 11 Feb 2026 01:40:48 +0800
kernel test robot <lkp@intel.com> wrote:
> url: https://github.com/intel-lab-lkp/linux/commits/Petr-Pavlu/tracing-Fix-checking-of-freed-trace_event_file-for-hist-files/20260210-194023
> base: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
> patch link: https://lore.kernel.org/r/20260210113427.1068932-2-petr.pavlu%40suse.com
> patch subject: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
> config: x86_64-buildonly-randconfig-004-20260210 (https://download.01.org/0day-ci/archive/20260211/202602110125.71oIfNm0-lkp@intel.com/config)
CONFIG_FTRACE is not set, and histograms have this as a dependency.
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260211/202602110125.71oIfNm0-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202602110125.71oIfNm0-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> kernel/trace/trace_events.c: In function 'remove_event_file_dir':
> >> kernel/trace/trace_events.c:1300:9: error: implicit declaration of function 'hist_poll_wakeup' [-Wimplicit-function-declaration]
> 1300 | hist_poll_wakeup();
> | ^~~~~~~~~~~~~~~~
>
This needs:
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 0a2b8229b999..8c627524c1d4 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -683,6 +683,8 @@ static inline void hist_poll_wakeup(void)
#define hist_poll_wait(file, wait) \
poll_wait(file, &hist_poll_wq, wait)
+#else
+# define hist_poll_wait(file, wait) do{ } while (0)
#endif
#define __TRACE_EVENT_FLAGS(name, value) \
-- Steve
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
2026-02-10 11:28 ` [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
2026-02-10 15:59 ` Masami Hiramatsu
2026-02-10 17:40 ` kernel test robot
@ 2026-02-10 18:57 ` kernel test robot
2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2026-02-10 18:57 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Masami Hiramatsu
Cc: llvm, oe-kbuild-all, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel, Petr Pavlu
Hi Petr,
kernel test robot noticed the following build errors:
[auto build test ERROR on 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b]
url: https://github.com/intel-lab-lkp/linux/commits/Petr-Pavlu/tracing-Fix-checking-of-freed-trace_event_file-for-hist-files/20260210-194023
base: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
patch link: https://lore.kernel.org/r/20260210113427.1068932-2-petr.pavlu%40suse.com
patch subject: [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files
config: sparc64-defconfig (https://download.01.org/0day-ci/archive/20260211/202602110247.8HeuKXss-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260211/202602110247.8HeuKXss-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602110247.8HeuKXss-lkp@intel.com/
All errors (new ones prefixed by >>):
>> kernel/trace/trace_events.c:1300:2: error: call to undeclared function 'hist_poll_wakeup'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1300 | hist_poll_wakeup();
| ^
1 error generated.
vim +/hist_poll_wakeup +1300 kernel/trace/trace_events.c
1289
1290 static void remove_event_file_dir(struct trace_event_file *file)
1291 {
1292 eventfs_remove_dir(file->ei);
1293 list_del(&file->list);
1294 remove_subsystem(file->system);
1295 free_event_filter(file->filter);
1296 file->flags |= EVENT_FILE_FL_FREED;
1297 event_file_put(file);
1298
1299 /* Wake up hist poll waiters to notice the EVENT_FILE_FL_FREED flag. */
> 1300 hist_poll_wakeup();
1301 }
1302
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files
2026-02-10 11:28 [PATCH 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
2026-02-10 11:28 ` [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
@ 2026-02-10 11:28 ` Petr Pavlu
2026-02-11 16:37 ` Steven Rostedt
2026-02-10 11:28 ` [PATCH 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED Petr Pavlu
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Petr Pavlu @ 2026-02-10 11:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
The code for reading an event id currently uses file->f_inode->i_private to
store the value of trace_event_file->event_call->event.type, unlike all
other event files which use it to store a pointer to the associated
trace_event_file data. The event_id_read() function retrieves this id value
from i_private and checks if it is non-0/NULL to determine whether the
event is still valid. This approach worked in the past when
remove_event_file_dir() would set i_private to NULL for all files in an
event directory upon removal. However, with the introduction of eventfs,
i_private is assigned when an eventfs inode is allocated and remains set
throughout its lifetime. As a result, event_id_read() may fail to detect
that an event is being removed.
Fix this issue by changing the event id file to use i_private to store
a trace_event_file pointer and utilize event_file_file() to check for the
EVENT_FILE_FL_FREED flag.
Fixes: 6fdac58c560e ("tracing: Remove unused trace_event_file dir field")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace_events.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e8ed6ba155cf..3d272b3cd688 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2090,11 +2090,18 @@ 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);
+ struct trace_event_file *file;
+ int id;
char buf[32];
int len;
- if (unlikely(!id))
+ mutex_lock(&event_mutex);
+ file = event_file_file(filp);
+ if (likely(file))
+ id = file->event_call->event.type;
+ mutex_unlock(&event_mutex);
+
+ if (!file)
return -ENODEV;
len = sprintf(buf, "%d\n", id);
@@ -2572,7 +2579,9 @@ static const struct file_operations ftrace_event_format_fops = {
#ifdef CONFIG_PERF_EVENTS
static const struct file_operations ftrace_event_id_fops = {
+ .open = tracing_open_file_tr,
.read = event_id_read,
+ .release = tracing_release_file_tr,
.llseek = default_llseek,
};
#endif
@@ -2936,7 +2945,6 @@ static int event_callback(const char *name, umode_t *mode, void **data,
if (call->event.type && call->class->reg &&
strcmp(name, "id") == 0) {
*mode = TRACE_MODE_READ;
- *data = (void *)(long)call->event.type;
*fops = &ftrace_event_id_fops;
return 1;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files
2026-02-10 11:28 ` [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files Petr Pavlu
@ 2026-02-11 16:37 ` Steven Rostedt
2026-02-12 8:15 ` Petr Pavlu
0 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-02-11 16:37 UTC (permalink / raw)
To: Petr Pavlu
Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel
On Tue, 10 Feb 2026 12:28:17 +0100
Petr Pavlu <petr.pavlu@suse.com> wrote:
> The code for reading an event id currently uses file->f_inode->i_private to
> store the value of trace_event_file->event_call->event.type, unlike all
> other event files which use it to store a pointer to the associated
> trace_event_file data. The event_id_read() function retrieves this id value
> from i_private and checks if it is non-0/NULL to determine whether the
> event is still valid. This approach worked in the past when
> remove_event_file_dir() would set i_private to NULL for all files in an
> event directory upon removal. However, with the introduction of eventfs,
> i_private is assigned when an eventfs inode is allocated and remains set
> throughout its lifetime. As a result, event_id_read() may fail to detect
> that an event is being removed.
Who cares? It's just an id. If the event is being removed, there's
nothing wrong with still returning its id. The code currently is very
simple, I don't want to make it complex for something nobody cares
about.
-- Steve
>
> Fix this issue by changing the event id file to use i_private to store
> a trace_event_file pointer and utilize event_file_file() to check for the
> EVENT_FILE_FL_FREED flag.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files
2026-02-11 16:37 ` Steven Rostedt
@ 2026-02-12 8:15 ` Petr Pavlu
0 siblings, 0 replies; 13+ messages in thread
From: Petr Pavlu @ 2026-02-12 8:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel
On 2/11/26 5:37 PM, Steven Rostedt wrote:
> On Tue, 10 Feb 2026 12:28:17 +0100
> Petr Pavlu <petr.pavlu@suse.com> wrote:
>
>> The code for reading an event id currently uses file->f_inode->i_private to
>> store the value of trace_event_file->event_call->event.type, unlike all
>> other event files which use it to store a pointer to the associated
>> trace_event_file data. The event_id_read() function retrieves this id value
>> from i_private and checks if it is non-0/NULL to determine whether the
>> event is still valid. This approach worked in the past when
>> remove_event_file_dir() would set i_private to NULL for all files in an
>> event directory upon removal. However, with the introduction of eventfs,
>> i_private is assigned when an eventfs inode is allocated and remains set
>
>> throughout its lifetime. As a result, event_id_read() may fail to detect
>> that an event is being removed.
>
> Who cares? It's just an id. If the event is being removed, there's
> nothing wrong with still returning its id. The code currently is very
> simple, I don't want to make it complex for something nobody cares
> about.
I think one could argue that handling all event files in the same way is
less complex. It would also allow for a minor simplification where
event_callback() would no longer need to modify the data pointer and
eventfs then wouldn't need to pass it down to lookup_file() because it
could always use the default ei->data.
However, this is all minor and I can see your point of view. I'll drop
this patch.
--
Thanks,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
2026-02-10 11:28 [PATCH 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
2026-02-10 11:28 ` [PATCH 1/5] tracing: Fix checking of freed trace_event_file for hist files Petr Pavlu
2026-02-10 11:28 ` [PATCH 2/5] tracing: Fix checking of freed trace_event_file for id files Petr Pavlu
@ 2026-02-10 11:28 ` Petr Pavlu
2026-02-10 11:28 ` [PATCH 4/5] tracing: Clean up access to trace_event_file from a file pointer Petr Pavlu
2026-02-10 11:28 ` [PATCH 5/5] tracing: Free up file->private_data for use by individual events Petr Pavlu
4 siblings, 0 replies; 13+ messages in thread
From: Petr Pavlu @ 2026-02-10 11:28 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 3d272b3cd688..401ab7ed869b 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2160,12 +2160,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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5] tracing: Clean up access to trace_event_file from a file pointer
2026-02-10 11:28 [PATCH 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
` (2 preceding siblings ...)
2026-02-10 11:28 ` [PATCH 3/5] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED Petr Pavlu
@ 2026-02-10 11:28 ` Petr Pavlu
2026-02-11 16:45 ` Steven Rostedt
2026-02-10 11:28 ` [PATCH 5/5] tracing: Free up file->private_data for use by individual events Petr Pavlu
4 siblings, 1 reply; 13+ messages in thread
From: Petr Pavlu @ 2026-02-10 11:28 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. A previous fix
also modified event_id_read() to read i_private while holding the
event_mutex.
Remove the now unnecessary READ_ONCE() access to i_private in both
event_file_file() and event_file_data(). 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 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index c11edec5d8f5..c6b450de93bc 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;
--
2.52.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] tracing: Clean up access to trace_event_file from a file pointer
2026-02-10 11:28 ` [PATCH 4/5] tracing: Clean up access to trace_event_file from a file pointer Petr Pavlu
@ 2026-02-11 16:45 ` Steven Rostedt
0 siblings, 0 replies; 13+ messages in thread
From: Steven Rostedt @ 2026-02-11 16:45 UTC (permalink / raw)
To: Petr Pavlu
Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi, linux-kernel,
linux-trace-kernel
On Tue, 10 Feb 2026 12:28:19 +0100
Petr Pavlu <petr.pavlu@suse.com> wrote:
> 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.
I'm OK with this change. Instead of removing the event_file_data() like
you did from event_id_read(), just open code it there. The id is
simple, let's keep it that way.
That is, something like:
int id = (long)READ_ONCE(file_inode(filp)->i_private);
Feel free to add comments to explain it.
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] tracing: Free up file->private_data for use by individual events
2026-02-10 11:28 [PATCH 0/5] Clean up access to trace_event_file from a file struct Petr Pavlu
` (3 preceding siblings ...)
2026-02-10 11:28 ` [PATCH 4/5] tracing: Clean up access to trace_event_file from a file pointer Petr Pavlu
@ 2026-02-10 11:28 ` Petr Pavlu
4 siblings, 0 replies; 13+ messages in thread
From: Petr Pavlu @ 2026-02-10 11:28 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 8bd4ec08fb36..7c9882e5ba22 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4805,8 +4805,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
^ permalink raw reply related [flat|nested] 13+ messages in thread