* [PATCH v12] dma-buf: add some tracepoints to debug.
From: Xiang Gao @ 2025-12-29 3:15 UTC (permalink / raw)
To: sumit.semwal, christian.koenig, rostedt, mhiramat
Cc: linux-media, dri-devel, linux-kernel, mathieu.desnoyers, dhowells,
kuba, brauner, akpm, linux-trace-kernel, gaoxiang17
From: gaoxiang17 <gaoxiang17@xiaomi.com>
Since we can only inspect dmabuf by iterating over process FDs or the
dmabuf_list, we need to add our own tracepoints to track its status in
real time in production.
For example:
binder:3016_1-3102 [006] ...1. 255.126521: dma_buf_export: exp_name=qcom,system size=12685312 ino=2738
binder:3016_1-3102 [006] ...1. 255.126528: dma_buf_fd: exp_name=qcom,system size=12685312 ino=2738 fd=8
binder:3016_1-3102 [006] ...1. 255.126642: dma_buf_mmap_internal: exp_name=qcom,system size=28672 ino=2739
kworker/6:1-86 [006] ...1. 255.127194: dma_buf_put: exp_name=qcom,system size=12685312 ino=2738
RenderThread-9293 [006] ...1. 316.618179: dma_buf_get: exp_name=qcom,system size=12771328 ino=2762 fd=176
RenderThread-9293 [006] ...1. 316.618195: dma_buf_dynamic_attach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
RenderThread-9293 [006] ...1. 318.878220: dma_buf_detach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
---
Changes since v11: https://lore.kernel.org/all/20251225121119.2194228-1-gxxa03070307@gmail.com/
- Lined up the backslashes nicely for the macro DMA_BUF_TRACE.
- Used DEFINE_EVENT_CONDITION() to move the condition branch into the tracepoint.
Changes since v10: https://lore.kernel.org/all/20251224013455.1649879-1-gxxa03070307@gmail.com/
- Always expose dmabuf->name_lock if lockdep is enabled.
Changes since v9: https://lore.kernel.org/all/20251223032749.1371913-1-gxxa03070307@gmail.com/
- Fixed some whitespace issues with the macro DMA_BUF_TRACE again.
Changes since v8: https://lore.kernel.org/all/20251218062853.819744-1-gxxa03070307@gmail.com/
- Expose dmabuf->name_lock when lockdep is enabled but trace event is not.
Changes since v7: https://lore.kernel.org/all/20251217105132.643300-1-gxxa03070307@gmail.com/
- Fixed some whitespace issues with the macro DMA_BUF_TRACE.
Changes since v6: https://lore.kernel.org/all/20251216063952.516364-1-gxxa03070307@gmail.com/
- Add a comment for the macro DMA_BUF_TRACE.
Changes since v5: https://lore.kernel.org/all/20251201112148.843572-1-gxxa03070307@gmail.com/
- Add the macro DMA_BUF_TRACE.
- Modify the commit message.
Changes since v4: https://lore.kernel.org/all/20251128085215.550100-1-gxxa03070307@gmail.com/
- Remove the tracepoints exporting.
- Remove the file refcount for the trace.
- Print the dev_name last.
- Add the parameter of whether dma_buf attachment is dynamic.
Changes since v3: https://lore.kernel.org/all/20251127004352.376307-1-gxxa03070307@gmail.com/
- Take the dmabuf->name_lock around the tracepoint call itself.
- Used DECLARE_EVENT_CLASS() and a DEFINE_EVENT().
Changes since v2: https://lore.kernel.org/all/20251125162949.220488-1-gxxa03070307@gmail.com/
- Add more explanation to the commit message.
drivers/dma-buf/dma-buf.c | 48 +++++++++-
include/trace/events/dma_buf.h | 159 +++++++++++++++++++++++++++++++++
2 files changed, 205 insertions(+), 2 deletions(-)
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 edaa9e4ee4ae..dee59d4c0b12 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -35,6 +35,26 @@
#include "dma-buf-sysfs-stats.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/dma_buf.h>
+
+/*
+ * dmabuf->name must be accessed with holding dmabuf->name_lock.
+ * we need to take the lock around the tracepoint call itself where
+ * it is called in the code.
+ *
+ * Note: FUNC##_enabled() is a static branch that will only
+ * be set when the trace event is enabled.
+ */
+#define DMA_BUF_TRACE(FUNC, ...) \
+ do { \
+ /* Always expose lock if lockdep is enabled */ \
+ if (IS_ENABLED(CONFIG_LOCKDEP) || FUNC##_enabled()) { \
+ guard(spinlock)(&dmabuf->name_lock); \
+ FUNC(__VA_ARGS__); \
+ } \
+ } while (0)
+
static inline int is_dma_buf_file(struct file *);
static DEFINE_MUTEX(dmabuf_list_mutex);
@@ -220,6 +240,8 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
dmabuf->size >> PAGE_SHIFT)
return -EINVAL;
+ DMA_BUF_TRACE(trace_dma_buf_mmap_internal, dmabuf);
+
return dmabuf->ops->mmap(dmabuf, vma);
}
@@ -745,6 +767,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
__dma_buf_list_add(dmabuf);
+ DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
+
return dmabuf;
err_dmabuf:
@@ -768,10 +792,15 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_export, "DMA_BUF");
*/
int dma_buf_fd(struct dma_buf *dmabuf, int flags)
{
+ int fd;
+
if (!dmabuf || !dmabuf->file)
return -EINVAL;
- return FD_ADD(flags, dmabuf->file);
+ fd = FD_ADD(flags, dmabuf->file);
+ DMA_BUF_TRACE(trace_dma_buf_fd, dmabuf, fd);
+
+ return fd;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
@@ -786,6 +815,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
struct dma_buf *dma_buf_get(int fd)
{
struct file *file;
+ struct dma_buf *dmabuf;
file = fget(fd);
@@ -797,7 +827,11 @@ struct dma_buf *dma_buf_get(int fd)
return ERR_PTR(-EINVAL);
}
- return file->private_data;
+ dmabuf = file->private_data;
+
+ DMA_BUF_TRACE(trace_dma_buf_get, dmabuf, fd);
+
+ return dmabuf;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_get, "DMA_BUF");
@@ -817,6 +851,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
return;
fput(dmabuf->file);
+
+ DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
@@ -971,6 +1007,9 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
list_add(&attach->node, &dmabuf->attachments);
dma_resv_unlock(dmabuf->resv);
+ DMA_BUF_TRACE(trace_dma_buf_dynamic_attach, dmabuf, attach,
+ dma_buf_attachment_is_dynamic(attach), dev);
+
return attach;
err_attach:
@@ -1015,6 +1054,9 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
if (dmabuf->ops->detach)
dmabuf->ops->detach(dmabuf, attach);
+ DMA_BUF_TRACE(trace_dma_buf_detach, dmabuf, attach,
+ dma_buf_attachment_is_dynamic(attach), attach->dev);
+
kfree(attach);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_detach, "DMA_BUF");
@@ -1480,6 +1522,8 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
vma_set_file(vma, dmabuf->file);
vma->vm_pgoff = pgoff;
+ DMA_BUF_TRACE(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..3bb88d05bcc8
--- /dev/null
+++ b/include/trace/events/dma_buf.h
@@ -0,0 +1,159 @@
+/* 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>
+
+DECLARE_EVENT_CLASS(dma_buf,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string( exp_name, dmabuf->exp_name)
+ __field( size_t, size)
+ __field( ino_t, ino)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino)
+);
+
+DECLARE_EVENT_CLASS(dma_buf_attach_dev,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev),
+
+ TP_STRUCT__entry(
+ __string( dev_name, dev_name(dev))
+ __string( exp_name, dmabuf->exp_name)
+ __field( size_t, size)
+ __field( ino_t, ino)
+ __field( struct dma_buf_attachment *, attach)
+ __field( bool, is_dynamic)
+ ),
+
+ TP_fast_assign(
+ __assign_str(dev_name);
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->is_dynamic = is_dynamic;
+ __entry->attach = attach;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu attachment:%p is_dynamic=%d dev_name=%s",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino,
+ __entry->attach,
+ __entry->is_dynamic,
+ __get_str(dev_name))
+);
+
+DECLARE_EVENT_CLASS(dma_buf_fd,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd),
+
+ TP_STRUCT__entry(
+ __string( exp_name, dmabuf->exp_name)
+ __field( size_t, size)
+ __field( ino_t, ino)
+ __field( int, fd)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->fd = fd;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu fd=%d",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino,
+ __entry->fd)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_export,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_mmap_internal,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_mmap,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_put,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf_attach_dev, dma_buf_dynamic_attach,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev)
+);
+
+DEFINE_EVENT(dma_buf_attach_dev, dma_buf_detach,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev)
+);
+
+DEFINE_EVENT_CONDITION(dma_buf_fd, dma_buf_fd,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd),
+
+ TP_CONDITION(fd >= 0)
+);
+
+DEFINE_EVENT(dma_buf_fd, dma_buf_get,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd)
+);
+
+#endif /* _TRACE_DMA_BUF_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 12/13] rv: Add deadline monitors
From: Gabriele Monaco @ 2025-12-29 7:00 UTC (permalink / raw)
To: Nam Cao, linux-kernel, Steven Rostedt, Masami Hiramatsu,
linux-trace-kernel
Cc: Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
In-Reply-To: <87y0mr9vaw.fsf@yellow.woof>
On Thu, 2025-12-25 at 08:58 +0700, Nam Cao wrote:
> Nam Cao <namcao@linutronix.de> writes:
> > There are some helper functions that I am a bit uncomfortable with
> > (e.g. pi_of, is_dl_boosted, dl_is_implicit, ...) because they are
> > relying on implementation details of deadline scheduler which can be
> > changed. So ideally this patch should have an Ack from the scheduler people.
Thanks for the review!
I'm going to ping scheduler guys some time after the holidays.
> Think about this again, perhaps we should move all these helpers to
> include/linux/sched/deadline.h instead? The scheduler people must be
> aware of these functions, and maintain them. I don't want a situation
> where the scheduler people make changes, and something else (which they
> do not know exist) breaks.
That's a fair point, I just copied those functions from kernel/sched/deadline.c
but that's dirty indeed.
Going to try exporting them to deadline.h and see if no one complains.
> Ideally, non-scheduler code should not look at private fields of
> scheduler's structs.
Yeah, one may argue those modules shouldn't be doing what they're doing with the
deadline entities. To be fair those modules are quite dependant on the data
structure implementation, but the alternative would be to maintain a set of
getter functions which might as well break.
Or at least I don't see a better way.
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH v2 2/2] mm: vmscan: add PIDs to vmscan tracepoints
From: Thomas Ballasi @ 2025-12-29 10:54 UTC (permalink / raw)
To: rostedt; +Cc: akpm, linux-mm, linux-trace-kernel, mhiramat, tballasi
In-Reply-To: <20251216130302.5202ca81@gandalf.local.home>
On Tue, Dec 16, 2025 at 01:03:02PM -0500, Steven Rostedt wrote:
> On Tue, 16 Dec 2025 06:02:52 -0800
> Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
>
> > The changes aims at adding additionnal tracepoints variables to help
> > debuggers attribute them to specific processes.
> >
> > The PID field uses in_task() to reliably detect when we're in process
> > context and can safely access current->pid. When not in process
> > context (such as in interrupt or in an asynchronous RCU context), the
> > field is set to -1 as a sentinel value.
> >
> > Signed-off-by: Thomas Ballasi <tballasi@linux.microsoft.com>
>
> Is this really needed? The trace events already show if you are in
> interrupt context or not.
>
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 25817/25817 #P:8
> #
> # _-----=> irqs-off/BH-disabled
> # / _----=> need-resched
> # | / _---=> hardirq/softirq <<<<------ Shows irq context
> # || / _--=> preempt-depth
> # ||| / _-=> migrate-disable
> # |||| / delay
> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
> # | | | ||||| | |
> <idle>-0 [002] d..1. 11429.293552: rcu_watching: Startirq 0 1 0x74c
> <idle>-0 [000] d.H1. 11429.293564: rcu_utilization: Start scheduler-tick
> <idle>-0 [000] d.H1. 11429.293566: rcu_utilization: End scheduler-tick
> <idle>-0 [002] dN.1. 11429.293567: rcu_watching: Endirq 1 0 0x74c
> <idle>-0 [002] dN.1. 11429.293568: rcu_watching: Start 0 1 0x754
> <idle>-0 [000] d.s1. 11429.293577: rcu_watching: --= 3 1 0xdf4
> <idle>-0 [002] dN.1. 11429.293579: rcu_utilization: Start context switch
> <idle>-0 [002] dN.1. 11429.293580: rcu_utilization: End context switch
> rcu_sched-15 [002] d..1. 11429.293589: rcu_grace_period: rcu_sched 132685 start
> <idle>-0 [000] dN.1. 11429.293592: rcu_watching: Endirq 1 0 0xdf4
> rcu_sched-15 [002] d..1. 11429.293592: rcu_grace_period: rcu_sched 132685 cpustart
> rcu_sched-15 [002] d..1. 11429.293592: rcu_grace_period_init: rcu_sched 132685 0 0 7 ff
> <idle>-0 [000] dN.1. 11429.293593: rcu_watching: Start 0 1 0xdfc
>
> Thus, you can already tell if you are in interrupt context or not, and you
> always get the current pid. The 'H', 'h' or 's' means you are in a
> interrupt type context. ('H' for hard interrupt interrupting a softirq, 'h'
> for just a hard interrupt, and 's' for a softirq).
>
> What's the point of adding another field to cover the same information
> that's already available?
>
> -- Steve
(re-sending the reply as I believe I missed the reply all)
It indeed shows whether or not we're in an IRQ, but I believe the
kernel shouldn't show erronous debugging values. Even though it can be
obvious that we're in an interrupt, some people might look directly at
the garbage PID value without having second thoughts and taking it for
granted. On the other hand, it takes just a small check to mark the
debugging information as clearly invalid, which complements the IRQ
context flag.
If we shouldn't put that check there, I'd happily remove it, but I'd
tend to think it's a trivial addition that can only be for the best.
Thomas
^ permalink raw reply
* Re: [PATCHv5 bpf-next 9/9] bpf,x86: Use single ftrace_ops for direct calls
From: Steven Rostedt @ 2025-12-29 16:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <aVFLKgZ56wt5aLci@krava>
On Sun, 28 Dec 2025 16:22:18 +0100
Jiri Olsa <olsajiri@gmail.com> wrote:
> actualy, it seems that having it the original way with adding the rest
> of the wrappers for !CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS case is
> easier AFAICS
I'm fine either way.
-- Steve
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Steven Rostedt @ 2025-12-29 16:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Yury Norov (NVIDIA), Masami Hiramatsu, Mathieu Desnoyers,
Andy Shevchenko, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <20251228133150.1d5731d04bc1b685b0fe81c1@linux-foundation.org>
On Sun, 28 Dec 2025 13:31:50 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:
> > trace_printk() should be as available to the kernel as printk() is.
>
> um, why? trace_printk is used 1% as often as is printk. Seems
> reasonable to include a header file to access such a rarely-used(!) and
> specialized thing?
It will waste a lot of kernel developers time. Go to conferences and talk
with developers. trace_printk() is now one of the most common ways to debug
your code. Having to add "#include <linux/trace_printk.h>" in every file
that you use trace_printk() (and after your build fails because you forgot
to include that file **WILL** slow down kernel debugging for hundreds of
developers! It *is* used more than printk() for debugging today. Because
it's fast and can be used in any context (NMI, interrupt handlers, etc).
But sure, if you want to save the few minutes that is added to "make
allyesconfig" by sacrificing minutes of kernel developer's time. Go ahead
and make this change.
I don't know how much you debug and develop today, but lots of people I
talk to at conferences thank me for trace_printk() because it makes
debugging their code so much easier.
The "shotgun" approach is very common. That is, you add:
trace_printk("%s:%d\n", __func__, __LINE__);
all over your code to find out where things are going wrong. With the
persistent ring buffer, you can even extract that information after a
crash and reboot.
There's very few instances of it in the kernel because I made it that way.
If you add a trace_printk() in the kernel, you get the banner:
**********************************************************
** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
** **
** trace_printk() being used. Allocating extra memory. **
** **
** This means that this is a DEBUG kernel and it is **
** unsafe for production use. **
** **
** If you see this message and you are not debugging **
** the kernel, report this immediately to your vendor! **
** **
** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
**********************************************************
in your dmesg.
I've been recommending people that if you find a "trace_printk()" useful,
you should convert it into a normal TRACE_EVENT() for submission. Which
many developers have done.
Yes, it's not in your kernel, but it is in several other people's kernels
as they develop it. And adding a requirement that they need to include a
header file for every place they add it (and then have to remember to
remove that header file when they are done debugging) is going to waste
more precious time that kernel developers don't have much of.
-- Steve
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Danilo Krummrich @ 2025-12-29 16:41 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Yury Norov (NVIDIA), Masami Hiramatsu,
Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy,
Randy Dunlap, Ingo Molnar, Jani Nikula, Joonas Lahtinen,
David Laight, Petr Pavlu, Andi Shyti, Rodrigo Vivi,
Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, linux-kernel, intel-gfx, dri-devel,
linux-modules, linux-trace-kernel
In-Reply-To: <20251229111748.3ba66311@gandalf.local.home>
On Mon Dec 29, 2025 at 5:17 PM CET, Steven Rostedt wrote:
> It will waste a lot of kernel developers time. Go to conferences and talk
> with developers. trace_printk() is now one of the most common ways to debug
> your code. Having to add "#include <linux/trace_printk.h>" in every file
> that you use trace_printk() (and after your build fails because you forgot
> to include that file **WILL** slow down kernel debugging for hundreds of
> developers! It *is* used more than printk() for debugging today. Because
> it's fast and can be used in any context (NMI, interrupt handlers, etc).
I strongly agree with this. I heavly use trace_printk() for debugging for a long
time and have recommended it to dozens of people that all have been very
thankful for it -- especially when it comes to debugging race conditions on a
tough timing, where a normal printk() simply "fixes" the race.
Having to include additional headers would be very painful, especially when
debugging large code bases with lots of files. For instance, one of the
components I maintain is the nouveau driver with 773 C files and 1390 files
overall.
I suppose it would be fair to argue that such codebases usually have their own
common header files that could be reused, but even in that case, I’d consider
the ergonomic cost too high.
- Danilo
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Borislav Petkov @ 2025-12-29 17:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Yury Norov (NVIDIA), Masami Hiramatsu,
Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy,
Randy Dunlap, Ingo Molnar, Jani Nikula, Joonas Lahtinen,
David Laight, Petr Pavlu, Andi Shyti, Rodrigo Vivi,
Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
dri-devel, linux-modules, linux-trace-kernel
In-Reply-To: <20251229111748.3ba66311@gandalf.local.home>
On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
> But sure, if you want to save the few minutes that is added to "make
> allyesconfig"
Nah, it is
"Removing trace_printk.h saves 1.5-2% of compilation time on my
Ubuntu-derived x86_64/localyesconfig"
which is:
localyesconfig - Update current config converting local mods to core
and which makes me wonder - who does that?
What are we actually optimizing here?
And 1-2% at that.
I don't see how this outweighs the goodness of using trace_printk()
everywhere.
So that's a NO on that patch from me too.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 2/2] mm: vmscan: add PIDs to vmscan tracepoints
From: Steven Rostedt @ 2025-12-29 18:29 UTC (permalink / raw)
To: Thomas Ballasi; +Cc: akpm, linux-mm, linux-trace-kernel, mhiramat
In-Reply-To: <20251229105427.14720-1-tballasi@linux.microsoft.com>
On Mon, 29 Dec 2025 02:54:27 -0800
Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
> It indeed shows whether or not we're in an IRQ, but I believe the
> kernel shouldn't show erronous debugging values. Even though it can be
> obvious that we're in an interrupt, some people might look directly at
> the garbage PID value without having second thoughts and taking it for
> granted. On the other hand, it takes just a small check to mark the
> debugging information as clearly invalid, which complements the IRQ
> context flag.
>
> If we shouldn't put that check there, I'd happily remove it, but I'd
> tend to think it's a trivial addition that can only be for the best.
I just don't like wasting valuable ring buffer space for something that can
be easily determined without it.
How about this. I just wrote up this patch, and it could be something you
use. I tested it against the sched waking events, by adding:
__entry->target_cpu = task_cpu(p);
),
- TP_printk("comm=%s pid=%d prio=%d target_cpu=%03d",
+ TP_printk("comm=%s pid=%d prio=%d target_cpu=%03d %s",
__entry->comm, __entry->pid, __entry->prio,
- __entry->target_cpu)
+ __entry->target_cpu,
+ __event_in_irq() ? "(in-irq)" : "")
);
Which produces:
<idle>-0 [003] d.h4. 44.832126: sched_waking: comm=in:imklog pid=619 prio=120 target_cpu=006 (in-irq)
<idle>-0 [003] d.s3. 44.832180: sched_waking: comm=rcu_preempt pid=15 prio=120 target_cpu=001 (in-irq)
in:imklog-619 [006] d..2. 44.832393: sched_waking: comm=rs:main Q:Reg pid=620 prio=120 target_cpu=003
You can see it adds "(in-irq)" when the even is executed from IRQ context
(soft or hard irq). But I also added __event_in_hardirq() and
__event_in_softirq() if you wanted to distinguish them.
Now you don't need to update what goes into the ring buffer (and waste its
space), but only update the output format that makes it obvious that the
task was in interrupt context or not.
I also used trace-cmd to record the events, and it still parses properly
with no updates to libtraceevent needed.
Would this work for you?
Below is the patch that allows for this:
-- Steve
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 1e7b0bef95f5..53a23988a3b8 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -150,3 +150,11 @@
#undef __get_buf
#define __get_buf(len) trace_seq_acquire(p, (len))
+
+#undef __event_in_hardirq
+#undef __event_in_softirq
+#undef __event_in_irq
+
+#define __event_in_hardirq() (__entry->ent.flags & TRACE_FLAG_HARDIRQ)
+#define __event_in_softirq() (__entry->ent.flags & TRACE_FLAG_SOFTIRQ)
+#define __event_in_irq() (__entry->ent.flags & (TRACE_FLAG_HARDIRQ | TRACE_FLAG_SOFTIRQ))
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index fcd564a590f4..47008897a795 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -26,6 +26,25 @@
#undef __print_hex_dump
#undef __get_buf
+#undef __event_in_hardirq
+#undef __event_in_softirq
+#undef __event_in_irq
+
+/*
+ * The TRACE_FLAG_* are enums. Instead of using TRACE_DEFINE_ENUM(),
+ * use their hardcoded values. These values are parsed by user space
+ * tooling elsewhere so they will never change.
+ *
+ * See "enum trace_flag_type" in linux/trace_events.h:
+ * TRACE_FLAG_HARDIRQ
+ * TRACE_FLAG_SOFTIRQ
+ */
+
+/* This is what is displayed in the format files */
+#define __event_in_hardirq() (REC->common_flags & 0x8)
+#define __event_in_softirq() (REC->common_flags & 0x10)
+#define __event_in_irq() (REC->common_flags & 0x18)
+
/*
* The below is not executed in the kernel. It is only what is
* displayed in the print format for userspace to parse.
^ permalink raw reply related
* [PATCH] tracing: Add __event_in_*irq() helpers
From: Steven Rostedt @ 2025-12-29 21:35 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Thomas Ballasi,
Andrew Morton
From: Steven Rostedt <rostedt@goodmis.org>
Some trace events want to expose in their output if they were triggered in
an interrupt or softirq context. Instead of recording this in the event
structure itself, as this information is stored in the flags portion of
the event header, add helper macros that can be used in the print format:
TP_printk("val=%d %s", __entry->val, __entry_in_irq() ? "(in-irq)" : "")
This will output "(in-irq)" for the event in the trace data if the event
was triggered in hard or soft interrupt context.
Link: https://lore.kernel.org/all/20251229132942.31a2b583@gandalf.local.home/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/trace/stages/stage3_trace_output.h | 8 ++++++++
include/trace/stages/stage7_class_define.h | 19 +++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 1e7b0bef95f5..53a23988a3b8 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -150,3 +150,11 @@
#undef __get_buf
#define __get_buf(len) trace_seq_acquire(p, (len))
+
+#undef __event_in_hardirq
+#undef __event_in_softirq
+#undef __event_in_irq
+
+#define __event_in_hardirq() (__entry->ent.flags & TRACE_FLAG_HARDIRQ)
+#define __event_in_softirq() (__entry->ent.flags & TRACE_FLAG_SOFTIRQ)
+#define __event_in_irq() (__entry->ent.flags & (TRACE_FLAG_HARDIRQ | TRACE_FLAG_SOFTIRQ))
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index fcd564a590f4..47008897a795 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -26,6 +26,25 @@
#undef __print_hex_dump
#undef __get_buf
+#undef __event_in_hardirq
+#undef __event_in_softirq
+#undef __event_in_irq
+
+/*
+ * The TRACE_FLAG_* are enums. Instead of using TRACE_DEFINE_ENUM(),
+ * use their hardcoded values. These values are parsed by user space
+ * tooling elsewhere so they will never change.
+ *
+ * See "enum trace_flag_type" in linux/trace_events.h:
+ * TRACE_FLAG_HARDIRQ
+ * TRACE_FLAG_SOFTIRQ
+ */
+
+/* This is what is displayed in the format files */
+#define __event_in_hardirq() (REC->common_flags & 0x8)
+#define __event_in_softirq() (REC->common_flags & 0x10)
+#define __event_in_irq() (REC->common_flags & 0x18)
+
/*
* The below is not executed in the kernel. It is only what is
* displayed in the print format for userspace to parse.
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] mm: vmscan: add PIDs to vmscan tracepoints
From: Steven Rostedt @ 2025-12-29 21:36 UTC (permalink / raw)
To: Thomas Ballasi; +Cc: akpm, linux-mm, linux-trace-kernel, mhiramat
In-Reply-To: <20251229132942.31a2b583@gandalf.local.home>
On Mon, 29 Dec 2025 13:29:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> I just don't like wasting valuable ring buffer space for something that can
> be easily determined without it.
>
> How about this. I just wrote up this patch, and it could be something you
> use. I tested it against the sched waking events, by adding:
>
> __entry->target_cpu = task_cpu(p);
> ),
>
> - TP_printk("comm=%s pid=%d prio=%d target_cpu=%03d",
> + TP_printk("comm=%s pid=%d prio=%d target_cpu=%03d %s",
> __entry->comm, __entry->pid, __entry->prio,
> - __entry->target_cpu)
> + __entry->target_cpu,
> + __event_in_irq() ? "(in-irq)" : "")
> );
>
> Which produces:
>
> <idle>-0 [003] d.h4. 44.832126: sched_waking: comm=in:imklog pid=619 prio=120 target_cpu=006 (in-irq)
> <idle>-0 [003] d.s3. 44.832180: sched_waking: comm=rcu_preempt pid=15 prio=120 target_cpu=001 (in-irq)
> in:imklog-619 [006] d..2. 44.832393: sched_waking: comm=rs:main Q:Reg pid=620 prio=120 target_cpu=003
>
> You can see it adds "(in-irq)" when the even is executed from IRQ context
> (soft or hard irq). But I also added __event_in_hardirq() and
> __event_in_softirq() if you wanted to distinguish them.
>
> Now you don't need to update what goes into the ring buffer (and waste its
> space), but only update the output format that makes it obvious that the
> task was in interrupt context or not.
>
> I also used trace-cmd to record the events, and it still parses properly
> with no updates to libtraceevent needed.
>
> Would this work for you?
If this would work for you. Feel free to take the patch I posted and use that:
https://lore.kernel.org/all/20251229163515.3d1b0bba@gandalf.local.home/
-- Steve
^ permalink raw reply
* Re: [PATCH v3 1/2] cpufreq: Replace trace_cpu_frequency with trace_policy_frequency
From: Steven Rostedt @ 2025-12-29 21:52 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Christian Loehle, Samuel Wu, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Jonathan Corbet, Viresh Kumar,
Masami Hiramatsu, Mathieu Desnoyers, Srinivas Pandruvada,
Len Brown, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter, James Clark, kernel-team, linux-pm, linux-doc,
linux-kernel, linux-trace-kernel, bpf, linux-perf-users
In-Reply-To: <CAJZ5v0irO1zmh=un+8vDQ8h2k-sHFTpCPCwr=iVRPcozHMRKHA@mail.gmail.com>
On Thu, 4 Dec 2025 18:24:57 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:
> My concern is that the patch effectively removes one trace point
> (cpu_frequency) and adds another one with a different format
> (policy_frequency), updates one utility in the kernel tree and expects
> everyone else to somehow know that they should switch over.
>
> I know about at least several people who have their own scripts using
> this tracepoint though.
Hi Rafael,
Can you reach out to those that have scripts that use this trace event to
see if it can be changed?
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v3 1/2] cpufreq: Replace trace_cpu_frequency with trace_policy_frequency
From: Steven Rostedt @ 2025-12-29 22:00 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Christian Loehle, Samuel Wu, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Jonathan Corbet, Viresh Kumar,
Masami Hiramatsu, Mathieu Desnoyers, Srinivas Pandruvada,
Len Brown, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter, James Clark, kernel-team, linux-pm, linux-doc,
linux-kernel, linux-trace-kernel, bpf, linux-perf-users
In-Reply-To: <20251229165212.5bd8508d@gandalf.local.home>
On Mon, 29 Dec 2025 16:52:12 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 4 Dec 2025 18:24:57 +0100
> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>
> > My concern is that the patch effectively removes one trace point
> > (cpu_frequency) and adds another one with a different format
> > (policy_frequency), updates one utility in the kernel tree and expects
> > everyone else to somehow know that they should switch over.
> >
> > I know about at least several people who have their own scripts using
> > this tracepoint though.
>
> Hi Rafael,
>
> Can you reach out to those that have scripts that use this trace event to
> see if it can be changed?
>
> Thanks,
I got a bunch of "Undelivered Mail Returned to Sender". It seems that gmail
thinks my goodmis.org account is now spam :-p
-- Steve
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Mathieu Desnoyers @ 2025-12-29 22:25 UTC (permalink / raw)
To: Steven Rostedt, Andrew Morton
Cc: Yury Norov (NVIDIA), Masami Hiramatsu, Andy Shevchenko,
Christophe Leroy, Randy Dunlap, Ingo Molnar, Jani Nikula,
Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
dri-devel, linux-modules, linux-trace-kernel
In-Reply-To: <20251229111748.3ba66311@gandalf.local.home>
On 2025-12-29 11:17, Steven Rostedt wrote:
> On Sun, 28 Dec 2025 13:31:50 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>>> trace_printk() should be as available to the kernel as printk() is.
>>
>> um, why? trace_printk is used 1% as often as is printk. Seems
>> reasonable to include a header file to access such a rarely-used(!) and
>> specialized thing?
[...]
> Yes, it's not in your kernel, but it is in several other people's kernels
> as they develop it. And adding a requirement that they need to include a
> header file for every place they add it (and then have to remember to
> remove that header file when they are done debugging) is going to waste
> more precious time that kernel developers don't have much of.
I agree with Steven. trace_printk() needs to stay convenient to use for
kernel developers. Part of this convenience comes from not having to
include additional header files by hand. It has been around for
16 years and documented as such in kernel documentation [1],
LWN articles [2], and conference presentation material. Changing
this would lead to confusion for people trying to use the feature.
I personally use trace_printk() to sprinkle temporary debug-style
trace events in frequently executed kernel code I need to follow
carefully.
One possible compromise would be to move it to its own header file,
and introduce a CONFIG_TRACE_PRINTK Kconfig option (default Y) that
would surround an include from linux/kernel.h with a preprocessor
conditional. But please make sure the default stays as it is today:
include the trace printk header by default.
Thanks,
Mathieu
[1] Debugging the kernel using Ftrace - part 1 https://lwn.net/Articles/365835/
[2] Documentation/trace/ftrace.txt
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCH] tracing: Allow perf to read synthetic events
From: Steven Rostedt @ 2025-12-29 22:49 UTC (permalink / raw)
To: Jiri Olsa
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Ian Rogers, Arnaldo Carvalho de Melo, Namhyung Kim,
Peter Zijlstra
In-Reply-To: <aVG2VoBpFoEEE1gG@krava>
On Sun, 28 Dec 2025 23:59:34 +0100
Jiri Olsa <olsajiri@gmail.com> wrote:
> hi,
> I don't see the crash, but perf record/script gives me 'FAILED TO PARSE' in perf script:
>
> # cd /sys/kernel/tracing
> # echo 's:block_lat pid_t pid; u64 delta; unsigned long[] stack;' > dynamic_events
> # echo 'hist:keys=next_pid:ts=common_timestamp.usecs,st=common_stacktrace if prev_state == 2' >> events/sched/sched_switch/trigger
> # echo 'hist:keys=prev_pid:delta=common_timestamp.usecs-$ts,s=$st:onmax($delta).trace(block_lat,prev_pid,$delta,$s)' >> events/sched/sched_switch/trigger
> # echo 1 > events/synthetic/block_lat/enable
>
> # perf record -e 'synthetic:block_lat' -a
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.259 MB perf.data (1 samples) ]
> # perf script
> kworker/u33:2-w 244 [000] 1707.836263: synthetic:block_lat: [FAILED TO PARSE] pid=244 delta=21 stack=ARRAY[0b, 00, 00, 00, 00, 00, 00, 00, 1d, 72, 9d, 82, ff, ff, ff, ff, 0d, 7d, 9d, 82, ff, ff, ff, ff, 3d, 3d, 9e, 82, ff, ff, ff, ff, 05, 91, 9d, 82, ff, ff, ff, ff, 40, 7a, 42, 81, ff, ff, ff, ff, 5e, f4, 0c, 82, ff, ff, ff, ff, 43, 8d, 0c, 82, ff, ff, ff, ff, 82, 2d, 89, 81, ff, ff, ff, ff, 9b, 39, 89, 81, ff, ff, ff, ff, a6, 5a, 9c, 82, ff, ff, ff, ff, 2f, 01, 00, 81, ff, ff, ff, ff]
>
> not sure it's fixed in the latest libtraceevent, mine is
> libtraceevent-1.8.4-3.fc42.x86_64
Yeah, it's not supported yet by libtraceevent (fails for trace-cmd record
as well). But this should be easily fixed:
># trace-cmd list -e block_lat -F --full
system: synthetic
name: block_lat
ID: 1871
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:pid_t pid; offset:8; size:4; signed:1;
field:u64 delta; offset:16; size:8; signed:0;
field:__data_loc unsigned long[] stack; offset:24; size:8; signed:1;
print fmt: "pid=%d delta=%llu stack=%s", REC->pid, REC->delta, __get_stacktrace(stack)
Looks like I only need to add "__get_stacktrace()" to the libtraceevent parsing.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: Allow perf to read synthetic events
From: Steven Rostedt @ 2025-12-29 23:36 UTC (permalink / raw)
To: Jiri Olsa
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Ian Rogers, Arnaldo Carvalho de Melo, Namhyung Kim,
Peter Zijlstra
In-Reply-To: <20251229174901.0e62eab0@gandalf.local.home>
On Mon, 29 Dec 2025 17:49:01 -0500
Steven Rostedt <steven@rostedt.org> wrote:
> Looks like I only need to add "__get_stacktrace()" to the libtraceevent parsing.
The below patch appears to work:
<...>-22 [001] d..5. 2891.837516: block_lat: pid=22 delta=159 stack=
=> __schedule+0xded
=> schedule+0x123
=> io_schedule+0x44
=> bit_wait_io+0x11
=> __wait_on_bit+0x4a
=> out_of_line_wait_on_bit+0x9d
=> ext4_read_bh+0x95
=> ext4_bread+0x51
=> __ext4_read_dirblock+0x45
=> htree_dirblock_to_tree+0x76
=> ext4_htree_fill_tree+0x3b1
=> ext4_readdir+0xa9b
=> iterate_dir+0xef
=> __se_sys_getdents64+0x76
=> do_syscall_64+0x93
<...>-254 [004] d..5. 2892.173786: block_lat: pid=254 delta=57 stack=
=> __schedule+0xded
=> schedule+0x123
=> io_schedule+0x44
=> bit_wait_io+0x11
=> __wait_on_bit+0x4a
=> out_of_line_wait_on_bit+0x9d
=> ext4_read_bh+0x95
=> ext4_bread+0x51
=> __ext4_read_dirblock+0x45
=> htree_dirblock_to_tree+0x76
=> ext4_htree_fill_tree+0x1e5
=> ext4_readdir+0xa9b
=> iterate_dir+0xef
=> __se_sys_getdents64+0x76
=> do_syscall_64+0x93
-- Steve
diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index ebfc2c7..9c1abfa 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -138,6 +138,11 @@ struct tep_print_arg_bitmask {
struct tep_format_field *field;
};
+struct tep_print_arg_stacktrace {
+ char *stacktrace;
+ struct tep_format_field *field;
+};
+
struct tep_print_arg_field {
char *name;
struct tep_format_field *field;
@@ -215,6 +220,7 @@ enum tep_print_arg_type {
TEP_PRINT_DYNAMIC_ARRAY_LEN,
TEP_PRINT_HEX_STR,
TEP_PRINT_CPUMASK,
+ TEP_PRINT_STACKTRACE,
};
struct tep_print_arg {
@@ -231,6 +237,7 @@ struct tep_print_arg {
struct tep_print_arg_func func;
struct tep_print_arg_string string;
struct tep_print_arg_bitmask bitmask;
+ struct tep_print_arg_stacktrace stacktrace;
struct tep_print_arg_op op;
struct tep_print_arg_dynarray dynarray;
};
diff --git a/src/event-parse.c b/src/event-parse.c
index 939b0a8..09d9092 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -1129,6 +1129,9 @@ static void free_arg(struct tep_print_arg *arg)
free_arg(arg->op.left);
free_arg(arg->op.right);
break;
+ case TEP_PRINT_STACKTRACE:
+ free(arg->stacktrace.stacktrace);
+ break;
case TEP_PRINT_FUNC:
while (arg->func.args) {
farg = arg->func.args;
@@ -2895,6 +2898,7 @@ static int arg_num_eval(struct tep_print_arg *arg, long long *val)
case TEP_PRINT_BSTRING:
case TEP_PRINT_BITMASK:
case TEP_PRINT_CPUMASK:
+ case TEP_PRINT_STACKTRACE:
default:
do_warning("invalid eval type %d", arg->type);
ret = 0;
@@ -2925,6 +2929,7 @@ static char *arg_eval (struct tep_print_arg *arg)
case TEP_PRINT_BSTRING:
case TEP_PRINT_BITMASK:
case TEP_PRINT_CPUMASK:
+ case TEP_PRINT_STACKTRACE:
default:
do_warning("invalid eval type %d", arg->type);
break;
@@ -3462,6 +3467,34 @@ process_cpumask(struct tep_event *event __maybe_unused, struct tep_print_arg *ar
return type;
}
+static enum tep_event_type
+process_stacktrace(struct tep_event *event, struct tep_print_arg *arg, char **tok)
+{
+ enum tep_event_type type;
+ char *token;
+
+ if (read_expect_type(event->tep, TEP_EVENT_ITEM, &token) < 0)
+ goto out_free;
+
+ arg->type = TEP_PRINT_STACKTRACE;
+ arg->stacktrace.stacktrace = token;
+ arg->stacktrace.field = NULL;
+
+ if (read_expected(event->tep, TEP_EVENT_DELIM, ")") < 0)
+ goto out_err;
+
+ type = read_token(event->tep, &token);
+ *tok = token;
+
+ return type;
+
+ out_free:
+ free_token(token);
+ out_err:
+ *tok = NULL;
+ return TEP_EVENT_ERROR;
+}
+
static struct tep_function_handler *
find_func_handler(struct tep_handle *tep, char *func_name)
{
@@ -3750,6 +3783,10 @@ process_function(struct tep_event *event, struct tep_print_arg *arg,
free_token(token);
return process_dynamic_array_len(event, arg, tok);
}
+ if (strcmp(token, "__get_stacktrace") == 0) {
+ free_token(token);
+ return process_stacktrace(event, arg, tok);
+ }
if (strcmp(token, "__builtin_expect") == 0) {
free_token(token);
return process_builtin_expect(event, arg, tok);
@@ -4414,6 +4451,7 @@ eval_num_arg(void *data, int size, struct tep_event *event, struct tep_print_arg
case TEP_PRINT_BSTRING:
case TEP_PRINT_BITMASK:
case TEP_PRINT_CPUMASK:
+ case TEP_PRINT_STACKTRACE:
return 0;
case TEP_PRINT_FUNC: {
struct trace_seq s;
@@ -4859,6 +4897,33 @@ more:
free(str);
}
+static void print_stacktrace_to_seq(struct tep_handle *tep,
+ struct trace_seq *s, const char *format,
+ int len_arg, const void *data, int size)
+{
+ int nr_funcs = size / tep->long_size;
+ struct func_map *func;
+ unsigned long long val;
+
+ trace_seq_putc(s, '\n');
+
+ /* The first entry is a counter, skip it */
+ data += tep->long_size;
+
+ for (int i = 1; i < nr_funcs; i++) {
+ trace_seq_puts(s, "=> ");
+ val = tep_read_number(tep, data, tep->long_size);
+ func = find_func(tep, val);
+ if (func) {
+ trace_seq_puts(s, func->func);
+ trace_seq_printf(s, "+0x%llx\n", val - func->addr);
+ } else {
+ trace_seq_printf(s, "%llx\n", val);
+ }
+ data += tep->long_size;
+ }
+}
+
static void print_str_arg(struct trace_seq *s, void *data, int size,
struct tep_event *event, const char *format,
int len_arg, struct tep_print_arg *arg)
@@ -5097,6 +5162,17 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
data + offset, len);
break;
}
+ case TEP_PRINT_STACKTRACE: {
+ if (!arg->stacktrace.field) {
+ arg->stacktrace.field = tep_find_any_field(event, arg->stacktrace.stacktrace);
+ if (!arg->stacktrace.field)
+ break;
+ }
+ dynamic_offset_field(tep, arg->stacktrace.field, data, size, &offset, &len);
+ print_stacktrace_to_seq(tep, s, format, len_arg,
+ data + offset, len);
+ break;
+ }
case TEP_PRINT_OP:
/*
* The only op for string should be ? :
^ permalink raw reply related
* [PATCH v2] rv: Fix documentation reference in da_monitor.h
From: Shubham Sharma @ 2025-12-30 7:53 UTC (permalink / raw)
To: rostedt, gmonaco; +Cc: linux-trace-kernel, linux-kernel, Shubham Sharma
Update documentation reference to reflect the file rename.
Monitor synthesis documentation was renamed in commit f40a7c060207
("Documentation/rv: Prepare monitor synthesis document for LTL inclusion")
from da_monitor_synthesis.rst to monitor_synthesis.rst.
Signed-off-by: Shubham Sharma <slopixelz@gmail.com>
---
v2: Removed backslashes as suggested by Gabriele Monaco
include/rv/da_monitor.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 0cef64366538..810cfb140131 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -8,7 +8,7 @@
* The dot2k tool is available at tools/verification/dot2k/
*
* For further information, see:
- * Documentation/trace/rv/da_monitor_synthesis.rst
+ * Documentation/trace/rv/monitor_synthesis.rst
*/
#include <rv/automata.h>
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Andy Shevchenko @ 2025-12-30 8:55 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, Andrew Morton, Yury Norov (NVIDIA),
Masami Hiramatsu, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <9833cb61-1ec5-4cc1-ad9d-3e07f3deff80@efficios.com>
On Mon, Dec 29, 2025 at 05:25:08PM -0500, Mathieu Desnoyers wrote:
...
> One possible compromise would be to move it to its own header file,
> and introduce a CONFIG_TRACE_PRINTK Kconfig option (default Y) that
> would surround an include from linux/kernel.h with a preprocessor
> conditional. But please make sure the default stays as it is today:
> include the trace printk header by default.
"by default" where exactly? The problem is that kernel.h is a total mess and
it's included in a lot of mysterious ways (indirectly), and in C you _must_
include a header anyway for a custom API, just define *which* one.
Based on the Steven's first replies I see a compromise in having it inside
printk.h. If you want to debug something with printf() (in general) the same
header should provide all species. Do you agree?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] tracing: Replace use of system_wq with system_percpu_wq
From: Marco Crivellari @ 2025-12-30 10:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Tejun Heo, Lai Jiangshan,
Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko,
Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20251224133210.7b62ba5e@gandalf.local.home>
Hi Steven,
On Wed, Dec 24, 2025 at 9:43 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> [...]
> > @@ -1375,7 +1375,7 @@ static void free_filter_list_tasks(struct rcu_head *rhp)
> > struct filter_head *filter_list = container_of(rhp, struct filter_head, rcu);
> >
> > INIT_RCU_WORK(&filter_list->rwork, free_filter_list_work);
> > - queue_rcu_work(system_wq, &filter_list->rwork);
> > + queue_rcu_work(system_percpu_wq, &filter_list->rwork);
>
> This is just garbage collection and has no need to be done on the CPU it
> was queued on. I guess this can use system_dfl_wq?
I will check and I will send a new version with the unbound wq.
Thanks!
--
Marco Crivellari
L3 Support Engineer
^ permalink raw reply
* Re: [PATCH v2] rv: Fix documentation reference in da_monitor.h
From: Gabriele Monaco @ 2025-12-30 11:29 UTC (permalink / raw)
To: Shubham Sharma; +Cc: linux-trace-kernel, linux-kernel, rostedt
In-Reply-To: <20251230075337.11993-1-slopixelz@gmail.com>
On Tue, 2025-12-30 at 13:23 +0530, Shubham Sharma wrote:
> Update documentation reference to reflect the file rename.
> Monitor synthesis documentation was renamed in commit f40a7c060207
> ("Documentation/rv: Prepare monitor synthesis document for LTL inclusion")
> from da_monitor_synthesis.rst to monitor_synthesis.rst.
>
> Signed-off-by: Shubham Sharma <slopixelz@gmail.com>
Looks good, thanks! I'm going to apply it to my tree.
Small note for next time, although it doesn't make a big difference with this
tiny patch, when you send a new version after getting a review, try to propagate
the review tag.
Simply copy:
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
(or whoever reviewed/acked) before the signoff in the commit message.
This helps when reviewing larger series and when using tools like patchwork.
(now you don't need to do it, most tools will pick it from this answer)
Thanks,
Gabriele
> ---
> v2: Removed backslashes as suggested by Gabriele Monaco
>
> include/rv/da_monitor.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 0cef64366538..810cfb140131 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -8,7 +8,7 @@
> * The dot2k tool is available at tools/verification/dot2k/
> *
> * For further information, see:
> - * Documentation/trace/rv/da_monitor_synthesis.rst
> + * Documentation/trace/rv/monitor_synthesis.rst
> */
>
> #include <rv/automata.h>
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Mathieu Desnoyers @ 2025-12-30 14:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Steven Rostedt, Andrew Morton, Yury Norov (NVIDIA),
Masami Hiramatsu, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <aVOTbArAxmbT5LY9@smile.fi.intel.com>
On 2025-12-30 03:55, Andy Shevchenko wrote:
> On Mon, Dec 29, 2025 at 05:25:08PM -0500, Mathieu Desnoyers wrote:
>
> ...
>
>> One possible compromise would be to move it to its own header file,
>> and introduce a CONFIG_TRACE_PRINTK Kconfig option (default Y) that
>> would surround an include from linux/kernel.h with a preprocessor
>> conditional. But please make sure the default stays as it is today:
>> include the trace printk header by default.
>
> "by default" where exactly? The problem is that kernel.h is a total mess and
> it's included in a lot of mysterious ways (indirectly), and in C you _must_
> include a header anyway for a custom API, just define *which* one.
This patch series moves the guts of trace_printk into its own header
file, which reduces clutter. So that's already progress. :)
>
> Based on the Steven's first replies I see a compromise in having it inside
> printk.h. If you want to debug something with printf() (in general) the same
> header should provide all species. Do you agree?
I don't have a strong opinion about including trace_printk.h from either
kernel.h or printk.h. As long as it's still included by a default kernel
config the same way it has been documented/used since 2009.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* [PATCH v2] tracing: Replace use of system_wq with system_dfl_wq
From: Marco Crivellari @ 2025-12-30 14:28 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
This patch continues the effort to refactor workqueue APIs, which has begun
with the changes introducing new workqueues and a new alloc_workqueue flag:
commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
The point of the refactoring is to eventually alter the default behavior of
workqueues to become unbound by default so that their workload placement is
optimized by the scheduler.
Before that to happen after a careful review and conversion of each individual
case, workqueue users must be converted to the better named new workqueues with
no intended behaviour changes:
system_wq -> system_percpu_wq
system_unbound_wq -> system_dfl_wq
This specific workflow has no benefits being per-cpu, so instead of
system_percpu_wq the new unbound workqueue has been used (system_dfl_wq).
This way the old obsolete workqueues (system_wq, system_unbound_wq) can be
removed in the future.
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
Changes in v2:
- use system_dfl_wq instead of system_percpu_wq because this workload has no
benefits being per-cpu.
- rebase on v6.19-rc3.
kernel/trace/trace_events_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 385af8405392..7001e34476ee 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1375,7 +1375,7 @@ static void free_filter_list_tasks(struct rcu_head *rhp)
struct filter_head *filter_list = container_of(rhp, struct filter_head, rcu);
INIT_RCU_WORK(&filter_list->rwork, free_filter_list_work);
- queue_rcu_work(system_wq, &filter_list->rwork);
+ queue_rcu_work(system_dfl_wq, &filter_list->rwork);
}
/*
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2] rv: Fix documentation reference in da_monitor.h
From: Steven Rostedt @ 2025-12-30 14:40 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Shubham Sharma, linux-trace-kernel, linux-kernel
In-Reply-To: <5a8b1ef45e775d4724237057f60ed49cf5432ae8.camel@redhat.com>
On Tue, 30 Dec 2025 12:29:13 +0100
Gabriele Monaco <gmonaco@redhat.com> wrote:
> Looks good, thanks! I'm going to apply it to my tree.
>
>
> Small note for next time, although it doesn't make a big difference with this
> tiny patch, when you send a new version after getting a review, try to propagate
> the review tag.
>
> Simply copy:
>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
>
> (or whoever reviewed/acked) before the signoff in the commit message.
> This helps when reviewing larger series and when using tools like patchwork.
>
> (now you don't need to do it, most tools will pick it from this answer)
As this is sent to linux-trace-kernel@vger.kernel.org, you can use
patchwork to automatically get all the tags of a patch or series.
When I make a new series, I first create a new git branch based on the
patches pulled from patchwork (b4 works too, but I don't use that).
The patches live here:
https://patchwork.kernel.org/project/linux-trace-kernel/
The old version of the patch:
https://patchwork.kernel.org/project/linux-trace-kernel/patch/20251207205546.73946-1-slopixelz@gmail.com/
If you go there and hit the "mbox" (for single patch) or "series" (for a
patch set), it will download a mbox that you can use "git am" on and
patchwork would have added all the necessary tags to the patches.
Then start with those patches to make the update for v2, and you don't need
to manually add the tags.
Just an FYI,
-- Steve
^ permalink raw reply
* Re: [PATCH v2] tracing: Replace use of system_wq with system_dfl_wq
From: Steven Rostedt @ 2025-12-30 14:42 UTC (permalink / raw)
To: Marco Crivellari
Cc: linux-kernel, linux-trace-kernel, Tejun Heo, Lai Jiangshan,
Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko,
Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20251230142820.173712-1-marco.crivellari@suse.com>
On Tue, 30 Dec 2025 15:28:20 +0100
Marco Crivellari <marco.crivellari@suse.com> wrote:
> This patch continues the effort to refactor workqueue APIs, which has begun
> with the changes introducing new workqueues and a new alloc_workqueue flag:
>
> commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
>
> The point of the refactoring is to eventually alter the default behavior of
> workqueues to become unbound by default so that their workload placement is
> optimized by the scheduler.
>
> Before that to happen after a careful review and conversion of each individual
> case, workqueue users must be converted to the better named new workqueues with
> no intended behaviour changes:
>
> system_wq -> system_percpu_wq
> system_unbound_wq -> system_dfl_wq
>
> This specific workflow has no benefits being per-cpu, so instead of
> system_percpu_wq the new unbound workqueue has been used (system_dfl_wq).
>
> This way the old obsolete workqueues (system_wq, system_unbound_wq) can be
> removed in the future.
>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> ---
> Changes in v2:
> - use system_dfl_wq instead of system_percpu_wq because this workload has no
> benefits being per-cpu.
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: Jiri Olsa @ 2025-12-30 14:50 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
hi,
while poking the multi-tracing interface I ended up with just one ftrace_ops
object to attach all trampolines.
This change allows to use less direct API calls during the attachment changes
in the future code, so in effect speeding up the attachment.
In current code we get a speed up from using just a single ftrace_ops object.
- with current code:
Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
6,364,157,902 cycles:k
828,728,902 cycles:u
1,064,803,824 instructions:u # 1.28 insn per cycle
23,797,500,067 instructions:k # 3.74 insn per cycle
4.416004987 seconds time elapsed
0.164121000 seconds user
1.289550000 seconds sys
- with the fix:
Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
6,535,857,905 cycles:k
810,809,429 cycles:u
1,064,594,027 instructions:u # 1.31 insn per cycle
23,962,552,894 instructions:k # 3.67 insn per cycle
1.666961239 seconds time elapsed
0.157412000 seconds user
1.283396000 seconds sys
The speedup seems to be related to the fact that with single ftrace_ops object
we don't call ftrace_shutdown anymore (we use ftrace_update_ops instead) and
we skip the synchronize rcu calls (each ~100ms) at the end of that function.
rfc: https://lore.kernel.org/bpf/20250729102813.1531457-1-jolsa@kernel.org/
v1: https://lore.kernel.org/bpf/20250923215147.1571952-1-jolsa@kernel.org/
v2: https://lore.kernel.org/bpf/20251113123750.2507435-1-jolsa@kernel.org/
v3: https://lore.kernel.org/bpf/20251120212402.466524-1-jolsa@kernel.org/
v4: https://lore.kernel.org/bpf/20251203082402.78816-1-jolsa@kernel.org/
v5: https://lore.kernel.org/bpf/20251215211402.353056-10-jolsa@kernel.org/
v6 changes:
- rename add_hash_entry_direct to add_ftrace_hash_entry_direct [Steven]
- factor hash_add/hash_sub [Steven]
- add kerneldoc header for update_ftrace_direct_* functions [Steven]
- few assorted smaller fixes [Steven]
- added missing direct_ops wrappers for !CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
case [Steven]
v5 changes:
- do not export ftrace_hash object [Steven]
- fix update_ftrace_direct_add new_filter_hash leak [ci]
v4 changes:
- rebased on top of bpf-next/master (with jmp attach changes)
added patch 1 to deal with that
- added extra checks for update_ftrace_direct_del/mod to address
the ci bot review
v3 changes:
- rebased on top of bpf-next/master
- fixed update_ftrace_direct_del cleanup path
- added missing inline to update_ftrace_direct_* stubs
v2 changes:
- rebased on top fo bpf-next/master plus Song's livepatch fixes [1]
- renamed the API functions [2] [Steven]
- do not export the new api [Steven]
- kept the original direct interface:
I'm not sure if we want to melt both *_ftrace_direct and the new interface
into single one. It's bit different in semantic (hence the name change as
Steven suggested [2]) and I don't think the changes are not that big so
we could easily keep both APIs.
v1 changes:
- make the change x86 specific, after discussing with Mark options for
arm64 [Mark]
thanks,
jirka
[1] https://lore.kernel.org/bpf/20251027175023.1521602-1-song@kernel.org/
[2] https://lore.kernel.org/bpf/20250924050415.4aefcb91@batman.local.home/
---
Jiri Olsa (9):
ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
ftrace: Make alloc_and_copy_ftrace_hash direct friendly
ftrace: Export some of hash related functions
ftrace: Add update_ftrace_direct_add function
ftrace: Add update_ftrace_direct_del function
ftrace: Add update_ftrace_direct_mod function
bpf: Add trampoline ip hash table
ftrace: Factor ftrace_ops ops_func interface
bpf,x86: Use single ftrace_ops for direct calls
arch/x86/Kconfig | 1 +
include/linux/bpf.h | 7 ++-
include/linux/ftrace.h | 31 +++++++++-
kernel/bpf/trampoline.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
kernel/trace/Kconfig | 3 +
kernel/trace/ftrace.c | 406 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
6 files changed, 632 insertions(+), 75 deletions(-)
^ permalink raw reply
* [PATCHv6 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
From: Jiri Olsa @ 2025-12-30 14:50 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251230145010.103439-1-jolsa@kernel.org>
At the moment the we allow the jmp attach only for ftrace_ops that
has FTRACE_OPS_FL_JMP set. This conflicts with following changes
where we use single ftrace_ops object for all direct call sites,
so all could be be attached via just call or jmp.
We already limit the jmp attach support with config option and bit
(LSB) set on the trampoline address. It turns out that's actually
enough to limit the jmp attach for architecture and only for chosen
addresses (with LSB bit set).
Each user of register_ftrace_direct or modify_ftrace_direct can set
the trampoline bit (LSB) to indicate it has to be attached by jmp.
The bpf trampoline generation code uses trampoline flags to generate
jmp-attach specific code and ftrace inner code uses the trampoline
bit (LSB) to handle return from jmp attachment, so there's no harm
to remove the FTRACE_OPS_FL_JMP bit.
The fexit/fmodret performance stays the same (did not drop),
current code:
fentry : 77.904 ± 0.546M/s
fexit : 62.430 ± 0.554M/s
fmodret : 66.503 ± 0.902M/s
with this change:
fentry : 80.472 ± 0.061M/s
fexit : 63.995 ± 0.127M/s
fmodret : 67.362 ± 0.175M/s
Fixes: 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/ftrace.h | 1 -
kernel/bpf/trampoline.c | 32 ++++++++++++++------------------
kernel/trace/ftrace.c | 14 --------------
3 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..41c9bb08d4e4 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -359,7 +359,6 @@ enum {
FTRACE_OPS_FL_DIRECT = BIT(17),
FTRACE_OPS_FL_SUBOP = BIT(18),
FTRACE_OPS_FL_GRAPH = BIT(19),
- FTRACE_OPS_FL_JMP = BIT(20),
};
#ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 2a125d063e62..789ff4e1f40b 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
int ret;
if (tr->func.ftrace_managed) {
+ unsigned long addr = (unsigned long) new_addr;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+
if (lock_direct_mutex)
- ret = modify_ftrace_direct(tr->fops, (long)new_addr);
+ ret = modify_ftrace_direct(tr->fops, addr);
else
- ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
+ ret = modify_ftrace_direct_nolock(tr->fops, addr);
} else {
ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
new_addr);
@@ -240,10 +245,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
}
if (tr->func.ftrace_managed) {
+ unsigned long addr = (unsigned long) new_addr;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+
ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
if (ret)
return ret;
- ret = register_ftrace_direct(tr->fops, (long)new_addr);
+ ret = register_ftrace_direct(tr->fops, addr);
} else {
ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
}
@@ -499,13 +509,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
if (err)
goto out_free;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
- if (bpf_trampoline_use_jmp(tr->flags))
- tr->fops->flags |= FTRACE_OPS_FL_JMP;
- else
- tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
-#endif
-
WARN_ON(tr->cur_image && total == 0);
if (tr->cur_image)
/* progs already running at this address */
@@ -533,15 +536,8 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
tr->cur_image = im;
out:
/* If any error happens, restore previous flags */
- if (err) {
+ if (err)
tr->flags = orig_flags;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
- if (bpf_trampoline_use_jmp(tr->flags))
- tr->fops->flags |= FTRACE_OPS_FL_JMP;
- else
- tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
-#endif
- }
kfree(tlinks);
return err;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3ec2033c0774..f5f042ea079e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6043,15 +6043,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
if (ftrace_hash_empty(hash))
return -EINVAL;
- /* This is a "raw" address, and this should never happen. */
- if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
- return -EINVAL;
-
mutex_lock(&direct_mutex);
- if (ops->flags & FTRACE_OPS_FL_JMP)
- addr = ftrace_jmp_set(addr);
-
/* Make sure requested entries are not already registered.. */
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
@@ -6172,13 +6165,6 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
lockdep_assert_held_once(&direct_mutex);
- /* This is a "raw" address, and this should never happen. */
- if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
- return -EINVAL;
-
- if (ops->flags & FTRACE_OPS_FL_JMP)
- addr = ftrace_jmp_set(addr);
-
/* Enable the tmp_ops to have the same functions as the direct ops */
ftrace_ops_init(&tmp_ops);
tmp_ops.func_hash = ops->func_hash;
--
2.52.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