* [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Steven Rostedt @ 2025-11-18 0:29 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Thomas Gleixner, Ian Rogers, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
This series adds a perf event to the ftrace ring buffer.
It is currently a proof of concept as I'm not happy with the interface
and I also think the recorded perf event format may be changed too.
This proof-of-concept interface (which I have no plans on using), currently
just adds 6 new trace options.
event_cache_misses
event_cpu_cycles
func-cache-misses
func-cpu-cycles
funcgraph-cache-misses
funcgraph-cpu-cycles
The first two trigger a perf event after every event, the second two trigger
a perf event after every function and the last two trigger a perf event
right after the start of a function and again at the end of the function.
As this will eventual work with many more perf events than just cache-misses
and cpu-cycles , using options is not appropriate. Especially since the
options are limited to a 64 bit bitmask, and that can easily go much higher.
I'm thinking about having a file instead that will act as a way to enable
perf events for events, function and function graph tracing.
set_event_perf, set_ftrace_perf, set_fgraph_perf
And an available_perf_events that show what can be written into these files,
(similar to how set_ftrace_filter works). But for now, it was just easier to
implement them as options.
As for the perf event that is triggered. It currently is a dynamic array of
64 bit values. Each value is broken up into 8 bits for what type of perf
event it is, and 56 bits for the counter. It only writes a per CPU raw
counter and does not do any math. That would be needed to be done by any
post processing.
Since the values are for user space to do the subtraction to figure out the
difference between events, for example, the function_graph tracer may have:
is_vmalloc_addr() {
/* cpu_cycles: 5582263593 cache_misses: 2869004572 */
/* cpu_cycles: 5582267527 cache_misses: 2869006049 */
}
User space would subtract 2869006049 - 2869004572 = 1477
Then 56 bits should be plenty.
2^55 / 1,000,000,000 / 60 / 60 / 24 = 416
416 / 4 = 104
If you have a 4GHz machine, the cpu-cycles will overflow the 55 bits in 104
days. This tooling is not for seeing how many cycles run over 104 days.
User space tooling would just need to be aware that the vale is 56 bits and
when calculating the difference between start and end do something like:
if (start > end)
end |= 1ULL << 56;
delta = end - start;
The next question is how to label the perf events to be in the 8 bit
portion. It could simply be a value that is registered, and listed in the
available_perf_events file.
cpu_cycles:1
cach_misses:2
[..]
And this would need to be recorded by any tooling reading the events
so that it knows how to map the events with their attached ids.
But again, this is just a proof-of-concept. How this will eventually be
implemented is yet to be determined.
But to test these patches (which are based on top of my linux-next branch,
which should now be in linux-next):
# cd /sys/kernel/tracing
# echo 1 > options/event_cpu_cycles
# echo 1 > options/event_cache_misses
# echo 1 > events/syscalls/enable
# cat trace
[..]
bash-995 [007] ..... 98.255252: sys_write -> 0x2
bash-995 [007] ..... 98.255257: cpu_cycles: 1557241774 cache_misses: 449901166
bash-995 [007] ..... 98.255284: sys_dup2(oldfd: 0xa, newfd: 1)
bash-995 [007] ..... 98.255285: cpu_cycles: 1557260057 cache_misses: 449902679
bash-995 [007] ..... 98.255305: sys_dup2 -> 0x1
bash-995 [007] ..... 98.255305: cpu_cycles: 1557280203 cache_misses: 449906196
bash-995 [007] ..... 98.255343: sys_fcntl(fd: 0xa, cmd: 1, arg: 0)
bash-995 [007] ..... 98.255344: cpu_cycles: 1557322304 cache_misses: 449915522
bash-995 [007] ..... 98.255352: sys_fcntl -> 0x1
bash-995 [007] ..... 98.255353: cpu_cycles: 1557327809 cache_misses: 449916844
bash-995 [007] ..... 98.255361: sys_close(fd: 0xa)
bash-995 [007] ..... 98.255362: cpu_cycles: 1557335383 cache_misses: 449918232
bash-995 [007] ..... 98.255369: sys_close -> 0x0
Comments welcomed.
Steven Rostedt (3):
tracing: Add perf events
ftrace: Add perf counters to function tracing
fgraph: Add perf counters to function graph tracer
----
include/linux/trace_recursion.h | 5 +-
kernel/trace/trace.c | 153 ++++++++++++++++++++++++++++++++-
kernel/trace/trace.h | 38 ++++++++
kernel/trace/trace_entries.h | 13 +++
kernel/trace/trace_event_perf.c | 162 +++++++++++++++++++++++++++++++++++
kernel/trace/trace_functions.c | 124 +++++++++++++++++++++++++--
kernel/trace/trace_functions_graph.c | 117 +++++++++++++++++++++++--
kernel/trace/trace_output.c | 70 +++++++++++++++
8 files changed, 670 insertions(+), 12 deletions(-)
^ permalink raw reply
* [POC][RFC][PATCH 2/3] ftrace: Add perf counters to function tracing
From: Steven Rostedt @ 2025-11-18 0:29 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Thomas Gleixner, Ian Rogers, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <20251118002950.680329246@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Add option to trigger perf events to function tracing.
Two new options are added: func-cpu-cycles and func-cache-misses
# cd /sys/kernel/tracing
# echo 1 > options/func-cache-misses
# echo function > current_tracer
# cat trace
[..]
sshd-session-1014 [005] ..... 327.836708: __x64_sys_read <-do_syscall_64
sshd-session-1014 [005] ..... 327.836708: cache_misses: 741719054
sshd-session-1014 [005] ..... 327.836712: ksys_read <-do_syscall_64
sshd-session-1014 [005] ..... 327.836713: cache_misses: 741720271
sshd-session-1014 [005] ..... 327.836716: fdget_pos <-ksys_read
sshd-session-1014 [005] ..... 327.836717: cache_misses: 741721483
sshd-session-1014 [005] ..... 327.836720: vfs_read <-ksys_read
sshd-session-1014 [005] ..... 327.836721: cache_misses: 741722726
sshd-session-1014 [005] ..... 327.836724: rw_verify_area <-vfs_read
sshd-session-1014 [005] ..... 327.836725: cache_misses: 741723940
sshd-session-1014 [005] ..... 327.836728: security_file_permission <-rw_verify_area
sshd-session-1014 [005] ..... 327.836729: cache_misses: 741725151
The option will cause the perf event to be triggered after every function
called.
If cpu_cycles is also enabled:
# echo 1 > options/func-cpu-cycles
# cat trace
[..]
sshd-session-1014 [005] b..1. 536.844538: preempt_count_sub <-_raw_spin_unlock
sshd-session-1014 [005] b..1. 536.844539: cpu_cycles: 1919425978 cache_misses: 3431216952
sshd-session-1014 [005] b.... 536.844545: validate_xmit_skb_list <-sch_direct_xmit
sshd-session-1014 [005] b.... 536.844545: cpu_cycles: 1919429935 cache_misses: 3431218535
sshd-session-1014 [005] b.... 536.844551: validate_xmit_skb.isra.0 <-validate_xmit_skb_list
sshd-session-1014 [005] b.... 536.844552: cpu_cycles: 1919433763 cache_misses: 3431220112
sshd-session-1014 [005] b.... 536.844557: netif_skb_features <-validate_xmit_skb.isra.0
sshd-session-1014 [005] b.... 536.844558: cpu_cycles: 1919437574 cache_misses: 3431221688
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_recursion.h | 5 +-
kernel/trace/trace.c | 58 ++++++++++++---
kernel/trace/trace.h | 6 ++
kernel/trace/trace_functions.c | 124 ++++++++++++++++++++++++++++++--
4 files changed, 178 insertions(+), 15 deletions(-)
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index ae04054a1be3..c42d86d81afa 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -132,9 +132,12 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
* will think a recursion occurred, and the event will be dropped.
* Let a single instance happen via the TRANSITION_BIT to
* not drop those events.
+ *
+ * When ip is zero, the caller is purposely trying causing
+ * recursion. Don't record it.
*/
bit = TRACE_CTX_TRANSITION + start;
- if (val & (1 << bit)) {
+ if ((val & (1 << bit)) && ip) {
do_ftrace_record_recursion(ip, pip);
return -1;
}
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 64d966a3ec8b..42bf1c046de1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2915,21 +2915,18 @@ void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
#ifdef CONFIG_PERF_EVENTS
-static inline void record_perf_event(struct trace_array *tr,
- struct trace_buffer *buffer,
- unsigned int trace_ctx)
+static inline void trace_perf_event(struct trace_array *tr,
+ struct trace_buffer *buffer,
+ int entries, u64 flags,
+ unsigned int trace_ctx)
{
struct ring_buffer_event *event;
struct perf_event_entry *entry;
- int entries = READ_ONCE(tr->perf_events);
struct trace_array_cpu *data;
u64 *value;
int size;
int cpu;
- if (!entries)
- return;
-
guard(preempt_notrace)();
cpu = smp_processor_id();
@@ -2949,12 +2946,12 @@ static inline void record_perf_event(struct trace_array *tr,
entry = ring_buffer_event_data(event);
value = entry->values;
- if (tr->trace_flags & TRACE_ITER(PERF_CYCLES)) {
+ if (flags & TRACE_ITER(PERF_CYCLES)) {
*value++ = TRACE_PERF_VALUE(PERF_TRACE_CYCLES);
entries--;
}
- if (entries && tr->trace_flags & TRACE_ITER(PERF_CACHE)) {
+ if (entries && flags & TRACE_ITER(PERF_CACHE)) {
*value++ = TRACE_PERF_VALUE(PERF_TRACE_CACHE);
entries--;
}
@@ -2968,6 +2965,49 @@ static inline void record_perf_event(struct trace_array *tr,
local_dec(&data->disabled);
}
+static inline void record_perf_event(struct trace_array *tr,
+ struct trace_buffer *buffer,
+ unsigned int trace_ctx)
+{
+ int entries = READ_ONCE(tr->perf_events);
+
+ if (!entries)
+ return;
+
+ trace_perf_event(tr, buffer, entries, tr->trace_flags, trace_ctx);
+}
+
+#ifdef CONFIG_FUNCTION_TRACER
+void ftrace_perf_events(struct trace_array *tr, int perf_events,
+ u64 perf_mask, unsigned int trace_ctx)
+{
+ struct trace_buffer *buffer;
+ int bit;
+
+ /*
+ * Prevent any ftrace recursion.
+ * The ftrace_test_recursion_trylock() allows one nested loop
+ * to handle the case where an interrupt comes in and traces
+ * before the preempt_count is updated to the new context.
+ * This one instance allows that function to still be traced.
+ *
+ * The trace_perf_cache_misses() will call functions that function
+ * tracing will want to trace. Prevent this one loop from happening
+ * by taking the the lock again. If an interrupt comes in now,
+ * it may still be dropped, but there's really nothing that can
+ * be done about that until all those locations get fixed.
+ */
+ bit = ftrace_test_recursion_trylock(0, 0);
+
+ buffer = tr->array_buffer.buffer;
+ trace_perf_event(tr, buffer, perf_events, perf_mask, trace_ctx);
+
+ /* bit < 0 means the trylock failed and does not need to be unlocked */
+ if (bit >= 0)
+ ftrace_test_recursion_unlock(bit);
+}
+#endif
+
static int handle_perf_event(struct trace_array *tr, u64 mask, int enabled)
{
int ret = 0;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 094a156b0c70..bb764a2255c7 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -365,6 +365,8 @@ struct trace_array {
int buffer_disabled;
int perf_events;
+ int ftrace_perf_events;
+ u64 ftrace_perf_mask;
struct trace_pid_list __rcu *filtered_pids;
struct trace_pid_list __rcu *filtered_no_pids;
@@ -1402,6 +1404,10 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
u64 do_trace_perf_event(int type);
int trace_perf_event_enable(int type);
void trace_perf_event_disable(int type);
+#ifdef CONFIG_FUNCTION_TRACER
+void ftrace_perf_events(struct trace_array *tr, int perf_events,
+ u64 perf_mask, unsigned int trace_ctx);
+#endif
#else
# define PERF_FLAGS
static inline u64 do_trace_perf_event(int type) { return 0; }
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index c12795c2fb39..97f46ac7ef21 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -47,8 +47,12 @@ enum {
TRACE_FUNC_OPT_NO_REPEATS = 0x2,
TRACE_FUNC_OPT_ARGS = 0x4,
- /* Update this to next highest bit. */
- TRACE_FUNC_OPT_HIGHEST_BIT = 0x8
+ /* Update this to next highest function bit. */
+ TRACE_FUNC_OPT_HIGHEST_BIT = 0x8,
+
+ /* These are just other options */
+ TRACE_FUNC_OPT_PERF_CYCLES = 0x10,
+ TRACE_FUNC_OPT_PERF_CACHE = 0x20,
};
#define TRACE_FUNC_OPT_MASK (TRACE_FUNC_OPT_HIGHEST_BIT - 1)
@@ -143,6 +147,105 @@ static bool handle_func_repeats(struct trace_array *tr, u32 flags_val)
return true;
}
+#ifdef CONFIG_PERF_EVENTS
+static inline void
+do_trace_function(struct trace_array *tr, unsigned long ip,
+ unsigned long parent_ip, unsigned int trace_ctx,
+ struct ftrace_regs *fregs)
+{
+ trace_function(tr, ip, parent_ip, trace_ctx, fregs);
+
+ if (likely(!tr->ftrace_perf_events))
+ return;
+
+ ftrace_perf_events(tr, tr->ftrace_perf_events, tr->ftrace_perf_mask, trace_ctx);
+}
+
+static bool handle_perf_event_flag(struct trace_array *tr, int bit, int set, int *err)
+{
+ u64 mask;
+ int type;
+
+ *err = 0;
+
+ switch (bit) {
+ case TRACE_FUNC_OPT_PERF_CYCLES:
+ mask = TRACE_ITER(PERF_CYCLES);
+ type = PERF_TRACE_CYCLES;
+ break;
+
+ case TRACE_FUNC_OPT_PERF_CACHE:
+ mask = TRACE_ITER(PERF_CACHE);
+ type = PERF_TRACE_CACHE;
+ break;
+
+ default:
+ return 0;
+ }
+
+ if (set)
+ *err = trace_perf_event_enable(type);
+ else
+ trace_perf_event_disable(type);
+
+ if (*err < 0)
+ return 1;
+
+ if (set) {
+ tr->ftrace_perf_events++;
+ tr->ftrace_perf_mask |= mask;
+ } else {
+ tr->ftrace_perf_mask &= ~mask;
+ tr->ftrace_perf_events--;
+ }
+ return 1;
+}
+
+static void ftrace_perf_enable(struct trace_array *tr, int bit)
+{
+ int err;
+
+ if (!(tr->current_trace_flags->val & bit))
+ return;
+
+ handle_perf_event_flag(tr, bit, 1, &err);
+ if (err < 0)
+ tr->current_trace_flags->val &= ~bit;
+}
+
+static void ftrace_perf_disable(struct trace_array *tr, int bit)
+{
+ int err;
+
+ /* Only disable if it was enabled */
+ if (!(tr->current_trace_flags->val & bit))
+ return;
+
+ handle_perf_event_flag(tr, bit, 0, &err);
+}
+
+static void ftrace_perf_init(struct trace_array *tr)
+{
+ ftrace_perf_enable(tr, TRACE_FUNC_OPT_PERF_CYCLES);
+ ftrace_perf_enable(tr, TRACE_FUNC_OPT_PERF_CACHE);
+}
+
+static void ftrace_perf_reset(struct trace_array *tr)
+{
+ ftrace_perf_disable(tr, TRACE_FUNC_OPT_PERF_CYCLES);
+ ftrace_perf_disable(tr, TRACE_FUNC_OPT_PERF_CACHE);
+}
+#else
+#define do_trace_function trace_function
+static inline bool handle_perf_event_flag(struct trace_array *tr, int bit,
+ int set, int *err)
+{
+ return 0;
+}
+static inline void ftrace_perf_init(struct trace_array *tr) { }
+static inline void ftrace_perf_reset(struct trace_array *tr) { }
+#endif /* CONFIG_PERF_EVENTS */
+
static int function_trace_init(struct trace_array *tr)
{
ftrace_func_t func;
@@ -165,6 +268,8 @@ static int function_trace_init(struct trace_array *tr)
tr->array_buffer.cpu = raw_smp_processor_id();
+ ftrace_perf_init(tr);
+
tracing_start_cmdline_record();
tracing_start_function_trace(tr);
return 0;
@@ -172,6 +277,7 @@ static int function_trace_init(struct trace_array *tr)
static void function_trace_reset(struct trace_array *tr)
{
+ ftrace_perf_reset(tr);
tracing_stop_function_trace(tr);
tracing_stop_cmdline_record();
ftrace_reset_array_ops(tr);
@@ -223,7 +329,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx_dec();
- trace_function(tr, ip, parent_ip, trace_ctx, NULL);
+ do_trace_function(tr, ip, parent_ip, trace_ctx, NULL);
ftrace_test_recursion_unlock(bit);
}
@@ -245,7 +351,7 @@ function_args_trace_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx();
- trace_function(tr, ip, parent_ip, trace_ctx, fregs);
+ do_trace_function(tr, ip, parent_ip, trace_ctx, fregs);
ftrace_test_recursion_unlock(bit);
}
@@ -372,7 +478,7 @@ function_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx_dec();
process_repeats(tr, ip, parent_ip, last_info, trace_ctx);
- trace_function(tr, ip, parent_ip, trace_ctx, NULL);
+ do_trace_function(tr, ip, parent_ip, trace_ctx, NULL);
out:
ftrace_test_recursion_unlock(bit);
@@ -428,6 +534,10 @@ static struct tracer_opt func_opts[] = {
{ TRACER_OPT(func-no-repeats, TRACE_FUNC_OPT_NO_REPEATS) },
#ifdef CONFIG_FUNCTION_TRACE_ARGS
{ TRACER_OPT(func-args, TRACE_FUNC_OPT_ARGS) },
+#endif
+#if CONFIG_PERF_EVENTS
+ { TRACER_OPT(func-cpu-cycles, TRACE_FUNC_OPT_PERF_CYCLES) },
+ { TRACER_OPT(func-cache-misses, TRACE_FUNC_OPT_PERF_CACHE) },
#endif
{ } /* Always set a last empty entry */
};
@@ -457,6 +567,7 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
{
ftrace_func_t func;
u32 new_flags;
+ int err;
/* Do nothing if already set. */
if (!!set == !!(tr->current_trace_flags->val & bit))
@@ -466,6 +577,9 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
if (tr->current_trace != &function_trace)
return 0;
+ if (handle_perf_event_flag(tr, bit, set, &err))
+ return err;
+
new_flags = (tr->current_trace_flags->val & ~bit) | (set ? bit : 0);
func = select_trace_function(new_flags);
if (!func)
--
2.51.0
^ permalink raw reply related
* [POC][RFC][PATCH 1/3] tracing: Add perf events
From: Steven Rostedt @ 2025-11-18 0:29 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Thomas Gleixner, Ian Rogers, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <20251118002950.680329246@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Add perf events into the ftrace ring buffer. Create a new ftrace event
called a "perf_event". This event contains a dynamic array of u64 words.
Each entry allows to read 56 bits of the raw content of a perf PMU value
into the word leaving 8 bits as an identifier for what word that is.
One may ask "what happens when the counter is greater than 56 bits". The
answer is that you really shouldn't care. The value is written for user
space to consume and do any calculations. If one wants to see the
difference between two events, they can simply subtract the previous one
from the next one. If there is a wrap over the 56 bits, then adding a
"1ULL << 56" to the second value if it is less than the first will give
the correct result.
"What happens if the difference of the counters is 1 << 55 apart?"
Let's look at CPU cycles, as they probably go up the quickest. At 4GHz,
that would be 4,000,000,000 times a second.
1 << 55 / 400000000 = 9007199 seconds
9007199 / 60 = 150119 minutes
150119 / 60 = 2501 hours
2501 / 24 = 104 days!
This will not work if you want to see the number of cycles between two
events if those two events are 104 days apart. Do we care?
Currently only cpu cycles and cache misses are supported, but more can be
added in the future.
Two new options are added: event_cache_misses and event_cpu_cycles
# cd /sys/kernel/tracing
# echo 1 > options/event_cache_misses
# echo 1 > events/syscalls/enable
# cat trace
[..]
bash-1009 [005] ..... 566.863956: sys_write -> 0x2
bash-1009 [005] ..... 566.863973: cache_misses: 26544738
bash-1009 [005] ..... 566.864003: sys_dup2(oldfd: 0xa, newfd: 1)
bash-1009 [005] ..... 566.864004: cache_misses: 26546241
bash-1009 [005] ..... 566.864021: sys_dup2 -> 0x1
bash-1009 [005] ..... 566.864022: cache_misses: 26549598
bash-1009 [005] ..... 566.864059: sys_fcntl(fd: 0xa, cmd: 1, arg: 0)
bash-1009 [005] ..... 566.864060: cache_misses: 26558778
The option will cause the perf event to be triggered after every event.
If cpu_cycles is also enabled:
# echo 1 > options/event_cpu_cycles
# cat trace
[..]
bash-1009 [006] ..... 683.223244: sys_write -> 0x2
bash-1009 [006] ..... 683.223245: cpu_cycles: 273245 cache_misses: 40481492
bash-1009 [006] ..... 683.223262: sys_dup2(oldfd: 0xa, newfd: 1)
bash-1009 [006] ..... 683.223263: cpu_cycles: 286640 cache_misses: 40483017
bash-1009 [006] ..... 683.223278: sys_dup2 -> 0x1
bash-1009 [006] ..... 683.223279: cpu_cycles: 301412 cache_misses: 40486560
bash-1009 [006] ..... 683.223309: sys_fcntl(fd: 0xa, cmd: 1, arg: 0)
bash-1009 [006] ..... 683.223310: cpu_cycles: 335188 cache_misses: 40495672
bash-1009 [006] ..... 683.223317: sys_fcntl -> 0x1
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 113 +++++++++++++++++++++-
kernel/trace/trace.h | 28 ++++++
kernel/trace/trace_entries.h | 13 +++
kernel/trace/trace_event_perf.c | 162 ++++++++++++++++++++++++++++++++
kernel/trace/trace_output.c | 70 ++++++++++++++
5 files changed, 385 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 59cd4ed8af6d..64d966a3ec8b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1110,7 +1110,6 @@ void tracing_on(void)
}
EXPORT_SYMBOL_GPL(tracing_on);
-
static __always_inline void
__buffer_unlock_commit(struct trace_buffer *buffer, struct ring_buffer_event *event)
{
@@ -2915,6 +2914,103 @@ void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
}
EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+#ifdef CONFIG_PERF_EVENTS
+static inline void record_perf_event(struct trace_array *tr,
+ struct trace_buffer *buffer,
+ unsigned int trace_ctx)
+{
+ struct ring_buffer_event *event;
+ struct perf_event_entry *entry;
+ int entries = READ_ONCE(tr->perf_events);
+ struct trace_array_cpu *data;
+ u64 *value;
+ int size;
+ int cpu;
+
+ if (!entries)
+ return;
+
+ guard(preempt_notrace)();
+ cpu = smp_processor_id();
+
+ /* Prevent this from recursing */
+ data = per_cpu_ptr(tr->array_buffer.data, cpu);
+ if (unlikely(!data) || local_read(&data->disabled))
+ return;
+
+ if (local_inc_return(&data->disabled) != 1)
+ goto out;
+
+ size = struct_size(entry, values, entries);
+ event = trace_buffer_lock_reserve(buffer, TRACE_PERF_EVENT, size,
+ trace_ctx);
+ if (!event)
+ goto out;
+ entry = ring_buffer_event_data(event);
+ value = entry->values;
+
+ if (tr->trace_flags & TRACE_ITER(PERF_CYCLES)) {
+ *value++ = TRACE_PERF_VALUE(PERF_TRACE_CYCLES);
+ entries--;
+ }
+
+ if (entries && tr->trace_flags & TRACE_ITER(PERF_CACHE)) {
+ *value++ = TRACE_PERF_VALUE(PERF_TRACE_CACHE);
+ entries--;
+ }
+
+ /* If something changed, zero the rest */
+ if (unlikely(entries))
+ memset(value, 0, sizeof(u64) * entries);
+
+ trace_buffer_unlock_commit_nostack(buffer, event);
+ out:
+ local_dec(&data->disabled);
+}
+
+static int handle_perf_event(struct trace_array *tr, u64 mask, int enabled)
+{
+ int ret = 0;
+ int type;
+
+ switch (mask) {
+
+ case TRACE_ITER(PERF_CYCLES):
+ type = PERF_TRACE_CYCLES;
+ break;
+ case TRACE_ITER(PERF_CACHE):
+ type = PERF_TRACE_CACHE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (enabled)
+ ret = trace_perf_event_enable(type);
+ else
+ trace_perf_event_disable(type);
+
+ if (ret < 0)
+ return ret;
+
+ if (enabled)
+ tr->perf_events++;
+ else
+ tr->perf_events--;
+
+ if (WARN_ON_ONCE(tr->perf_events < 0))
+ tr->perf_events = 0;
+
+ return 0;
+}
+#else
+static inline void record_perf_event(struct trace_array *tr,
+ struct trace_buffer *buffer,
+ unsigned int trace_ctx)
+{
+}
+#endif
+
/*
* Skip 3:
*
@@ -2932,6 +3028,8 @@ void trace_buffer_unlock_commit_regs(struct trace_array *tr,
{
__buffer_unlock_commit(buffer, event);
+ record_perf_event(tr, buffer, trace_ctx);
+
/*
* If regs is not set, then skip the necessary functions.
* Note, we can still get here via blktrace, wakeup tracer
@@ -5287,7 +5385,20 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
update_marker_trace(tr, enabled);
/* update_marker_trace updates the tr->trace_flags */
return 0;
+
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_ITER(PERF_CACHE):
+ case TRACE_ITER(PERF_CYCLES):
+ {
+ int ret = 0;
+
+ ret = handle_perf_event(tr, mask, enabled);
+ if (ret < 0)
+ return ret;
+ break;
}
+#endif
+ } /* switch (mask) */
if (enabled)
tr->trace_flags |= mask;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 58be6d741d72..094a156b0c70 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -56,6 +56,7 @@ enum trace_type {
TRACE_TIMERLAT,
TRACE_RAW_DATA,
TRACE_FUNC_REPEATS,
+ TRACE_PERF_EVENT,
__TRACE_LAST_TYPE,
};
@@ -363,6 +364,8 @@ struct trace_array {
int buffer_disabled;
+ int perf_events;
+
struct trace_pid_list __rcu *filtered_pids;
struct trace_pid_list __rcu *filtered_no_pids;
/*
@@ -537,6 +540,7 @@ extern void __ftrace_bad_type(void);
IF_ASSIGN(var, ent, struct hwlat_entry, TRACE_HWLAT); \
IF_ASSIGN(var, ent, struct osnoise_entry, TRACE_OSNOISE);\
IF_ASSIGN(var, ent, struct timerlat_entry, TRACE_TIMERLAT);\
+ IF_ASSIGN(var, ent, struct perf_event_entry, TRACE_PERF_EVENT); \
IF_ASSIGN(var, ent, struct raw_data_entry, TRACE_RAW_DATA);\
IF_ASSIGN(var, ent, struct trace_mmiotrace_rw, \
TRACE_MMIO_RW); \
@@ -1382,6 +1386,29 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
# define TRACE_ITER_PROF_TEXT_OFFSET_BIT -1
#endif
+#ifdef CONFIG_PERF_EVENTS
+#define PERF_MAKE_VALUE(type, val) (((type) << 56) | ((val) & ~(0xffULL << 56)))
+/* Not required, but keep consistent with include/uapi/linux/perf_event.h */
+#define PERF_TRACE_CYCLES 0ULL
+#define PERF_TRACE_CACHE 5ULL
+#define TRACE_PERF_VALUE(type) \
+ PERF_MAKE_VALUE((type), do_trace_perf_event(type))
+#define PERF_TRACE_VALUE(val) ((val) & ~(0xffULL << 56))
+#define PERF_TRACE_TYPE(val) ((val) >> 56)
+# define PERF_FLAGS \
+ C(PERF_CACHE, "event_cache_misses"), \
+ C(PERF_CYCLES, "event_cpu_cycles"),
+
+u64 do_trace_perf_event(int type);
+int trace_perf_event_enable(int type);
+void trace_perf_event_disable(int type);
+#else
+# define PERF_FLAGS
+static inline u64 do_trace_perf_event(int type) { return 0; }
+static inline int trace_perf_event_enable(int type) { return -ENOTSUPP; }
+static inline void trace_perf_event_disable(int type) { }
+#endif /* CONFIG_PERF_EVENTS */
+
/*
* trace_iterator_flags is an enumeration that defines bit
* positions into trace_flags that controls the output.
@@ -1420,6 +1447,7 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
FUNCTION_FLAGS \
FGRAPH_FLAGS \
STACK_FLAGS \
+ PERF_FLAGS \
BRANCH_FLAGS \
PROFILER_FLAGS \
FPROFILE_FLAGS
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index de294ae2c5c5..ecda463a9d8e 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -456,3 +456,16 @@ FTRACE_ENTRY(timerlat, timerlat_entry,
__entry->context,
__entry->timer_latency)
);
+
+#ifdef CONFIG_PERF_EVENTS
+FTRACE_ENTRY(perf_event, perf_event_entry,
+
+ TRACE_PERF_EVENT,
+
+ F_STRUCT(
+ __dynamic_array(u64, values )
+ ),
+
+ F_printk("values: %lld\n", __entry->values[0])
+);
+#endif
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c5..ff864d300251 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -430,6 +430,168 @@ void perf_trace_buf_update(void *record, u16 type)
}
NOKPROBE_SYMBOL(perf_trace_buf_update);
+static void perf_callback(struct perf_event *event,
+ struct perf_sample_data *data,
+ struct pt_regs *regs)
+{
+ /* nop */
+}
+
+struct trace_perf_event {
+ struct perf_event *event;
+};
+
+static struct trace_perf_event __percpu *perf_cache_events;
+static struct trace_perf_event __percpu *perf_cycles_events;
+static DEFINE_MUTEX(perf_event_mutex);
+static int perf_cache_cnt;
+static int perf_cycles_cnt;
+
+static inline int set_perf_type(int type, int *ptype, int *pconfig, int **pcount,
+ struct trace_perf_event __percpu ***pevents)
+{
+ switch (type) {
+ case PERF_TRACE_CYCLES:
+ if (ptype)
+ *ptype = PERF_TYPE_HARDWARE;
+ if (pconfig)
+ *pconfig = PERF_COUNT_HW_CPU_CYCLES;
+ *pcount = &perf_cycles_cnt;
+ *pevents = &perf_cycles_events;
+ return 0;
+
+ case PERF_TRACE_CACHE:
+ if (ptype)
+ *ptype = PERF_TYPE_HW_CACHE;
+ if (pconfig)
+ *pconfig = PERF_COUNT_HW_CACHE_MISSES;
+ *pcount = &perf_cache_cnt;
+ *pevents = &perf_cache_events;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+u64 do_trace_perf_event(int type)
+{
+ struct trace_perf_event __percpu **pevents;
+ struct trace_perf_event __percpu *events;
+ struct perf_event *e;
+ int *count;
+ int cpu;
+
+ if (set_perf_type(type, NULL, NULL, &count, &pevents) < 0)
+ return 0;
+
+ if (!*count)
+ return 0;
+
+ guard(preempt)();
+
+ events = READ_ONCE(*pevents);
+ if (!events)
+ return 0;
+
+ cpu = smp_processor_id();
+
+ e = per_cpu_ptr(events, cpu)->event;
+ if (!e)
+ return 0;
+
+ e->pmu->read(e);
+ return local64_read(&e->count);
+}
+
+static void __free_trace_perf_events(struct trace_perf_event __percpu *events)
+{
+ struct perf_event *e;
+ int cpu;
+
+ for_each_online_cpu(cpu) {
+ e = per_cpu_ptr(events, cpu)->event;
+ per_cpu_ptr(events, cpu)->event = NULL;
+ perf_event_release_kernel(e);
+ }
+}
+
+int trace_perf_event_enable(int type)
+{
+ struct perf_event_attr __free(kfree) *attr = NULL;
+ struct trace_perf_event __percpu **pevents;
+ struct trace_perf_event __percpu *events;
+ struct perf_event *e;
+ int *count;
+ int config;
+ int cpu;
+
+ if (set_perf_type(type, &config, &type, &count, &pevents) < 0)
+ return -EINVAL;
+
+ guard(mutex)(&perf_event_mutex);
+
+ if (*count) {
+ (*count)++;
+ return 0;
+ }
+
+ attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+ if (!attr)
+ return -ENOMEM;
+
+ events = alloc_percpu(struct trace_perf_event);
+ if (!events)
+ return -ENOMEM;
+
+ attr->type = type;
+ attr->config = config;
+ attr->size = sizeof(struct perf_event_attr);
+ attr->pinned = 1;
+
+ /* initialize in case of failure */
+ for_each_possible_cpu(cpu) {
+ per_cpu_ptr(events, cpu)->event = NULL;
+ }
+
+ for_each_online_cpu(cpu) {
+ e = perf_event_create_kernel_counter(attr, cpu, NULL,
+ perf_callback, NULL);
+ if (IS_ERR_OR_NULL(e)) {
+ __free_trace_perf_events(events);
+ return PTR_ERR(e);;
+ }
+ per_cpu_ptr(events, cpu)->event = e;
+ }
+
+ WRITE_ONCE(*pevents, events);
+ (*count)++;
+
+ return 0;
+}
+
+void trace_perf_event_disable(int type)
+{
+ struct trace_perf_event __percpu **pevents;
+ struct trace_perf_event __percpu *events;
+ int *count;
+
+ if (set_perf_type(type, NULL, NULL, &count, &pevents) < 0)
+ return;
+
+ guard(mutex)(&perf_event_mutex);
+
+ if (WARN_ON_ONCE(!*count))
+ return;
+
+ if (--(*count))
+ return;
+
+ events = READ_ONCE(*pevents);
+ WRITE_ONCE(*pevents, NULL);
+
+ __free_trace_perf_events(events);
+}
+
#ifdef CONFIG_FUNCTION_TRACER
static void
perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index ebbab3e9622b..a0f21cec9eed 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1661,6 +1661,75 @@ static struct trace_event trace_timerlat_event = {
.funcs = &trace_timerlat_funcs,
};
+/* TRACE_PERF_EVENT */
+
+static enum print_line_t
+trace_perf_event_print(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
+{
+ struct trace_entry *entry = iter->ent;
+ struct trace_seq *s = &iter->seq;
+ struct perf_event_entry *field;
+ u64 value;
+ u64 *val;
+ u64 *end;
+
+ end = (u64 *)((long)iter->ent + iter->ent_size);
+
+ trace_assign_type(field, entry);
+
+ for (val = field->values; val < end; val++) {
+ if (val != field->values)
+ trace_seq_putc(s, ' ');
+ value = PERF_TRACE_VALUE(*val);
+ switch (PERF_TRACE_TYPE(*val)) {
+ case PERF_TRACE_CYCLES:
+ trace_seq_printf(s, "cpu_cycles: %lld", value);
+ break;
+ case PERF_TRACE_CACHE:
+ trace_seq_printf(s, "cache_misses: %lld", value);
+ break;
+ default:
+ trace_seq_printf(s, "unkown(%d): %lld",
+ (int)PERF_TRACE_TYPE(*val), value);
+ }
+ }
+ trace_seq_putc(s, '\n');
+ return trace_handle_return(s);
+}
+
+static enum print_line_t
+trace_perf_event_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
+{
+ struct perf_event_entry *field;
+ struct trace_seq *s = &iter->seq;
+ u64 *val;
+ u64 *end;
+
+ end = (u64 *)((long)iter->ent + iter->ent_size);
+
+ trace_assign_type(field, iter->ent);
+
+ for (val = field->values; val < end; val++) {
+ if (val != field->values)
+ trace_seq_putc(s, ' ');
+ trace_seq_printf(s, "%lld\n", *val);
+ }
+ trace_seq_putc(s, '\n');
+ return trace_handle_return(s);
+}
+
+static struct trace_event_functions trace_perf_event_funcs = {
+ .trace = trace_perf_event_print,
+ .raw = trace_perf_event_raw,
+};
+
+static struct trace_event trace_perf_event_event = {
+ .type = TRACE_PERF_EVENT,
+ .funcs = &trace_perf_event_funcs,
+};
+
/* TRACE_BPUTS */
static enum print_line_t
trace_bputs_print(struct trace_iterator *iter, int flags,
@@ -1878,6 +1947,7 @@ static struct trace_event *events[] __initdata = {
&trace_timerlat_event,
&trace_raw_data_event,
&trace_func_repeats_event,
+ &trace_perf_event_event,
NULL
};
--
2.51.0
^ permalink raw reply related
* [POC][RFC][PATCH 3/3] fgraph: Add perf counters to function graph tracer
From: Steven Rostedt @ 2025-11-18 0:29 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Thomas Gleixner, Ian Rogers, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <20251118002950.680329246@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Add option to trigger perf events to function graph tracing.
Two new options are added: funcgraph-cpu-cycles and funcgraph-cache-misses
This adds the perf event right after the start of a function and again
just before the end of a function.
# cd /sys/kernel/tracing
# echo 1 > options/funcgraph-cache-misses
# echo vfs_read > set_graph_function
# echo function_graph > current_tracer
# cat trace
[..]
5) | vfs_read() {
5) | /* cache_misses: 822565 */
5) | rw_verify_area() {
5) | /* cache_misses: 824003 */
5) | security_file_permission() {
5) | /* cache_misses: 825440 */
5) | apparmor_file_permission() {
5) | /* cache_misses: 826875 */
5) | aa_file_perm() {
5) | /* cache_misses: 828326 */
5) | __rcu_read_lock() {
5) | /* cache_misses: 829766 */
5) | /* cache_misses: 830785 */
5) 5.116 us | }
5) | __rcu_read_unlock() {
5) | /* cache_misses: 832611 */
5) | /* cache_misses: 833632 */
5) 5.223 us | }
5) | /* cache_misses: 835043 */
5) + 25.462 us | }
5) | /* cache_misses: 836454 */
5) + 35.518 us | }
5) | bpf_lsm_file_permission() {
5) | /* cache_misses: 838276 */
5) | /* cache_misses: 839292 */
5) 4.613 us | }
5) | /* cache_misses: 840697 */
5) + 54.684 us | }
5) | /* cache_misses: 842107 */
5) + 64.449 us | }
The option will cause the perf event to be triggered after every function
called.
If cpu_cycles is also enabled:
# echo 1 > options/funcgraph-cpu-cycles
# cat trace
[..]
3) | vfs_read() {
3) | /* cpu_cycles: 2947481793 cache_misses: 2002984031 */
3) | rw_verify_area() {
3) | /* cpu_cycles: 2947488061 cache_misses: 2002985922 */
3) | security_file_permission() {
3) | /* cpu_cycles: 2947492867 cache_misses: 2002987812 */
3) | apparmor_file_permission() {
3) | /* cpu_cycles: 2947497713 cache_misses: 2002989700 */
3) | aa_file_perm() {
3) | /* cpu_cycles: 2947502560 cache_misses: 2002991604 */
3) | __rcu_read_lock() {
3) | /* cpu_cycles: 2947507398 cache_misses: 2002993497 */
3) | /* cpu_cycles: 2947512435 cache_misses: 2002994969 */
3) 7.586 us | }
3) | __rcu_read_unlock() {
3) | /* cpu_cycles: 2947518226 cache_misses: 2002997248 */
3) | /* cpu_cycles: 2947522328 cache_misses: 2002998722 */
3) 7.211 us | }
3) | /* cpu_cycles: 2947527067 cache_misses: 2003000586 */
3) + 37.581 us | }
3) | /* cpu_cycles: 2947531727 cache_misses: 2003002450 */
3) + 52.061 us | }
3) | bpf_lsm_file_permission() {
3) | /* cpu_cycles: 2947537274 cache_misses: 2003004725 */
3) | /* cpu_cycles: 2947541104 cache_misses: 2003006194 */
3) 7.029 us | }
3) | /* cpu_cycles: 2947545762 cache_misses: 2003008052 */
3) + 80.971 us | }
3) | /* cpu_cycles: 2947550459 cache_misses: 2003009915 */
3) + 95.515 us | }
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 4 +
kernel/trace/trace_functions_graph.c | 117 +++++++++++++++++++++++++--
2 files changed, 116 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bb764a2255c7..64cdb6fda3fb 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -366,7 +366,9 @@ struct trace_array {
int perf_events;
int ftrace_perf_events;
+ int fgraph_perf_events;
u64 ftrace_perf_mask;
+ u64 fgraph_perf_mask;
struct trace_pid_list __rcu *filtered_pids;
struct trace_pid_list __rcu *filtered_no_pids;
@@ -946,6 +948,8 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
#define TRACE_GRAPH_PRINT_RETVAL_HEX 0x1000
#define TRACE_GRAPH_PRINT_RETADDR 0x2000
#define TRACE_GRAPH_ARGS 0x4000
+#define TRACE_GRAPH_PERF_CACHE 0x8000
+#define TRACE_GRAPH_PERF_CYCLES 0x10000
#define TRACE_GRAPH_PRINT_FILL_SHIFT 28
#define TRACE_GRAPH_PRINT_FILL_MASK (0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 44d5dc5031e2..e618dd12ca0c 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -22,6 +22,8 @@ static int ftrace_graph_skip_irqs;
/* Do not record function time when task is sleeping */
unsigned int fgraph_no_sleep_time;
+static struct tracer graph_trace;
+
struct fgraph_cpu_data {
pid_t last_pid;
int depth;
@@ -88,6 +90,11 @@ static struct tracer_opt trace_opts[] = {
/* Include sleep time (scheduled out) between entry and return */
{ TRACER_OPT(sleep-time, TRACE_GRAPH_SLEEP_TIME) },
+#ifdef CONFIG_PERF_EVENTS
+ { TRACER_OPT(funcgraph-cache-misses, TRACE_GRAPH_PERF_CACHE) },
+ { TRACER_OPT(funcgraph-cpu-cycles, TRACE_GRAPH_PERF_CYCLES) },
+#endif
+
{ } /* Empty entry */
};
@@ -104,6 +111,97 @@ static bool tracer_flags_is_set(struct trace_array *tr, u32 flags)
return (tr->current_trace_flags->val & flags) == flags;
}
+#ifdef CONFIG_PERF_EVENTS
+static inline void handle_perf_event(struct trace_array *tr, unsigned int trace_ctx)
+{
+ if (!tr->fgraph_perf_events)
+ return;
+ ftrace_perf_events(tr, tr->fgraph_perf_events, tr->fgraph_perf_mask, trace_ctx);
+}
+
+static int ftrace_graph_perf_event(struct trace_array *tr, int set, int bit)
+{
+ u64 mask;
+ int type;
+ int ret = 0;
+
+ /* Do nothing if the current tracer is not this tracer */
+ if (tr->current_trace != &graph_trace)
+ return 0;
+
+ switch (bit) {
+ case TRACE_GRAPH_PERF_CACHE:
+ mask = TRACE_ITER(PERF_CACHE);
+ type = PERF_TRACE_CACHE;
+ break;
+ case TRACE_GRAPH_PERF_CYCLES:
+ mask = TRACE_ITER(PERF_CYCLES);
+ type = PERF_TRACE_CYCLES;
+ break;
+ }
+
+ if (set)
+ ret = trace_perf_event_enable(type);
+ else
+ trace_perf_event_disable(type);
+
+ if (ret < 0)
+ return ret;
+
+ if (set) {
+ tr->fgraph_perf_events++;
+ tr->fgraph_perf_mask |= mask;
+ } else {
+ tr->fgraph_perf_mask &= ~mask;
+ tr->fgraph_perf_events--;
+ }
+ return 0;
+}
+
+static void ftrace_graph_perf_enable(struct trace_array *tr, int bit)
+{
+ int err;
+
+ if (!(tr->current_trace_flags->val & bit))
+ return;
+
+ err = ftrace_graph_perf_event(tr, 1, bit);
+ if (err < 0)
+ tr->current_trace_flags->val &= ~bit;
+}
+
+static void ftrace_graph_perf_disable(struct trace_array *tr, int bit)
+{
+ /* Only disable if it was enabled */
+ if (!(tr->current_trace_flags->val & bit))
+ return;
+
+ ftrace_graph_perf_event(tr, 0, bit);
+}
+
+static void fgraph_perf_init(struct trace_array *tr)
+{
+ ftrace_graph_perf_enable(tr, TRACE_GRAPH_PERF_CYCLES);
+ ftrace_graph_perf_enable(tr, TRACE_GRAPH_PERF_CACHE);
+}
+
+static void fgraph_perf_reset(struct trace_array *tr)
+{
+ ftrace_graph_perf_disable(tr, TRACE_GRAPH_PERF_CYCLES);
+ ftrace_graph_perf_disable(tr, TRACE_GRAPH_PERF_CACHE);
+}
+#else
+static inline void handle_perf_event(struct trace_array *tr, unsigned int trace_ctx)
+{
+}
+static inline void fgraph_perf_init(struct trace_array *tr)
+{
+}
+static inline void fgraph_perf_reset(struct trace_array *tr)
+{
+}
+#endif
+
/*
* DURATION column is being also used to display IRQ signs,
* following values are used by print_graph_irq and others
@@ -272,6 +370,9 @@ static int graph_entry(struct ftrace_graph_ent *trace,
ret = __graph_entry(tr, trace, trace_ctx, fregs);
}
+ if (ret)
+ handle_perf_event(tr, trace_ctx);
+
return ret;
}
@@ -324,6 +425,8 @@ void __trace_graph_return(struct trace_array *tr,
struct trace_buffer *buffer = tr->array_buffer.buffer;
struct ftrace_graph_ret_entry *entry;
+ handle_perf_event(tr, trace_ctx);
+
event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_RET,
sizeof(*entry), trace_ctx);
if (!event)
@@ -465,6 +568,8 @@ static int graph_trace_init(struct trace_array *tr)
if (!tracer_flags_is_set(tr, TRACE_GRAPH_SLEEP_TIME))
fgraph_no_sleep_time++;
+ fgraph_perf_init(tr);
+
/* Make gops functions visible before we start tracing */
smp_mb();
@@ -476,8 +581,6 @@ static int graph_trace_init(struct trace_array *tr)
return 0;
}
-static struct tracer graph_trace;
-
static int ftrace_graph_trace_args(struct trace_array *tr, int set)
{
trace_func_graph_ent_t entry;
@@ -512,6 +615,7 @@ static void graph_trace_reset(struct trace_array *tr)
if (WARN_ON_ONCE(fgraph_no_sleep_time < 0))
fgraph_no_sleep_time = 0;
+ fgraph_perf_reset(tr);
tracing_stop_cmdline_record();
unregister_ftrace_graph(tr->gops);
}
@@ -1684,9 +1788,12 @@ func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
ftrace_graph_skip_irqs = 0;
break;
- case TRACE_GRAPH_ARGS:
- return ftrace_graph_trace_args(tr, set);
- }
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_GRAPH_PERF_CACHE:
+ case TRACE_GRAPH_PERF_CYCLES:
+ return ftrace_graph_perf_event(tr, set, bit);
+#endif
+ };
return 0;
}
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v3 1/8] mm: introduce VM_MAYBE_GUARD and make visible in /proc/$pid/smaps
From: jane.chu @ 2025-11-18 1:05 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Jonathan Corbet, David Hildenbrand, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jann Horn,
Pedro Falcato, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, linux-kernel, linux-fsdevel,
linux-doc, linux-mm, linux-trace-kernel, linux-kselftest,
Andrei Vagin
In-Reply-To: <94d1e9c6c6dd8a4de1f2a8022ca92e2e320730ff.1762531708.git.lorenzo.stoakes@oracle.com>
On 11/7/2025 8:11 AM, Lorenzo Stoakes wrote:
> Currently, if a user needs to determine if guard regions are present in a
> range, they have to scan all VMAs (or have knowledge of which ones might
> have guard regions).
>
> Since commit 8e2f2aeb8b48 ("fs/proc/task_mmu: add guard region bit to
> pagemap") and the related commit a516403787e0 ("fs/proc: extend the
> PAGEMAP_SCAN ioctl to report guard regions"), users can use either
> /proc/$pid/pagemap or the PAGEMAP_SCAN functionality to perform this
> operation at a virtual address level.
>
> This is not ideal, and it gives no visibility at a /proc/$pid/smaps level
> that guard regions exist in ranges.
>
> This patch remedies the situation by establishing a new VMA flag,
> VM_MAYBE_GUARD, to indicate that a VMA may contain guard regions (it is
> uncertain because we cannot reasonably determine whether a
> MADV_GUARD_REMOVE call has removed all of the guard regions in a VMA, and
> additionally VMAs may change across merge/split).
>
> We utilise 0x800 for this flag which makes it available to 32-bit
> architectures also, a flag that was previously used by VM_DENYWRITE, which
> was removed in commit 8d0920bde5eb ("mm: remove VM_DENYWRITE") and hasn't
> bee reused yet.
>
> We also update the smaps logic and documentation to identify these VMAs.
>
> Another major use of this functionality is that we can use it to identify
> that we ought to copy page tables on fork.
>
> We do not actually implement usage of this flag in mm/madvise.c yet as we
> need to allow some VMA flags to be applied atomically under mmap/VMA read
> lock in order to avoid the need to acquire a write lock for this purpose.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> Documentation/filesystems/proc.rst | 5 +++--
> fs/proc/task_mmu.c | 1 +
> include/linux/mm.h | 3 +++
> include/trace/events/mmflags.h | 1 +
> mm/memory.c | 4 ++++
> tools/testing/vma/vma_internal.h | 1 +
> 6 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 0b86a8022fa1..8256e857e2d7 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -553,7 +553,7 @@ otherwise.
> kernel flags associated with the particular virtual memory area in two letter
> encoded manner. The codes are the following:
>
> - == =======================================
> + == =============================================================
> rd readable
> wr writeable
> ex executable
> @@ -591,7 +591,8 @@ encoded manner. The codes are the following:
> sl sealed
> lf lock on fault pages
> dp always lazily freeable mapping
> - == =======================================
> + gu maybe contains guard regions (if not set, definitely doesn't)
> + == =============================================================
>
> Note that there is no guarantee that every flag and associated mnemonic will
> be present in all further kernel releases. Things get changed, the flags may
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 8a9894aefbca..a420dcf9ffbb 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1147,6 +1147,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> [ilog2(VM_MAYSHARE)] = "ms",
> [ilog2(VM_GROWSDOWN)] = "gd",
> [ilog2(VM_PFNMAP)] = "pf",
> + [ilog2(VM_MAYBE_GUARD)] = "gu",
> [ilog2(VM_LOCKED)] = "lo",
> [ilog2(VM_IO)] = "io",
> [ilog2(VM_SEQ_READ)] = "sr",
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6e5ca5287e21..2a5516bff75a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -271,6 +271,8 @@ extern struct rw_semaphore nommu_region_sem;
> extern unsigned int kobjsize(const void *objp);
> #endif
>
> +#define VM_MAYBE_GUARD_BIT 11
> +
> /*
> * vm_flags in vm_area_struct, see mm_types.h.
> * When changing, update also include/trace/events/mmflags.h
> @@ -296,6 +298,7 @@ extern unsigned int kobjsize(const void *objp);
> #define VM_UFFD_MISSING 0
> #endif /* CONFIG_MMU */
> #define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
> +#define VM_MAYBE_GUARD BIT(VM_MAYBE_GUARD_BIT) /* The VMA maybe contains guard regions. */
> #define VM_UFFD_WP 0x00001000 /* wrprotect pages tracking */
>
> #define VM_LOCKED 0x00002000
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index aa441f593e9a..a6e5a44c9b42 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -213,6 +213,7 @@ IF_HAVE_PG_ARCH_3(arch_3)
> {VM_UFFD_MISSING, "uffd_missing" }, \
> IF_HAVE_UFFD_MINOR(VM_UFFD_MINOR, "uffd_minor" ) \
> {VM_PFNMAP, "pfnmap" }, \
> + {VM_MAYBE_GUARD, "maybe_guard" }, \
> {VM_UFFD_WP, "uffd_wp" }, \
> {VM_LOCKED, "locked" }, \
> {VM_IO, "io" }, \
> diff --git a/mm/memory.c b/mm/memory.c
> index 046579a6ec2f..334732ab6733 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1480,6 +1480,10 @@ vma_needs_copy(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
> if (src_vma->anon_vma)
> return true;
>
> + /* Guard regions have momdified page tables that require copying. */
Nit: s/momdified/modified.
> + if (src_vma->vm_flags & VM_MAYBE_GUARD)
> + return true;
> +
> /*
> * Don't copy ptes where a page fault will fill them correctly. Fork
> * becomes much lighter when there are big shared or private readonly
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index c68d382dac81..46acb4df45de 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -56,6 +56,7 @@ extern unsigned long dac_mmap_min_addr;
> #define VM_MAYEXEC 0x00000040
> #define VM_GROWSDOWN 0x00000100
> #define VM_PFNMAP 0x00000400
> +#define VM_MAYBE_GUARD 0x00000800
> #define VM_LOCKED 0x00002000
> #define VM_IO 0x00004000
> #define VM_SEQ_READ 0x00008000 /* App will access data sequentially */
^ permalink raw reply
* Re: [PATCH v1] cpufreq: Add policy_frequency trace event
From: Samuel Wu @ 2025-11-18 1:12 UTC (permalink / raw)
To: Christian Loehle
Cc: Viresh Kumar, peterz, vincent.guittot, Rafael J. Wysocki,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, kernel-team,
linux-pm, linux-kernel, linux-trace-kernel, Dietmar Eggemann,
Qais Yousef, John Stultz
In-Reply-To: <28868c0e-2a46-47ed-9bd7-439056cf94c0@arm.com>
On Mon, Nov 17, 2025 at 1:18 AM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 11/14/25 05:11, Viresh Kumar wrote:
> > On 13-11-25, 19:41, Samuel Wu wrote:
> >> On Wed, Nov 12, 2025 at 10:45 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >>>
> >>> On 12-11-25, 15:51, Samuel Wu wrote:
> >>>> The existing cpu_frequency trace_event can be verbose, emitting an event
> >>>> for every CPU in the policy even when their frequencies are identical.
> >>>>
> >>>> This patch adds a new policy_frequency trace event, which provides a
> >>>> more efficient alternative to cpu_frequency trace event. This option
> >>>> allows users who only need frequency at a policy level more concise logs
> >>>> with simpler analysis.
> >>>>
> >>>> Signed-off-by: Samuel Wu <wusamuel@google.com>
> >>>> ---
> >>>> drivers/cpufreq/cpufreq.c | 2 ++
> >>>> include/trace/events/power.h | 21 +++++++++++++++++++++
> >>>> 2 files changed, 23 insertions(+)
> >>>>
> >>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> >>>> index 4472bb1ec83c..b65534a4fd9a 100644
> >>>> --- a/drivers/cpufreq/cpufreq.c
> >>>> +++ b/drivers/cpufreq/cpufreq.c
> >>>> @@ -345,6 +345,7 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
> >>>> pr_debug("FREQ: %u - CPUs: %*pbl\n", freqs->new,
> >>>> cpumask_pr_args(policy->cpus));
> >>>>
> >>>> + trace_policy_frequency(freqs->new, policy->cpu);
> >>>> for_each_cpu(cpu, policy->cpus)
> >>>> trace_cpu_frequency(freqs->new, cpu);
> >>>
> >>> I don't see much value in almost duplicate trace events. If we feel that a
> >>> per-policy event is a better fit (which makes sens), then we can just drop the
> >>> trace_cpu_frequency() events and print policy->cpus (or related_cpus)
> >>> information along with the per-policy events.
> >>
> >> Thank you for the feedback Viresh. Fair enough, I've done some testing
> >> and a single trace event should work and would be cleaner. Please let
> >> me know what you think of this proposal for v2.
> >>
> >> We can append a bitmask of policy->cpus field to
> >> trace_cpu_frequency(). This way we maintain backwards compatibility:
> >> trace_cpu_frequency() is not removed, and its pre-existing fields are
> >> not disturbed.
> >>
> >> Call flow wise, we can delete all the for_each_cpu() loops, and we
> >> still retain the benefits of the trace emitting once per policy
> >> instead of once per cpu.
> >
> > Fine by me. I have added Scheduler maintainers in the loop to see if they have a
> > different view.
> >
>
> And IIUC your proposal is to fold policy_frequency into cpu_frequency but then
> only have one cpu_frequency event per policy emitted?
That's right, emit the trace event once per policy instead of once per
cpu- which I think is the most valuable element of this patch. And
yes, the latest idea was to append bitmask of policy->cpus into the
cpu_frequency event such that relevant policy info is encapsulated in
the trace event.
> I think from a tooling perspective it would be easier to remove cpu_frequency
> entirely, then tools can probe on the presence of policy_frequency / cpu_frequency.
This can be handled perfectly fine by the tools I know of that consume
this trace event. The points you and Viresh have brought up are valid,
and as this solution is not in conflict with those points,
"policy_frequency replacing cpu_frequency" can be the frontrunner for
now.
^ permalink raw reply
* Re: [PATCH bpf-next v2 6/6] bpf: implement "jmp" mode for trampoline
From: Menglong Dong @ 2025-11-18 1:20 UTC (permalink / raw)
To: ast, rostedt, Menglong Dong
Cc: daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251117034906.32036-7-dongml2@chinatelecom.cn>
On 2025/11/17 11:49, Menglong Dong wrote:
> Implement the "jmp" mode for the bpf trampoline. For the ftrace_managed
> case, we need only to set the FTRACE_OPS_FL_JMP on the tr->fops if "jmp"
> is needed.
>
> For the bpf poke case, we will check the origin poke type with the
> "origin_flags", and current poke type with "tr->flags". The function
> bpf_trampoline_update_fentry() is introduced to do the job.
>
> The "jmp" mode will only be enabled with CONFIG_DYNAMIC_FTRACE_WITH_JMP
> enabled and BPF_TRAMP_F_SHARE_IPMODIFY is not set. With
> BPF_TRAMP_F_SHARE_IPMODIFY, we need to get the origin call ip from the
> stack, so we can't use the "jmp" mode.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> v2:
> - rename bpf_text_poke to bpf_trampoline_update_fentry
> - remove the BPF_TRAMP_F_JMPED and check the current mode with the origin
> flags instead.
> ---
> kernel/bpf/trampoline.c | 68 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 2dcc999a411f..80ab435d6e00 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -175,24 +175,42 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
> return tr;
> }
>
> -static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
> +static int bpf_trampoline_update_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> + void *old_addr, void *new_addr)
> {
> + enum bpf_text_poke_type new_t = BPF_MOD_CALL, old_t = BPF_MOD_CALL;
> void *ip = tr->func.addr;
> +
> + if (!new_addr)
> + new_t = BPF_MOD_NOP;
> + else if (bpf_trampoline_use_jmp(tr->flags))
> + new_t = BPF_MOD_JUMP;
> +
> + if (!old_addr)
> + old_t = BPF_MOD_NOP;
> + else if (bpf_trampoline_use_jmp(orig_flags))
> + old_t = BPF_MOD_JUMP;
> +
> + return bpf_arch_text_poke(ip, old_t, new_t, old_addr, new_addr);
> +}
> +
> +static int unregister_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> + void *old_addr)
> +{
> int ret;
>
> if (tr->func.ftrace_managed)
> ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
> else
> - ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, BPF_MOD_NOP,
> - old_addr, NULL);
> + ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr, NULL);
>
> return ret;
> }
>
> -static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_addr,
> +static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> + void *old_addr, void *new_addr,
> bool lock_direct_mutex)
> {
> - void *ip = tr->func.addr;
> int ret;
>
> if (tr->func.ftrace_managed) {
> @@ -201,10 +219,8 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
> else
> ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
> } else {
> - ret = bpf_arch_text_poke(ip,
> - old_addr ? BPF_MOD_CALL : BPF_MOD_NOP,
> - new_addr ? BPF_MOD_CALL : BPF_MOD_NOP,
> - old_addr, new_addr);
> + ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
> + new_addr);
> }
> return ret;
> }
> @@ -229,8 +245,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
> return ret;
> ret = register_ftrace_direct(tr->fops, (long)new_addr);
> } else {
> - ret = bpf_arch_text_poke(ip, BPF_MOD_NOP, BPF_MOD_CALL,
> - NULL, new_addr);
> + ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
> }
>
> return ret;
> @@ -416,7 +431,7 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
> return PTR_ERR(tlinks);
>
> if (total == 0) {
> - err = unregister_fentry(tr, tr->cur_image->image);
> + err = unregister_fentry(tr, orig_flags, tr->cur_image->image);
> bpf_tramp_image_put(tr->cur_image);
> tr->cur_image = NULL;
> goto out;
> @@ -440,9 +455,17 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> again:
> - if ((tr->flags & BPF_TRAMP_F_SHARE_IPMODIFY) &&
> - (tr->flags & BPF_TRAMP_F_CALL_ORIG))
> - tr->flags |= BPF_TRAMP_F_ORIG_STACK;
> + if (tr->flags & BPF_TRAMP_F_CALL_ORIG) {
> + if (tr->flags & BPF_TRAMP_F_SHARE_IPMODIFY) {
> + tr->flags |= BPF_TRAMP_F_ORIG_STACK;
> + } else if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_JMP)) {
> + /* Use "jmp" instead of "call" for the trampoline
> + * in the origin call case, and we don't need to
> + * skip the frame.
> + */
> + tr->flags &= ~BPF_TRAMP_F_SKIP_FRAME;
> + }
> + }
> #endif
>
> size = arch_bpf_trampoline_size(&tr->func.model, tr->flags,
> @@ -473,10 +496,16 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
> if (err)
> goto out_free;
>
> + if (bpf_trampoline_use_jmp(tr->flags))
> + tr->fops->flags |= FTRACE_OPS_FL_JMP;
> + else
> + tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
This should be wrapped by "#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP".
I'll change it in v3 after more human comments.
> +
> WARN_ON(tr->cur_image && total == 0);
> if (tr->cur_image)
> /* progs already running at this address */
> - err = modify_fentry(tr, tr->cur_image->image, im->image, lock_direct_mutex);
> + err = modify_fentry(tr, orig_flags, tr->cur_image->image,
> + im->image, lock_direct_mutex);
> else
> /* first time registering */
> err = register_fentry(tr, im->image);
> @@ -499,8 +528,13 @@ 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;
> + if (bpf_trampoline_use_jmp(tr->flags))
> + tr->fops->flags |= FTRACE_OPS_FL_JMP;
> + else
> + tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
> + }
> kfree(tlinks);
> return err;
>
>
^ permalink raw reply
* Re: [PATCH v12 mm-new 13/15] khugepaged: avoid unnecessary mTHP collapse attempts
From: Wei Yang @ 2025-11-18 2:00 UTC (permalink / raw)
To: Nico Pache
Cc: Wei Yang, linux-kernel, linux-trace-kernel, linux-mm, linux-doc,
david, ziy, baolin.wang, lorenzo.stoakes, Liam.Howlett,
ryan.roberts, dev.jain, corbet, rostedt, mhiramat,
mathieu.desnoyers, akpm, baohua, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, surenb, zokeefe, hannes,
rientjes, mhocko, rdunlap, hughd, lance.yang, vbabka, rppt, jannh,
pfalcato
In-Reply-To: <CAA1CXcA9zaGqLPHnJWH=fKPUjb02dV+rKgfmsRZOGdeukiC8eg@mail.gmail.com>
On Mon, Nov 17, 2025 at 11:16:53AM -0700, Nico Pache wrote:
>On Sat, Nov 8, 2025 at 7:40 PM Wei Yang <richard.weiyang@gmail.com> wrote:
>>
>> On Wed, Oct 22, 2025 at 12:37:15PM -0600, Nico Pache wrote:
>> >There are cases where, if an attempted collapse fails, all subsequent
>> >orders are guaranteed to also fail. Avoid these collapse attempts by
>> >bailing out early.
>> >
>> >Signed-off-by: Nico Pache <npache@redhat.com>
>> >---
>> > mm/khugepaged.c | 31 ++++++++++++++++++++++++++++++-
>> > 1 file changed, 30 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> >index e2319bfd0065..54f5c7888e46 100644
>> >--- a/mm/khugepaged.c
>> >+++ b/mm/khugepaged.c
>> >@@ -1431,10 +1431,39 @@ static int collapse_scan_bitmap(struct mm_struct *mm, unsigned long address,
>> > ret = collapse_huge_page(mm, address, referenced,
>> > unmapped, cc, mmap_locked,
>> > order, offset);
>> >- if (ret == SCAN_SUCCEED) {
>> >+
>> >+ /*
>> >+ * Analyze failure reason to determine next action:
>> >+ * - goto next_order: try smaller orders in same region
>> >+ * - continue: try other regions at same order
>> >+ * - break: stop all attempts (system-wide failure)
>> >+ */
>> >+ switch (ret) {
>> >+ /* Cases were we should continue to the next region */
>> >+ case SCAN_SUCCEED:
>> > collapsed += 1UL << order;
>> >+ fallthrough;
>> >+ case SCAN_PTE_MAPPED_HUGEPAGE:
>> > continue;
>> >+ /* Cases were lower orders might still succeed */
>> >+ case SCAN_LACK_REFERENCED_PAGE:
>> >+ case SCAN_EXCEED_NONE_PTE:
>> >+ case SCAN_EXCEED_SWAP_PTE:
>> >+ case SCAN_EXCEED_SHARED_PTE:
>> >+ case SCAN_PAGE_LOCK:
>> >+ case SCAN_PAGE_COUNT:
>> >+ case SCAN_PAGE_LRU:
>> >+ case SCAN_PAGE_NULL:
>> >+ case SCAN_DEL_PAGE_LRU:
>> >+ case SCAN_PTE_NON_PRESENT:
>> >+ case SCAN_PTE_UFFD_WP:
>> >+ case SCAN_ALLOC_HUGE_PAGE_FAIL:
>> >+ goto next_order;
>> >+ /* All other cases should stop collapse attempts */
>> >+ default:
>> >+ break;
>> > }
>> >+ break;
>>
>> One question here:
>
>Hi Wei Yang,
>
>Sorry I forgot to get back to this email.
>
No problem, thanks for taking a look.
>>
>> Suppose we have iterated several orders and not collapse successfully yet. So
>> the mthp_bitmap_stack[] would look like this:
>>
>> [8 7 6 6]
>> ^
>> |
>
>so we always pop before pushing. So it would go
>
>[9]
>pop
>if (collapse fails)
>[8 8]
>lets say we pop and successfully collapse a order 8
>[8]
>Then we fail the other order 8
>[7 7]
>now if we succeed the first order 7
>[7 6 6]
>I believe we are now in the state you wanted to describe.
>
>>
>> Now we found this one pass the threshold check, but it fails with other
>> result.
>
>ok lets say we pass the threshold checks, but the collapse fails for
>any reason that is described in the
>/* Cases were lower orders might still succeed */
>In this case we would continue to order 5 (or lower). Once we are done
>with this branch of the tree we go back to the other order 6 collapse.
>and eventually the order 7.
>
>>
>> Current code looks it would give up at all, but we may still have a chance to
>> collapse the above 3 range?
>
>for cases under /* All other cases should stop collapse attempts */
>Yes we would bail out and skip some collapses. I tried to think about
>all the cases were we would still want to continue trying, vs cases
>where the system is probably out of resources or hitting some major
>failure, and we should just break out (as others will probably fail
>too).
>
Thanks, your explanation is very clear.
>But this is also why I separated this patch out on its own. I was
>hoping to have some more focus on the different cases, and make sure I
>handled them in the best possible way. So I really appreciate the
>question :)
>
>* I did some digging through old message to find this *
>
>I believe these are the remaining cases. If these are hit I figured
>it's better to abort.
>
I agree we need to take care of those cases.
>/* cases where we must stop collapse attempts */
>case SCAN_CGROUP_CHARGE_FAIL:
>case SCAN_COPY_MC:
>case SCAN_ADDRESS_RANGE:
>case SCAN_PMD_NULL:
>case SCAN_ANY_PROCESS:
>case SCAN_VMA_NULL:
>case SCAN_VMA_CHECK:
>case SCAN_SCAN_ABORT:
>case SCAN_PMD_NONE:
>case SCAN_PAGE_ANON:
>case SCAN_PMD_MAPPED:
>case SCAN_FAIL:
>
>Please let me know if you think we should move these to either the
>`continue` or `next order` cases.
Take a look into these cases, it looks good to me now.
Also one of my concern is this coding style is a little hard to maintain. In
case we introduce a new result, we should remember to add it here. Otherwise
we may stop the collapse too early.
While it maybe a separate work after this patch set merged.
>
>Cheers,
>-- Nico
>
>>
>> > }
>> >
>> > next_order:
>> >--
>> >2.51.0
>>
>> --
>> Wei Yang
>> Help you, Help me
>>
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: [rtla 01/13] rtla: Check for memory allocation failures
From: Masami Hiramatsu @ 2025-11-18 2:09 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Tomas Glozar, Ivan Pravdin, Crystal Wood,
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: <20251117184409.42831-2-wander@redhat.com>
On Mon, 17 Nov 2025 15:41:08 -0300
Wander Lairson Costa <wander@redhat.com> wrote:
> The actions_init() and actions_new() functions did not check the
> return value of calloc() and realloc() respectively. In a low
> memory situation, this could lead to a NULL pointer dereference.
>
> Add checks for the return value of memory allocation functions
> and return an error in case of failure. Update the callers to
> handle the error properly.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
> tools/tracing/rtla/src/actions.c | 26 +++++++++++++++++++++++---
> tools/tracing/rtla/src/actions.h | 2 +-
> tools/tracing/rtla/src/timerlat_hist.c | 7 +++++--
> tools/tracing/rtla/src/timerlat_top.c | 7 +++++--
> 4 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> index 8945aee58d511..01648a1425c10 100644
> --- a/tools/tracing/rtla/src/actions.c
> +++ b/tools/tracing/rtla/src/actions.c
> @@ -11,11 +11,13 @@
> /*
> * actions_init - initialize struct actions
> */
> -void
> +int
> actions_init(struct actions *self)
> {
> self->size = action_default_size;
> self->list = calloc(self->size, sizeof(struct action));
> + if (!self->list)
> + return -1;
Can you return -ENOMEM?
> self->len = 0;
> self->continue_flag = false;
>
> @@ -23,6 +25,7 @@ actions_init(struct actions *self)
>
> /* This has to be set by the user */
> self->trace_output_inst = NULL;
> + return 0;
> }
>
> /*
> @@ -50,8 +53,13 @@ static struct action *
> actions_new(struct actions *self)
> {
> if (self->len >= self->size) {
> - self->size *= 2;
> - self->list = realloc(self->list, self->size * sizeof(struct action));
> + const size_t new_size = self->size * 2;
> + void *p = reallocarray(self->list, new_size, sizeof(struct action));
> +
> + if (!p)
> + return NULL;
> + self->list = p;
> + self->size = new_size;
> }
>
> return &self->list[self->len++];
> @@ -65,6 +73,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
> {
> struct action *action = actions_new(self);
>
> + if (!action)
> + return -1;
I think !action should return -ENOMEM too.
> +
> self->present[ACTION_TRACE_OUTPUT] = true;
> action->type = ACTION_TRACE_OUTPUT;
> action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
> @@ -83,6 +94,9 @@ actions_add_signal(struct actions *self, int signal, int pid)
> {
> struct action *action = actions_new(self);
>
> + if (!action)
> + return -1;
> +
> self->present[ACTION_SIGNAL] = true;
> action->type = ACTION_SIGNAL;
> action->signal = signal;
> @@ -99,6 +113,9 @@ actions_add_shell(struct actions *self, const char *command)
> {
> struct action *action = actions_new(self);
>
> + if (!action)
> + return -1;
> +
> self->present[ACTION_SHELL] = true;
> action->type = ACTION_SHELL;
> action->command = calloc(strlen(command) + 1, sizeof(char));
> @@ -117,6 +134,9 @@ actions_add_continue(struct actions *self)
> {
> struct action *action = actions_new(self);
>
> + if (!action)
> + return -1;
> +
> self->present[ACTION_CONTINUE] = true;
> action->type = ACTION_CONTINUE;
>
The above same patterns too.
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [rtla 01/13] rtla: Check for memory allocation failures
From: Steven Rostedt @ 2025-11-18 3:06 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Wander Lairson Costa, Tomas Glozar, Ivan Pravdin, Crystal Wood,
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: <20251118110946.2e154e8c88b3edd31cc3113a@kernel.org>
On Tue, 18 Nov 2025 11:09:46 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> On Mon, 17 Nov 2025 15:41:08 -0300
> Wander Lairson Costa <wander@redhat.com> wrote:
>
> > The actions_init() and actions_new() functions did not check the
> > return value of calloc() and realloc() respectively. In a low
> > memory situation, this could lead to a NULL pointer dereference.
> >
> > Add checks for the return value of memory allocation functions
> > and return an error in case of failure. Update the callers to
> > handle the error properly.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > ---
> > tools/tracing/rtla/src/actions.c | 26 +++++++++++++++++++++++---
> > tools/tracing/rtla/src/actions.h | 2 +-
> > tools/tracing/rtla/src/timerlat_hist.c | 7 +++++--
> > tools/tracing/rtla/src/timerlat_top.c | 7 +++++--
> > 4 files changed, 34 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > index 8945aee58d511..01648a1425c10 100644
> > --- a/tools/tracing/rtla/src/actions.c
> > +++ b/tools/tracing/rtla/src/actions.c
> > @@ -11,11 +11,13 @@
> > /*
> > * actions_init - initialize struct actions
> > */
> > -void
> > +int
> > actions_init(struct actions *self)
> > {
> > self->size = action_default_size;
> > self->list = calloc(self->size, sizeof(struct action));
> > + if (!self->list)
> > + return -1;
>
> Can you return -ENOMEM?
Does it need to? This is user space not the kernel. Errno is already
set by calloc() failing.
-- Steve
^ permalink raw reply
* Re: [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Masami Hiramatsu @ 2025-11-18 3:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Peter Zijlstra, Thomas Gleixner,
Ian Rogers, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Douglas Raillard
In-Reply-To: <20251118002950.680329246@kernel.org>
Hi Steve,
Thanks for the great idea!
On Mon, 17 Nov 2025 19:29:50 -0500
Steven Rostedt <rostedt@kernel.org> wrote:
>
> This series adds a perf event to the ftrace ring buffer.
> It is currently a proof of concept as I'm not happy with the interface
> and I also think the recorded perf event format may be changed too.
>
> This proof-of-concept interface (which I have no plans on using), currently
> just adds 6 new trace options.
>
> event_cache_misses
> event_cpu_cycles
> func-cache-misses
> func-cpu-cycles
> funcgraph-cache-misses
> funcgraph-cpu-cycles
>
> The first two trigger a perf event after every event, the second two trigger
> a perf event after every function and the last two trigger a perf event
> right after the start of a function and again at the end of the function.
>
> As this will eventual work with many more perf events than just cache-misses
> and cpu-cycles , using options is not appropriate. Especially since the
> options are limited to a 64 bit bitmask, and that can easily go much higher.
> I'm thinking about having a file instead that will act as a way to enable
> perf events for events, function and function graph tracing.
>
> set_event_perf, set_ftrace_perf, set_fgraph_perf
What about adding a global `trigger` action file so that user can
add these "perf" actions to write into it. It is something like
stacktrace for events. (Maybe we can move stacktrace/user-stacktrace
into it too)
For pre-defined/software counters:
# echo "perf:cpu_cycles" >> /sys/kernel/tracing/trigger
For some hardware event sources (see /sys/bus/event_source/devices/):
# echo "perf:cstate_core.c3-residency" >> /sys/kernel/tracing/trigger
echo "perf:my_counter=pmu/config=M,config1=N" >> /sys/kernel/tracing/trigger
If we need to set those counters for tracers and events separately,
we can add `events/trigger` and `tracer-trigger` files.
echo "perf:cpu_cycles" >> /sys/kernel/tracing/events/trigger
To disable counters, we can use '!' as same as event triggers.
echo !perf:cpu_cycles > trigger
To add more than 2 counters, connect it with ':'.
(or, we will allow to append new perf counters)
This allows user to set perf counter options for each events.
Maybe we also should move 'stacktrace'/'userstacktrace' option
flags to it too eventually.
>
> And an available_perf_events that show what can be written into these files,
> (similar to how set_ftrace_filter works). But for now, it was just easier to
> implement them as options.
>
> As for the perf event that is triggered. It currently is a dynamic array of
> 64 bit values. Each value is broken up into 8 bits for what type of perf
> event it is, and 56 bits for the counter. It only writes a per CPU raw
> counter and does not do any math. That would be needed to be done by any
> post processing.
>
> Since the values are for user space to do the subtraction to figure out the
> difference between events, for example, the function_graph tracer may have:
>
> is_vmalloc_addr() {
> /* cpu_cycles: 5582263593 cache_misses: 2869004572 */
> /* cpu_cycles: 5582267527 cache_misses: 2869006049 */
> }
Just a style question: Would this mean the first line is for function entry
and the second one is function return?
>
> User space would subtract 2869006049 - 2869004572 = 1477
>
> Then 56 bits should be plenty.
>
> 2^55 / 1,000,000,000 / 60 / 60 / 24 = 416
> 416 / 4 = 104
>
> If you have a 4GHz machine, the cpu-cycles will overflow the 55 bits in 104
> days. This tooling is not for seeing how many cycles run over 104 days.
> User space tooling would just need to be aware that the vale is 56 bits and
> when calculating the difference between start and end do something like:
>
> if (start > end)
> end |= 1ULL << 56;
>
> delta = end - start;
>
> The next question is how to label the perf events to be in the 8 bit
> portion. It could simply be a value that is registered, and listed in the
> available_perf_events file.
>
> cpu_cycles:1
> cach_misses:2
> [..]
Looks good to me. I think pre-definied events of `perf list`
will be there and have fixed numbers.
Thank you,
>
> And this would need to be recorded by any tooling reading the events
> so that it knows how to map the events with their attached ids.
>
> But again, this is just a proof-of-concept. How this will eventually be
> implemented is yet to be determined.
>
> But to test these patches (which are based on top of my linux-next branch,
> which should now be in linux-next):
>
> # cd /sys/kernel/tracing
> # echo 1 > options/event_cpu_cycles
> # echo 1 > options/event_cache_misses
> # echo 1 > events/syscalls/enable
> # cat trace
> [..]
> bash-995 [007] ..... 98.255252: sys_write -> 0x2
> bash-995 [007] ..... 98.255257: cpu_cycles: 1557241774 cache_misses: 449901166
> bash-995 [007] ..... 98.255284: sys_dup2(oldfd: 0xa, newfd: 1)
> bash-995 [007] ..... 98.255285: cpu_cycles: 1557260057 cache_misses: 449902679
> bash-995 [007] ..... 98.255305: sys_dup2 -> 0x1
> bash-995 [007] ..... 98.255305: cpu_cycles: 1557280203 cache_misses: 449906196
> bash-995 [007] ..... 98.255343: sys_fcntl(fd: 0xa, cmd: 1, arg: 0)
> bash-995 [007] ..... 98.255344: cpu_cycles: 1557322304 cache_misses: 449915522
> bash-995 [007] ..... 98.255352: sys_fcntl -> 0x1
> bash-995 [007] ..... 98.255353: cpu_cycles: 1557327809 cache_misses: 449916844
> bash-995 [007] ..... 98.255361: sys_close(fd: 0xa)
> bash-995 [007] ..... 98.255362: cpu_cycles: 1557335383 cache_misses: 449918232
> bash-995 [007] ..... 98.255369: sys_close -> 0x0
>
>
>
> Comments welcomed.
>
>
> Steven Rostedt (3):
> tracing: Add perf events
> ftrace: Add perf counters to function tracing
> fgraph: Add perf counters to function graph tracer
>
> ----
> include/linux/trace_recursion.h | 5 +-
> kernel/trace/trace.c | 153 ++++++++++++++++++++++++++++++++-
> kernel/trace/trace.h | 38 ++++++++
> kernel/trace/trace_entries.h | 13 +++
> kernel/trace/trace_event_perf.c | 162 +++++++++++++++++++++++++++++++++++
> kernel/trace/trace_functions.c | 124 +++++++++++++++++++++++++--
> kernel/trace/trace_functions_graph.c | 117 +++++++++++++++++++++++--
> kernel/trace/trace_output.c | 70 +++++++++++++++
> 8 files changed, 670 insertions(+), 12 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Steven Rostedt @ 2025-11-18 3:42 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, linux-trace-kernel, Mark Rutland, Mathieu Desnoyers,
Andrew Morton, Peter Zijlstra, Thomas Gleixner, Ian Rogers,
Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Douglas Raillard
In-Reply-To: <20251118120821.0c47ef684b53d5d9a2d6dc83@kernel.org>
On Tue, 18 Nov 2025 12:08:21 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hi Steve,
>
> Thanks for the great idea!
Thanks!
> >
> > As this will eventual work with many more perf events than just cache-misses
> > and cpu-cycles , using options is not appropriate. Especially since the
> > options are limited to a 64 bit bitmask, and that can easily go much higher.
> > I'm thinking about having a file instead that will act as a way to enable
> > perf events for events, function and function graph tracing.
> >
> > set_event_perf, set_ftrace_perf, set_fgraph_perf
>
> What about adding a global `trigger` action file so that user can
> add these "perf" actions to write into it. It is something like
> stacktrace for events. (Maybe we can move stacktrace/user-stacktrace
> into it too)
>
> For pre-defined/software counters:
> # echo "perf:cpu_cycles" >> /sys/kernel/tracing/trigger
For events, it would make more sense to put it into the events directory:
# echo "perf:cpu_cycles" >> /sys/kernel/tracing/events/trigger
As there is already a events/enable
Heck we could even add it per system:
# echo "perf:cpu_cycles" >> /sys/kernel/tracing/events/syscalls/trigger
>
> For some hardware event sources (see /sys/bus/event_source/devices/):
> # echo "perf:cstate_core.c3-residency" >> /sys/kernel/tracing/trigger
>
> echo "perf:my_counter=pmu/config=M,config1=N" >> /sys/kernel/tracing/trigger
Still need a way to add an identifier list. Currently, if the size of
the type identifier is one byte, then it can only support up to 256 events.
Do we need every event for this? Or just have a subset of events that
would be supported?
>
> If we need to set those counters for tracers and events separately,
> we can add `events/trigger` and `tracer-trigger` files.
As I mentioned, the trigger for events should be in the events directory.
We could add a ftrace_trigger that can affect both function and
function graph tracer.
>
> echo "perf:cpu_cycles" >> /sys/kernel/tracing/events/trigger
>
> To disable counters, we can use '!' as same as event triggers.
>
> echo !perf:cpu_cycles > trigger
Yes, it would follow the current way to disable a trigger.
>
> To add more than 2 counters, connect it with ':'.
> (or, we will allow to append new perf counters)
> This allows user to set perf counter options for each events.
>
> Maybe we also should move 'stacktrace'/'userstacktrace' option
> flags to it too eventually.
We can add them, but may never be able to remove them due to backward
compatibility.
> >
> > And an available_perf_events that show what can be written into these files,
> > (similar to how set_ftrace_filter works). But for now, it was just easier to
> > implement them as options.
> >
> > As for the perf event that is triggered. It currently is a dynamic array of
> > 64 bit values. Each value is broken up into 8 bits for what type of perf
> > event it is, and 56 bits for the counter. It only writes a per CPU raw
> > counter and does not do any math. That would be needed to be done by any
> > post processing.
> >
> > Since the values are for user space to do the subtraction to figure out the
> > difference between events, for example, the function_graph tracer may have:
> >
> > is_vmalloc_addr() {
> > /* cpu_cycles: 5582263593 cache_misses: 2869004572 */
> > /* cpu_cycles: 5582267527 cache_misses: 2869006049 */
> > }
>
> Just a style question: Would this mean the first line is for function entry
> and the second one is function return?
Yes.
Perhaps we could add field to the perf event to allow for annotation,
so the above could look like:
is_vmalloc_addr() {
/* --> cpu_cycles: 5582263593 cache_misses: 2869004572 */
/* <-- cpu_cycles: 5582267527 cache_misses: 2869006049 */
}
Or something similar?
> > The next question is how to label the perf events to be in the 8 bit
> > portion. It could simply be a value that is registered, and listed in the
> > available_perf_events file.
> >
> > cpu_cycles:1
> > cach_misses:2
> > [..]
>
> Looks good to me. I think pre-definied events of `perf list`
> will be there and have fixed numbers.
Thanks for looking at this,
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next v2 6/6] bpf: implement "jmp" mode for trampoline
From: kernel test robot @ 2025-11-18 5:09 UTC (permalink / raw)
To: Menglong Dong, ast, rostedt
Cc: oe-kbuild-all, daniel, john.fastabend, andrii, martin.lau,
eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa,
mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel
In-Reply-To: <20251117034906.32036-7-dongml2@chinatelecom.cn>
Hi Menglong,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Menglong-Dong/ftrace-introduce-FTRACE_OPS_FL_JMP/20251117-115243
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20251117034906.32036-7-dongml2%40chinatelecom.cn
patch subject: [PATCH bpf-next v2 6/6] bpf: implement "jmp" mode for trampoline
config: riscv-randconfig-001-20251118 (https://download.01.org/0day-ci/archive/20251118/202511181238.cVO5ERaA-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251118/202511181238.cVO5ERaA-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511181238.cVO5ERaA-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/bpf/trampoline.c: In function 'bpf_trampoline_update':
>> kernel/bpf/trampoline.c:500:11: error: invalid use of undefined type 'struct ftrace_ops'
500 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ^~
>> kernel/bpf/trampoline.c:500:22: error: 'FTRACE_OPS_FL_JMP' undeclared (first use in this function)
500 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ^~~~~~~~~~~~~~~~~
kernel/bpf/trampoline.c:500:22: note: each undeclared identifier is reported only once for each function it appears in
kernel/bpf/trampoline.c:502:11: error: invalid use of undefined type 'struct ftrace_ops'
502 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ^~
kernel/bpf/trampoline.c:534:12: error: invalid use of undefined type 'struct ftrace_ops'
534 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ^~
kernel/bpf/trampoline.c:536:12: error: invalid use of undefined type 'struct ftrace_ops'
536 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ^~
vim +500 kernel/bpf/trampoline.c
470
471 size = arch_bpf_trampoline_size(&tr->func.model, tr->flags,
472 tlinks, tr->func.addr);
473 if (size < 0) {
474 err = size;
475 goto out;
476 }
477
478 if (size > PAGE_SIZE) {
479 err = -E2BIG;
480 goto out;
481 }
482
483 im = bpf_tramp_image_alloc(tr->key, size);
484 if (IS_ERR(im)) {
485 err = PTR_ERR(im);
486 goto out;
487 }
488
489 err = arch_prepare_bpf_trampoline(im, im->image, im->image + size,
490 &tr->func.model, tr->flags, tlinks,
491 tr->func.addr);
492 if (err < 0)
493 goto out_free;
494
495 err = arch_protect_bpf_trampoline(im->image, im->size);
496 if (err)
497 goto out_free;
498
499 if (bpf_trampoline_use_jmp(tr->flags))
> 500 tr->fops->flags |= FTRACE_OPS_FL_JMP;
501 else
502 tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
503
504 WARN_ON(tr->cur_image && total == 0);
505 if (tr->cur_image)
506 /* progs already running at this address */
507 err = modify_fentry(tr, orig_flags, tr->cur_image->image,
508 im->image, lock_direct_mutex);
509 else
510 /* first time registering */
511 err = register_fentry(tr, im->image);
512
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [rtla 01/13] rtla: Check for memory allocation failures
From: Masami Hiramatsu @ 2025-11-18 5:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: Wander Lairson Costa, Tomas Glozar, Ivan Pravdin, Crystal Wood,
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: <20251117220615.079bce82@batman.local.home>
On Mon, 17 Nov 2025 22:06:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 18 Nov 2025 11:09:46 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > On Mon, 17 Nov 2025 15:41:08 -0300
> > Wander Lairson Costa <wander@redhat.com> wrote:
> >
> > > The actions_init() and actions_new() functions did not check the
> > > return value of calloc() and realloc() respectively. In a low
> > > memory situation, this could lead to a NULL pointer dereference.
> > >
> > > Add checks for the return value of memory allocation functions
> > > and return an error in case of failure. Update the callers to
> > > handle the error properly.
> > >
> > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > ---
> > > tools/tracing/rtla/src/actions.c | 26 +++++++++++++++++++++++---
> > > tools/tracing/rtla/src/actions.h | 2 +-
> > > tools/tracing/rtla/src/timerlat_hist.c | 7 +++++--
> > > tools/tracing/rtla/src/timerlat_top.c | 7 +++++--
> > > 4 files changed, 34 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > > index 8945aee58d511..01648a1425c10 100644
> > > --- a/tools/tracing/rtla/src/actions.c
> > > +++ b/tools/tracing/rtla/src/actions.c
> > > @@ -11,11 +11,13 @@
> > > /*
> > > * actions_init - initialize struct actions
> > > */
> > > -void
> > > +int
> > > actions_init(struct actions *self)
> > > {
> > > self->size = action_default_size;
> > > self->list = calloc(self->size, sizeof(struct action));
> > > + if (!self->list)
> > > + return -1;
> >
> > Can you return -ENOMEM?
>
> Does it need to? This is user space not the kernel. Errno is already
> set by calloc() failing.
Ah, indeed! I agree to just return -1.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you,
>
> -- Steve
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/6] ftrace: introduce FTRACE_OPS_FL_JMP
From: Masami Hiramatsu @ 2025-11-18 5:19 UTC (permalink / raw)
To: Menglong Dong
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251117034906.32036-2-dongml2@chinatelecom.cn>
On Mon, 17 Nov 2025 11:49:01 +0800
Menglong Dong <menglong8.dong@gmail.com> wrote:
> For now, the "nop" will be replaced with a "call" instruction when a
> function is hooked by the ftrace. However, sometimes the "call" can break
> the RSB and introduce extra overhead. Therefore, introduce the flag
> FTRACE_OPS_FL_JMP, which indicate that the ftrace_ops should be called
> with a "jmp" instead of "call". For now, it is only used by the direct
> call case.
>
> When a direct ftrace_ops is marked with FTRACE_OPS_FL_JMP, the last bit of
> the ops->direct_call will be set to 1. Therefore, we can tell if we should
> use "jmp" for the callback in ftrace_call_replace().
>
nit: Is it sure the last bit is always 0?
At least register_ftrace_direct() needs to reject if @addr
parameter has the last bit.
Thanks,
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> include/linux/ftrace.h | 33 +++++++++++++++++++++++++++++++++
> kernel/trace/Kconfig | 12 ++++++++++++
> kernel/trace/ftrace.c | 9 ++++++++-
> 3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 07f8c309e432..015dd1049bea 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -359,6 +359,7 @@ 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
> @@ -577,6 +578,38 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
> unsigned long addr) { }
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
> +static inline bool ftrace_is_jmp(unsigned long addr)
> +{
> + return addr & 1;
> +}
> +
> +static inline unsigned long ftrace_jmp_set(unsigned long addr)
> +{
> + return addr | 1UL;
> +}
> +
> +static inline unsigned long ftrace_jmp_get(unsigned long addr)
> +{
> + return addr & ~1UL;
> +}
> +#else
> +static inline bool ftrace_is_jmp(unsigned long addr)
> +{
> + return false;
> +}
> +
> +static inline unsigned long ftrace_jmp_set(unsigned long addr)
> +{
> + return addr;
> +}
> +
> +static inline unsigned long ftrace_jmp_get(unsigned long addr)
> +{
> + return addr;
> +}
> +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_JMP */
> +
> #ifdef CONFIG_STACK_TRACER
>
> int stack_trace_sysctl(const struct ctl_table *table, int write, void *buffer,
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index d2c79da81e4f..4661b9e606e0 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -80,6 +80,12 @@ config HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
> If the architecture generates __patchable_function_entries sections
> but does not want them included in the ftrace locations.
>
> +config HAVE_DYNAMIC_FTRACE_WITH_JMP
> + bool
> + help
> + If the architecture supports to replace the __fentry__ with a
> + "jmp" instruction.
> +
> config HAVE_SYSCALL_TRACEPOINTS
> bool
> help
> @@ -330,6 +336,12 @@ config DYNAMIC_FTRACE_WITH_ARGS
> depends on DYNAMIC_FTRACE
> depends on HAVE_DYNAMIC_FTRACE_WITH_ARGS
>
> +config DYNAMIC_FTRACE_WITH_JMP
> + def_bool y
> + depends on DYNAMIC_FTRACE
> + depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> + depends on HAVE_DYNAMIC_FTRACE_WITH_JMP
> +
> config FPROBE
> bool "Kernel Function Probe (fprobe)"
> depends on HAVE_FUNCTION_GRAPH_FREGS && HAVE_FTRACE_GRAPH_FUNC
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 59cfacb8a5bb..a6c060a4f50b 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5951,7 +5951,8 @@ static void remove_direct_functions_hash(struct ftrace_hash *hash, unsigned long
> for (i = 0; i < size; i++) {
> hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> del = __ftrace_lookup_ip(direct_functions, entry->ip);
> - if (del && del->direct == addr) {
> + if (del && ftrace_jmp_get(del->direct) ==
> + ftrace_jmp_get(addr)) {
> remove_hash_entry(direct_functions, del);
> kfree(del);
> }
> @@ -6018,6 +6019,9 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>
> 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++) {
> @@ -6138,6 +6142,9 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>
> lockdep_assert_held_once(&direct_mutex);
>
> + 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.51.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/6] ftrace: introduce FTRACE_OPS_FL_JMP
From: Menglong Dong @ 2025-11-18 6:14 UTC (permalink / raw)
To: Menglong Dong, Masami Hiramatsu
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251118141934.ddf14aabf371d0939415b588@kernel.org>
On 2025/11/18 13:19, Masami Hiramatsu wrote:
> On Mon, 17 Nov 2025 11:49:01 +0800
> Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> > For now, the "nop" will be replaced with a "call" instruction when a
> > function is hooked by the ftrace. However, sometimes the "call" can break
> > the RSB and introduce extra overhead. Therefore, introduce the flag
> > FTRACE_OPS_FL_JMP, which indicate that the ftrace_ops should be called
> > with a "jmp" instead of "call". For now, it is only used by the direct
> > call case.
> >
> > When a direct ftrace_ops is marked with FTRACE_OPS_FL_JMP, the last bit of
> > the ops->direct_call will be set to 1. Therefore, we can tell if we should
> > use "jmp" for the callback in ftrace_call_replace().
> >
>
> nit: Is it sure the last bit is always 0?
AFAIK, the function address is 16-bytes aligned for x86_64, and
8-bytes aligned for arm64, so I guess it is.
In the feature, if there is a exception, we can make ftrace_jmp_set,
ftrace_jmp_get arch-specification.
> At least register_ftrace_direct() needs to reject if @addr
> parameter has the last bit.
That make sense, I'll add such checking in the next version.
Thanks!
Menglong Dong
>
> Thanks,
>
>
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > include/linux/ftrace.h | 33 +++++++++++++++++++++++++++++++++
> > kernel/trace/Kconfig | 12 ++++++++++++
> > kernel/trace/ftrace.c | 9 ++++++++-
> > 3 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 07f8c309e432..015dd1049bea 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -359,6 +359,7 @@ 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
> > @@ -577,6 +578,38 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
> > unsigned long addr) { }
> > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> >
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
> > +static inline bool ftrace_is_jmp(unsigned long addr)
> > +{
> > + return addr & 1;
> > +}
> > +
> > +static inline unsigned long ftrace_jmp_set(unsigned long addr)
> > +{
> > + return addr | 1UL;
> > +}
> > +
> > +static inline unsigned long ftrace_jmp_get(unsigned long addr)
> > +{
> > + return addr & ~1UL;
> > +}
> > +#else
> > +static inline bool ftrace_is_jmp(unsigned long addr)
> > +{
> > + return false;
> > +}
> > +
> > +static inline unsigned long ftrace_jmp_set(unsigned long addr)
> > +{
> > + return addr;
> > +}
> > +
> > +static inline unsigned long ftrace_jmp_get(unsigned long addr)
> > +{
> > + return addr;
> > +}
> > +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_JMP */
> > +
> > #ifdef CONFIG_STACK_TRACER
> >
> > int stack_trace_sysctl(const struct ctl_table *table, int write, void *buffer,
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index d2c79da81e4f..4661b9e606e0 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -80,6 +80,12 @@ config HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
> > If the architecture generates __patchable_function_entries sections
> > but does not want them included in the ftrace locations.
> >
> > +config HAVE_DYNAMIC_FTRACE_WITH_JMP
> > + bool
> > + help
> > + If the architecture supports to replace the __fentry__ with a
> > + "jmp" instruction.
> > +
> > config HAVE_SYSCALL_TRACEPOINTS
> > bool
> > help
> > @@ -330,6 +336,12 @@ config DYNAMIC_FTRACE_WITH_ARGS
> > depends on DYNAMIC_FTRACE
> > depends on HAVE_DYNAMIC_FTRACE_WITH_ARGS
> >
> > +config DYNAMIC_FTRACE_WITH_JMP
> > + def_bool y
> > + depends on DYNAMIC_FTRACE
> > + depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > + depends on HAVE_DYNAMIC_FTRACE_WITH_JMP
> > +
> > config FPROBE
> > bool "Kernel Function Probe (fprobe)"
> > depends on HAVE_FUNCTION_GRAPH_FREGS && HAVE_FTRACE_GRAPH_FUNC
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 59cfacb8a5bb..a6c060a4f50b 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -5951,7 +5951,8 @@ static void remove_direct_functions_hash(struct ftrace_hash *hash, unsigned long
> > for (i = 0; i < size; i++) {
> > hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> > del = __ftrace_lookup_ip(direct_functions, entry->ip);
> > - if (del && del->direct == addr) {
> > + if (del && ftrace_jmp_get(del->direct) ==
> > + ftrace_jmp_get(addr)) {
> > remove_hash_entry(direct_functions, del);
> > kfree(del);
> > }
> > @@ -6018,6 +6019,9 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> >
> > 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++) {
> > @@ -6138,6 +6142,9 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> >
> > lockdep_assert_held_once(&direct_mutex);
> >
> > + 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;
>
>
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/6] bpf trampoline support "jmp" mode
From: Alexei Starovoitov @ 2025-11-18 6:31 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <20251117034906.32036-1-dongml2@chinatelecom.cn>
On Sun, Nov 16, 2025 at 7:49 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> For now, the bpf trampoline is called by the "call" instruction. However,
> it break the RSB and introduce extra overhead in x86_64 arch.
Please include performance numbers in the cover letter when you respin.
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/6] bpf trampoline support "jmp" mode
From: Menglong Dong @ 2025-11-18 6:34 UTC (permalink / raw)
To: Menglong Dong, Alexei Starovoitov
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <CAADnVQK5U28Wv2tSkymZY6ixCoUrSDoohB5wJmpyZL7t-Czk4w@mail.gmail.com>
On 2025/11/18 14:31, Alexei Starovoitov wrote:
> On Sun, Nov 16, 2025 at 7:49 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > For now, the bpf trampoline is called by the "call" instruction. However,
> > it break the RSB and introduce extra overhead in x86_64 arch.
>
> Please include performance numbers in the cover letter when you respin.
Hmm...I included a little performance, do you mean more performance
data? Current description:
As we can see above, the RSB is totally balanced. After the modification,
the performance of fexit increases from 76M/s to 130M/s.
>
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/6] bpf trampoline support "jmp" mode
From: Alexei Starovoitov @ 2025-11-18 6:41 UTC (permalink / raw)
To: Menglong Dong
Cc: Menglong Dong, Alexei Starovoitov, Steven Rostedt,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard, Song Liu, Yonghong Song, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, jiang.biao, bpf, LKML,
linux-trace-kernel
In-Reply-To: <5027922.GXAFRqVoOG@7950hx>
On Mon, Nov 17, 2025 at 10:34 PM Menglong Dong <menglong.dong@linux.dev> wrote:
>
> On 2025/11/18 14:31, Alexei Starovoitov wrote:
> > On Sun, Nov 16, 2025 at 7:49 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > >
> > > For now, the bpf trampoline is called by the "call" instruction. However,
> > > it break the RSB and introduce extra overhead in x86_64 arch.
> >
> > Please include performance numbers in the cover letter when you respin.
>
> Hmm...I included a little performance, do you mean more performance
> data? Current description:
>
> As we can see above, the RSB is totally balanced. After the modification,
> the performance of fexit increases from 76M/s to 130M/s.
I saw that. I meant full comparison with fentry and fmodret.
I suspect fmodret improved as well, right?
And include the command line that you used to measure.
selftests/bpf/bench...
so there is a way to reproduce what patchset claims.
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/6] bpf trampoline support "jmp" mode
From: Menglong Dong @ 2025-11-18 6:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Menglong Dong, Alexei Starovoitov, Steven Rostedt,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard, Song Liu, Yonghong Song, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, jiang.biao, bpf, LKML,
linux-trace-kernel
In-Reply-To: <CAADnVQJtm3pHFxYD=_FPJFiMNXwo-scj5CoNL5jHbUn+E0zvrQ@mail.gmail.com>
On Tue, Nov 18, 2025 at 2:41 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Nov 17, 2025 at 10:34 PM Menglong Dong <menglong.dong@linux.dev> wrote:
> >
> > On 2025/11/18 14:31, Alexei Starovoitov wrote:
> > > On Sun, Nov 16, 2025 at 7:49 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > > >
> > > > For now, the bpf trampoline is called by the "call" instruction. However,
> > > > it break the RSB and introduce extra overhead in x86_64 arch.
> > >
> > > Please include performance numbers in the cover letter when you respin.
> >
> > Hmm...I included a little performance, do you mean more performance
> > data? Current description:
> >
> > As we can see above, the RSB is totally balanced. After the modification,
> > the performance of fexit increases from 76M/s to 130M/s.
>
> I saw that. I meant full comparison with fentry and fmodret.
> I suspect fmodret improved as well, right?
> And include the command line that you used to measure.
> selftests/bpf/bench...
> so there is a way to reproduce what patchset claims.
I see. "fmodret" improved too, and all the BPF prog that based on
bpf trampoline origin call have a performance improvement.
I'll add the full comparison results in the next version.
Thanks!
Menglong Dong
^ permalink raw reply
* Re: [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Namhyung Kim @ 2025-11-18 7:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Peter Zijlstra, Thomas Gleixner,
Ian Rogers, Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <20251118002950.680329246@kernel.org>
Hi Steve,
On Mon, Nov 17, 2025 at 07:29:50PM -0500, Steven Rostedt wrote:
>
> This series adds a perf event to the ftrace ring buffer.
> It is currently a proof of concept as I'm not happy with the interface
> and I also think the recorded perf event format may be changed too.
>
> This proof-of-concept interface (which I have no plans on using), currently
> just adds 6 new trace options.
>
> event_cache_misses
> event_cpu_cycles
> func-cache-misses
> func-cpu-cycles
> funcgraph-cache-misses
> funcgraph-cpu-cycles
Unfortunately the hardware cache event is ambiguous on which level it
refers to and architectures define it differently. There are
encodings to clearly define the cache levels and accesses but the
support depends on the hardware capabilities.
>
> The first two trigger a perf event after every event, the second two trigger
> a perf event after every function and the last two trigger a perf event
> right after the start of a function and again at the end of the function.
>
> As this will eventual work with many more perf events than just cache-misses
> and cpu-cycles , using options is not appropriate. Especially since the
> options are limited to a 64 bit bitmask, and that can easily go much higher.
> I'm thinking about having a file instead that will act as a way to enable
> perf events for events, function and function graph tracing.
>
> set_event_perf, set_ftrace_perf, set_fgraph_perf
>
> And an available_perf_events that show what can be written into these files,
> (similar to how set_ftrace_filter works). But for now, it was just easier to
> implement them as options.
>
> As for the perf event that is triggered. It currently is a dynamic array of
> 64 bit values. Each value is broken up into 8 bits for what type of perf
> event it is, and 56 bits for the counter. It only writes a per CPU raw
> counter and does not do any math. That would be needed to be done by any
> post processing.
If you want to keep the perf events per CPU, you may consider CPU
migrations for the func-graph case. Otherwise userspace may not
calculate the diff from the begining correctly.
>
> Since the values are for user space to do the subtraction to figure out the
> difference between events, for example, the function_graph tracer may have:
>
> is_vmalloc_addr() {
> /* cpu_cycles: 5582263593 cache_misses: 2869004572 */
> /* cpu_cycles: 5582267527 cache_misses: 2869006049 */
> }
>
> User space would subtract 2869006049 - 2869004572 = 1477
>
> Then 56 bits should be plenty.
>
> 2^55 / 1,000,000,000 / 60 / 60 / 24 = 416
> 416 / 4 = 104
>
> If you have a 4GHz machine, the cpu-cycles will overflow the 55 bits in 104
> days. This tooling is not for seeing how many cycles run over 104 days.
> User space tooling would just need to be aware that the vale is 56 bits and
> when calculating the difference between start and end do something like:
>
> if (start > end)
> end |= 1ULL << 56;
>
> delta = end - start;
>
> The next question is how to label the perf events to be in the 8 bit
> portion. It could simply be a value that is registered, and listed in the
> available_perf_events file.
>
> cpu_cycles:1
> cach_misses:2
> [..]
>
> And this would need to be recorded by any tooling reading the events
> so that it knows how to map the events with their attached ids.
>
> But again, this is just a proof-of-concept. How this will eventually be
> implemented is yet to be determined.
>
> But to test these patches (which are based on top of my linux-next branch,
> which should now be in linux-next):
>
> # cd /sys/kernel/tracing
> # echo 1 > options/event_cpu_cycles
> # echo 1 > options/event_cache_misses
> # echo 1 > events/syscalls/enable
> # cat trace
> [..]
> bash-995 [007] ..... 98.255252: sys_write -> 0x2
> bash-995 [007] ..... 98.255257: cpu_cycles: 1557241774 cache_misses: 449901166
> bash-995 [007] ..... 98.255284: sys_dup2(oldfd: 0xa, newfd: 1)
> bash-995 [007] ..... 98.255285: cpu_cycles: 1557260057 cache_misses: 449902679
> bash-995 [007] ..... 98.255305: sys_dup2 -> 0x1
> bash-995 [007] ..... 98.255305: cpu_cycles: 1557280203 cache_misses: 449906196
> bash-995 [007] ..... 98.255343: sys_fcntl(fd: 0xa, cmd: 1, arg: 0)
> bash-995 [007] ..... 98.255344: cpu_cycles: 1557322304 cache_misses: 449915522
> bash-995 [007] ..... 98.255352: sys_fcntl -> 0x1
> bash-995 [007] ..... 98.255353: cpu_cycles: 1557327809 cache_misses: 449916844
> bash-995 [007] ..... 98.255361: sys_close(fd: 0xa)
> bash-995 [007] ..... 98.255362: cpu_cycles: 1557335383 cache_misses: 449918232
> bash-995 [007] ..... 98.255369: sys_close -> 0x0
>
>
>
> Comments welcomed.
Just FYI, I did the similar thing (like fgraph case) in uftrace and I
grouped two related events to produce a metric.
$ uftrace -T a@read=pmu-cycle ~/tmp/abc
# DURATION TID FUNCTION
[ 521741] | main() {
[ 521741] | a() {
[ 521741] | /* read:pmu-cycle (cycles=482 instructions=38) */
[ 521741] | b() {
[ 521741] | c() {
0.659 us [ 521741] | getpid();
1.600 us [ 521741] | } /* c */
1.780 us [ 521741] | } /* b */
[ 521741] | /* diff:pmu-cycle (cycles=+7361 instructions=+3955 IPC=0.54) */
24.485 us [ 521741] | } /* a */
34.797 us [ 521741] | } /* main */
It reads cycles and instructions events (specified by 'pmu-cycle') at
entry and exit of the given function ('a') and shows the diff with the
metric IPC.
Thanks,
Namhyung
>
>
> Steven Rostedt (3):
> tracing: Add perf events
> ftrace: Add perf counters to function tracing
> fgraph: Add perf counters to function graph tracer
>
> ----
> include/linux/trace_recursion.h | 5 +-
> kernel/trace/trace.c | 153 ++++++++++++++++++++++++++++++++-
> kernel/trace/trace.h | 38 ++++++++
> kernel/trace/trace_entries.h | 13 +++
> kernel/trace/trace_event_perf.c | 162 +++++++++++++++++++++++++++++++++++
> kernel/trace/trace_functions.c | 124 +++++++++++++++++++++++++--
> kernel/trace/trace_functions_graph.c | 117 +++++++++++++++++++++++--
> kernel/trace/trace_output.c | 70 +++++++++++++++
> 8 files changed, 670 insertions(+), 12 deletions(-)
^ permalink raw reply
* Re: [PATCH v3 1/8] mm: introduce VM_MAYBE_GUARD and make visible in /proc/$pid/smaps
From: Lorenzo Stoakes @ 2025-11-18 7:33 UTC (permalink / raw)
To: jane.chu
Cc: Andrew Morton, Jonathan Corbet, David Hildenbrand,
Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jann Horn, Pedro Falcato,
Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, linux-kernel, linux-fsdevel, linux-doc,
linux-mm, linux-trace-kernel, linux-kselftest, Andrei Vagin
In-Reply-To: <1c4b521a-f84c-4a3a-99f7-ecb9ace3baf1@oracle.com>
On Mon, Nov 17, 2025 at 05:05:26PM -0800, jane.chu@oracle.com wrote:
>
>
> On 11/7/2025 8:11 AM, Lorenzo Stoakes wrote:
[snip]
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 046579a6ec2f..334732ab6733 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1480,6 +1480,10 @@ vma_needs_copy(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
> > if (src_vma->anon_vma)
> > return true;
> > + /* Guard regions have momdified page tables that require copying. */
>
> Nit: s/momdified/modified.
Thanks, Liam mentioned this too and I forgot to fix too, going to respin and
resolve :) I must have been tired :P
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v3 1/8] mm: introduce VM_MAYBE_GUARD and make visible in /proc/$pid/smaps
From: Lorenzo Stoakes @ 2025-11-18 7:36 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Andrew Morton, Jonathan Corbet, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jann Horn, Pedro Falcato,
Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, linux-kernel, linux-fsdevel, linux-doc,
linux-mm, linux-trace-kernel, linux-kselftest, Andrei Vagin
In-Reply-To: <9e617d7b-afd7-465b-b075-32b02257b90b@kernel.org>
On Mon, Nov 17, 2025 at 04:11:47PM +0100, David Hildenbrand (Red Hat) wrote:
> On 07.11.25 17:11, Lorenzo Stoakes wrote:
> > Currently, if a user needs to determine if guard regions are present in a
> > range, they have to scan all VMAs (or have knowledge of which ones might
> > have guard regions).
> >
> > Since commit 8e2f2aeb8b48 ("fs/proc/task_mmu: add guard region bit to
> > pagemap") and the related commit a516403787e0 ("fs/proc: extend the
> > PAGEMAP_SCAN ioctl to report guard regions"), users can use either
> > /proc/$pid/pagemap or the PAGEMAP_SCAN functionality to perform this
> > operation at a virtual address level.
> >
> > This is not ideal, and it gives no visibility at a /proc/$pid/smaps level
> > that guard regions exist in ranges.
> >
> > This patch remedies the situation by establishing a new VMA flag,
> > VM_MAYBE_GUARD, to indicate that a VMA may contain guard regions (it is
> > uncertain because we cannot reasonably determine whether a
> > MADV_GUARD_REMOVE call has removed all of the guard regions in a VMA, and
> > additionally VMAs may change across merge/split).
> >
> > We utilise 0x800 for this flag which makes it available to 32-bit
> > architectures also, a flag that was previously used by VM_DENYWRITE, which
> > was removed in commit 8d0920bde5eb ("mm: remove VM_DENYWRITE") and hasn't
> > bee reused yet.
> >
> > We also update the smaps logic and documentation to identify these VMAs.
> >
> > Another major use of this functionality is that we can use it to identify
> > that we ought to copy page tables on fork.
> >
> > We do not actually implement usage of this flag in mm/madvise.c yet as we
> > need to allow some VMA flags to be applied atomically under mmap/VMA read
> > lock in order to avoid the need to acquire a write lock for this purpose.
> >
> > Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> > Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> > Documentation/filesystems/proc.rst | 5 +++--
> > fs/proc/task_mmu.c | 1 +
> > include/linux/mm.h | 3 +++
> > include/trace/events/mmflags.h | 1 +
> > mm/memory.c | 4 ++++
> > tools/testing/vma/vma_internal.h | 1 +
> > 6 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > index 0b86a8022fa1..8256e857e2d7 100644
> > --- a/Documentation/filesystems/proc.rst
> > +++ b/Documentation/filesystems/proc.rst
> > @@ -553,7 +553,7 @@ otherwise.
> > kernel flags associated with the particular virtual memory area in two letter
> > encoded manner. The codes are the following:
> > - == =======================================
> > + == =============================================================
> > rd readable
> > wr writeable
> > ex executable
> > @@ -591,7 +591,8 @@ encoded manner. The codes are the following:
> > sl sealed
> > lf lock on fault pages
> > dp always lazily freeable mapping
> > - == =======================================
> > + gu maybe contains guard regions (if not set, definitely doesn't)
> > + == =============================================================
>
>
> In general LGTM, BUT in the context of this patch where the flag is never
> set, that's not entirely correct ;) It made sense after staring at patch #5.
Yeah I realise that's a bit of compromise but I think relatively benign
in order to split up the series sensibly :)
>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
Thanks!
>
> --
> Cheers
>
> David
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v3 3/8] mm: implement sticky VMA flags
From: Lorenzo Stoakes @ 2025-11-18 7:46 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, David Hildenbrand, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jann Horn,
Pedro Falcato, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, linux-kernel, linux-fsdevel,
linux-doc, linux-mm, linux-trace-kernel, linux-kselftest,
Andrei Vagin
In-Reply-To: <20251117144332.d338e8368d59c3ab665db986@linux-foundation.org>
On Mon, Nov 17, 2025 at 02:43:32PM -0800, Andrew Morton wrote:
> On Mon, 17 Nov 2025 20:02:03 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> > Sorry to be a pain here, and can respin if it's easier, but can we update the
> > text of the comments below? As in discussion with Liam off-list we agreed that
> > the current wording is rather unclear and we can do a lot better.
> >
> > I provide the improved version inline below:
>
> np,
>
> include/linux/mm.h | 7 +++----
> tools/testing/vma/vma_internal.h | 7 +++----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> --- a/include/linux/mm.h~mm-implement-sticky-vma-flags-fix-2
> +++ a/include/linux/mm.h
> @@ -549,10 +549,9 @@ extern unsigned int kobjsize(const void
> * pressure on the memory system forcing the kernel to generate
> * new VMAs when old one could be extended instead.
> *
> - * VM_STICKY - If one VMA has flags which most be 'sticky', that is ones
> - * which should propagate to all VMAs, but the other does not,
> - * the merge should still proceed with the merge logic applying
> - * sticky flags to the final VMA.
> + * VM_STICKY - When merging VMAs, VMA flags must match, unless they are
> + * 'sticky'. If any sticky flags exist in either VMA, we simply
> + * set all of them on the merged VMA.
> */
> #define VM_IGNORE_MERGE (VM_SOFTDIRTY | VM_STICKY)
>
> --- a/tools/testing/vma/vma_internal.h~mm-implement-sticky-vma-flags-fix-2
> +++ a/tools/testing/vma/vma_internal.h
> @@ -139,10 +139,9 @@ extern unsigned long dac_mmap_min_addr;
> * pressure on the memory system forcing the kernel to generate
> * new VMAs when old one could be extended instead.
> *
> - * VM_STICKY - If one VMA has flags which most be 'sticky', that is ones
> - * which should propagate to all VMAs, but the other does not,
> - * the merge should still proceed with the merge logic applying
> - * sticky flags to the final VMA.
> + * VM_STICKY - When merging VMAs, VMA flags must match, unless they are
> + * 'sticky'. If any sticky flags exist in either VMA, we simply
> + * set all of them on the merged VMA.
> */
> #define VM_IGNORE_MERGE (VM_SOFTDIRTY | VM_STICKY)
>
> _
>
Thanks, much appreciated!
Despite your having very kindly done this, I apologise as I realise now after
all I have to respin... doh!
There was yet another typo as pointed out by Jane (and previously off-list,
Liam) plus Pedro (off-list) mentioned a silly mistake in my code fixup, and at
this point I think it'll keep us all a lot saner to just respin :>)
Cheers, Lorenzo
^ permalink raw reply
* [PATCH] tracing: Show the tracer options in boot-time created instance
From: Masami Hiramatsu (Google) @ 2025-11-18 7:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since tracer_init_tracefs_work_func() only updates the tracer options
for the global_trace, the instances created by the kernel cmdline
do not have those options.
Fix to update tracer options for those boot-time created instances
to show those options.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Fixes: f20a580627f43 ("ftrace: Allow instances to use function tracing")
---
kernel/trace/trace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8ae95800592d..2e87060e1d5a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10903,6 +10903,7 @@ static struct notifier_block trace_module_nb = {
static __init void tracer_init_tracefs_work_func(struct work_struct *work)
{
+ struct trace_array *tr;
event_trace_init();
@@ -10937,7 +10938,8 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
create_trace_instances(NULL);
- update_tracer_options(&global_trace);
+ list_for_each_entry(tr, &ftrace_trace_arrays, list)
+ update_tracer_options(tr);
}
static __init int tracer_init_tracefs(void)
^ 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