* [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 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 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
* Re: [PATCH bpf-next v2 6/6] bpf: implement "jmp" mode for trampoline
From: kernel test robot @ 2025-11-17 22:49 UTC (permalink / raw)
To: Menglong Dong, ast, rostedt
Cc: llvm, 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: arm64-randconfig-002-20251118 (https://download.01.org/0day-ci/archive/20251118/202511180613.Om7k1nP1-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 0bba1e76581bad04e7d7f09f5115ae5e2989e0d9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251118/202511180613.Om7k1nP1-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/202511180613.Om7k1nP1-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/bpf/trampoline.c:500:11: error: incomplete definition of type 'struct ftrace_ops'
500 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ~~~~~~~~^
include/linux/ftrace.h:40:8: note: forward declaration of 'struct ftrace_ops'
40 | struct ftrace_ops;
| ^
>> kernel/bpf/trampoline.c:500:22: error: use of undeclared identifier 'FTRACE_OPS_FL_JMP'
500 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ^~~~~~~~~~~~~~~~~
kernel/bpf/trampoline.c:502:11: error: incomplete definition of type 'struct ftrace_ops'
502 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ~~~~~~~~^
include/linux/ftrace.h:40:8: note: forward declaration of 'struct ftrace_ops'
40 | struct ftrace_ops;
| ^
kernel/bpf/trampoline.c:502:23: error: use of undeclared identifier 'FTRACE_OPS_FL_JMP'
502 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ^~~~~~~~~~~~~~~~~
kernel/bpf/trampoline.c:534:12: error: incomplete definition of type 'struct ftrace_ops'
534 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ~~~~~~~~^
include/linux/ftrace.h:40:8: note: forward declaration of 'struct ftrace_ops'
40 | struct ftrace_ops;
| ^
kernel/bpf/trampoline.c:534:23: error: use of undeclared identifier 'FTRACE_OPS_FL_JMP'
534 | tr->fops->flags |= FTRACE_OPS_FL_JMP;
| ^~~~~~~~~~~~~~~~~
kernel/bpf/trampoline.c:536:12: error: incomplete definition of type 'struct ftrace_ops'
536 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ~~~~~~~~^
include/linux/ftrace.h:40:8: note: forward declaration of 'struct ftrace_ops'
40 | struct ftrace_ops;
| ^
kernel/bpf/trampoline.c:536:24: error: use of undeclared identifier 'FTRACE_OPS_FL_JMP'
536 | tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
| ^~~~~~~~~~~~~~~~~
8 errors generated.
vim +/FTRACE_OPS_FL_JMP +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: [PATCH v3 3/8] mm: implement sticky VMA flags
From: Andrew Morton @ 2025-11-17 22:43 UTC (permalink / raw)
To: Lorenzo Stoakes
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: <cf58c518-05d0-494f-8fe4-571879834031@lucifer.local>
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)
_
^ permalink raw reply
* Re: [PATCH] unwind: Show that entries of struct unwind_cache is not bound by nr_entries
From: Kees Cook @ 2025-11-17 21:28 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Thorsten Blum, Josh Poimboeuf,
Peter Zijlstra, Gustavo A. R. Silva, David Laight
In-Reply-To: <20251114121352.35108fb8@gandalf.local.home>
On Fri, Nov 14, 2025 at 12:13:52PM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The structure unwind_cache has:
>
> struct unwind_cache {
> unsigned long unwind_completed;
> unsigned int nr_entries;
> unsigned long entries[];
> };
>
> Which triggers lots of scripts to convert this to:
>
> struct unwind_cache {
> unsigned long unwind_completed;
> unsigned int nr_entries;
> unsigned long entries[] __counted_by(nr_entries);
> };
>
> But that is incorrect. The structure is created via:
>
> #define UNWIND_MAX_ENTRIES \
> ((SZ_4K - sizeof(struct unwind_cache)) / sizeof(long))
>
> info->cache = kzalloc(struct_size(cache, entries, UNWIND_MAX_ENTRIES), GFP_KERNEL);
>
> Where the size of entries is determined by the size of the rest of the
> structure subtracted from 4K. But because the size of entries has a
> dependency on the structure itself, it can't be used to define it.
>
> The entries are filled by another function that returns how many entries it
> added and that is what nr_entries gets set to. This would most definitely
> trigger a false-positive out-of-bounds bug if the __counted_by() was added.
Just so I'm clear: "nr_entries" shows how many _valid_ entries exist in
entries[] (even if more are allocated there)? (Wouldn't you still want to
know if something tried to access an invalid entry? But I digress...)
> To stop scripts from thinking this needs a counted_by(), move the
> UNWIND_MAX_ENTRIES macro to the header, and add a comment in the entries
> size:
>
> unsigned long entries[ /* UNWIND_MAX_ENTRIES */ ];
This doesn't solve the problem that we've hidden the actual size of the
(fixed size!) object from the compiler, forcing it to avoid doing bounds
checking on it.
The problem is that the non-"entries" portion of the struct doesn't have
a "name" associated with it at declaration time, but we have a solution
for that already: struct_group. I would propose this form instead, which
requires no changes to how unwind_cache is used, and allows for the true
size of the "entries" array to be exposed to the compiler (and allows
for "normal" methods of finding the max entries):
struct unwind_cache {
struct_group_tagged(unwind_cache_hdr, hdr,
unsigned long unwind_completed;
unsigned int nr_entries;
);
unsigned long entries[(SZ_4K - sizeof(struct unwind_cache_hdr)) / sizeof(long)];
};
#define UNWIND_MAX_ENTRIES ARRAY_SIZE(((struct unwind_cache*)NULL)->entries)
And this checks out for me:
UNWIND_MAX_ENTRIES:510
sizeof(struct unwind_cache):4096
No hiding things from the compiler, and you can treat "entries" like a
real array (since it is one now).
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH bpf-next v2 5/6] bpf: specify the old and new poke_type for bpf_arch_text_poke
From: kernel test robot @ 2025-11-17 20:55 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-6-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-6-dongml2%40chinatelecom.cn
patch subject: [PATCH bpf-next v2 5/6] bpf: specify the old and new poke_type for bpf_arch_text_poke
config: powerpc64-randconfig-002-20251118 (https://download.01.org/0day-ci/archive/20251118/202511180431.JVOEm6SO-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251118/202511180431.JVOEm6SO-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/202511180431.JVOEm6SO-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/powerpc/net/bpf_jit_comp.c: In function 'bpf_arch_text_poke':
>> arch/powerpc/net/bpf_jit_comp.c:1135:7: error: 'poke_type' undeclared (first use in this function); did you mean 'probe_type'?
if (poke_type != BPF_MOD_JUMP) {
^~~~~~~~~
probe_type
arch/powerpc/net/bpf_jit_comp.c:1135:7: note: each undeclared identifier is reported only once for each function it appears in
vim +1135 arch/powerpc/net/bpf_jit_comp.c
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1070
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1071 /*
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1072 * A 3-step process for bpf prog entry:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1073 * 1. At bpf prog entry, a single nop/b:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1074 * bpf_func:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1075 * [nop|b] ool_stub
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1076 * 2. Out-of-line stub:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1077 * ool_stub:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1078 * mflr r0
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1079 * [b|bl] <bpf_prog>/<long_branch_stub>
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1080 * mtlr r0 // CONFIG_PPC_FTRACE_OUT_OF_LINE only
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1081 * b bpf_func + 4
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1082 * 3. Long branch stub:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1083 * long_branch_stub:
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1084 * .long <branch_addr>/<dummy_tramp>
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1085 * mflr r11
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1086 * bcl 20,31,$+4
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1087 * mflr r12
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1088 * ld r12, -16(r12)
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1089 * mtctr r12
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1090 * mtlr r11 // needed to retain ftrace ABI
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1091 * bctr
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1092 *
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1093 * dummy_tramp is used to reduce synchronization requirements.
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1094 *
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1095 * When attaching a bpf trampoline to a bpf prog, we do not need any
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1096 * synchronization here since we always have a valid branch target regardless
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1097 * of the order in which the above stores are seen. dummy_tramp ensures that
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1098 * the long_branch stub goes to a valid destination on other cpus, even when
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1099 * the branch to the long_branch stub is seen before the updated trampoline
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1100 * address.
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1101 *
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1102 * However, when detaching a bpf trampoline from a bpf prog, or if changing
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1103 * the bpf trampoline address, we need synchronization to ensure that other
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1104 * cpus can no longer branch into the older trampoline so that it can be
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1105 * safely freed. bpf_tramp_image_put() uses rcu_tasks to ensure all cpus
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1106 * make forward progress, but we still need to ensure that other cpus
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1107 * execute isync (or some CSI) so that they don't go back into the
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1108 * trampoline again.
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1109 */
7cc5910294285d Menglong Dong 2025-11-17 1110 int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
7cc5910294285d Menglong Dong 2025-11-17 1111 enum bpf_text_poke_type new_t, void *old_addr,
7cc5910294285d Menglong Dong 2025-11-17 1112 void *new_addr)
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1113 {
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1114 unsigned long bpf_func, bpf_func_end, size, offset;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1115 ppc_inst_t old_inst, new_inst;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1116 int ret = 0, branch_flags;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1117 char name[KSYM_NAME_LEN];
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1118
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1119 if (IS_ENABLED(CONFIG_PPC32))
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1120 return -EOPNOTSUPP;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1121
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1122 bpf_func = (unsigned long)ip;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1123
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1124 /* We currently only support poking bpf programs */
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1125 if (!__bpf_address_lookup(bpf_func, &size, &offset, name)) {
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1126 pr_err("%s (0x%lx): kernel/modules are not supported\n", __func__, bpf_func);
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1127 return -EOPNOTSUPP;
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1128 }
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1129
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1130 /*
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1131 * If we are not poking at bpf prog entry, then we are simply patching in/out
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1132 * an unconditional branch instruction at im->ip_after_call
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1133 */
d243b62b7bd3d5 Naveen N Rao 2024-10-30 1134 if (offset) {
d243b62b7bd3d5 Naveen N Rao 2024-10-30 @1135 if (poke_type != BPF_MOD_JUMP) {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Dave Chinner @ 2025-11-17 20:51 UTC (permalink / raw)
To: John Garry
Cc: Ojaswin Mujoo, Ritesh Harjani, Christoph Hellwig,
Christian Brauner, djwong, tytso, willy, dchinner, linux-xfs,
linux-kernel, linux-ext4, linux-fsdevel, linux-mm, jack, nilay,
martin.petersen, rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <8d645cb5-7589-4544-a547-19729610d44d@oracle.com>
On Mon, Nov 17, 2025 at 10:59:55AM +0000, John Garry wrote:
> On 16/11/2025 08:11, Dave Chinner wrote:
> > > This patch set focuses on HW accelerated single block atomic writes with
> > > buffered IO, to get some early reviews on the core design.
> > What hardware acceleration? Hardware atomic writes are do not make
> > IO faster; they only change IO failure semantics in certain corner
> > cases.
>
> I think that he references using REQ_ATOMIC-based bio vs xfs software-based
> atomic writes (which reuse the CoW infrastructure). And the former is
> considerably faster from my testing (for DIO, obvs). But the latter has not
> been optimized.
For DIO, REQ_ATOMIC IO will generally be faster than the software
fallback because no page cache interactions or data copy is required
by the DIO REQ_ATOMIC fast path.
But we are considering buffered writes, which *must* do a data copy,
and so the behaviour and performance differential of doing a COW vs
trying to force writeback to do REQ_ATOMIC IO is going to be much
different.
Consider that the way atomic buffered writes have been implemented
in writeback - turning off all folio and IO merging. This means
writeback efficiency of atomic writes is going to be horrendous
compared to COW writes that don't use REQ_ATOMIC.
Further, REQ_ATOMIC buffered writes need to turn off delayed
allocation because if you can't allocate aligned extents then the
atomic write can *never* be performed. Hence we have to allocate up
front where we can return errors to userspace immediately, rather
than just reserve space and punt allocation to writeback. i.e. we
have to avoid the situation where we have dirty "atomic" data in the
page cache that cannot be written because physical allocation fails.
The likely outcome of turning off delalloc is that it further
degrades buffered atomic write writeback efficiency because it
removes the ability for the filesystem to optimise physical locality
of writeback IO. e.g. adjacent allocation across multiple small
files or packing of random writes in a single file to allow them to
merge at the block layer into one big IO...
REQ_ATOMIC is a natural fit for DIO because DIO is largely a "one
write syscall, one physical IO" style interface. Buffered writes,
OTOH, completely decouples application IO from physical IO, and so
there is no real "atomic" connection between the data being written
into the page caceh and the physical IO that is performed at some
time later.
This decoupling of physical IO is what brings all the problems and
inefficiencies. The filesystem being able to mark the RWF_ATOMIC
write range as a COW range at submission time creates a natural
"atomic IO" behaviour without requiring the page cache or writeback
to even care that the data needs to be written atomically.
From there, we optimise the COW IO path to record that
the new COW extent was created for the purpose of an atomic write.
Then when we go to write back data over that extent, the filesystem
can chose to do a REQ_ATOMIC write to do an atomic overwrite instead
of allocating a new extent and swapping the BMBT extent pointers at
IO completion time.
We really don't care if 4x16kB adjacent RWF_ATOMIC writes are
submitted as 1x64kB REQ_ATOMIC IO or 4 individual 16kB REQ_ATOMIC
IOs. The former is much more efficient from an IO perspective, and
the COW path can actually optimise for this because it can track the
atomic write ranges in cache exactly. If the range is larger (or
unaligned) than what REQ_ATOMIC can handle, we use COW writeback to
optimise for maximum writeback bandwidth, otherwise we use
REQ_ATOMIC to optimise for minimum writeback submission and
completion overhead...
IOWs, I think that for XFS (and other COW-capable filesystems) we
should be looking at optimising the COW IO path to use REQ_ATOMIC
where appropriate to create a direct overwrite fast path for
RWF_ATOMIC buffered writes. This seems a more natural and a lot less
intrusive than trying to blast through the page caceh abstractions
to directly couple userspace IO boundaries to physical writeback IO
boundaries...
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH v3 3/8] mm: implement sticky VMA flags
From: Lorenzo Stoakes @ 2025-11-17 20:02 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: <1ee529ff912f71b3460d0d21bc5b32ca89d63513.1762531708.git.lorenzo.stoakes@oracle.com>
Hi Andrew,
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:
On Fri, Nov 07, 2025 at 04:11:48PM +0000, Lorenzo Stoakes wrote:
> It is useful to be able to designate that certain flags are 'sticky', that
> is, if two VMAs are merged one with a flag of this nature and one without,
> the merged VMA sets this flag.
>
> As a result we ignore these flags for the purposes of determining VMA flag
> differences between VMAs being considered for merge.
>
> This patch therefore updates the VMA merge logic to perform this action,
> with flags possessing this property being described in the VM_STICKY
> bitmap.
>
> Those flags which ought to be ignored for the purposes of VMA merge are
> described in the VM_IGNORE_MERGE bitmap, which the VMA merge logic is also
> updated to use.
>
> As part of this change we place VM_SOFTDIRTY in VM_IGNORE_MERGE as it
> already had this behaviour, alongside VM_STICKY as sticky flags by
> implication must not disallow merge.
>
> Ultimately it seems that we should make VM_SOFTDIRTY a sticky flag in its
> own right, but this change is out of scope for this series.
>
> The only sticky flag designated as such is VM_MAYBE_GUARD, so as a result
> of this change, once the VMA flag is set upon guard region installation,
> VMAs with guard ranges will now not have their merge behaviour impacted as
> a result and can be freely merged with other VMAs without VM_MAYBE_GUARD
> set.
>
> We also update the VMA userland tests to account for the changes.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> include/linux/mm.h | 29 +++++++++++++++++++++++++++++
> mm/vma.c | 22 ++++++++++++----------
> tools/testing/vma/vma_internal.h | 29 +++++++++++++++++++++++++++++
> 3 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 699566c21ff7..6c1c459e9acb 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -527,6 +527,35 @@ extern unsigned int kobjsize(const void *objp);
> #endif
> #define VM_FLAGS_CLEAR (ARCH_VM_PKEY_FLAGS | VM_ARCH_CLEAR)
>
> +/*
> + * Flags which should be 'sticky' on merge - that is, flags which, when one VMA
> + * possesses it but the other does not, the merged VMA should nonetheless have
> + * applied to it:
> + *
> + * VM_MAYBE_GUARD - If a VMA may have guard regions in place it implies that
> + * mapped page tables may contain metadata not described by the
> + * VMA and thus any merged VMA may also contain this metadata,
> + * and thus we must make this flag sticky.
> + */
> +#define VM_STICKY VM_MAYBE_GUARD
> +
> +/*
> + * VMA flags we ignore for the purposes of merge, i.e. one VMA possessing one
> + * of these flags and the other not does not preclude a merge.
> + *
> + * VM_SOFTDIRTY - Should not prevent from VMA merging, if we match the flags but
> + * dirty bit -- the caller should mark merged VMA as dirty. If
> + * dirty bit won't be excluded from comparison, we increase
> + * pressure on the memory system forcing the kernel to generate
> + * new VMAs when old one could be extended instead.
> + *
Could you replace this:
> + * 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.
With this:
* 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)
> +
> /*
> * mapping from the currently active vm_flags protection bits (the
> * low four bits) to a page protection mask..
> diff --git a/mm/vma.c b/mm/vma.c
> index 0c5e391fe2e2..6cb082bc5e29 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -89,15 +89,7 @@ static inline bool is_mergeable_vma(struct vma_merge_struct *vmg, bool merge_nex
>
> if (!mpol_equal(vmg->policy, vma_policy(vma)))
> return false;
> - /*
> - * VM_SOFTDIRTY should not prevent from VMA merging, if we
> - * match the flags but dirty bit -- the caller should mark
> - * merged VMA as dirty. If dirty bit won't be excluded from
> - * comparison, we increase pressure on the memory system forcing
> - * the kernel to generate new VMAs when old one could be
> - * extended instead.
> - */
> - if ((vma->vm_flags ^ vmg->vm_flags) & ~VM_SOFTDIRTY)
> + if ((vma->vm_flags ^ vmg->vm_flags) & ~VM_IGNORE_MERGE)
> return false;
> if (vma->vm_file != vmg->file)
> return false;
> @@ -808,6 +800,7 @@ static bool can_merge_remove_vma(struct vm_area_struct *vma)
> static __must_check struct vm_area_struct *vma_merge_existing_range(
> struct vma_merge_struct *vmg)
> {
> + vm_flags_t sticky_flags = vmg->vm_flags & VM_STICKY;
> struct vm_area_struct *middle = vmg->middle;
> struct vm_area_struct *prev = vmg->prev;
> struct vm_area_struct *next;
> @@ -900,11 +893,13 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> if (merge_right) {
> vma_start_write(next);
> vmg->target = next;
> + sticky_flags |= (next->vm_flags & VM_STICKY);
> }
>
> if (merge_left) {
> vma_start_write(prev);
> vmg->target = prev;
> + sticky_flags |= (prev->vm_flags & VM_STICKY);
> }
>
> if (merge_both) {
> @@ -974,6 +969,7 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> if (err || commit_merge(vmg))
> goto abort;
>
> + vm_flags_set(vmg->target, sticky_flags);
> khugepaged_enter_vma(vmg->target, vmg->vm_flags);
> vmg->state = VMA_MERGE_SUCCESS;
> return vmg->target;
> @@ -1124,6 +1120,10 @@ int vma_expand(struct vma_merge_struct *vmg)
> bool remove_next = false;
> struct vm_area_struct *target = vmg->target;
> struct vm_area_struct *next = vmg->next;
> + vm_flags_t sticky_flags;
> +
> + sticky_flags = vmg->vm_flags & VM_STICKY;
> + sticky_flags |= target->vm_flags & VM_STICKY;
>
> VM_WARN_ON_VMG(!target, vmg);
>
> @@ -1133,6 +1133,7 @@ int vma_expand(struct vma_merge_struct *vmg)
> if (next && (target != next) && (vmg->end == next->vm_end)) {
> int ret;
>
> + sticky_flags |= next->vm_flags & VM_STICKY;
> remove_next = true;
> /* This should already have been checked by this point. */
> VM_WARN_ON_VMG(!can_merge_remove_vma(next), vmg);
> @@ -1159,6 +1160,7 @@ int vma_expand(struct vma_merge_struct *vmg)
> if (commit_merge(vmg))
> goto nomem;
>
> + vm_flags_set(target, sticky_flags);
> return 0;
>
> nomem:
> @@ -1902,7 +1904,7 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
> return a->vm_end == b->vm_start &&
> mpol_equal(vma_policy(a), vma_policy(b)) &&
> a->vm_file == b->vm_file &&
> - !((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_SOFTDIRTY)) &&
> + !((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_IGNORE_MERGE)) &&
> b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
> }
>
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index 46acb4df45de..a54990aa3009 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -117,6 +117,35 @@ extern unsigned long dac_mmap_min_addr;
> #define VM_SEALED VM_NONE
> #endif
>
> +/*
> + * Flags which should be 'sticky' on merge - that is, flags which, when one VMA
> + * possesses it but the other does not, the merged VMA should nonetheless have
> + * applied to it:
> + *
> + * VM_MAYBE_GUARD - If a VMA may have guard regions in place it implies that
> + * mapped page tables may contain metadata not described by the
> + * VMA and thus any merged VMA may also contain this metadata,
> + * and thus we must make this flag sticky.
> + */
> +#define VM_STICKY VM_MAYBE_GUARD
> +
> +/*
> + * VMA flags we ignore for the purposes of merge, i.e. one VMA possessing one
> + * of these flags and the other not does not preclude a merge.
> + *
> + * VM_SOFTDIRTY - Should not prevent from VMA merging, if we match the flags but
> + * dirty bit -- the caller should mark merged VMA as dirty. If
> + * dirty bit won't be excluded from comparison, we increase
> + * pressure on the memory system forcing the kernel to generate
> + * new VMAs when old one could be extended instead.
> + *
Also here, we dupicate this for the VMA unit tests, could we replace:
> + * 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.
With:
* 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)
> +
> #define FIRST_USER_ADDRESS 0UL
> #define USER_PGTABLES_CEILING 0UL
>
> --
> 2.51.0
>
Thanks, Lorenzo
^ permalink raw reply
* [rtla 13/13] rtla: Fix inconsistent state in actions_add_* functions
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The actions_add_trace_output() and actions_add_shell() functions
leave the action list in an inconsistent state when strdup() fails.
The actions_new() function increments self->len before returning a
pointer to the new action slot, but if the subsequent strdup()
allocation fails, the function returns an error without decrementing
self->len back.
This leaves an action object in an invalid state within the list.
When actions_destroy() or other functions iterate over the list
using for_each_action(), they will access this invalid entry with
uninitialized fields, potentially leading to undefined behavior.
Fix this by decrementing self->len when strdup() fails, effectively
returning the allocated slot back to the pool and maintaining list
consistency even when memory allocation fails.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 2d153d5efdea2..4aaaedadcc42a 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -76,11 +76,13 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
if (!action)
return -1;
- self->present[ACTION_TRACE_OUTPUT] = true;
action->type = ACTION_TRACE_OUTPUT;
action->trace_output = strdup(trace_output);
- if (!action->trace_output)
+ if (!action->trace_output) {
+ self->len--; // return the action object to the pool
return -1;
+ }
+ self->present[ACTION_TRACE_OUTPUT] = true;
return 0;
}
@@ -115,11 +117,13 @@ actions_add_shell(struct actions *self, const char *command)
if (!action)
return -1;
- self->present[ACTION_SHELL] = true;
action->type = ACTION_SHELL;
action->command = strdup(command);
- if (!action->command)
+ if (!action->command) {
+ self->len--;
return -1;
+ }
+ self->present[ACTION_SHELL] = true;
return 0;
}
--
2.51.1
^ permalink raw reply related
* [rtla 12/13] rtla: Remove unused headers
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, Wander Lairson Costa, Tomas Glozar, Crystal Wood,
Ivan Pravdin, John Kacur, Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20251117184409.42831-1-wander@redhat.com>
Remove unused includes for <errno.h> and <signal.h> to clean up the
code and reduce unnecessary dependencies.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/osnoise_hist.c | 1 -
tools/tracing/rtla/src/timerlat.c | 1 -
tools/tracing/rtla/src/timerlat_top.c | 1 -
tools/tracing/rtla/src/trace.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index dffb6d0a98d7d..44614f56493f2 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -9,7 +9,6 @@
#include <string.h>
#include <signal.h>
#include <unistd.h>
-#include <errno.h>
#include <stdio.h>
#include <time.h>
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 50c7eb00fd6b7..9b77dbcbd1375 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -9,7 +9,6 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <sched.h>
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index d831a9e1818f4..7f2491e72e495 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -11,7 +11,6 @@
#include <unistd.h>
#include <stdio.h>
#include <time.h>
-#include <errno.h>
#include <sched.h>
#include <pthread.h>
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 658a6e94edfba..ab2bf6d04f2ee 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -2,7 +2,6 @@
#define _GNU_SOURCE
#include <sys/sendfile.h>
#include <tracefs.h>
-#include <signal.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
--
2.51.1
^ permalink raw reply related
* [rtla 11/13] rtla: Replace magic number with MAX_PATH
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The trace functions use a buffer to manipulate strings that will be
written to tracefs files. These buffers are defined with a magic number
of 1024, which is a common source of vulnerabilities.
Replace the magic number 1024 with the MAX_PATH macro to make the code
safer and more readable. While at it, replace other instances of the
magic number with ARRAY_SIZE() when the buffer is locally defined.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/osnoise.c | 4 ++--
tools/tracing/rtla/src/timerlat_u.c | 4 ++--
tools/tracing/rtla/src/trace.c | 20 ++++++++++----------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 5075e0f485c77..d502cbbcea91b 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -52,7 +52,7 @@ char *osnoise_get_cpus(struct osnoise_context *context)
int osnoise_set_cpus(struct osnoise_context *context, char *cpus)
{
char *orig_cpus = osnoise_get_cpus(context);
- char buffer[1024];
+ char buffer[MAX_PATH];
int retval;
if (!orig_cpus)
@@ -62,7 +62,7 @@ int osnoise_set_cpus(struct osnoise_context *context, char *cpus)
if (!context->curr_cpus)
return -1;
- snprintf(buffer, 1024, "%s\n", cpus);
+ snprintf(buffer, ARRAY_SIZE(buffer), "%s\n", cpus);
debug_msg("setting cpus to %s from %s", cpus, context->orig_cpus);
diff --git a/tools/tracing/rtla/src/timerlat_u.c b/tools/tracing/rtla/src/timerlat_u.c
index 01dbf9a6b5a51..52977e725c79c 100644
--- a/tools/tracing/rtla/src/timerlat_u.c
+++ b/tools/tracing/rtla/src/timerlat_u.c
@@ -32,7 +32,7 @@
static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
{
struct sched_param sp = { .sched_priority = 95 };
- char buffer[1024];
+ char buffer[MAX_PATH];
int timerlat_fd;
cpu_set_t set;
int retval;
@@ -87,7 +87,7 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
/* add should continue with a signal handler */
while (true) {
- retval = read(timerlat_fd, buffer, 1024);
+ retval = read(timerlat_fd, buffer, ARRAY_SIZE(buffer));
if (retval < 0)
break;
}
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 813f4368f104b..658a6e94edfba 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -330,7 +330,7 @@ int trace_event_add_trigger(struct trace_events *event, char *trigger)
static void trace_event_disable_filter(struct trace_instance *instance,
struct trace_events *tevent)
{
- char filter[1024];
+ char filter[MAX_PATH];
int retval;
if (!tevent->filter)
@@ -342,7 +342,7 @@ static void trace_event_disable_filter(struct trace_instance *instance,
debug_msg("Disabling %s:%s filter %s\n", tevent->system,
tevent->event ? : "*", tevent->filter);
- snprintf(filter, 1024, "!%s\n", tevent->filter);
+ snprintf(filter, ARRAY_SIZE(filter), "!%s\n", tevent->filter);
retval = tracefs_event_file_write(instance->inst, tevent->system,
tevent->event, "filter", filter);
@@ -361,7 +361,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
{
int retval, index, out_fd;
mode_t mode = 0644;
- char path[1024];
+ char path[MAX_PATH];
char *hist;
if (!tevent)
@@ -376,7 +376,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
if (retval)
return;
- snprintf(path, 1024, "%s_%s_hist.txt", tevent->system, tevent->event);
+ snprintf(path, ARRAY_SIZE(path), "%s_%s_hist.txt", tevent->system, tevent->event);
printf(" Saving event %s:%s hist to %s\n", tevent->system, tevent->event, path);
@@ -408,7 +408,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
static void trace_event_disable_trigger(struct trace_instance *instance,
struct trace_events *tevent)
{
- char trigger[1024];
+ char trigger[MAX_PATH];
int retval;
if (!tevent->trigger)
@@ -422,7 +422,7 @@ static void trace_event_disable_trigger(struct trace_instance *instance,
trace_event_save_hist(instance, tevent);
- snprintf(trigger, 1024, "!%s\n", tevent->trigger);
+ snprintf(trigger, ARRAY_SIZE(trigger), "!%s\n", tevent->trigger);
retval = tracefs_event_file_write(instance->inst, tevent->system,
tevent->event, "trigger", trigger);
@@ -461,7 +461,7 @@ void trace_events_disable(struct trace_instance *instance,
static int trace_event_enable_filter(struct trace_instance *instance,
struct trace_events *tevent)
{
- char filter[1024];
+ char filter[MAX_PATH];
int retval;
if (!tevent->filter)
@@ -473,7 +473,7 @@ static int trace_event_enable_filter(struct trace_instance *instance,
return 1;
}
- snprintf(filter, 1024, "%s\n", tevent->filter);
+ snprintf(filter, ARRAY_SIZE(filter), "%s\n", tevent->filter);
debug_msg("Enabling %s:%s filter %s\n", tevent->system,
tevent->event ? : "*", tevent->filter);
@@ -496,7 +496,7 @@ static int trace_event_enable_filter(struct trace_instance *instance,
static int trace_event_enable_trigger(struct trace_instance *instance,
struct trace_events *tevent)
{
- char trigger[1024];
+ char trigger[MAX_PATH];
int retval;
if (!tevent->trigger)
@@ -508,7 +508,7 @@ static int trace_event_enable_trigger(struct trace_instance *instance,
return 1;
}
- snprintf(trigger, 1024, "%s\n", tevent->trigger);
+ snprintf(trigger, ARRAY_SIZE(trigger), "%s\n", tevent->trigger);
debug_msg("Enabling %s:%s trigger %s\n", tevent->system,
tevent->event ? : "*", tevent->trigger);
--
2.51.1
^ permalink raw reply related
* [rtla 10/13] rtla: Remove redundant memset after calloc
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, Wander Lairson Costa, Tomas Glozar, Crystal Wood,
Ivan Pravdin, John Kacur, Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20251117184409.42831-1-wander@redhat.com>
The actions struct is allocated using calloc, which already returns
zeroed memory. The subsequent memset call to zero the 'present' member
is therefore redundant.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 2a01ece78454c..2d153d5efdea2 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -21,8 +21,6 @@ actions_init(struct actions *self)
self->len = 0;
self->continue_flag = false;
- memset(&self->present, 0, sizeof(self->present));
-
/* This has to be set by the user */
self->trace_output_inst = NULL;
return 0;
--
2.51.1
^ permalink raw reply related
* [rtla 09/13] rtla: Exit if trace output action fails
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The actions_add_trace_output() function can fail if it's unable to
allocate memory for a new action. Currently, the return value is not
checked, and the program continues to run, which can lead to
unexpected behavior.
Add a check to the return value of actions_add_trace_output() and
exit with a proper error message if it fails.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/timerlat_hist.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index f14fc56c5b4a5..39a14c4e07de8 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -1070,8 +1070,10 @@ static struct common_params
}
}
- if (trace_output)
- actions_add_trace_output(¶ms->common.threshold_actions, trace_output);
+ if (trace_output && actions_add_trace_output(¶ms->common.threshold_actions, trace_output)) {
+ err_msg("Could not add a new trace output");
+ exit(EXIT_FAILURE);
+ }
if (geteuid()) {
err_msg("rtla needs root permission\n");
--
2.51.1
^ permalink raw reply related
* [rtla 08/13] rtla: Use standard exit codes for result enum
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The result enum defines custom values for PASSED, ERROR, and FAILED.
These values correspond to standard exit codes EXIT_SUCCESS and
EXIT_FAILURE.
Update the enum to use the standard macros EXIT_SUCCESS and
EXIT_FAILURE to improve readability and adherence to standard C
practices.
The FAILED value is implicitly assigned EXIT_FAILURE + 1, so there
is no need to assign an explicit value.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/utils.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index f7ff548f7fba7..beff211fdae2d 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -4,6 +4,7 @@
#include <time.h>
#include <sched.h>
#include <stdbool.h>
+#include <stdlib.h>
/*
* '18446744073709551615\0'
@@ -97,7 +98,7 @@ bool strtoi(const char *s, int *res);
#define ns_to_per(total, part) ((part * 100) / (double)total)
enum result {
- PASSED = 0, /* same as EXIT_SUCCESS */
- ERROR = 1, /* same as EXIT_FAILURE, an error in arguments */
- FAILED = 2, /* test hit the stop tracing condition */
+ PASSED = EXIT_SUCCESS,
+ ERROR = EXIT_FAILURE,
+ FAILED, /* test hit the stop tracing condition */
};
--
2.51.1
^ permalink raw reply related
* [rtla 07/13] rtla: Introduce timerlat_restart() helper
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, Wander Lairson Costa, Tomas Glozar, Crystal Wood,
Ivan Pravdin, John Kacur, Costa Shulyupin, Tiezhu Yang,
open list:Real-time Linux Analysis (RTLA) tools, open list,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20251117184409.42831-1-wander@redhat.com>
The timerlat_hist and timerlat_top commands duplicate the logic for
handling threshold actions. When a threshold is reached, both commands
stop the trace, perform actions, and restart the trace if configured to
continue.
Create a new helper function, timerlat_restart(), to centralize this
shared logic and avoid code duplication. This function now handles the
threshold actions and restarts the necessary trace instances.
Refactor timerlat_hist_main() and the timerlat_top main loops to call
the new helper. This makes the code cleaner and more maintainable.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/timerlat.c | 31 ++++++++++++++++++++++++++
tools/tracing/rtla/src/timerlat.h | 9 ++++++++
tools/tracing/rtla/src/timerlat_hist.c | 19 ++++++++--------
tools/tracing/rtla/src/timerlat_top.c | 19 ++++++++--------
4 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 56e0b8af041d7..50c7eb00fd6b7 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -22,6 +22,37 @@
static int dma_latency_fd = -1;
+/**
+ * timerlat_restart - handle threshold actions and optionally restart tracing
+ * @tool: pointer to the osnoise_tool instance containing trace contexts
+ * @params: timerlat parameters with threshold action configuration
+ *
+ * Return:
+ * RESTART_OK - Actions executed successfully and tracing restarted
+ * RESTART_STOP - Actions executed but 'continue' flag not set, stop tracing
+ * RESTART_ERROR - Failed to restart tracing after executing actions
+ */
+enum restart_result
+timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params)
+{
+ actions_perform(¶ms->common.threshold_actions);
+
+ if (!params->common.threshold_actions.continue_flag)
+ /* continue flag not set, break */
+ return RESTART_STOP;
+
+ /* continue action reached, re-enable tracing */
+ if (tool->record && trace_instance_start(&tool->record->trace))
+ goto err;
+ if (tool->aa && trace_instance_start(&tool->aa->trace))
+ goto err;
+ return RESTART_OK;
+
+err:
+ err_msg("Error restarting trace\n");
+ return RESTART_ERROR;
+}
+
/*
* timerlat_apply_config - apply common configs to the initialized tool
*/
diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
index fd6065f48bb7f..47a34bb443fa0 100644
--- a/tools/tracing/rtla/src/timerlat.h
+++ b/tools/tracing/rtla/src/timerlat.h
@@ -31,6 +31,15 @@ struct timerlat_params {
#define to_timerlat_params(ptr) container_of(ptr, struct timerlat_params, common)
+enum restart_result {
+ RESTART_OK,
+ RESTART_STOP,
+ RESTART_ERROR = -1,
+};
+
+enum restart_result
+timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params);
+
int timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params);
int timerlat_main(int argc, char *argv[]);
int timerlat_enable(struct osnoise_tool *tool);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 09a3da3f58630..f14fc56c5b4a5 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct osnoise_tool *tool)
if (!stop_tracing) {
/* Threshold overflow, perform actions on threshold */
- actions_perform(¶ms->common.threshold_actions);
+ enum restart_result result;
- if (!params->common.threshold_actions.continue_flag)
- /* continue flag not set, break */
+ result = timerlat_restart(tool, params);
+ if (result == RESTART_STOP)
break;
- /* continue action reached, re-enable tracing */
- if (tool->record)
- trace_instance_start(&tool->record->trace);
- if (tool->aa)
- trace_instance_start(&tool->aa->trace);
- timerlat_bpf_restart_tracing();
+ if (result == RESTART_ERROR)
+ return -1;
+
+ if (timerlat_bpf_restart_tracing()) {
+ err_msg("Error restarting BPF trace\n");
+ return -1;
+ }
}
}
timerlat_bpf_detach();
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index 7679820e72db5..d831a9e1818f4 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -921,18 +921,19 @@ timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
if (wait_retval == 1) {
/* Stopping requested by tracer */
- actions_perform(¶ms->common.threshold_actions);
+ enum restart_result result;
- if (!params->common.threshold_actions.continue_flag)
- /* continue flag not set, break */
+ result = timerlat_restart(tool, params);
+ if (result == RESTART_STOP)
break;
- /* continue action reached, re-enable tracing */
- if (tool->record)
- trace_instance_start(&tool->record->trace);
- if (tool->aa)
- trace_instance_start(&tool->aa->trace);
- timerlat_bpf_restart_tracing();
+ if (result == RESTART_ERROR)
+ return -1;
+
+ if (timerlat_bpf_restart_tracing()) {
+ err_msg("Error restarting BPF trace\n");
+ return -1;
+ }
}
/* is there still any user-threads ? */
--
2.51.1
^ permalink raw reply related
* [rtla 06/13] rtla: Use strncmp_static() in more places
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The recently introduced strncmp_static() helper provides a safer way
to compare strings with static strings by determining the length at
compile time.
Replace several open-coded strncmp() calls with strncmp_static() to
improve code readability and robustness. This change affects the
parsing of command-line arguments and environment variables.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/osnoise.c | 2 +-
tools/tracing/rtla/src/timerlat.c | 4 ++--
tools/tracing/rtla/src/trace.c | 2 +-
tools/tracing/rtla/src/utils.c | 8 ++++----
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 312c511fa0044..5075e0f485c77 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1229,7 +1229,7 @@ int osnoise_main(int argc, char *argv[])
if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
osnoise_usage(0);
- } else if (strncmp(argv[1], "-", 1) == 0) {
+ } else if (strncmp_static(argv[1], "-") == 0) {
/* the user skipped the tool, call the default one */
run_tool(&osnoise_top_ops, argc, argv);
exit(0);
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index b692128741279..56e0b8af041d7 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -34,7 +34,7 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
* Try to enable BPF, unless disabled explicitly.
* If BPF enablement fails, fall back to tracefs mode.
*/
- if (getenv("RTLA_NO_BPF") && strncmp(getenv("RTLA_NO_BPF"), "1", 2) == 0) {
+ if (getenv("RTLA_NO_BPF") && strncmp_static(getenv("RTLA_NO_BPF"), "1") == 0) {
debug_msg("RTLA_NO_BPF set, disabling BPF\n");
params->mode = TRACING_MODE_TRACEFS;
} else if (!tep_find_event_by_name(tool->trace.tep, "osnoise", "timerlat_sample")) {
@@ -275,7 +275,7 @@ int timerlat_main(int argc, char *argv[])
if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
timerlat_usage(0);
- } else if (strncmp(argv[1], "-", 1) == 0) {
+ } else if (strncmp_static(argv[1], "-") == 0) {
/* the user skipped the tool, call the default one */
run_tool(&timerlat_top_ops, argc, argv);
exit(0);
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 69cbc48d53d3a..813f4368f104b 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -372,7 +372,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
return;
/* is this a hist: trigger? */
- retval = strncmp(tevent->trigger, "hist:", strlen("hist:"));
+ retval = strncmp_static(tevent->trigger, "hist:");
if (retval)
return;
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 4cb765b94feec..f13d00d7b6bfe 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -196,15 +196,15 @@ long parse_ns_duration(char *val)
t = strtol(val, &end, 10);
if (end) {
- if (!strncmp(end, "ns", 2)) {
+ if (!strncmp_static(end, "ns")) {
return t;
- } else if (!strncmp(end, "us", 2)) {
+ } else if (!strncmp_static(end, "us")) {
t *= 1000;
return t;
- } else if (!strncmp(end, "ms", 2)) {
+ } else if (!strncmp_static(end, "ms")) {
t *= 1000 * 1000;
return t;
- } else if (!strncmp(end, "s", 1)) {
+ } else if (!strncmp_static(end, "s")) {
t *= 1000 * 1000 * 1000;
return t;
}
--
2.51.1
^ permalink raw reply related
* [rtla 05/13] rtla: Simplify argument parsing
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The actions_parse() function uses open-coded logic to extract arguments
from a string. This includes manual length checks and strncmp() calls,
which can be verbose and error-prone.
To simplify and improve the robustness of argument parsing, introduce a
new extract_arg() helper macro. This macro extracts the value from a
"key=value" pair, making the code more concise and readable.
Also, introduce STRING_LENGTH() and strncmp_static() macros to
perform compile-time calculations of string lengths and safer string
comparisons.
Refactor actions_parse() to use these new helpers, resulting in
cleaner and more maintainable code.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 37 ++++++++++++++++++++++----------
tools/tracing/rtla/src/utils.h | 14 ++++++++++--
2 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index e23d4f1c5a592..2a01ece78454c 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -143,15 +143,30 @@ actions_add_continue(struct actions *self)
return 0;
}
+/*
+ * extract_arg - extract argument value from option token
+ * @token: option token (e.g., "file=trace.txt")
+ * @opt: option name to match (e.g., "file")
+ *
+ * Returns pointer to argument value after "=" if token matches "opt=",
+ * otherwise returns NULL.
+ */
+#define extract_arg(token, opt) ( \
+ strlen(token) > STRING_LENGTH(opt "=") && \
+ !strncmp_static(token, opt "=") \
+ ? (token) + STRING_LENGTH(opt "=") : NULL )
+
/*
* actions_parse - add an action based on text specification
*/
int
actions_parse(struct actions *self, const char *trigger, const char *tracefn)
{
+
enum action_type type = ACTION_NONE;
const char *token;
char trigger_c[strlen(trigger) + 1];
+ const char *arg_value;
/* For ACTION_SIGNAL */
int signal = 0, pid = 0;
@@ -182,12 +197,10 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
if (token == NULL)
trace_output = tracefn;
else {
- if (strlen(token) > 5 && strncmp(token, "file=", 5) == 0) {
- trace_output = token + 5;
- } else {
+ trace_output = extract_arg(token, "file");
+ if (!trace_output)
/* Invalid argument */
return -1;
- }
token = strtok(NULL, ",");
if (token != NULL)
@@ -198,14 +211,15 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
case ACTION_SIGNAL:
/* Takes two arguments, num (signal) and pid */
while (token != NULL) {
- if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
- if(!strtoi(token + 4, &signal))
+ arg_value = extract_arg(token, "num");
+ if (arg_value) {
+ if (!strtoi(arg_value, &signal))
return -1;
- } else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
- if (strncmp(token + 4, "parent", 7) == 0)
+ } else if ((arg_value = extract_arg(token, "pid"))) {
+ if (strncmp_static(arg_value, "parent") == 0)
pid = -1;
else
- if (!strtoi(token + 4, &pid))
+ if (!strtoi(arg_value, &pid))
return -1;
} else {
/* Invalid argument */
@@ -223,8 +237,9 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
case ACTION_SHELL:
if (token == NULL)
return -1;
- if (strlen(token) > 8 && strncmp(token, "command=", 8) == 0)
- return actions_add_shell(self, token + 8);
+ arg_value = extract_arg(token, "command");
+ if (arg_value)
+ return actions_add_shell(self, arg_value);
return -1;
case ACTION_CONTINUE:
/* Takes no argument */
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index 160491f5de91c..f7ff548f7fba7 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -13,8 +13,18 @@
#define MAX_NICE 20
#define MIN_NICE -19
-#define container_of(ptr, type, member)({ \
- const typeof(((type *)0)->member) *__mptr = (ptr); \
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+#endif
+
+/* Calculate string length at compile time (excluding null terminator) */
+#define STRING_LENGTH(s) (ARRAY_SIZE(s) - sizeof(*(s)))
+
+/* Compare string with static string, length determined at compile time */
+#define strncmp_static(s1, s2) strncmp(s1, s2, STRING_LENGTH(s2))
+
+#define container_of(ptr, type, member)({ \
+ const typeof(((type *)0)->member) *__mptr = (ptr); \
(type *)((char *)__mptr - offsetof(type, member)) ; })
extern int config_debug;
--
2.51.1
^ permalink raw reply related
* [rtla 04/13] rtla: Replace atoi() with a robust strtoi()
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The atoi() function does not perform error checking, which can lead to
undefined behavior when parsing invalid or out-of-range strings. This
can cause issues when parsing user-provided numerical inputs, such as
signal numbers, PIDs, or CPU lists.
To address this, introduce a new strtoi() helper function that safely
converts a string to an integer. This function validates the input and
checks for overflows, returning a boolean to indicate success or failure.
Replace all calls to atoi() with the new strtoi() function and add
proper error handling to make the parsing more robust and prevent
potential issues.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 6 +++--
tools/tracing/rtla/src/utils.c | 40 ++++++++++++++++++++++++++++----
tools/tracing/rtla/src/utils.h | 2 ++
3 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index efa17290926da..e23d4f1c5a592 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
/* Takes two arguments, num (signal) and pid */
while (token != NULL) {
if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
- signal = atoi(token + 4);
+ if(!strtoi(token + 4, &signal))
+ return -1;
} else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
if (strncmp(token + 4, "parent", 7) == 0)
pid = -1;
else
- pid = atoi(token + 4);
+ if (!strtoi(token + 4, &pid))
+ return -1;
} else {
/* Invalid argument */
return -1;
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index d6ab15dcb4907..4cb765b94feec 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -17,6 +17,7 @@
#include <fcntl.h>
#include <sched.h>
#include <stdio.h>
+#include <limits.h>
#include "utils.h"
@@ -112,16 +113,18 @@ int parse_cpu_set(char *cpu_list, cpu_set_t *set)
nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
for (p = cpu_list; *p; ) {
- cpu = atoi(p);
- if (cpu < 0 || (!cpu && *p != '0') || cpu >= nr_cpus)
+ if (!strtoi(p, &cpu))
+ goto err;
+ if (cpu < 0 || cpu >= nr_cpus)
goto err;
while (isdigit(*p))
p++;
if (*p == '-') {
p++;
- end_cpu = atoi(p);
- if (end_cpu < cpu || (!end_cpu && *p != '0') || end_cpu >= nr_cpus)
+ if (!strtoi(p, &end_cpu))
+ goto err;
+ if (end_cpu < cpu || end_cpu >= nr_cpus)
goto err;
while (isdigit(*p))
p++;
@@ -322,6 +325,7 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
struct dirent *proc_entry;
DIR *procfs;
int retval;
+ int pid;
if (strlen(comm_prefix) >= MAX_PATH) {
err_msg("Command prefix is too long: %d < strlen(%s)\n",
@@ -341,8 +345,12 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
if (!retval)
continue;
+ if (!strtoi(proc_entry->d_name, &pid)) {
+ err_msg("'%s' is not a valid pid", proc_entry->d_name);
+ goto out_err;
+ }
/* procfs_is_workload_pid confirmed it is a pid */
- retval = __set_sched_attr(atoi(proc_entry->d_name), attr);
+ retval = __set_sched_attr(pid, attr);
if (retval) {
err_msg("Error setting sched attributes for pid:%s\n", proc_entry->d_name);
goto out_err;
@@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
return 1;
}
+
+/*
+ * strtoi - convert string to integer with error checking
+ *
+ * Returns true on success, false if conversion fails or result is out of int range.
+ */
+bool strtoi(const char *s, int *res)
+{
+ char *end_ptr;
+ long lres;
+
+ if (!*s)
+ return false;
+
+ errno = 0;
+ lres = strtol(s, &end_ptr, 0);
+ if (errno || *end_ptr || lres > INT_MAX || lres < INT_MIN)
+ return false;
+
+ *res = (int) lres;
+ return true;
+}
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index a2a6f89f342d0..160491f5de91c 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -3,6 +3,7 @@
#include <stdint.h>
#include <time.h>
#include <sched.h>
+#include <stdbool.h>
/*
* '18446744073709551615\0'
@@ -80,6 +81,7 @@ static inline int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int stat
static inline int have_libcpupower_support(void) { return 0; }
#endif /* HAVE_LIBCPUPOWER_SUPPORT */
int auto_house_keeping(cpu_set_t *monitored_cpus);
+bool strtoi(const char *s, int *res);
#define ns_to_usf(x) (((double)x/1000))
#define ns_to_per(total, part) ((part * 100) / (double)total)
--
2.51.1
^ permalink raw reply related
* [rtla 03/13] rtla: Introduce for_each_action() helper
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The for loop to iterate over the list of actions is used in
more than one place. To avoid code duplication and improve
readability, introduce a for_each_action() helper macro.
Replace the open-coded for loops with the new helper.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 6 ++++--
tools/tracing/rtla/src/actions.h | 5 +++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 696dd1ed98d9a..efa17290926da 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -35,7 +35,9 @@ void
actions_destroy(struct actions *self)
{
/* Free any action-specific data */
- for (struct action *action = self->list; action < self->list + self->len; action++) {
+ struct action *action;
+
+ for_each_action(self, action) {
if (action->type == ACTION_SHELL)
free(action->command);
if (action->type == ACTION_TRACE_OUTPUT)
@@ -241,7 +243,7 @@ actions_perform(struct actions *self)
int pid, retval;
const struct action *action;
- for (action = self->list; action < self->list + self->len; action++) {
+ for_each_action(self, action) {
switch (action->type) {
case ACTION_TRACE_OUTPUT:
retval = save_trace_to_file(self->trace_output_inst, action->trace_output);
diff --git a/tools/tracing/rtla/src/actions.h b/tools/tracing/rtla/src/actions.h
index 439bcc58ac93a..9b9df57a0cb34 100644
--- a/tools/tracing/rtla/src/actions.h
+++ b/tools/tracing/rtla/src/actions.h
@@ -42,6 +42,11 @@ struct actions {
struct tracefs_instance *trace_output_inst;
};
+#define for_each_action(actions, action) \
+ for ((action) = (actions)->list; \
+ (action) < (actions)->list + (actions)->len; \
+ (action)++)
+
int actions_init(struct actions *self);
void actions_destroy(struct actions *self);
int actions_add_trace_output(struct actions *self, const char *trace_output);
--
2.51.1
^ permalink raw reply related
* [rtla 02/13] rtla: Use strdup() to simplify code
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
The actions_add_trace_output() and actions_add_shell() functions were
using calloc() followed by strcpy() to allocate and copy a string.
This can be simplified by using strdup(), which allocates memory and
copies the string in a single step.
Replace the calloc() and strcpy() calls with strdup(), making the
code more concise and readable.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 01648a1425c10..696dd1ed98d9a 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -78,10 +78,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
self->present[ACTION_TRACE_OUTPUT] = true;
action->type = ACTION_TRACE_OUTPUT;
- action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
+ action->trace_output = strdup(trace_output);
if (!action->trace_output)
return -1;
- strcpy(action->trace_output, trace_output);
return 0;
}
@@ -118,10 +117,9 @@ actions_add_shell(struct actions *self, const char *command)
self->present[ACTION_SHELL] = true;
action->type = ACTION_SHELL;
- action->command = calloc(strlen(command) + 1, sizeof(char));
+ action->command = strdup(command);
if (!action->command)
return -1;
- strcpy(action->command, command);
return 0;
}
--
2.51.1
^ permalink raw reply related
* [rtla 01/13] rtla: Check for memory allocation failures
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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: <20251117184409.42831-1-wander@redhat.com>
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;
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;
+
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;
diff --git a/tools/tracing/rtla/src/actions.h b/tools/tracing/rtla/src/actions.h
index a4f9b570775b5..439bcc58ac93a 100644
--- a/tools/tracing/rtla/src/actions.h
+++ b/tools/tracing/rtla/src/actions.h
@@ -42,7 +42,7 @@ struct actions {
struct tracefs_instance *trace_output_inst;
};
-void actions_init(struct actions *self);
+int actions_init(struct actions *self);
void actions_destroy(struct actions *self);
int actions_add_trace_output(struct actions *self, const char *trace_output);
int actions_add_signal(struct actions *self, int signal, int pid);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 606c1688057b2..09a3da3f58630 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -798,8 +798,11 @@ static struct common_params
if (!params)
exit(1);
- actions_init(¶ms->common.threshold_actions);
- actions_init(¶ms->common.end_actions);
+ if (actions_init(¶ms->common.threshold_actions) ||
+ actions_init(¶ms->common.end_actions)) {
+ err_msg("Error initializing actions");
+ exit(EXIT_FAILURE);
+ }
/* disabled by default */
params->dma_latency = -1;
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index fc479a0dcb597..7679820e72db5 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -556,8 +556,11 @@ static struct common_params
if (!params)
exit(1);
- actions_init(¶ms->common.threshold_actions);
- actions_init(¶ms->common.end_actions);
+ if (actions_init(¶ms->common.threshold_actions) ||
+ actions_init(¶ms->common.end_actions)) {
+ err_msg("Error initializing actions");
+ exit(EXIT_FAILURE);
+ }
/* disabled by default */
params->dma_latency = -1;
--
2.51.1
^ permalink raw reply related
* [PATCH 0/13] rtla: Code robustness and maintainability improvements
From: Wander Lairson Costa @ 2025-11-17 18:41 UTC (permalink / raw)
To: Steven Rostedt, 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|_)
This patch series enhances the robustness and maintainability of the
RTLA (Real-Time Linux Analysis) tool through systematic improvements
to error handling, code clarity, and consistency.
The changes strengthen defensive programming practices throughout the
codebase by improving input validation and memory management. Several
new helper functions and macros are introduced to reduce code duplication
and provide safer, more readable alternatives to common operations. The
series also consolidates duplicate logic across modules and adopts
standard conventions where appropriate.
These improvements make the code more resilient and easier to maintain
while preserving existing functionality and behavior.
Wander Lairson Costa (13):
rtla: Check for memory allocation failures
rtla: Use strdup() to simplify code
rtla: Introduce for_each_action() helper
rtla: Replace atoi() with a robust strtoi()
rtla: Simplify argument parsing
rtla: Use strncmp_static() in more places
rtla: Introduce timerlat_restart() helper
rtla: Use standard exit codes for result enum
rtla: Exit if trace output action fails
rtla: Remove redundant memset after calloc
rtla: Replace magic number with MAX_PATH
rtla: Remove unused headers
rtla: Fix inconsistent state in actions_add_* functions
tools/tracing/rtla/src/actions.c | 91 ++++++++++++++++++--------
tools/tracing/rtla/src/actions.h | 7 +-
tools/tracing/rtla/src/osnoise.c | 6 +-
tools/tracing/rtla/src/osnoise_hist.c | 1 -
tools/tracing/rtla/src/timerlat.c | 36 +++++++++-
tools/tracing/rtla/src/timerlat.h | 9 +++
tools/tracing/rtla/src/timerlat_hist.c | 32 +++++----
tools/tracing/rtla/src/timerlat_top.c | 27 ++++----
tools/tracing/rtla/src/timerlat_u.c | 4 +-
tools/tracing/rtla/src/trace.c | 23 ++++---
tools/tracing/rtla/src/utils.c | 48 +++++++++++---
tools/tracing/rtla/src/utils.h | 23 +++++--
12 files changed, 220 insertions(+), 87 deletions(-)
--
2.51.1
^ permalink raw reply
* Re: [PATCH v12 mm-new 13/15] khugepaged: avoid unnecessary mTHP collapse attempts
From: Nico Pache @ 2025-11-17 18:16 UTC (permalink / raw)
To: Wei Yang
Cc: 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: <20251109024013.fzt7xxpmxwi75xgr@master>
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.
>
> 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).
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.
/* 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.
Cheers,
-- Nico
>
> > }
> >
> > next_order:
> >--
> >2.51.0
>
> --
> Wei Yang
> Help you, Help me
>
^ permalink raw reply
* Re: [PATCH] tracing: Remove unused variable in tracing_trace_options_show()
From: Andy Shevchenko @ 2025-11-17 17:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20251117120637.43ef995d@gandalf.local.home>
On Mon, Nov 17, 2025 at 12:06:37PM -0500, Steven Rostedt wrote:
> The flags and opts used in tracing_trace_options_show() now come directly
> from the trace array "current_trace_flags" and not the current_trace. The
> variable "trace" was still being assigned to tr->current_trace but never
> used. This caused a warning in clang.
Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
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