* Re: [rtla 04/13] rtla: Replace atoi() with a robust strtoi()
From: Wander Lairson Costa @ 2025-11-25 13:49 UTC (permalink / raw)
To: Costa Shulyupin
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, Crystal Wood,
John Kacur, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <CADDUTFyAAAv641OfGf_U4hVdegyAVyp5rgruF=NSNd+UPkjOzQ@mail.gmail.com>
On Tue, Nov 25, 2025 at 10:35:39AM +0200, Costa Shulyupin wrote:
> On Mon, 17 Nov 2025 at 20:55, Wander Lairson Costa <wander@redhat.com> wrote:
> > To address this, introduce a new strtoi() helper function that safely
> > converts a string to an integer. This function validates the input and
> > checks for overflows, returning a boolean to indicate success or failure.
>
> Why not use sscanf() for this purpose instead of adding a new utility function?
>
The strtoi implementation properly detects:
1. Empty strings - via the !*s check
2. Conversion errors - via errno from strtol
3. Trailing garbage - via *end_ptr check ensuring entire string was consumed
4. Integer overflow/underflow - via explicit lres > INT_MAX || lres < INT_MIN
bounds checking
sscanf has the following limitations:
1. Trailing garbage is silently ignored
int val;
sscanf("123abc", "%d", &val); /* Returns 1 (success), val=123, "abc" ignored */
While you could use "%d%n" with character count checking, this becomes
cumbersome and defeats the purpose of simplification.
2. Integer overflow has undefined behavior
sscanf with %d doesn't guarantee overflow detection and may silently wrap
values (e.g., 2147483648 -> -2147483648). There's no standard way to detect
this has occurred.
3. No detailed error reporting (this is minor, though)
sscanf only returns match count, not error type. You cannot distinguish
"bad format" from "overflow" from "trailing garbage".
> Also, using a boolean to return success or failure does not conform to
> POSIX standards and is confusing in Linux/POSIX code.
>
Ok, I will change it.
> Costa
>
^ permalink raw reply
* Re: [RFC PATCH v7 30/31] x86/mm, mm/vmalloc: Defer kernel TLB flush IPIs under CONFIG_COALESCE_TLBI=y
From: Valentin Schneider @ 2025-11-25 14:13 UTC (permalink / raw)
To: Dave Hansen, linux-kernel, linux-mm, rcu, x86, linux-arm-kernel,
loongarch, linux-riscv, linux-arch, linux-trace-kernel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra,
Arnaldo Carvalho de Melo, Josh Poimboeuf, Paolo Bonzini,
Arnd Bergmann, Frederic Weisbecker, Paul E. McKenney, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Sami Tolvanen, David S. Miller,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Mathieu Desnoyers, Mel Gorman, Andrew Morton,
Masahiro Yamada, Han Shen, Rik van Riel, Jann Horn, Dan Carpenter,
Oleg Nesterov, Juri Lelli, Clark Williams, Yair Podemsky,
Marcelo Tosatti, Daniel Wagner, Petr Tesarik, Shrikanth Hegde
In-Reply-To: <2837ea3e-c0b8-46b0-b8da-bf06906d124d@intel.com>
On 21/11/25 09:50, Dave Hansen wrote:
> On 11/21/25 09:37, Valentin Schneider wrote:
>> On 19/11/25 10:31, Dave Hansen wrote:
>>> On 11/14/25 07:14, Valentin Schneider wrote:
>>>> +static bool flush_tlb_kernel_cond(int cpu, void *info)
>>>> +{
>>>> + return housekeeping_cpu(cpu, HK_TYPE_KERNEL_NOISE) ||
>>>> + per_cpu(kernel_cr3_loaded, cpu);
>>>> +}
>>>
>>> Is it OK that 'kernel_cr3_loaded' can be be stale? Since it's not part
>>> of the instruction that actually sets CR3, there's a window between when
>>> 'kernel_cr3_loaded' is set (or cleared) and CR3 is actually written.
>>>
>>> Is that OK?
>>>
>>> It seems like it could lead to both unnecessary IPIs being sent and for
>>> IPIs to be missed.
>>>
>>
>> So the pattern is
>>
>> SWITCH_TO_KERNEL_CR3
>> FLUSH
>> KERNEL_CR3_LOADED := 1
>>
>> KERNEL_CR3_LOADED := 0
>> SWITCH_TO_USER_CR3
>>
>>
>> The 0 -> 1 transition has a window between the unconditional flush and the
>> write to 1 where a remote flush IPI may be omitted. Given that the write is
>> immediately following the unconditional flush, that would really be just
>> two flushes racing with each other,
>
> Let me fix that for you. When you wrote "a remote flush IPI may be
> omitted" you meant to write: "there's a bug." ;)
>
Something like that :-)
> In the end, KERNEL_CR3_LOADED==0 means, "you don't need to send this CPU
> flushing IPIs because it will flush the TLB itself before touching
> memory that needs a flush".
>
> SWITCH_TO_KERNEL_CR3
> FLUSH
> // On kernel CR3, *AND* not getting IPIs
> KERNEL_CR3_LOADED := 1
>
>> but I could punt the kernel_cr3_loaded
>> write above the unconditional flush.
>
> Yes, that would eliminate the window, as long as the memory ordering is
> right. You not only need to have the KERNEL_CR3_LOADED:=1 CPU set that
> variable, you need to ensure that it has seen the page table update.
>
I assumed the page table update would be a self-synchronizing operation,
but that betrays how little I know about x86; /me goes back to reading
>> The 1 -> 0 transition is less problematic, worst case a remote flush races
>> with the CPU returning to userspace and it'll get interrupted back to
>> kernelspace.
>
> It's also not just "returning to userspace". It could well be *in*
> userspace by the point the IPI shows up. It's not the end of the world,
> and the window isn't infinitely long. But there certainly is still a
> possibility of getting spurious interrupts for the precious NOHZ_FULL
> task while it's in userspace.
IME it's okay if the application is just starting as it needs to do some
initialization anyway (mlockall & friends), i.e. it's not executing actual
useful payload from the get go.
If it's resuming from an interference, well we'd be making things worse.
I'm thinking the worst case is if this becomes a repeating pattern, but
then that means even without those deferral hacks the isolated CPUs would
be bombarded by IPIs in the first place.
^ permalink raw reply
* Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper
From: Wander Lairson Costa @ 2025-11-25 14:20 UTC (permalink / raw)
To: Crystal Wood
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, John Kacur,
Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:\b|_)bpf(?:\b|_)
In-Reply-To: <fb5b468b38ac9570a5f3fb948452d1b5b03c9f9c.camel@redhat.com>
On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> > +enum restart_result
> > +timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params)
> > +{
> > + actions_perform(¶ms->common.threshold_actions);
> > +
> > + if (!params->common.threshold_actions.continue_flag)
> > + /* continue flag not set, break */
> > + return RESTART_STOP;
> > +
> > + /* continue action reached, re-enable tracing */
> > + if (tool->record && trace_instance_start(&tool->record->trace))
> > + goto err;
> > + if (tool->aa && trace_instance_start(&tool->aa->trace))
> > + goto err;
> > + return RESTART_OK;
> > +
> > +err:
> > + err_msg("Error restarting trace\n");
> > + return RESTART_ERROR;
> > +}
>
> The non-BPF functions in common.c have the same logic and should also
> call this. This isn't timerlat-specific.
>
I will replace them here, thanks.
>
> > diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
> > index 09a3da3f58630..f14fc56c5b4a5 100644
> > --- a/tools/tracing/rtla/src/timerlat_hist.c
> > +++ b/tools/tracing/rtla/src/timerlat_hist.c
> > @@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct osnoise_tool *tool)
> >
> > if (!stop_tracing) {
> > /* Threshold overflow, perform actions on threshold */
> > - actions_perform(¶ms->common.threshold_actions);
> > + enum restart_result result;
> >
> > - if (!params->common.threshold_actions.continue_flag)
> > - /* continue flag not set, break */
> > + result = timerlat_restart(tool, params);
> > + if (result == RESTART_STOP)
> > break;
> >
> > - /* continue action reached, re-enable tracing */
> > - if (tool->record)
> > - trace_instance_start(&tool->record->trace);
> > - if (tool->aa)
> > - trace_instance_start(&tool->aa->trace);
> > - timerlat_bpf_restart_tracing();
> > + if (result == RESTART_ERROR)
> > + return -1;
>
> Does it matter that we're not detaching on an error here? Is this
> something that gets cleaned up automatically (and if so, why do we ever
> need to do it explicitly)?
>
On process exit, it does.
> > +
> > + if (timerlat_bpf_restart_tracing()) {
> > + err_msg("Error restarting BPF trace\n");
> > + return -1;
> > + }
>
> [insert rant about not being able to use exceptions in userspace code in
> the year 2025]
>
I actually find exceptions an anti-pattern. Modern languages like Zig,
Go and Rust came back to error returning.
> -Crystal
>
^ permalink raw reply
* [PATCH] fgraph: Make fgraph_no_sleep_time signed
From: Steven Rostedt @ 2025-11-25 15:47 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Dan Carpenter
From: Steven Rostedt <rostedt@goodmis.org>
The variable fgraph_no_sleep_time changed from being a boolean to being a
counter. A check is made to make sure that it never goes below zero. But
the variable being unsigned makes the check always fail even if it does go
below zero.
Make the variable a signed int so that checking it going below zero still
works.
Fixes: 5abb6ccb58f0 ("tracing: Have function graph tracer option sleep-time be per instance")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aR1yRQxDmlfLZzoo@stanley.mountain/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 2 +-
kernel/trace/trace_functions_graph.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 58be6d741d72..da5d9527ebd6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1113,7 +1113,7 @@ static inline void ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftra
#endif /* CONFIG_DYNAMIC_FTRACE */
extern unsigned int fgraph_max_depth;
-extern unsigned int fgraph_no_sleep_time;
+extern int fgraph_no_sleep_time;
extern bool fprofile_no_sleep_time;
static inline bool
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 44d5dc5031e2..d0513cfcd936 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -20,7 +20,7 @@
static int ftrace_graph_skip_irqs;
/* Do not record function time when task is sleeping */
-unsigned int fgraph_no_sleep_time;
+int fgraph_no_sleep_time;
struct fgraph_cpu_data {
pid_t last_pid;
--
2.51.0
^ permalink raw reply related
* Re: [RFC PATCH net-next] page_pool: Add page_pool_release_stalled tracepoint
From: Steven Rostedt @ 2025-11-25 16:23 UTC (permalink / raw)
To: Leon Hwang
Cc: netdev, hawk, ilias.apalodimas, mhiramat, mathieu.desnoyers,
davem, edumazet, kuba, pabeni, horms, kerneljasonxing, lance.yang,
jiayuan.chen, linux-kernel, linux-trace-kernel, Leon Huang Fu
In-Reply-To: <20251125082207.356075-1-leon.hwang@linux.dev>
On Tue, 25 Nov 2025 16:22:07 +0800
Leon Hwang <leon.hwang@linux.dev> wrote:
> +TRACE_EVENT(page_pool_release_stalled,
> +
> + TP_PROTO(const struct page_pool *pool, int inflight, int sec),
> +
> + TP_ARGS(pool, inflight, sec),
> +
> + TP_STRUCT__entry(
> + __field(const struct page_pool *, pool)
> + __field(int, inflight)
> + __field(int, sec)
> + ),
> +
> + TP_fast_assign(
> + __entry->pool = pool;
> + __entry->inflight = inflight;
> + __entry->sec = sec;
> + ),
> +
> + TP_printk("page_pool=%p id=%d inflight=%d sec=%d",
> + __entry->pool, __entry->pool->user.id, __entry->inflight, __entry->sec)
You can't do: __entry->pool->user.id
The TP_fast_assign() is executed when the tracepoint is triggered. The
TP_printk() is executed when the trace is read. That can happen seconds,
minutes, hours, days, even months after the pool was assigned.
That __entry->pool can very well be freed a long time ago.
If you need the id, you need to record it in the TP_fast_assign():
__entry->id = pool->user.id
and print that.
-- Steve
> +);
> +
^ permalink raw reply
* [PATCH v2] dma-buf: add some tracepoints to debug.
From: Xiang Gao @ 2025-11-25 16:29 UTC (permalink / raw)
To: sumit.semwal, christian.koenig, rostedt, mhiramat
Cc: linux-media, dri-devel, linaro-mm-sig, linux-kernel,
mathieu.desnoyers, dhowells, kuba, brauner, akpm,
linux-trace-kernel, gaoxiang17
From: gaoxiang17 <gaoxiang17@xiaomi.com>
I want to track the status of dmabuf in real time in the production environment.
But now we can only check it by traversing the fd in the process or dmabuf_list.
For example:
<...>-8342 [006] ..... 199.626433: dma_buf_export: exp_name=system name=(null) size=32768 ino=2337 f_refcnt=2
<...>-8342 [006] ..... 199.626436: dma_buf_fd: exp_name=system name=(null) size=32768 ino=2337 fd=853 f_refcnt=2
<...>-8342 [006] ..... 199.626472: dma_buf_mmap_internal: exp_name=system name=system size=32768 ino=2337 f_refcnt=6
<...>-3199 [006] ..... 200.719182: dma_buf_get: exp_name=qcom,system name=acb size=184320 ino=2331 fd=111 f_refcnt=6
<...>-894 [006] ..... 199.632342: dma_buf_put: exp_name=system name=system size=32768 ino=2337 f_refcnt=2
<...>-3199 [003] ..... 213.402200: dma_buf_attach: dev_name=soc:qcom,xxx exp_name=qcom,system name=acb size=184320 ino=2331 f_refcnt=7
<...>-1229 [004] ..... 213.850270: dma_buf_detach: exp_name=qcom,system name=acb size=184320 ino=2331 f_refcnt=6
Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
---
drivers/dma-buf/dma-buf.c | 19 +++
include/trace/events/dma_buf.h | 281 +++++++++++++++++++++++++++++++++
2 files changed, 300 insertions(+)
create mode 100644 include/trace/events/dma_buf.h
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 2bcf9ceca997..8b5af73f0218 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -35,6 +35,9 @@
#include "dma-buf-sysfs-stats.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/dma_buf.h>
+
static inline int is_dma_buf_file(struct file *);
static DEFINE_MUTEX(dmabuf_list_mutex);
@@ -220,6 +223,8 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
dmabuf->size >> PAGE_SHIFT)
return -EINVAL;
+ trace_dma_buf_mmap_internal(dmabuf);
+
return dmabuf->ops->mmap(dmabuf, vma);
}
@@ -745,6 +750,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
__dma_buf_list_add(dmabuf);
+ trace_dma_buf_export(dmabuf);
+
return dmabuf;
err_dmabuf:
@@ -779,6 +786,8 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
fd_install(fd, dmabuf->file);
+ trace_dma_buf_fd(dmabuf, fd);
+
return fd;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
@@ -805,6 +814,8 @@ struct dma_buf *dma_buf_get(int fd)
return ERR_PTR(-EINVAL);
}
+ trace_dma_buf_get(fd, file);
+
return file->private_data;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_get, "DMA_BUF");
@@ -825,6 +836,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
return;
fput(dmabuf->file);
+
+ trace_dma_buf_put(dmabuf);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
@@ -998,6 +1011,8 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_dynamic_attach, "DMA_BUF");
struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
struct device *dev)
{
+ trace_dma_buf_attach(dmabuf, dev);
+
return dma_buf_dynamic_attach(dmabuf, dev, NULL, NULL);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_attach, "DMA_BUF");
@@ -1024,6 +1039,8 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
dmabuf->ops->detach(dmabuf, attach);
kfree(attach);
+
+ trace_dma_buf_detach(dmabuf);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_detach, "DMA_BUF");
@@ -1488,6 +1505,8 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
vma_set_file(vma, dmabuf->file);
vma->vm_pgoff = pgoff;
+ trace_dma_buf_mmap(dmabuf);
+
return dmabuf->ops->mmap(dmabuf, vma);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, "DMA_BUF");
diff --git a/include/trace/events/dma_buf.h b/include/trace/events/dma_buf.h
new file mode 100644
index 000000000000..ab593dea4617
--- /dev/null
+++ b/include/trace/events/dma_buf.h
@@ -0,0 +1,281 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dma_buf
+
+#if !defined(_TRACE_DMA_BUF_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_DMA_BUF_H
+
+#include <linux/dma-buf.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(dma_buf_export,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_fd,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(int, fd)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->fd = fd;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu fd=%d f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->fd,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_mmap_internal,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_mmap,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_attach,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct device *dev),
+
+ TP_ARGS(dmabuf, dev),
+
+ TP_STRUCT__entry(
+ __string(dname, dev_name(dev))
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(dname);
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("dev_name=%s exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(dname),
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_detach,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_get,
+
+ TP_PROTO(int fd, struct file *file),
+
+ TP_ARGS(fd, file),
+
+ TP_STRUCT__entry(
+ __string(exp_name, ((struct dma_buf *)file->private_data)->exp_name)
+ __string(name, ((struct dma_buf *)file->private_data)->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(int, fd)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ struct dma_buf *dmabuf = (struct dma_buf *)file->private_data;
+
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->fd = fd;
+ __entry->f_refcnt = file_count(file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu fd=%d f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->fd,
+ __entry->f_refcnt)
+);
+
+TRACE_EVENT(dma_buf_put,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __string(name, dmabuf->name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(long, f_refcnt)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ spin_lock(&dmabuf->name_lock);
+ __assign_str(name);
+ spin_unlock(&dmabuf->name_lock);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->f_refcnt = file_count(dmabuf->file);
+ ),
+
+ TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
+ __get_str(exp_name),
+ __get_str(name),
+ __entry->size,
+ __entry->ino,
+ __entry->f_refcnt)
+);
+
+#endif /* _TRACE_DMA_BUF_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] unwind_user/fp: Use dummies instead of ifdef
From: Jens Remus @ 2025-11-25 16:43 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
Peter Zijlstra
Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
In-Reply-To: <20251125164349.2592874-1-jremus@linux.ibm.com>
This simplifies the code. unwind_user_next_fp() does not need to
return -EINVAL if config option HAVE_UNWIND_USER_FP is disabled, as
unwind_user_start() will then not select this unwind method and
unwind_user_next() will therefore not call it.
Note that enabling the config option HAVE_UNWIND_USER_FP without
defining ARCH_INIT_USER_FP_FRAME, ARCH_INIT_USER_FP_ENTRY_FRAME, and
unwind_user_at_function_start() will result in a compile error, which
is helpful when implementing support for unwind user fp in an
architecture.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
include/linux/unwind_user.h | 14 +++++++++++---
kernel/unwind/user.c | 4 ----
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 7f7282516bf5..c3ff690a43e2 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -5,9 +5,17 @@
#include <linux/unwind_user_types.h>
#include <asm/unwind_user.h>
-#ifndef ARCH_INIT_USER_FP_FRAME
- #define ARCH_INIT_USER_FP_FRAME
-#endif
+#ifndef CONFIG_HAVE_UNWIND_USER_FP
+
+#define ARCH_INIT_USER_FP_FRAME
+#define ARCH_INIT_USER_FP_ENTRY_FRAME
+
+static inline bool unwind_user_at_function_start(struct pt_regs *regs)
+{
+ return false;
+}
+
+#endif /* !CONFIG_HAVE_UNWIND_USER_FP */
int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 0ca434f86e73..90ab3c1a205e 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -67,7 +67,6 @@ static int unwind_user_next_common(struct unwind_user_state *state,
static int unwind_user_next_fp(struct unwind_user_state *state)
{
-#ifdef CONFIG_HAVE_UNWIND_USER_FP
struct pt_regs *regs = task_pt_regs(current);
if (state->topmost && unwind_user_at_function_start(regs)) {
@@ -81,9 +80,6 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
ARCH_INIT_USER_FP_FRAME(state->ws)
};
return unwind_user_next_common(state, &fp_frame);
-#else
- return -EINVAL;
-#endif
}
static int unwind_user_next(struct unwind_user_state *state)
--
2.51.0
^ permalink raw reply related
* [PATCH 1/3] unwind_user: Enhance comments on get CFA, FP, and RA
From: Jens Remus @ 2025-11-25 16:43 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
Peter Zijlstra
Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
In-Reply-To: <20251125164349.2592874-1-jremus@linux.ibm.com>
Move the comment "Get the Canonical Frame Address (CFA)" to the top
of the sequence of statements that actually get the CFA. Reword the
comment "Find the Return Address (RA)" to "Get ...", as the statements
actually get the RA. Add a respective comment to the statements that
get the FP. This will be useful once future commits extend the logic
to get the RA and FP.
While at it align the comment on the "stack going in wrong direction"
check to the following one on the "address is word aligned" check.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
kernel/unwind/user.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 39e270789444..0ca434f86e73 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -31,6 +31,7 @@ static int unwind_user_next_common(struct unwind_user_state *state,
{
unsigned long cfa, fp, ra;
+ /* Get the Canonical Frame Address (CFA) */
if (frame->use_fp) {
if (state->fp < state->sp)
return -EINVAL;
@@ -38,11 +39,9 @@ static int unwind_user_next_common(struct unwind_user_state *state,
} else {
cfa = state->sp;
}
-
- /* Get the Canonical Frame Address (CFA) */
cfa += frame->cfa_off;
- /* stack going in wrong direction? */
+ /* Make sure that stack is not going in wrong direction */
if (cfa <= state->sp)
return -EINVAL;
@@ -50,10 +49,11 @@ static int unwind_user_next_common(struct unwind_user_state *state,
if (cfa & (state->ws - 1))
return -EINVAL;
- /* Find the Return Address (RA) */
+ /* Get the Return Address (RA) */
if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
return -EINVAL;
+ /* Get the Frame Pointer (FP) */
if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
return -EINVAL;
--
2.51.0
^ permalink raw reply related
* [PATCH 0/3] unwind_user: Cleanups
From: Jens Remus @ 2025-11-25 16:43 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
Peter Zijlstra
Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
This patch series applies on top of Peter Zijlstras' latest unwind user
enhancements (and perf deferred callchain support) on his tip perf/core
branch:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core
Which has already been merged to tip/master and linux-next/master.
Patch 1 enhances a few comments in unwind_user_next_common().
Patch 2 gets rid of an ifdef in unwind_user_next_fp() by moving it to
linux/unwind_user.h.
Patch 3 ensures the x86 unwind_user_word_size() implementation is
available whenever config option UNWIND_USER is enabled, as it is
required by unwind user in general and is not specific to its FP
unwind method.
Regards,
Jens
Jens Remus (3):
unwind_user: Enhance comments on get CFA, FP, and RA
unwind_user/fp: Use dummies instead of ifdef
x86/unwind_user: Guard unwind_user_word_size() by UNWIND_USER
arch/x86/include/asm/unwind_user.h | 28 ++++++++++++++++------------
include/linux/unwind_user.h | 14 +++++++++++---
kernel/unwind/user.c | 12 ++++--------
3 files changed, 31 insertions(+), 23 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH 3/3] x86/unwind_user: Guard unwind_user_word_size() by UNWIND_USER
From: Jens Remus @ 2025-11-25 16:43 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
Peter Zijlstra
Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
In-Reply-To: <20251125164349.2592874-1-jremus@linux.ibm.com>
The unwind user framework in general requires an architecture-specific
implementation of unwind_user_word_size() to be present for any unwind
method, whether that is fp or a future other method, such as potentially
sframe.
Guard unwind_user_word_size() by the availability of the UNWIND_USER
framework instead of the specific HAVE_UNWIND_USER_FP method.
This facilitates to selectively disable HAVE_UNWIND_USER_FP on x86
(e.g. for test purposes) once a new unwind method is added to unwind
user.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
arch/x86/include/asm/unwind_user.h | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 12064284bc4e..77d776c7fca9 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -2,6 +2,22 @@
#ifndef _ASM_X86_UNWIND_USER_H
#define _ASM_X86_UNWIND_USER_H
+#ifdef CONFIG_UNWIND_USER
+
+static inline int unwind_user_word_size(struct pt_regs *regs)
+{
+ /* We can't unwind VM86 stacks */
+ if (regs->flags & X86_VM_MASK)
+ return 0;
+#ifdef CONFIG_X86_64
+ if (!user_64bit_mode(regs))
+ return sizeof(int);
+#endif
+ return sizeof(long);
+}
+
+#endif /* CONFIG_UNWIND_USER */
+
#ifdef CONFIG_HAVE_UNWIND_USER_FP
#include <asm/ptrace.h>
@@ -19,18 +35,6 @@
.fp_off = 0, \
.use_fp = false,
-static inline int unwind_user_word_size(struct pt_regs *regs)
-{
- /* We can't unwind VM86 stacks */
- if (regs->flags & X86_VM_MASK)
- return 0;
-#ifdef CONFIG_X86_64
- if (!user_64bit_mode(regs))
- return sizeof(int);
-#endif
- return sizeof(long);
-}
-
static inline bool unwind_user_at_function_start(struct pt_regs *regs)
{
return is_uprobe_at_func_entry(regs);
--
2.51.0
^ permalink raw reply related
* Re: [rtla 05/13] rtla: Simplify argument parsing
From: Crystal Wood @ 2025-11-25 16:53 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, John Kacur,
Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:\b|_)bpf(?:\b|_)
In-Reply-To: <oamfaffwyj6y3mtmhjxlk5u552xvdc4xd6is4dg2mxyh55ebe5@y6fsy73ig5ez>
On Tue, 2025-11-25 at 10:45 -0300, Wander Lairson Costa wrote:
> On Mon, Nov 24, 2025 at 06:46:33PM -0600, Crystal Wood wrote:
> > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> > > diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> > > index 160491f5de91c..f7ff548f7fba7 100644
> > > --- a/tools/tracing/rtla/src/utils.h
> > > +++ b/tools/tracing/rtla/src/utils.h
> > > @@ -13,8 +13,18 @@
> > > #define MAX_NICE 20
> > > #define MIN_NICE -19
> > >
> > > -#define container_of(ptr, type, member)({ \
> > > - const typeof(((type *)0)->member) *__mptr = (ptr); \
> > [snip]
> > > +#define container_of(ptr, type, member)({ \
> > > + const typeof(((type *)0)->member) *__mptr = (ptr); \
> > > (type *)((char *)__mptr - offsetof(type, member)) ; })
> >
> > It's easier to review patches when they don't make unrelated aesthetic
> > changes...
> >
>
> It is just git messing up the diff. No actual change.
The change was to the position of the backslashes. Not a big deal, just
something that's helpful to avoid to get a cleaner diff.
-Crystal
^ permalink raw reply
* [PATCH v2] ring-buffer: Add helper functions for allocations
From: Steven Rostedt @ 2025-11-25 17:11 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Linus Torvalds
From: Steven Rostedt <rostedt@goodmis.org>
The allocation of the per CPU buffer descriptor, the buffer page
descriptors and the buffer page data itself can be pretty ugly:
kzalloc_node(ALIGN(sizeof(struct buffer_page), cache_line_size()),
GFP_KERNEL, cpu_to_node(cpu));
And the data pages:
page = alloc_pages_node(cpu_to_node(cpu),
GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO, order);
if (!page)
return NULL;
bpage->page = page_address(page);
rb_init_page(bpage->page);
Add helper functions to make the code easier to read.
This does make all allocations of the data page (bpage->page) allocated
with the __GFP_RETRY_MAYFAIL flag (and not just the bulk allocator). Which
is actually better, as allocating the data page for the ring buffer tracing
should try hard but not trigger the OOM killer.
Link: https://lore.kernel.org/all/CAHk-=wjMMSAaqTjBSfYenfuzE1bMjLj+2DLtLWJuGt07UGCH_Q@mail.gmail.com/
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20251124140906.71a2abf6@gandalf.local.home
- Removed set but unused variables (kernel test robot)
kernel/trace/ring_buffer.c | 97 +++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 44 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1244d2c5c384..6295443b0944 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -401,6 +401,41 @@ static void free_buffer_page(struct buffer_page *bpage)
kfree(bpage);
}
+/*
+ * For best performance, allocate cpu buffer data cache line sized
+ * and per CPU.
+ */
+#define alloc_cpu_buffer(cpu) (struct ring_buffer_per_cpu *) \
+ kzalloc_node(ALIGN(sizeof(struct ring_buffer_per_cpu), \
+ cache_line_size()), GFP_KERNEL, cpu_to_node(cpu));
+
+#define alloc_cpu_page(cpu) (struct buffer_page *) \
+ kzalloc_node(ALIGN(sizeof(struct buffer_page), \
+ cache_line_size()), GFP_KERNEL, cpu_to_node(cpu));
+
+static struct buffer_data_page *alloc_cpu_data(int cpu, int order)
+{
+ struct buffer_data_page *dpage;
+ struct page *page;
+ gfp_t mflags;
+
+ /*
+ * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
+ * gracefully without invoking oom-killer and the system is not
+ * destabilized.
+ */
+ mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO;
+
+ page = alloc_pages_node(cpu_to_node(cpu), mflags, order);
+ if (!page)
+ return NULL;
+
+ dpage = page_address(page);
+ rb_init_page(dpage);
+
+ return dpage;
+}
+
/*
* We need to fit the time_stamp delta into 27 bits.
*/
@@ -2204,7 +2239,6 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
struct ring_buffer_cpu_meta *meta = NULL;
struct buffer_page *bpage, *tmp;
bool user_thread = current->mm != NULL;
- gfp_t mflags;
long i;
/*
@@ -2218,13 +2252,6 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
if (i < nr_pages)
return -ENOMEM;
- /*
- * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
- * gracefully without invoking oom-killer and the system is not
- * destabilized.
- */
- mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL;
-
/*
* If a user thread allocates too much, and si_mem_available()
* reports there's enough memory, even though there is not.
@@ -2241,10 +2268,8 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
meta = rb_range_meta(buffer, nr_pages, cpu_buffer->cpu);
for (i = 0; i < nr_pages; i++) {
- struct page *page;
- bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
- mflags, cpu_to_node(cpu_buffer->cpu));
+ bpage = alloc_cpu_page(cpu_buffer->cpu);
if (!bpage)
goto free_pages;
@@ -2267,13 +2292,10 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
bpage->range = 1;
bpage->id = i + 1;
} else {
- page = alloc_pages_node(cpu_to_node(cpu_buffer->cpu),
- mflags | __GFP_COMP | __GFP_ZERO,
- cpu_buffer->buffer->subbuf_order);
- if (!page)
+ int order = cpu_buffer->buffer->subbuf_order;
+ bpage->page = alloc_cpu_data(cpu_buffer->cpu, order);
+ if (!bpage->page)
goto free_pages;
- bpage->page = page_address(page);
- rb_init_page(bpage->page);
}
bpage->order = cpu_buffer->buffer->subbuf_order;
@@ -2324,14 +2346,12 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
static struct ring_buffer_per_cpu *
rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
{
- struct ring_buffer_per_cpu *cpu_buffer __free(kfree) = NULL;
+ struct ring_buffer_per_cpu *cpu_buffer __free(kfree) =
+ alloc_cpu_buffer(cpu);
struct ring_buffer_cpu_meta *meta;
struct buffer_page *bpage;
- struct page *page;
int ret;
- cpu_buffer = kzalloc_node(ALIGN(sizeof(*cpu_buffer), cache_line_size()),
- GFP_KERNEL, cpu_to_node(cpu));
if (!cpu_buffer)
return NULL;
@@ -2347,8 +2367,7 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
init_waitqueue_head(&cpu_buffer->irq_work.full_waiters);
mutex_init(&cpu_buffer->mapping_lock);
- bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
- GFP_KERNEL, cpu_to_node(cpu));
+ bpage = alloc_cpu_page(cpu);
if (!bpage)
return NULL;
@@ -2370,13 +2389,10 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
rb_meta_buffer_update(cpu_buffer, bpage);
bpage->range = 1;
} else {
- page = alloc_pages_node(cpu_to_node(cpu),
- GFP_KERNEL | __GFP_COMP | __GFP_ZERO,
- cpu_buffer->buffer->subbuf_order);
- if (!page)
+ int order = cpu_buffer->buffer->subbuf_order;
+ bpage->page = alloc_cpu_data(cpu, order);
+ if (!bpage->page)
goto fail_free_reader;
- bpage->page = page_address(page);
- rb_init_page(bpage->page);
}
INIT_LIST_HEAD(&cpu_buffer->reader_page->list);
@@ -6464,7 +6480,6 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
struct ring_buffer_per_cpu *cpu_buffer;
struct buffer_data_read_page *bpage = NULL;
unsigned long flags;
- struct page *page;
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return ERR_PTR(-ENODEV);
@@ -6486,22 +6501,16 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
arch_spin_unlock(&cpu_buffer->lock);
local_irq_restore(flags);
- if (bpage->data)
- goto out;
-
- page = alloc_pages_node(cpu_to_node(cpu),
- GFP_KERNEL | __GFP_NORETRY | __GFP_COMP | __GFP_ZERO,
- cpu_buffer->buffer->subbuf_order);
- if (!page) {
- kfree(bpage);
- return ERR_PTR(-ENOMEM);
+ if (bpage->data) {
+ rb_init_page(bpage->data);
+ } else {
+ bpage->data = alloc_cpu_data(cpu, cpu_buffer->buffer->subbuf_order);
+ if (!bpage->data) {
+ kfree(bpage);
+ return ERR_PTR(-ENOMEM);
+ }
}
- bpage->data = page_address(page);
-
- out:
- rb_init_page(bpage->data);
-
return bpage;
}
EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
--
2.51.0
^ permalink raw reply related
* Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper
From: Crystal Wood @ 2025-11-25 17:35 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, John Kacur,
Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:\b|_)bpf(?:\b|_)
In-Reply-To: <CAAq0SUn=eK+9YZZhdL_bs0S2cfVMhuuV-v8DSRMkTOqoL=SEWA@mail.gmail.com>
On Tue, 2025-11-25 at 11:20 -0300, Wander Lairson Costa wrote:
> On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
> >
> > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> > > +
> > > + if (timerlat_bpf_restart_tracing()) {
> > > + err_msg("Error restarting BPF trace\n");
> > > + return -1;
> > > + }
> >
> > [insert rant about not being able to use exceptions in userspace code in
> > the year 2025]
> >
>
> I actually find exceptions an anti-pattern. Modern languages like Zig,
> Go and Rust came back to error returning.
Maybe I'm behind the times, but I see exceptions and error returns as
complementary... not everything should be an exception and I can
certainly see how they could be overused in an anti-pattern way, but
they're nice for getting useful information out rather than "something
failed" without having to add a bunch of debug prints.
-Crystal
^ permalink raw reply
* Re: [PATCH linux-next] fgraph: Fix and tighten PID filtering support
From: Steven Rostedt @ 2025-11-25 18:05 UTC (permalink / raw)
To: wang.yaxin
Cc: mhiramat, mark.rutland, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, hu.shengming, hang.run, yang.yang29
In-Reply-To: <20251113135627439rGwzvG7JzdmnN8VMHAfMs@zte.com.cn>
On Thu, 13 Nov 2025 13:56:27 +0800 (CST)
<wang.yaxin@zte.com.cn> wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
>
> Function graph tracing did not honor set_ftrace_pid() rules properly.
>
> The root cause is that for fgraph_ops, the underlying ftrace_ops->private
> was left uninitialized. As a result, ftrace_pids_enabled(op) always
> returned false, effectively disabling PID filtering in the function graph
> tracer.
>
> PID filtering seemed to "work" only because graph_entry() performed an
> extra coarse-grained check via ftrace_trace_task(). Specifically,
> ftrace_ignore_pid is updated by ftrace_filter_pid_sched_switch_probe
> during sched_switch events. Under the original logic, when the intent
> is to trace only PID A, a context switch from task B to A sets
> ftrace_ignore_pid to A’s PID. However, there remains a window
> where B’s functions are still captured by the function-graph tracer.
> The following trace demonstrates this leakage
> (B = haveged-213, A = test.sh-385):
Thanks for the patch.
> Fix this by:
The below should really be three different patches.
> 1. Properly initializing gops->ops->private so that
> ftrace_pids_enabled() works as expected.
> 2. Removing the imprecise fallback check in graph_entry().
> 3. Updating register_ftrace_graph() to set gops->entryfunc =
> fgraph_pid_func whenever PID filtering is active, so the correct
> per-task filtering is enforced at entry time.
>
> With this change, function graph tracing will respect the configured
> PID filter list consistently, and the redundant coarse check is no
> longer needed.
>
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
> kernel/trace/fgraph.c | 9 +++++++--
> kernel/trace/trace.h | 9 ---------
> kernel/trace/trace_functions_graph.c | 3 ---
> 3 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 484ad7a18..00df3d4ac 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -1019,6 +1019,7 @@ void fgraph_init_ops(struct ftrace_ops *dst_ops,
> mutex_init(&dst_ops->local_hash.regex_lock);
> INIT_LIST_HEAD(&dst_ops->subop_list);
> dst_ops->flags |= FTRACE_OPS_FL_INITIALIZED;
> + dst_ops->private = src_ops->private;
> }
> #endif
> }
> @@ -1375,6 +1376,12 @@ int register_ftrace_graph(struct fgraph_ops *gops)
> gops->idx = i;
>
> ftrace_graph_active++;
Please keep a space here.
> + /* Always save the function, and reset at unregistering */
> + gops->saved_func = gops->entryfunc;
> +#ifdef CONFIG_DYNAMIC_FTRACE
> + if (ftrace_pids_enabled(&gops->ops))
> + gops->entryfunc = fgraph_pid_func;
> +#endif
Thanks,
-- Steve
>
> if (ftrace_graph_active == 2)
> ftrace_graph_disable_direct(true);
> @@ -1395,8 +1402,6 @@ int register_ftrace_graph(struct fgraph_ops *gops)
> } else {
> init_task_vars(gops->idx);
> }
> - /* Always save the function, and reset at unregistering */
> - gops->saved_func = gops->entryfunc;
>
> gops->ops.flags |= FTRACE_OPS_FL_GRAPH;
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index a3a15cfab..048a53282 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1162,11 +1162,6 @@ struct ftrace_func_command {
> char *params, int enable);
> };
> extern bool ftrace_filter_param __initdata;
> -static inline int ftrace_trace_task(struct trace_array *tr)
> -{
> - return this_cpu_read(tr->array_buffer.data->ftrace_ignore_pid) !=
> - FTRACE_PID_IGNORE;
> -}
> extern int ftrace_is_dead(void);
> int ftrace_create_function_files(struct trace_array *tr,
> struct dentry *parent);
> @@ -1184,10 +1179,6 @@ void ftrace_clear_pids(struct trace_array *tr);
> int init_function_trace(void);
> void ftrace_pid_follow_fork(struct trace_array *tr, bool enable);
> #else
> -static inline int ftrace_trace_task(struct trace_array *tr)
> -{
> - return 1;
> -}
> static inline int ftrace_is_dead(void) { return 0; }
> static inline int
> ftrace_create_function_files(struct trace_array *tr,
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index fe9607edc..0efe831e4 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -232,9 +232,6 @@ static int graph_entry(struct ftrace_graph_ent *trace,
> return 1;
> }
>
> - if (!ftrace_trace_task(tr))
> - return 0;
> -
> if (ftrace_graph_ignore_func(gops, trace))
> return 0;
>
^ permalink raw reply
* Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper
From: Wander Lairson Costa @ 2025-11-25 18:09 UTC (permalink / raw)
To: Crystal Wood
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, John Kacur,
Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:\b|_)bpf(?:\b|_)
In-Reply-To: <99c2109dd8c4b65be34f5ee00575a267da10b002.camel@redhat.com>
On Tue, Nov 25, 2025 at 2:36 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Tue, 2025-11-25 at 11:20 -0300, Wander Lairson Costa wrote:
> > On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
> > >
> > > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> >
> > > > +
> > > > + if (timerlat_bpf_restart_tracing()) {
> > > > + err_msg("Error restarting BPF trace\n");
> > > > + return -1;
> > > > + }
> > >
> > > [insert rant about not being able to use exceptions in userspace code in
> > > the year 2025]
> > >
> >
> > I actually find exceptions an anti-pattern. Modern languages like Zig,
> > Go and Rust came back to error returning.
>
> Maybe I'm behind the times, but I see exceptions and error returns as
> complementary... not everything should be an exception and I can
> certainly see how they could be overused in an anti-pattern way, but
> they're nice for getting useful information out rather than "something
> failed" without having to add a bunch of debug prints.
>
IIRC, you do get stack trace from the error returns. I think Golang
didn't/don't by
default. Exception as an alias for "panic()" is fine, IMO. What I've
seen in production
is services down due to unhandled exceptions. One of the reasons is
that there is nothing
in the function signature that says if or what of exceptions it raises.
> -Crystal
>
^ permalink raw reply
* Re: [PATCH v3 2/7] rtla/timerlat: Add --bpf-action option
From: Wander Lairson Costa @ 2025-11-25 18:10 UTC (permalink / raw)
To: Tomas Glozar
Cc: Steven Rostedt, LKML, Linux Trace Kernel, John Kacur,
Luis Goncalves, Costa Shulyupin, Crystal Wood,
Arnaldo Carvalho de Melo
In-Reply-To: <CAP4=nvS6zDkb-yo+uu0LHk=OBKP+UO-p46tLygGOUG0zkrkbiw@mail.gmail.com>
On Tue, Nov 25, 2025 at 10:49 AM Tomas Glozar <tglozar@redhat.com> wrote:
>
> po 3. 11. 2025 v 15:45 odesílatel Wander Lairson Costa
> <wander@redhat.com> napsal:
> > >
> > > Executing additional BPF code on latency threshold overflow allows doing
> > > doing low-latency and in-kernel troubleshooting of the cause of the
> >
> > typo: double "doing"
> >
>
> Thanks, I'll fix that :)
>
> > > --- a/tools/tracing/rtla/src/timerlat.c
> > > +++ b/tools/tracing/rtla/src/timerlat.c
> > > @@ -48,6 +48,17 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
> > > }
> > > }
> > >
> > > + /* Check if BPF action program is requested but BPF is not available */
> > > + if (params->bpf_action_program) {
> > > + if (params->mode == TRACING_MODE_TRACEFS) {
> > > + err_msg("BPF actions are not supported in tracefs-only mode\n");
> >
> > I would just emit a warning to the user and proceed ignoring the bpf action argument.
> >
>
> I believe if the user explicitly requests BPF actions to be used,
> measurement should not proceed without the action. Imagine someone
> setting --bpf-action in an automated test, expecting it to report
> something. But the action never fires, because they do not notice they
> are running an old kernel that does not support this.
>
> The user can always restart/reconfigure RTLA to skip the option.
>
I was thinking in a more interactive debug session where you go back and forth
with little changes in the command line. But you have a good point.
> Tomas
>
^ permalink raw reply
* Re: [PATCH v2 1/2] rv: Convert to use lock guard
From: Steven Rostedt @ 2025-11-25 19:57 UTC (permalink / raw)
To: Nam Cao
Cc: Gabriele Monaco, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <dbefeb868093c40d4b29fd6b57294a6aa011b719.1763370183.git.namcao@linutronix.de>
On Mon, 17 Nov 2025 09:06:02 +0000
Nam Cao <namcao@linutronix.de> wrote:
> @@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file *filp, const char __user *user
> else
> retval = rv_disable_monitor(mon);
>
> - if (!retval)
> - retval = count;
> -
> - break;
> + if (retval)
> + return retval;
> + return count;
No biggy, but I wonder if this would look better as:
return retval ? : count;
-- Steve
> }
>
> - mutex_unlock(&rv_interface_lock);
> return retval;
> }
>
^ permalink raw reply
* Re: [PATCH 2/2] tracing: Merge struct event_trigger_ops into struct event_command
From: Steven Rostedt @ 2025-11-25 20:05 UTC (permalink / raw)
To: Tom Zanussi
Cc: Steven Rostedt, linux-kernel, linux-trace-kernel,
Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <cfa03e4c70ff706b5373bf5314f453930019e1a1.camel@kernel.org>
On Tue, 25 Nov 2025 07:01:43 -0600
Tom Zanussi <zanussi@kernel.org> wrote:
> > -static const struct event_trigger_ops event_hist_trigger_ops = {
> > - .trigger = event_hist_trigger,
> > - .print = event_hist_trigger_print,
> > - .init = event_hist_trigger_init,
> > - .free = event_hist_trigger_free,
> > -};
> > +static struct event_command trigger_hist_cmd;
> >
>
> I don't think this is needed, since the same declaration already
> appears just above the event_hist_trigger_parse() declaration.
No it doesn't. Nice catch. I'll send a v2.
Thanks for the review.
-- Steve
^ permalink raw reply
* [PATCH v2 0/2] tracing: Clean up trigger code my merging structures
From: Steven Rostedt @ 2025-11-25 20:08 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
The struct event_command has a callback function called get_trigger_ops().
This callback returns the "trigger_ops" to use for the trigger. These ops
define the trigger function, how to init the trigger, how to print the
trigger and how to free it.
The only reason there's a callback function to get these ops is because
some triggers have two types of operations. One is an "always on"
operation, and the other is a "count down" operation. If a user passes in
a parameter to say how many times the trigger should execute. For example:
echo stacktrace:5 > events/kmem/kmem_cache_alloc/trigger
It will trigger the stacktrace for the first 5 times the kmem_cache_alloc
event is hit.
Instead of having two different trigger_ops since the only difference
between them is the tigger itself (the print, init and free functions are
all the same), just use a single ops that the event_command points to and
add a function field to the trigger_ops to have a count_func.
When a trigger is added to an event, if there's a count attached to it and
the trigger ops has the count_func field, the data allocated to represent
this trigger will have a new flag set called COUNT.
Then when the trigger executes, it will check if the COUNT data flag is
set, and if so, it will call the ops count_func(). If that returns false,
it returns without executing the trigger.
After making the struct event_trigger_ops be mapped one to one with the
struct event_command, merge the former into the latter.
Changes since v1: https://lore.kernel.org/linux-trace-kernel/20251119031042.328864818@kernel.org
- Removed duplicate declaration of trigger_hist_cmd (Tom Zanussi)
Steven Rostedt (2):
tracing: Remove get_trigger_ops() and add count_func() from trigger ops
tracing: Merge struct event_trigger_ops into struct event_command
----
kernel/trace/trace.h | 124 ++++++-------
kernel/trace/trace_eprobe.c | 19 +-
kernel/trace/trace_events_hist.c | 143 +++++----------
kernel/trace/trace_events_trigger.c | 344 +++++++++++++-----------------------
4 files changed, 230 insertions(+), 400 deletions(-)
^ permalink raw reply
* [PATCH v2 1/2] tracing: Remove get_trigger_ops() and add count_func() from trigger ops
From: Steven Rostedt @ 2025-11-25 20:08 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251125200857.975524485@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The struct event_command has a callback function called get_trigger_ops().
This callback returns the "trigger_ops" to use for the trigger. These ops
define the trigger function, how to init the trigger, how to print the
trigger and how to free it.
The only reason there's a callback function to get these ops is because
some triggers have two types of operations. One is an "always on"
operation, and the other is a "count down" operation. If a user passes in
a parameter to say how many times the trigger should execute. For example:
echo stacktrace:5 > events/kmem/kmem_cache_alloc/trigger
It will trigger the stacktrace for the first 5 times the kmem_cache_alloc
event is hit.
Instead of having two different trigger_ops since the only difference
between them is the tigger itself (the print, init and free functions are
all the same), just use a single ops that the event_command points to and
add a function field to the trigger_ops to have a count_func.
When a trigger is added to an event, if there's a count attached to it and
the trigger ops has the count_func field, the data allocated to represent
this trigger will have a new flag set called COUNT.
Then when the trigger executes, it will check if the COUNT data flag is
set, and if so, it will call the ops count_func(). If that returns false,
it returns without executing the trigger.
This removes the need for duplicate event_trigger_ops structures.
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 26 ++-
kernel/trace/trace_eprobe.c | 8 +-
kernel/trace/trace_events_hist.c | 60 +------
kernel/trace/trace_events_trigger.c | 257 ++++++++++------------------
4 files changed, 116 insertions(+), 235 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index da5d9527ebd6..b9c59d9f9a0c 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1791,6 +1791,7 @@ extern void clear_event_triggers(struct trace_array *tr);
enum {
EVENT_TRIGGER_FL_PROBE = BIT(0),
+ EVENT_TRIGGER_FL_COUNT = BIT(1),
};
struct event_trigger_data {
@@ -1822,6 +1823,10 @@ struct enable_trigger_data {
bool hist;
};
+bool event_trigger_count(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event);
+
extern int event_enable_trigger_print(struct seq_file *m,
struct event_trigger_data *data);
extern void event_enable_trigger_free(struct event_trigger_data *data);
@@ -1909,6 +1914,11 @@ extern void event_file_put(struct trace_event_file *file);
* registered the trigger (see struct event_command) along with
* the trace record, rec.
*
+ * @count_func: If defined and a numeric parameter is passed to the
+ * trigger, then this function will be called before @trigger
+ * is called. If this function returns false, then @trigger is not
+ * executed.
+ *
* @init: An optional initialization function called for the trigger
* when the trigger is registered (via the event_command reg()
* function). This can be used to perform per-trigger
@@ -1936,6 +1946,10 @@ struct event_trigger_ops {
struct trace_buffer *buffer,
void *rec,
struct ring_buffer_event *rbe);
+ bool (*count_func)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
int (*init)(struct event_trigger_data *data);
void (*free)(struct event_trigger_data *data);
int (*print)(struct seq_file *m,
@@ -1962,6 +1976,9 @@ struct event_trigger_ops {
* @name: The unique name that identifies the event command. This is
* the name used when setting triggers via trigger files.
*
+ * @trigger_ops: The event_trigger_ops implementation associated with
+ * the command.
+ *
* @trigger_type: A unique id that identifies the event command
* 'type'. This value has two purposes, the first to ensure that
* only one trigger of the same type can be set at a given time
@@ -2013,17 +2030,11 @@ struct event_trigger_ops {
* event command, filters set by the user for the command will be
* ignored. This is usually implemented by the generic utility
* function @set_trigger_filter() (see trace_event_triggers.c).
- *
- * @get_trigger_ops: The callback function invoked to retrieve the
- * event_trigger_ops implementation associated with the command.
- * This callback function allows a single event_command to
- * support multiple trigger implementations via different sets of
- * event_trigger_ops, depending on the value of the @param
- * string.
*/
struct event_command {
struct list_head list;
char *name;
+ const struct event_trigger_ops *trigger_ops;
enum event_trigger_type trigger_type;
int flags;
int (*parse)(struct event_command *cmd_ops,
@@ -2040,7 +2051,6 @@ struct event_command {
int (*set_filter)(char *filter_str,
struct event_trigger_data *data,
struct trace_event_file *file);
- const struct event_trigger_ops *(*get_trigger_ops)(char *cmd, char *param);
};
/**
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index a1d402124836..14ae896dbe75 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -513,21 +513,15 @@ static void eprobe_trigger_unreg_func(char *glob,
}
-static const struct event_trigger_ops *eprobe_trigger_get_ops(char *cmd,
- char *param)
-{
- return &eprobe_trigger_ops;
-}
-
static struct event_command event_trigger_cmd = {
.name = "eprobe",
.trigger_type = ETT_EVENT_EPROBE,
.flags = EVENT_CMD_FL_NEEDS_REC,
+ .trigger_ops = &eprobe_trigger_ops,
.parse = eprobe_trigger_cmd_parse,
.reg = eprobe_trigger_reg_func,
.unreg = eprobe_trigger_unreg_func,
.unreg_all = NULL,
- .get_trigger_ops = eprobe_trigger_get_ops,
.set_filter = NULL,
};
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 1d536219b624..f9cc8d6a215b 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6363,12 +6363,6 @@ static const struct event_trigger_ops event_hist_trigger_named_ops = {
.free = event_hist_trigger_named_free,
};
-static const struct event_trigger_ops *event_hist_get_trigger_ops(char *cmd,
- char *param)
-{
- return &event_hist_trigger_ops;
-}
-
static void hist_clear(struct event_trigger_data *data)
{
struct hist_trigger_data *hist_data = data->private_data;
@@ -6908,11 +6902,11 @@ static struct event_command trigger_hist_cmd = {
.name = "hist",
.trigger_type = ETT_EVENT_HIST,
.flags = EVENT_CMD_FL_NEEDS_REC,
+ .trigger_ops = &event_hist_trigger_ops,
.parse = event_hist_trigger_parse,
.reg = hist_register_trigger,
.unreg = hist_unregister_trigger,
.unreg_all = hist_unreg_all,
- .get_trigger_ops = event_hist_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -6945,29 +6939,9 @@ hist_enable_trigger(struct event_trigger_data *data,
}
}
-static void
-hist_enable_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- hist_enable_trigger(data, buffer, rec, event);
-}
-
static const struct event_trigger_ops hist_enable_trigger_ops = {
.trigger = hist_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops hist_enable_count_trigger_ops = {
- .trigger = hist_enable_count_trigger,
+ .count_func = event_trigger_count,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -6975,36 +6949,12 @@ static const struct event_trigger_ops hist_enable_count_trigger_ops = {
static const struct event_trigger_ops hist_disable_trigger_ops = {
.trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
};
-static const struct event_trigger_ops hist_disable_count_trigger_ops = {
- .trigger = hist_enable_count_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops *
-hist_enable_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
- bool enable;
-
- enable = (strcmp(cmd, ENABLE_HIST_STR) == 0);
-
- if (enable)
- ops = param ? &hist_enable_count_trigger_ops :
- &hist_enable_trigger_ops;
- else
- ops = param ? &hist_disable_count_trigger_ops :
- &hist_disable_trigger_ops;
-
- return ops;
-}
-
static void hist_enable_unreg_all(struct trace_event_file *file)
{
struct event_trigger_data *test, *n;
@@ -7023,22 +6973,22 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
static struct event_command trigger_hist_enable_cmd = {
.name = ENABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
+ .trigger_ops = &hist_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
- .get_trigger_ops = hist_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
static struct event_command trigger_hist_disable_cmd = {
.name = DISABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
+ .trigger_ops = &hist_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
- .get_trigger_ops = hist_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index cbfc306c0159..576bad18bcdb 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,20 @@ void trigger_data_free(struct event_trigger_data *data)
kfree(data);
}
+static inline void data_ops_trigger(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
+{
+ const struct event_trigger_ops *ops = data->ops;
+
+ if (data->flags & EVENT_TRIGGER_FL_COUNT) {
+ if (!ops->count_func(data, buffer, rec, event))
+ return;
+ }
+
+ ops->trigger(data, buffer, rec, event);
+}
+
/**
* event_triggers_call - Call triggers associated with a trace event
* @file: The trace_event_file associated with the event
@@ -70,7 +84,7 @@ event_triggers_call(struct trace_event_file *file,
if (data->paused)
continue;
if (!rec) {
- data->ops->trigger(data, buffer, rec, event);
+ data_ops_trigger(data, buffer, rec, event);
continue;
}
filter = rcu_dereference_sched(data->filter);
@@ -80,7 +94,7 @@ event_triggers_call(struct trace_event_file *file,
tt |= data->cmd_ops->trigger_type;
continue;
}
- data->ops->trigger(data, buffer, rec, event);
+ data_ops_trigger(data, buffer, rec, event);
}
return tt;
}
@@ -122,7 +136,7 @@ event_triggers_post_call(struct trace_event_file *file,
if (data->paused)
continue;
if (data->cmd_ops->trigger_type & tt)
- data->ops->trigger(data, NULL, NULL, NULL);
+ data_ops_trigger(data, NULL, NULL, NULL);
}
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
@@ -377,6 +391,36 @@ __init int unregister_event_command(struct event_command *cmd)
return -ENODEV;
}
+/**
+ * event_trigger_count - Optional count function for event triggers
+ * @data: Trigger-specific data
+ * @buffer: The ring buffer that the event is being written to
+ * @rec: The trace entry for the event, NULL for unconditional invocation
+ * @event: The event meta data in the ring buffer
+ *
+ * For triggers that can take a count parameter that doesn't do anything
+ * special, they can use this function to assign to their .count_func
+ * field.
+ *
+ * This simply does a count down of the @data->count field.
+ *
+ * If the @data->count is greater than zero, it will decrement it.
+ *
+ * Returns false if @data->count is zero, otherwise true.
+ */
+bool event_trigger_count(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
+{
+ if (!data->count)
+ return false;
+
+ if (data->count != -1)
+ (data->count)--;
+
+ return true;
+}
+
/**
* event_trigger_print - Generic event_trigger_ops @print implementation
* @name: The name of the event trigger
@@ -807,9 +851,13 @@ int event_trigger_separate_filter(char *param_and_filter, char **param,
* @private_data: User data to associate with the event trigger
*
* Allocate an event_trigger_data instance and initialize it. The
- * @cmd_ops are used along with the @cmd and @param to get the
- * trigger_ops to assign to the event_trigger_data. @private_data can
- * also be passed in and associated with the event_trigger_data.
+ * @cmd_ops defines how the trigger will operate. If @param is set,
+ * and @cmd_ops->trigger_ops->count_func is non NULL, then the
+ * data->count is set to @param and before the trigger is executed, the
+ * @cmd_ops->trigger_ops->count_func() is called. If that function returns
+ * false, the @cmd_ops->trigger_ops->trigger() function will not be called.
+ * @private_data can also be passed in and associated with the
+ * event_trigger_data.
*
* Use trigger_data_free() to free an event_trigger_data object.
*
@@ -821,18 +869,17 @@ struct event_trigger_data *trigger_data_alloc(struct event_command *cmd_ops,
void *private_data)
{
struct event_trigger_data *trigger_data;
- const struct event_trigger_ops *trigger_ops;
-
- trigger_ops = cmd_ops->get_trigger_ops(cmd, param);
trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
if (!trigger_data)
return NULL;
trigger_data->count = -1;
- trigger_data->ops = trigger_ops;
+ trigger_data->ops = cmd_ops->trigger_ops;
trigger_data->cmd_ops = cmd_ops;
trigger_data->private_data = private_data;
+ if (param && cmd_ops->trigger_ops->count_func)
+ trigger_data->flags |= EVENT_TRIGGER_FL_COUNT;
INIT_LIST_HEAD(&trigger_data->list);
INIT_LIST_HEAD(&trigger_data->named_list);
@@ -1271,31 +1318,28 @@ traceon_trigger(struct event_trigger_data *data,
tracing_on();
}
-static void
-traceon_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+traceon_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct trace_event_file *file = data->private_data;
if (file) {
if (tracer_tracing_is_on(file->tr))
- return;
+ return false;
} else {
if (tracing_is_on())
- return;
+ return false;
}
if (!data->count)
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- if (file)
- tracer_tracing_on(file->tr);
- else
- tracing_on();
+ return true;
}
static void
@@ -1319,31 +1363,28 @@ traceoff_trigger(struct event_trigger_data *data,
tracing_off();
}
-static void
-traceoff_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+traceoff_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct trace_event_file *file = data->private_data;
if (file) {
if (!tracer_tracing_is_on(file->tr))
- return;
+ return false;
} else {
if (!tracing_is_on())
- return;
+ return false;
}
if (!data->count)
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- if (file)
- tracer_tracing_off(file->tr);
- else
- tracing_off();
+ return true;
}
static int
@@ -1362,13 +1403,7 @@ traceoff_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops traceon_trigger_ops = {
.trigger = traceon_trigger,
- .print = traceon_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops traceon_count_trigger_ops = {
- .trigger = traceon_count_trigger,
+ .count_func = traceon_count_func,
.print = traceon_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
@@ -1376,41 +1411,19 @@ static const struct event_trigger_ops traceon_count_trigger_ops = {
static const struct event_trigger_ops traceoff_trigger_ops = {
.trigger = traceoff_trigger,
+ .count_func = traceoff_count_func,
.print = traceoff_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops traceoff_count_trigger_ops = {
- .trigger = traceoff_count_trigger,
- .print = traceoff_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-onoff_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
-
- /* we register both traceon and traceoff to this callback */
- if (strcmp(cmd, "traceon") == 0)
- ops = param ? &traceon_count_trigger_ops :
- &traceon_trigger_ops;
- else
- ops = param ? &traceoff_count_trigger_ops :
- &traceoff_trigger_ops;
-
- return ops;
-}
-
static struct event_command trigger_traceon_cmd = {
.name = "traceon",
.trigger_type = ETT_TRACE_ONOFF,
+ .trigger_ops = &traceon_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = onoff_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1418,10 +1431,10 @@ static struct event_command trigger_traceoff_cmd = {
.name = "traceoff",
.trigger_type = ETT_TRACE_ONOFF,
.flags = EVENT_CMD_FL_POST_TRIGGER,
+ .trigger_ops = &traceoff_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = onoff_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1439,20 +1452,6 @@ snapshot_trigger(struct event_trigger_data *data,
tracing_snapshot();
}
-static void
-snapshot_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- snapshot_trigger(data, buffer, rec, event);
-}
-
static int
register_snapshot_trigger(char *glob,
struct event_trigger_data *data,
@@ -1486,31 +1485,19 @@ snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops snapshot_trigger_ops = {
.trigger = snapshot_trigger,
+ .count_func = event_trigger_count,
.print = snapshot_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops snapshot_count_trigger_ops = {
- .trigger = snapshot_count_trigger,
- .print = snapshot_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-snapshot_get_trigger_ops(char *cmd, char *param)
-{
- return param ? &snapshot_count_trigger_ops : &snapshot_trigger_ops;
-}
-
static struct event_command trigger_snapshot_cmd = {
.name = "snapshot",
.trigger_type = ETT_SNAPSHOT,
+ .trigger_ops = &snapshot_trigger_ops,
.parse = event_trigger_parse,
.reg = register_snapshot_trigger,
.unreg = unregister_snapshot_trigger,
- .get_trigger_ops = snapshot_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1558,20 +1545,6 @@ stacktrace_trigger(struct event_trigger_data *data,
trace_dump_stack(STACK_SKIP);
}
-static void
-stacktrace_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- stacktrace_trigger(data, buffer, rec, event);
-}
-
static int
stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
{
@@ -1581,32 +1554,20 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops stacktrace_trigger_ops = {
.trigger = stacktrace_trigger,
+ .count_func = event_trigger_count,
.print = stacktrace_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops stacktrace_count_trigger_ops = {
- .trigger = stacktrace_count_trigger,
- .print = stacktrace_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-stacktrace_get_trigger_ops(char *cmd, char *param)
-{
- return param ? &stacktrace_count_trigger_ops : &stacktrace_trigger_ops;
-}
-
static struct event_command trigger_stacktrace_cmd = {
.name = "stacktrace",
.trigger_type = ETT_STACKTRACE,
+ .trigger_ops = &stacktrace_trigger_ops,
.flags = EVENT_CMD_FL_POST_TRIGGER,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = stacktrace_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1642,24 +1603,24 @@ event_enable_trigger(struct event_trigger_data *data,
set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &enable_data->file->flags);
}
-static void
-event_enable_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+event_enable_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct enable_trigger_data *enable_data = data->private_data;
if (!data->count)
- return;
+ return false;
/* Skip if the event is in a state we want to switch to */
if (enable_data->enable == !(enable_data->file->flags & EVENT_FILE_FL_SOFT_DISABLED))
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- event_enable_trigger(data, buffer, rec, event);
+ return true;
}
int event_enable_trigger_print(struct seq_file *m,
@@ -1706,13 +1667,7 @@ void event_enable_trigger_free(struct event_trigger_data *data)
static const struct event_trigger_ops event_enable_trigger_ops = {
.trigger = event_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_enable_count_trigger_ops = {
- .trigger = event_enable_count_trigger,
+ .count_func = event_enable_count_func,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -1720,13 +1675,7 @@ static const struct event_trigger_ops event_enable_count_trigger_ops = {
static const struct event_trigger_ops event_disable_trigger_ops = {
.trigger = event_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_disable_count_trigger_ops = {
- .trigger = event_enable_count_trigger,
+ .count_func = event_enable_count_func,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -1906,45 +1855,23 @@ void event_enable_unregister_trigger(char *glob,
data->ops->free(data);
}
-static const struct event_trigger_ops *
-event_enable_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
- bool enable;
-
-#ifdef CONFIG_HIST_TRIGGERS
- enable = ((strcmp(cmd, ENABLE_EVENT_STR) == 0) ||
- (strcmp(cmd, ENABLE_HIST_STR) == 0));
-#else
- enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
-#endif
- if (enable)
- ops = param ? &event_enable_count_trigger_ops :
- &event_enable_trigger_ops;
- else
- ops = param ? &event_disable_count_trigger_ops :
- &event_disable_trigger_ops;
-
- return ops;
-}
-
static struct event_command trigger_enable_cmd = {
.name = ENABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
+ .trigger_ops = &event_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
- .get_trigger_ops = event_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
static struct event_command trigger_disable_cmd = {
.name = DISABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
+ .trigger_ops = &event_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
- .get_trigger_ops = event_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/2] tracing: Merge struct event_trigger_ops into struct event_command
From: Steven Rostedt @ 2025-11-25 20:08 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251125200857.975524485@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Now that there's pretty much a one to one mapping between the struct
event_trigger_ops and struct event_command, there's no reason to have two
different structures. Merge the function pointers of event_trigger_ops
into event_command.
There's one exception in trace_events_hist.c for the
event_hist_trigger_named_ops. This has special logic for the init and free
function pointers for "named histograms". In this case, allocate the
cmd_ops of the event_trigger_data and set it to the proper init and free
functions, which are used to initialize and free the event_trigger_data
respectively. Have the free function and the init function (on failure)
free the cmd_ops of the data element.
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20251119031603.658997916@kernel.org
- Removed duplicate declaration of trigger_hist_cmd (Tom Zanussi)
kernel/trace/trace.h | 126 +++++++++++-----------------
kernel/trace/trace_eprobe.c | 13 +--
kernel/trace/trace_events_hist.c | 93 ++++++++++----------
kernel/trace/trace_events_trigger.c | 121 +++++++++++---------------
4 files changed, 151 insertions(+), 202 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b9c59d9f9a0c..901aad30099b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1798,7 +1798,6 @@ struct event_trigger_data {
unsigned long count;
int ref;
int flags;
- const struct event_trigger_ops *ops;
struct event_command *cmd_ops;
struct event_filter __rcu *filter;
char *filter_str;
@@ -1889,73 +1888,6 @@ extern void event_trigger_unregister(struct event_command *cmd_ops,
extern void event_file_get(struct trace_event_file *file);
extern void event_file_put(struct trace_event_file *file);
-/**
- * struct event_trigger_ops - callbacks for trace event triggers
- *
- * The methods in this structure provide per-event trigger hooks for
- * various trigger operations.
- *
- * The @init and @free methods are used during trigger setup and
- * teardown, typically called from an event_command's @parse()
- * function implementation.
- *
- * The @print method is used to print the trigger spec.
- *
- * The @trigger method is the function that actually implements the
- * trigger and is called in the context of the triggering event
- * whenever that event occurs.
- *
- * All the methods below, except for @init() and @free(), must be
- * implemented.
- *
- * @trigger: The trigger 'probe' function called when the triggering
- * event occurs. The data passed into this callback is the data
- * that was supplied to the event_command @reg() function that
- * registered the trigger (see struct event_command) along with
- * the trace record, rec.
- *
- * @count_func: If defined and a numeric parameter is passed to the
- * trigger, then this function will be called before @trigger
- * is called. If this function returns false, then @trigger is not
- * executed.
- *
- * @init: An optional initialization function called for the trigger
- * when the trigger is registered (via the event_command reg()
- * function). This can be used to perform per-trigger
- * initialization such as incrementing a per-trigger reference
- * count, for instance. This is usually implemented by the
- * generic utility function @event_trigger_init() (see
- * trace_event_triggers.c).
- *
- * @free: An optional de-initialization function called for the
- * trigger when the trigger is unregistered (via the
- * event_command @reg() function). This can be used to perform
- * per-trigger de-initialization such as decrementing a
- * per-trigger reference count and freeing corresponding trigger
- * data, for instance. This is usually implemented by the
- * generic utility function @event_trigger_free() (see
- * trace_event_triggers.c).
- *
- * @print: The callback function invoked to have the trigger print
- * itself. This is usually implemented by a wrapper function
- * that calls the generic utility function @event_trigger_print()
- * (see trace_event_triggers.c).
- */
-struct event_trigger_ops {
- void (*trigger)(struct event_trigger_data *data,
- struct trace_buffer *buffer,
- void *rec,
- struct ring_buffer_event *rbe);
- bool (*count_func)(struct event_trigger_data *data,
- struct trace_buffer *buffer,
- void *rec,
- struct ring_buffer_event *rbe);
- int (*init)(struct event_trigger_data *data);
- void (*free)(struct event_trigger_data *data);
- int (*print)(struct seq_file *m,
- struct event_trigger_data *data);
-};
-
/**
* struct event_command - callbacks and data members for event commands
*
@@ -1976,9 +1908,6 @@ struct event_trigger_ops {
* @name: The unique name that identifies the event command. This is
* the name used when setting triggers via trigger files.
*
- * @trigger_ops: The event_trigger_ops implementation associated with
- * the command.
- *
* @trigger_type: A unique id that identifies the event command
* 'type'. This value has two purposes, the first to ensure that
* only one trigger of the same type can be set at a given time
@@ -2008,7 +1937,7 @@ struct event_trigger_ops {
*
* @reg: Adds the trigger to the list of triggers associated with the
* event, and enables the event trigger itself, after
- * initializing it (via the event_trigger_ops @init() function).
+ * initializing it (via the event_command @init() function).
* This is also where commands can use the @trigger_type value to
* make the decision as to whether or not multiple instances of
* the trigger should be allowed. This is usually implemented by
@@ -2017,7 +1946,7 @@ struct event_trigger_ops {
*
* @unreg: Removes the trigger from the list of triggers associated
* with the event, and disables the event trigger itself, after
- * initializing it (via the event_trigger_ops @free() function).
+ * initializing it (via the event_command @free() function).
* This is usually implemented by the generic utility function
* @unregister_trigger() (see trace_event_triggers.c).
*
@@ -2030,11 +1959,46 @@ struct event_trigger_ops {
* event command, filters set by the user for the command will be
* ignored. This is usually implemented by the generic utility
* function @set_trigger_filter() (see trace_event_triggers.c).
+ *
+ * All the methods below, except for @init() and @free(), must be
+ * implemented.
+ *
+ * @trigger: The trigger 'probe' function called when the triggering
+ * event occurs. The data passed into this callback is the data
+ * that was supplied to the event_command @reg() function that
+ * registered the trigger (see struct event_command) along with
+ * the trace record, rec.
+ *
+ * @count_func: If defined and a numeric parameter is passed to the
+ * trigger, then this function will be called before @trigger
+ * is called. If this function returns false, then @trigger is not
+ * executed.
+ *
+ * @init: An optional initialization function called for the trigger
+ * when the trigger is registered (via the event_command reg()
+ * function). This can be used to perform per-trigger
+ * initialization such as incrementing a per-trigger reference
+ * count, for instance. This is usually implemented by the
+ * generic utility function @event_trigger_init() (see
+ * trace_event_triggers.c).
+ *
+ * @free: An optional de-initialization function called for the
+ * trigger when the trigger is unregistered (via the
+ * event_command @reg() function). This can be used to perform
+ * per-trigger de-initialization such as decrementing a
+ * per-trigger reference count and freeing corresponding trigger
+ * data, for instance. This is usually implemented by the
+ * generic utility function @event_trigger_free() (see
+ * trace_event_triggers.c).
+ *
+ * @print: The callback function invoked to have the trigger print
+ * itself. This is usually implemented by a wrapper function
+ * that calls the generic utility function @event_trigger_print()
+ * (see trace_event_triggers.c).
*/
struct event_command {
struct list_head list;
char *name;
- const struct event_trigger_ops *trigger_ops;
enum event_trigger_type trigger_type;
int flags;
int (*parse)(struct event_command *cmd_ops,
@@ -2051,6 +2015,18 @@ struct event_command {
int (*set_filter)(char *filter_str,
struct event_trigger_data *data,
struct trace_event_file *file);
+ void (*trigger)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
+ bool (*count_func)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
+ int (*init)(struct event_trigger_data *data);
+ void (*free)(struct event_trigger_data *data);
+ int (*print)(struct seq_file *m,
+ struct event_trigger_data *data);
};
/**
@@ -2071,7 +2047,7 @@ struct event_command {
* either committed or discarded. At that point, if any commands
* have deferred their triggers, those commands are finally
* invoked following the close of the current event. In other
- * words, if the event_trigger_ops @func() probe implementation
+ * words, if the event_command @func() probe implementation
* itself logs to the trace buffer, this flag should be set,
* otherwise it can be left unspecified.
*
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 14ae896dbe75..f3e0442c3b96 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -484,13 +484,6 @@ static void eprobe_trigger_func(struct event_trigger_data *data,
__eprobe_trace_func(edata, rec);
}
-static const struct event_trigger_ops eprobe_trigger_ops = {
- .trigger = eprobe_trigger_func,
- .print = eprobe_trigger_print,
- .init = eprobe_trigger_init,
- .free = eprobe_trigger_free,
-};
-
static int eprobe_trigger_cmd_parse(struct event_command *cmd_ops,
struct trace_event_file *file,
char *glob, char *cmd,
@@ -517,12 +510,15 @@ static struct event_command event_trigger_cmd = {
.name = "eprobe",
.trigger_type = ETT_EVENT_EPROBE,
.flags = EVENT_CMD_FL_NEEDS_REC,
- .trigger_ops = &eprobe_trigger_ops,
.parse = eprobe_trigger_cmd_parse,
.reg = eprobe_trigger_reg_func,
.unreg = eprobe_trigger_unreg_func,
.unreg_all = NULL,
.set_filter = NULL,
+ .trigger = eprobe_trigger_func,
+ .print = eprobe_trigger_print,
+ .init = eprobe_trigger_init,
+ .free = eprobe_trigger_free,
};
static struct event_trigger_data *
@@ -542,7 +538,6 @@ new_eprobe_trigger(struct trace_eprobe *ep, struct trace_event_file *file)
trigger->flags = EVENT_TRIGGER_FL_PROBE;
trigger->count = -1;
- trigger->ops = &eprobe_trigger_ops;
/*
* EVENT PROBE triggers are not registered as commands with
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f9cc8d6a215b..f0dafc1f2787 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5694,7 +5694,7 @@ static void hist_trigger_show(struct seq_file *m,
seq_puts(m, "\n\n");
seq_puts(m, "# event histogram\n#\n# trigger info: ");
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
seq_puts(m, "#\n\n");
hist_data = data->private_data;
@@ -6016,7 +6016,7 @@ static void hist_trigger_debug_show(struct seq_file *m,
seq_puts(m, "\n\n");
seq_puts(m, "# event histogram\n#\n# trigger info: ");
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
seq_puts(m, "#\n\n");
hist_data = data->private_data;
@@ -6326,20 +6326,21 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
free_hist_pad();
}
-static const struct event_trigger_ops event_hist_trigger_ops = {
- .trigger = event_hist_trigger,
- .print = event_hist_trigger_print,
- .init = event_hist_trigger_init,
- .free = event_hist_trigger_free,
-};
-
static int event_hist_trigger_named_init(struct event_trigger_data *data)
{
+ int ret;
+
data->ref++;
save_named_trigger(data->named_data->name, data);
- return event_hist_trigger_init(data->named_data);
+ ret = event_hist_trigger_init(data->named_data);
+ if (ret < 0) {
+ kfree(data->cmd_ops);
+ data->cmd_ops = &trigger_hist_cmd;
+ }
+
+ return ret;
}
static void event_hist_trigger_named_free(struct event_trigger_data *data)
@@ -6351,18 +6352,14 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
data->ref--;
if (!data->ref) {
+ struct event_command *cmd_ops = data->cmd_ops;
+
del_named_trigger(data);
trigger_data_free(data);
+ kfree(cmd_ops);
}
}
-static const struct event_trigger_ops event_hist_trigger_named_ops = {
- .trigger = event_hist_trigger,
- .print = event_hist_trigger_print,
- .init = event_hist_trigger_named_init,
- .free = event_hist_trigger_named_free,
-};
-
static void hist_clear(struct event_trigger_data *data)
{
struct hist_trigger_data *hist_data = data->private_data;
@@ -6556,13 +6553,24 @@ static int hist_register_trigger(char *glob,
data->paused = true;
if (named_data) {
+ struct event_command *cmd_ops;
+
data->private_data = named_data->private_data;
set_named_trigger_data(data, named_data);
- data->ops = &event_hist_trigger_named_ops;
+ /* Copy the command ops and update some of the functions */
+ cmd_ops = kmalloc(sizeof(*cmd_ops), GFP_KERNEL);
+ if (!cmd_ops) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ *cmd_ops = *data->cmd_ops;
+ cmd_ops->init = event_hist_trigger_named_init;
+ cmd_ops->free = event_hist_trigger_named_free;
+ data->cmd_ops = cmd_ops;
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
goto out;
}
@@ -6676,8 +6684,8 @@ static void hist_unregister_trigger(char *glob,
}
}
- if (test && test->ops->free)
- test->ops->free(test);
+ if (test && test->cmd_ops->free)
+ test->cmd_ops->free(test);
if (hist_data->enable_timestamps) {
if (!hist_data->remove || test)
@@ -6729,8 +6737,8 @@ static void hist_unreg_all(struct trace_event_file *file)
update_cond_flag(file);
if (hist_data->enable_timestamps)
tracing_set_filter_buffering(file->tr, false);
- if (test->ops->free)
- test->ops->free(test);
+ if (test->cmd_ops->free)
+ test->cmd_ops->free(test);
}
}
}
@@ -6902,12 +6910,15 @@ static struct event_command trigger_hist_cmd = {
.name = "hist",
.trigger_type = ETT_EVENT_HIST,
.flags = EVENT_CMD_FL_NEEDS_REC,
- .trigger_ops = &event_hist_trigger_ops,
.parse = event_hist_trigger_parse,
.reg = hist_register_trigger,
.unreg = hist_unregister_trigger,
.unreg_all = hist_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = event_hist_trigger,
+ .print = event_hist_trigger_print,
+ .init = event_hist_trigger_init,
+ .free = event_hist_trigger_free,
};
__init int register_trigger_hist_cmd(void)
@@ -6939,22 +6950,6 @@ hist_enable_trigger(struct event_trigger_data *data,
}
}
-static const struct event_trigger_ops hist_enable_trigger_ops = {
- .trigger = hist_enable_trigger,
- .count_func = event_trigger_count,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops hist_disable_trigger_ops = {
- .trigger = hist_enable_trigger,
- .count_func = event_trigger_count,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
static void hist_enable_unreg_all(struct trace_event_file *file)
{
struct event_trigger_data *test, *n;
@@ -6964,8 +6959,8 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
list_del_rcu(&test->list);
update_cond_flag(file);
trace_event_trigger_enable_disable(file, 0);
- if (test->ops->free)
- test->ops->free(test);
+ if (test->cmd_ops->free)
+ test->cmd_ops->free(test);
}
}
}
@@ -6973,23 +6968,31 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
static struct event_command trigger_hist_enable_cmd = {
.name = ENABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
- .trigger_ops = &hist_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static struct event_command trigger_hist_disable_cmd = {
.name = DISABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
- .trigger_ops = &hist_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static __init void unregister_trigger_hist_enable_disable_cmds(void)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 576bad18bcdb..7795af600466 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -32,14 +32,14 @@ static inline void data_ops_trigger(struct event_trigger_data *data,
struct trace_buffer *buffer, void *rec,
struct ring_buffer_event *event)
{
- const struct event_trigger_ops *ops = data->ops;
+ const struct event_command *cmd_ops = data->cmd_ops;
if (data->flags & EVENT_TRIGGER_FL_COUNT) {
- if (!ops->count_func(data, buffer, rec, event))
+ if (!cmd_ops->count_func(data, buffer, rec, event))
return;
}
- ops->trigger(data, buffer, rec, event);
+ cmd_ops->trigger(data, buffer, rec, event);
}
/**
@@ -205,7 +205,7 @@ static int trigger_show(struct seq_file *m, void *v)
}
data = list_entry(v, struct event_trigger_data, list);
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
return 0;
}
@@ -422,7 +422,7 @@ bool event_trigger_count(struct event_trigger_data *data,
}
/**
- * event_trigger_print - Generic event_trigger_ops @print implementation
+ * event_trigger_print - Generic event_command @print implementation
* @name: The name of the event trigger
* @m: The seq_file being printed to
* @data: Trigger-specific data
@@ -457,7 +457,7 @@ event_trigger_print(const char *name, struct seq_file *m,
}
/**
- * event_trigger_init - Generic event_trigger_ops @init implementation
+ * event_trigger_init - Generic event_command @init implementation
* @data: Trigger-specific data
*
* Common implementation of event trigger initialization.
@@ -474,7 +474,7 @@ int event_trigger_init(struct event_trigger_data *data)
}
/**
- * event_trigger_free - Generic event_trigger_ops @free implementation
+ * event_trigger_free - Generic event_command @free implementation
* @data: Trigger-specific data
*
* Common implementation of event trigger de-initialization.
@@ -536,8 +536,8 @@ clear_event_triggers(struct trace_array *tr)
list_for_each_entry_safe(data, n, &file->triggers, list) {
trace_event_trigger_enable_disable(file, 0);
list_del_rcu(&data->list);
- if (data->ops->free)
- data->ops->free(data);
+ if (data->cmd_ops->free)
+ data->cmd_ops->free(data);
}
}
}
@@ -600,8 +600,8 @@ static int register_trigger(char *glob,
return -EEXIST;
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
return ret;
}
@@ -639,8 +639,8 @@ static bool try_unregister_trigger(char *glob,
}
if (data) {
- if (data->ops->free)
- data->ops->free(data);
+ if (data->cmd_ops->free)
+ data->cmd_ops->free(data);
return true;
}
@@ -875,10 +875,9 @@ struct event_trigger_data *trigger_data_alloc(struct event_command *cmd_ops,
return NULL;
trigger_data->count = -1;
- trigger_data->ops = cmd_ops->trigger_ops;
trigger_data->cmd_ops = cmd_ops;
trigger_data->private_data = private_data;
- if (param && cmd_ops->trigger_ops->count_func)
+ if (param && cmd_ops->count_func)
trigger_data->flags |= EVENT_TRIGGER_FL_COUNT;
INIT_LIST_HEAD(&trigger_data->list);
@@ -1401,41 +1400,33 @@ traceoff_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops traceon_trigger_ops = {
- .trigger = traceon_trigger,
- .count_func = traceon_count_func,
- .print = traceon_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops traceoff_trigger_ops = {
- .trigger = traceoff_trigger,
- .count_func = traceoff_count_func,
- .print = traceoff_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_traceon_cmd = {
.name = "traceon",
.trigger_type = ETT_TRACE_ONOFF,
- .trigger_ops = &traceon_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = traceon_trigger,
+ .count_func = traceon_count_func,
+ .print = traceon_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static struct event_command trigger_traceoff_cmd = {
.name = "traceoff",
.trigger_type = ETT_TRACE_ONOFF,
.flags = EVENT_CMD_FL_POST_TRIGGER,
- .trigger_ops = &traceoff_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = traceoff_trigger,
+ .count_func = traceoff_count_func,
+ .print = traceoff_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
#ifdef CONFIG_TRACER_SNAPSHOT
@@ -1483,22 +1474,18 @@ snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops snapshot_trigger_ops = {
- .trigger = snapshot_trigger,
- .count_func = event_trigger_count,
- .print = snapshot_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_snapshot_cmd = {
.name = "snapshot",
.trigger_type = ETT_SNAPSHOT,
- .trigger_ops = &snapshot_trigger_ops,
.parse = event_trigger_parse,
.reg = register_snapshot_trigger,
.unreg = unregister_snapshot_trigger,
.set_filter = set_trigger_filter,
+ .trigger = snapshot_trigger,
+ .count_func = event_trigger_count,
+ .print = snapshot_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static __init int register_trigger_snapshot_cmd(void)
@@ -1552,23 +1539,19 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops stacktrace_trigger_ops = {
- .trigger = stacktrace_trigger,
- .count_func = event_trigger_count,
- .print = stacktrace_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_stacktrace_cmd = {
.name = "stacktrace",
.trigger_type = ETT_STACKTRACE,
- .trigger_ops = &stacktrace_trigger_ops,
.flags = EVENT_CMD_FL_POST_TRIGGER,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = stacktrace_trigger,
+ .count_func = event_trigger_count,
+ .print = stacktrace_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static __init int register_trigger_stacktrace_cmd(void)
@@ -1665,22 +1648,6 @@ void event_enable_trigger_free(struct event_trigger_data *data)
}
}
-static const struct event_trigger_ops event_enable_trigger_ops = {
- .trigger = event_enable_trigger,
- .count_func = event_enable_count_func,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_disable_trigger_ops = {
- .trigger = event_enable_trigger,
- .count_func = event_enable_count_func,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
int event_enable_trigger_parse(struct event_command *cmd_ops,
struct trace_event_file *file,
char *glob, char *cmd, char *param_and_filter)
@@ -1810,8 +1777,8 @@ int event_enable_register_trigger(char *glob,
}
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
return ret;
}
@@ -1851,28 +1818,36 @@ void event_enable_unregister_trigger(char *glob,
}
}
- if (data && data->ops->free)
- data->ops->free(data);
+ if (data && data->cmd_ops->free)
+ data->cmd_ops->free(data);
}
static struct event_command trigger_enable_cmd = {
.name = ENABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
- .trigger_ops = &event_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = event_enable_trigger,
+ .count_func = event_enable_count_func,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static struct event_command trigger_disable_cmd = {
.name = DISABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
- .trigger_ops = &event_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = event_enable_trigger,
+ .count_func = event_enable_count_func,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static __init void unregister_trigger_enable_disable_cmds(void)
--
2.51.0
^ permalink raw reply related
* [PATCH v2 0/3] tracing: More clean ups of triggers
From: Steven Rostedt @ 2025-11-25 21:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
This is based on top of:
https://lore.kernel.org/linux-trace-kernel/20251125200857.975524485@kernel.org
Changes since v1: https://lore.kernel.org/linux-trace-kernel/20251120205600.570673392@kernel.org/
- 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(-)
^ permalink raw reply
* [PATCH v2 1/3] tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()
From: Steven Rostedt @ 2025-11-25 21:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251125214004.396482401@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
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.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 7795af600466..e5dcfcbb2cd5 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -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;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/3] tracing: Add bulk garbage collection of freeing event_trigger_data
From: Steven Rostedt @ 2025-11-25 21:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251125214004.396482401@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
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).
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20251120205710.151041470@kernel.org
- 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(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 5863800b1ab3..911fc75dc6c4 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -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 */
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index e5dcfcbb2cd5..3b97c242b795 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -6,6 +6,7 @@
*/
#include <linux/security.h>
+#include <linux/kthread.h>
#include <linux/module.h>
#include <linux/ctype.h>
#include <linux/mutex.h>
@@ -17,15 +18,63 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+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);
- /* 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;
+ }
- 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,
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/3] tracing: Use strim() in trigger_process_regex() instead of skip_spaces()
From: Steven Rostedt @ 2025-11-25 21:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251125214004.396482401@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
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.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 3b97c242b795..96aad82b1628 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -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);
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).