| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251125093425.2563849-1-dolinux.peng@gmail.com>
Date: Tue, 25 Nov 2025 17:34:25 +0800
From: Donglin Peng <dolinux.peng@...il.com>
To: rostedt@...dmis.org
Cc: linux-trace-kernel@...r.kernel.org,
linux-kernel@...r.kernel.org,
pengdonglin <pengdonglin@...omi.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Xiaoqin Zhang <zhangxiaoqin@...omi.com>
Subject: [PATCH v9] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: pengdonglin <pengdonglin@...omi.com>
Currently, the funcgraph-args and funcgraph-retaddr features are
mutually exclusive. This patch resolves this limitation by allowing
funcgraph-retaddr to have an args array.
To verify the change, use perf to trace vfs_write with both options
enabled:
Before:
# perf ftrace -G vfs_write --graph-opts args,retaddr
......
down_read() { /* <-n_tty_write+0xa3/0x540 */
__cond_resched(); /* <-down_read+0x12/0x160 */
preempt_count_add(); /* <-down_read+0x3b/0x160 */
preempt_count_sub(); /* <-down_read+0x8b/0x160 */
}
After:
# perf ftrace -G vfs_write --graph-opts args,retaddr
......
down_read(sem=0xffff8880100bea78) { /* <-n_tty_write+0xa3/0x540 */
__cond_resched(); /* <-down_read+0x12/0x160 */
preempt_count_add(val=1); /* <-down_read+0x3b/0x160 */
preempt_count_sub(val=1); /* <-down_read+0x8b/0x160 */
}
Cc: Steven Rostedt (Google) <rostedt@...dmis.org>
Cc: Sven Schnelle <svens@...ux.ibm.com>
Cc: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Xiaoqin Zhang <zhangxiaoqin@...omi.com>
Signed-off-by: pengdonglin <pengdonglin@...omi.com>
---
v9:
- Fix typo
- Link to v8: https://lore.kernel.org/linux-trace-kernel/20251125092711.2558787-1-dolinux.peng@gmail.com/
v8:
- Add the retaddr member back and reconstruct the patch
- Rebase on the trace/for-next branch
- Link to v7: https://lore.kernel.org/linux-trace-kernel/20251114025522.2115359-1-dolinux.peng@gmail.com/
v7:
- Remove nr_args to eliminate a multiplication instruction
- Link to v6: https://lore.kernel.org/all/20251114023453.2061590-1-dolinux.peng@gmail.com/
v6:
- Initialize retaddr to NULL to silence uninitialized variable warning.
- Use ARGS_OFFS(size) macro instead of args_offs variable to eliminate
"set but not used" warning
- Link to v5: https://lore.kernel.org/all/20251113072938.333657-1-dolinux.peng@gmail.com/
v5:
- Avoid unnecessary branch overhead by first verifying
CONFIG_FUNCTION_GRAPH_RETADDR is enabled prior to testing
the TRACE_GRAPH_PRINT_RETADDR flag
- Link to v4: https://lore.kernel.org/all/20251112093650.3345108-1-dolinux.peng@gmail.com/
v4:
- Remove redundant TRACE_GRAPH_ARGS flag check
- Eliminate unnecessary retaddr initialization
- Resend to add missing add Acked-by tags
- Link to v3: https://lore.kernel.org/all/20251112034333.2901601-1-dolinux.peng@gmail.com/
v3:
- Replace min() with min_t() to improve type safety and maintainability
- Keep only one Signed-off-by for cleaner attribution
- Code refactoring for improved readability
- Link to v2: https://lore.kernel.org/all/20251111135432.2143993-1-dolinux.peng@gmail.com/
v2:
- Preserve retaddr event functionality (suggested by Steven)
- Store the retaddr in args[0] (suggested by Steven)
- Refactor implementation logic and commit message clarity
- Link to v1: https://lore.kernel.org/all/20251011164156.3678012-1-dolinux.peng@gmail.com/
---
include/linux/ftrace.h | 7 +--
kernel/trace/trace.h | 24 +++++++++-
kernel/trace/trace_entries.h | 15 +++---
kernel/trace/trace_functions_graph.c | 71 ++++++++++++++++++----------
4 files changed, 80 insertions(+), 37 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..6ca9c6229d93 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1126,17 +1126,14 @@ static inline void ftrace_init(void) { }
*/
struct ftrace_graph_ent {
unsigned long func; /* Current function */
- int depth;
+ unsigned long depth;
} __packed;
/*
* Structure that defines an entry function trace with retaddr.
- * It's already packed but the attribute "packed" is needed
- * to remove extra padding at the end.
*/
struct fgraph_retaddr_ent {
- unsigned long func; /* Current function */
- int depth;
+ struct ftrace_graph_ent ent;
unsigned long retaddr; /* Return address */
} __packed;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 58be6d741d72..a06d836b7d92 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -962,7 +962,8 @@ extern int __trace_graph_entry(struct trace_array *tr,
extern int __trace_graph_retaddr_entry(struct trace_array *tr,
struct ftrace_graph_ent *trace,
unsigned int trace_ctx,
- unsigned long retaddr);
+ unsigned long retaddr,
+ struct ftrace_regs *fregs);
extern void __trace_graph_return(struct trace_array *tr,
struct ftrace_graph_ret *trace,
unsigned int trace_ctx,
@@ -2287,4 +2288,25 @@ static inline int rv_init_interface(void)
*/
#define FTRACE_TRAMPOLINE_MARKER ((unsigned long) INT_MAX)
+/*
+ * This is used to get the address of the args array based on
+ * the type of the entry.
+ */
+#define FGRAPH_ENTRY_ARGS(e) \
+ ({ \
+ unsigned long *_args; \
+ struct ftrace_graph_ent_entry *_e = e; \
+ \
+ if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) && \
+ e->ent.type == TRACE_GRAPH_RETADDR_ENT) { \
+ struct fgraph_retaddr_ent_entry *_re; \
+ \
+ _re = (typeof(_re))_e; \
+ _args = _re->args; \
+ } else { \
+ _args = _e->args; \
+ } \
+ _args; \
+ })
+
#endif /* _LINUX_KERNEL_TRACE_H */
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index de294ae2c5c5..f6a8d29c0d76 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -80,11 +80,11 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry,
F_STRUCT(
__field_struct( struct ftrace_graph_ent, graph_ent )
__field_packed( unsigned long, graph_ent, func )
- __field_packed( unsigned int, graph_ent, depth )
+ __field_packed( unsigned long, graph_ent, depth )
__dynamic_array(unsigned long, args )
),
- F_printk("--> %ps (%u)", (void *)__entry->func, __entry->depth)
+ F_printk("--> %ps (%lu)", (void *)__entry->func, __entry->depth)
);
#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
@@ -95,13 +95,14 @@ FTRACE_ENTRY_PACKED(fgraph_retaddr_entry, fgraph_retaddr_ent_entry,
TRACE_GRAPH_RETADDR_ENT,
F_STRUCT(
- __field_struct( struct fgraph_retaddr_ent, graph_ent )
- __field_packed( unsigned long, graph_ent, func )
- __field_packed( unsigned int, graph_ent, depth )
- __field_packed( unsigned long, graph_ent, retaddr )
+ __field_struct( struct fgraph_retaddr_ent, graph_rent )
+ __field_packed( unsigned long, graph_rent.ent, func )
+ __field_packed( unsigned long, graph_rent.ent, depth )
+ __field_packed( unsigned long, graph_rent, retaddr )
+ __dynamic_array(unsigned long, args )
),
- F_printk("--> %ps (%u) <- %ps", (void *)__entry->func, __entry->depth,
+ F_printk("--> %ps (%lu) <- %ps", (void *)__entry->func, __entry->depth,
(void *)__entry->retaddr)
);
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 44d5dc5031e2..8a075d3c4563 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -36,14 +36,19 @@ struct fgraph_ent_args {
unsigned long args[FTRACE_REGS_MAX_ARGS];
};
+struct fgraph_retaddr_ent_args {
+ struct fgraph_retaddr_ent_entry ent;
+ /* Force the sizeof of args[] to have FTRACE_REGS_MAX_ARGS entries */
+ unsigned long args[FTRACE_REGS_MAX_ARGS];
+};
+
struct fgraph_data {
struct fgraph_cpu_data __percpu *cpu_data;
/* Place to preserve last processed entry. */
union {
struct fgraph_ent_args ent;
- /* TODO allow retaddr to have args */
- struct fgraph_retaddr_ent_entry rent;
+ struct fgraph_retaddr_ent_args rent;
};
struct ftrace_graph_ret_entry ret;
int failed;
@@ -160,20 +165,32 @@ int __trace_graph_entry(struct trace_array *tr,
int __trace_graph_retaddr_entry(struct trace_array *tr,
struct ftrace_graph_ent *trace,
unsigned int trace_ctx,
- unsigned long retaddr)
+ unsigned long retaddr,
+ struct ftrace_regs *fregs)
{
struct ring_buffer_event *event;
struct trace_buffer *buffer = tr->array_buffer.buffer;
struct fgraph_retaddr_ent_entry *entry;
+ int size;
+
+ /* If fregs is defined, add FTRACE_REGS_MAX_ARGS long size words */
+ size = sizeof(*entry) + (FTRACE_REGS_MAX_ARGS * !!fregs * sizeof(long));
event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_RETADDR_ENT,
- sizeof(*entry), trace_ctx);
+ size, trace_ctx);
if (!event)
return 0;
entry = ring_buffer_event_data(event);
- entry->graph_ent.func = trace->func;
- entry->graph_ent.depth = trace->depth;
- entry->graph_ent.retaddr = retaddr;
+ entry->graph_rent.ent = *trace;
+ entry->graph_rent.retaddr = retaddr;
+
+#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
+ if (fregs) {
+ for (int i = 0; i < FTRACE_REGS_MAX_ARGS; i++)
+ entry->args[i] = ftrace_regs_get_argument(fregs, i);
+ }
+#endif
+
trace_buffer_unlock_commit_nostack(buffer, event);
return 1;
@@ -182,7 +199,8 @@ int __trace_graph_retaddr_entry(struct trace_array *tr,
int __trace_graph_retaddr_entry(struct trace_array *tr,
struct ftrace_graph_ent *trace,
unsigned int trace_ctx,
- unsigned long retaddr)
+ unsigned long retaddr,
+ struct ftrace_regs *fregs)
{
return 1;
}
@@ -267,7 +285,8 @@ static int graph_entry(struct ftrace_graph_ent *trace,
if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) &&
tracer_flags_is_set(tr, TRACE_GRAPH_PRINT_RETADDR)) {
unsigned long retaddr = ftrace_graph_top_ret_addr(current);
- ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx, retaddr);
+ ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx,
+ retaddr, fregs);
} else {
ret = __graph_entry(tr, trace, trace_ctx, fregs);
}
@@ -654,13 +673,9 @@ get_return_for_leaf(struct trace_iterator *iter,
* Save current and next entries for later reference
* if the output fails.
*/
- if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT)) {
- data->rent = *(struct fgraph_retaddr_ent_entry *)curr;
- } else {
- int size = min((int)sizeof(data->ent), (int)iter->ent_size);
+ int size = min_t(int, sizeof(data->rent), iter->ent_size);
- memcpy(&data->ent, curr, size);
- }
+ memcpy(&data->rent, curr, size);
/*
* If the next event is not a return type, then
* we only care about what type it is. Otherwise we can
@@ -838,7 +853,7 @@ static void print_graph_retaddr(struct trace_seq *s, struct fgraph_retaddr_ent_e
trace_seq_puts(s, " /*");
trace_seq_puts(s, " <-");
- seq_print_ip_sym_offset(s, entry->graph_ent.retaddr, trace_flags);
+ seq_print_ip_sym_offset(s, entry->graph_rent.retaddr, trace_flags);
if (comment)
trace_seq_puts(s, " */");
@@ -984,7 +999,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,
trace_seq_printf(s, "%ps", (void *)ret_func);
if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
- print_function_args(s, entry->args, ret_func);
+ print_function_args(s, FGRAPH_ENTRY_ARGS(entry), ret_func);
trace_seq_putc(s, ';');
} else
trace_seq_puts(s, "();");
@@ -1036,7 +1051,7 @@ print_graph_entry_nested(struct trace_iterator *iter,
args_size = iter->ent_size - offsetof(struct ftrace_graph_ent_entry, args);
if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long))
- print_function_args(s, entry->args, func);
+ print_function_args(s, FGRAPH_ENTRY_ARGS(entry), func);
else
trace_seq_puts(s, "()");
@@ -1218,11 +1233,14 @@ print_graph_entry(struct ftrace_graph_ent_entry *field, struct trace_seq *s,
/*
* print_graph_entry() may consume the current event,
* thus @field may become invalid, so we need to save it.
- * sizeof(struct ftrace_graph_ent_entry) is very small,
- * it can be safely saved at the stack.
+ * This function is shared by ftrace_graph_ent_entry and
+ * fgraph_retaddr_ent_entry, the size of the latter one
+ * is larger, but it is very small and can be safely saved
+ * at the stack.
*/
struct ftrace_graph_ent_entry *entry;
- u8 save_buf[sizeof(*entry) + FTRACE_REGS_MAX_ARGS * sizeof(long)];
+ struct fgraph_retaddr_ent_entry *rentry;
+ u8 save_buf[sizeof(*rentry) + FTRACE_REGS_MAX_ARGS * sizeof(long)];
/* The ent_size is expected to be as big as the entry */
if (iter->ent_size > sizeof(save_buf))
@@ -1451,12 +1469,17 @@ print_graph_function_flags(struct trace_iterator *iter, u32 flags)
}
#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
case TRACE_GRAPH_RETADDR_ENT: {
- struct fgraph_retaddr_ent_entry saved;
+ /*
+ * ftrace_graph_ent_entry and fgraph_retaddr_ent_entry have
+ * similar functions and memory layouts. The only difference
+ * is that the latter one has an extra retaddr member, so
+ * they can share most of the logic.
+ */
struct fgraph_retaddr_ent_entry *rfield;
trace_assign_type(rfield, entry);
- saved = *rfield;
- return print_graph_entry((struct ftrace_graph_ent_entry *)&saved, s, iter, flags);
+ return print_graph_entry((struct ftrace_graph_ent_entry *)rfield,
+ s, iter, flags);
}
#endif
case TRACE_GRAPH_RET: {
--
2.34.1
Powered by blists - more mailing lists