* Re: [RFC PATCH 1/1] psi: Introduce in-kernel PSI auto monitor feature
From: K Prateek Nayak @ 2026-07-13 3:16 UTC (permalink / raw)
To: pintu.k
Cc: linux-kernel, linux-trace-kernel, hannes, surenb, rostedt,
mhiramat, peterz, mathieu.desnoyers, mingo, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
pintu.ping, nathan, ojeda, nsc, gary, tglx, thomas.weissschuh,
aliceryhl, dianders, linux.amoon, rdunlap, akpm, shuah
In-Reply-To: <CANqwyPiE1vtoGH6Zu==VxBkkwGq_E7-Zswi57msy_G97Rz2xSw@mail.gmail.com>
Hello Pintu,
On 7/10/2026 9:31 PM, Pintu Kumar Agarwal wrote:
>>> There is nothing here that warrants putting this in kernel/sched.
>> The feature depends on sched/psi so I decided to keep it close.
>> But I am open for any location.
>>
>>> Also this gets included by default when config is enabled and starts
>>> dumping a bunch of stats to dmesg without anyone asking. No?
>>>
>> This is included as a dependent feature of PSI.
>> If someone enables this CONFIG as part of PSI this indicates that they
>> are interested in getting auto-monitor stats.
>> Also, the dump will happen only if threshold is breached with high
>> default values.
>> However, for RFC stage I wanted to keep things simple.
>> Later, we can add an enable/disable flag in cmdline just like PSI.
>>
>>> Afaict, almost all of the detail used here is also available from
>>> procfs and people can easily put together a userspace tool if they
>>> need it. Why do we need an in-kernel module?
>>>
>> This is the most fundamental aspect of this auto-monitor feature.
>> This point is already described in the cover letter.
>> Let me put it again:
>> - Get kernel stats early during boot_time before userspace comes up.
People care about PSI signals at boot, and even before the userspace
is up? Why?
>> -> Set slightly lower threshold and boot stats (helps in analysing boot time)
>> - No user intervention or continuous polling or daemons needed
>> (Just enable config and start auto monitoring)
So overehead for everyone?
>> - userspace scheduling delays under high pressure
Even workqueues need scheduling. They don't just automagically run
at the end of the set timer.
>> - risk of missing short-lived spikes
>> - capturing details as soon as pressure hits and at same timestamp
But your data is sampled. So these will still be missed.
>> - useful for analysing real-time latency workload.
How often is PSI enabled there. I know for a fact there are certain
locking overheads associated with PSI (like ttwu trying to grab the
rq_lock to migrate PSI signals) and wouldn't most RT be better off
running without those overheads?
>> - useful for minimal environment like initramfs or busybox
Who is doing PSI analysis in these environments?
>>
>> The motivation is not to replace existing PSI interfaces or the ability
>> to build userspace monitoring tools.
>> The goal is attribution at the moment pressure thresholds are crossed.
>> A userspace implementation observes the system after being scheduled,
Workqueue threads still need scheduling. Just because it is a
kernel thread doesn't mean it gets any privilege.
>> whereas the in-kernel implementation captures contributors at the point
>> where pressure is detected.
Again, your implementation is based on sampling so signals can still
get missed.
>> During LPC-2024 I have done significant changes to core psi module
>> to implement the similar logic.
>> But the feedback was not to disturb the core psi interface, instead
>> develop a separate interface and make it configurable.
>> So, I came up with this auto-monitor idea.
I'll defer to Johannes on this since he knows these bits best.
>>
>> For more details please have a look at my OSS paper with data.
>> https://hosted-files.sched.co/ossindia2026/19/OSS-IND-26-PSI-Auto-Monitor.pdf
>> And also the reference data here:
>> https://github.com/pintuk/KERNEL/tree/master/PSI_WORK
>>
>> I am also looking out for someone who can test this on a larger
>> workload and capture data.
>> This will help us to gather insights, how the feature behaves.
I still think periodically walking the entire task tree on a large
server is a very bad idea,especially when people have tens of
thousands of task running.
>>
>>>> +
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_AUTHOR("Pintu Kumar Agarwal");
>>>> +MODULE_DESCRIPTION("In-kernel PSI automatic monitor with sysfs, weighted scoring and tracepoints");
>>>> --
>
> Any other feedback before I post v2 ?
It is the European holiday season so the response might be slow.
I suggest iterating on this only after checking with Johannes.
> Another thing that I wanted feedback is, whether to keep under
> kernel/sched/ or move under tools/sched/ ?
I think the only dependency here is the "psi_system" which is why you've
kept this in kernel/sched but if you can have an wrapper with an
EXPORT_SYMBOL_GPL(), you should be able to place it in tools/sched.
Again, I would check with Johannes on whether it is acceptable to expose
the PSI internals to modules.
I still think most of this is already visible via sysfs and that might
be a better way to get this information. OOMD is able to use PSI signals
and act on it without needing to be in kernel. What warrants an
in-kernel monitor?
--
Thanks and Regards,
Prateek
^ permalink raw reply
* [PATCH v4] tracing: Use seq_buf for string concatenation
From: Woradorn Laodhanadhaworn @ 2026-07-13 4:52 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers
Cc: linux-kernel, linux-trace-kernel, linux-hardening,
linux-kernel-mentees, shuah, skhan, me, jkoolstra, woradorn.laon
In preparation for removing the strlcat API[1],
replace the string concatenation logic with a struct seq_buf,
which tracks the current position and the remaining space internally.
Use seq_buf_str() to NUL-terminate before passing to early_enable_events().
Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>
---
v1 -> v2:
- Fixed WARN_ON when booting with empty trace_event.
v2 -> v3:
- Addressed Sashiko's concern about the compound literal backing buffer.
- Replaced the compund literal with an explicit declared buffer and pointed
seq_buf.buffer to it. This guarantees the backing storage is placed in
`.init.data` and reclaimed after boot.
v3 -> v4:
- Removed typecast a const char* to non const.
v1: https://lore.kernel.org/all/20260620175441.223342-1-woradorn.laon@gmail.com
v2: https://lore.kernel.org/all/20260622094623.18469-1-woradorn.laon@gmail.com
v3: https://lore.kernel.org/all/20260623145147.12145-1-woradorn.laon@gmail.com
Sashiko: https://sashiko.dev/#/patchset/20260622094623.18469-1-woradorn.laon%40gmail.com
kernel/trace/trace_events.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c46e623e7e0d..4f5a0e73a89b 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -22,6 +22,7 @@
#include <linux/sort.h>
#include <linux/slab.h>
#include <linux/delay.h>
+#include <linux/seq_buf.h>
#include <trace/events/sched.h>
#include <trace/syscall.h>
@@ -4501,13 +4502,20 @@ extern struct trace_event_call *__start_ftrace_events[];
extern struct trace_event_call *__stop_ftrace_events[];
static char bootup_event_buf[COMMAND_LINE_SIZE] __initdata;
+static struct seq_buf bootup_event_seq __initdata = {
+ .buffer = bootup_event_buf,
+ .size = sizeof(bootup_event_buf),
+};
static __init int setup_trace_event(char *str)
{
- if (bootup_event_buf[0] != '\0')
- strlcat(bootup_event_buf, ",", COMMAND_LINE_SIZE);
+ if (seq_buf_used(&bootup_event_seq) > 0)
+ seq_buf_puts(&bootup_event_seq, ",");
+
+ seq_buf_puts(&bootup_event_seq, str);
- strlcat(bootup_event_buf, str, COMMAND_LINE_SIZE);
+ if (seq_buf_has_overflowed(&bootup_event_seq))
+ return -ENOMEM;
trace_set_ring_buffer_expanded(NULL);
disable_tracing_selftest("running event tracing");
@@ -4766,6 +4774,7 @@ static __init int event_trace_enable(void)
*/
__trace_early_add_events(tr);
+ seq_buf_str(&bootup_event_seq);
early_enable_events(tr, bootup_event_buf, false);
trace_printk_start_comm();
@@ -4794,6 +4803,7 @@ static __init int event_trace_enable_again(void)
if (!tr)
return -ENODEV;
+ seq_buf_str(&bootup_event_seq);
early_enable_events(tr, bootup_event_buf, true);
return 0;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] tracing: Use seq_buf for string concatenation
From: Woradorn Laodhanadhaworn @ 2026-07-13 4:57 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel,
linux-hardening, linux-kernel-mentees, shuah, skhan, me,
jkoolstra
In-Reply-To: <20260629143947.2216574e@robin>
On 30/6/2569 BE 01:39, Steven Rostedt wrote:
> On Mon, 22 Jun 2026 16:46:23 +0700
> Woradorn Laodhanadhaworn <woradorn.laon@gmail.com> wrote:
>
>>
>> #include <trace/events/sched.h>
>> #include <trace/syscall.h>
>> @@ -4500,14 +4501,20 @@ static void __add_event_to_tracers(struct trace_event_call *call)
>> extern struct trace_event_call *__start_ftrace_events[];
>> extern struct trace_event_call *__stop_ftrace_events[];
>>
>> -static char bootup_event_buf[COMMAND_LINE_SIZE] __initdata;
>
> Keep the above string and just assign it.
>
>> +static struct seq_buf bootup_event_buf __initdata = {
>> + .buffer = (char[COMMAND_LINE_SIZE]) {},
>> + .size = COMMAND_LINE_SIZE,
>> +};
>
> static struct seq_buf bootup_event_seq __initdata = {
> .buffer = bootup_event_buf;
> .size = sizeof(bootup_event_buf);
> };
>
>>
>> static __init int setup_trace_event(char *str)
>> {
>> - if (bootup_event_buf[0] != '\0')
>> - strlcat(bootup_event_buf, ",", COMMAND_LINE_SIZE);
>> + if (seq_buf_used(&bootup_event_buf) > 0)
>> + seq_buf_puts(&bootup_event_buf, ",");
>> +
>> + seq_buf_puts(&bootup_event_buf, str);
>>
>> - strlcat(bootup_event_buf, str, COMMAND_LINE_SIZE);
>> + if (seq_buf_has_overflowed(&bootup_event_buf))
>> + return -ENOMEM;
>>
>> trace_set_ring_buffer_expanded(NULL);
>> disable_tracing_selftest("running event tracing");
>> @@ -4766,7 +4773,7 @@ static __init int event_trace_enable(void)
>> */
>> __trace_early_add_events(tr);
>>
>> - early_enable_events(tr, bootup_event_buf, false);
>> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_buf), false);
>
> The above then would be:
>
> seq_buf_str(&bootup_event_seq);
> early_enable_events(tr, bootup_event_buf, false);
>
> Don't typecast a const char* to non const.
>
>>
>> trace_printk_start_comm();
>>
>> @@ -4794,7 +4801,7 @@ static __init int event_trace_enable_again(void)
>> if (!tr)
>> return -ENODEV;
>>
>> - early_enable_events(tr, bootup_event_buf, true);
>> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_buf), true);
>
> Same here.
>
> -- Steve
>
>>
>> return 0;
>> }
>
Thank you, Steven, for your review. I've sent v4:
https://lore.kernel.org/all/20260713045249.69942-1-woradorn.laon@gmail.com
Thanks,
Woradorn
^ permalink raw reply
* [PATCH 0/5] tracing/probes: Clean up and refactor argument parser
From: Masami Hiramatsu (Google) @ 2026-07-13 7:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
Hi,
This is the 2nd version of the series which refactors and cleans up
the fetcharg parser in the trace_probe. The previous version is here:
https://lore.kernel.org/all/178386876526.3174487.5142283230157728964.stgit@devnote2/
The main goals are to improve code readability, prevent kernel stack
overflow by eliminating recursive function calls, and extend the
argument string length limit to support complex BTF typecast
expressions.
In this version, I fixed the double error log issue in [1/5], make
MAX_ARGSTR_LEN to 255 [4/5] and update patch description for clarify
this change updates the behavior (not a cleanup) and
make TRACEPROBE_MAX_NESTED_LEVEL to 8 (also update testcases)[5/5].
Patches:
- Patch 1: Decomposes parse_probe_vars() into smaller, dedicated helper
functions for parsing specific variable types ($retval, $stack, etc.).
- Patch 2: Decomposes parse_probe_arg() into static helper subroutines for
handling registers, memory/symbols, and immediates.
- Patch 3: Alphabetizes the ERRORS macros list in trace_probe.h to improve
readability.
- Patch 4: Extends MAX_ARGSTR_LEN from 63 to 255 for long BTF cast/dereference
arguments, and introduces MAX_COMMON_HEAD_LEN to prevent regression on match
head commands.
- Patch 5: Eliminates recursion in parse_probe_arg() using a loop and a parse
state stack integrated into traceprobe_parse_context. It also raises the
nesting limit to 8 and updates the selftest scripts.
Thanks,
---
Masami Hiramatsu (5):
tracing/probes: Refactor parse_probe_vars()
tracing/probes: Refactor parse_probe_arg()
tracing/probes: Sort ERRORS list in trace_probe.h alphabetically
tracing/probes: Extend max length of argument string
tracing/probes: Eliminate recursion in parse_probe_arg()
kernel/trace/trace_fprobe.c | 2
kernel/trace/trace_kprobe.c | 2
kernel/trace/trace_probe.c | 829 ++++++++++++--------
kernel/trace/trace_probe.h | 196 +++--
kernel/trace/trace_uprobe.c | 2
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 6
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 4
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 6
8 files changed, 621 insertions(+), 426 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH 1/5] tracing/probes: Refactor parse_probe_vars()
From: Masami Hiramatsu (Google) @ 2026-07-13 7:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178392766002.3229912.15711868107734330750.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Decompose parse_probe_vars() by extracting retval, stack, current task
struct, and function argument parsing logic into dedicated static
helper functions (parse_probe_var_retval, parse_probe_var_stack,
parse_probe_var_current, and parse_probe_var_arg). This simplifies
parse_probe_vars() and improves its readability.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Fix double error logging by removing err code passing, but just
record errors inside subfunctions.
- Simplify parse_probe_vars().
---
kernel/trace/trace_probe.c | 237 ++++++++++++++++++++++++++------------------
1 file changed, 140 insertions(+), 97 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 18c212122344..8af97e29f096 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1319,132 +1319,175 @@ NOKPROBE_SYMBOL(store_trace_entry_data)
#define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
-/* Parse $vars. @orig_arg points '$', which syncs to @ctx->offset */
-static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
- struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_var_retval(char *orig_arg, char *arg,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
- int err = TP_ERR_BAD_VAR;
- char *arg = orig_arg + 1;
+
+ if (!(ctx->flags & TPARG_FL_RETURN)) {
+ trace_probe_log_err(ctx->offset, RETVAL_ON_PROBE);
+ return -EINVAL;
+ }
+ if (!(ctx->flags & TPARG_FL_KERNEL) ||
+ !IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS)) {
+ code->op = FETCH_OP_RETVAL;
+ return 0;
+ }
+ return parse_btf_arg(orig_arg, pcode, end, ctx);
+}
+
+static int parse_probe_var_stack(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
unsigned long param;
- int ret = 0;
- int len;
+ int ret;
- if (ctx->flags & TPARG_FL_TEVENT) {
- if (parse_trace_event(arg, code, ctx) < 0) {
- /* 'comm' should be checked after field parsing. */
- if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
- code->op = FETCH_OP_COMM;
- return 0;
- }
- goto inval;
- }
+ if (arg[len] == '\0') {
+ code->op = FETCH_OP_STACKP;
return 0;
}
- if (str_has_prefix(arg, "retval")) {
- if (!(ctx->flags & TPARG_FL_RETURN)) {
- err = TP_ERR_RETVAL_ON_PROBE;
- goto inval;
+ if (isdigit(arg[len])) {
+ ret = kstrtoul(arg + len, 10, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return ret;
}
- if (!(ctx->flags & TPARG_FL_KERNEL) ||
- !IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS)) {
- code->op = FETCH_OP_RETVAL;
- return 0;
+
+ if ((ctx->flags & TPARG_FL_KERNEL) &&
+ param > PARAM_MAX_STACK) {
+ trace_probe_log_err(ctx->offset, BAD_STACK_NUM);
+ return -EINVAL;
}
+ code->op = FETCH_OP_STACK;
+ code->param = (unsigned int)param;
+ return 0;
+ }
+
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+}
+
+static int parse_probe_var_current(char *orig_arg, char *arg,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+
+ /* $current is only supported by kernel probe. */
+ if (!(ctx->flags & TPARG_FL_KERNEL)) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+ }
+ arg += strlen("current");
+ if (*arg == '-' && IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS))
return parse_btf_arg(orig_arg, pcode, end, ctx);
+
+ if (*arg != '\0') {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
}
- len = str_has_prefix(arg, "stack");
- if (len) {
+ code->op = FETCH_OP_CURRENT;
+ return 0;
+}
- if (arg[len] == '\0') {
- code->op = FETCH_OP_STACKP;
- return 0;
- }
+#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
+static int parse_probe_var_arg(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ unsigned long param;
+ int ret;
- if (isdigit(arg[len])) {
- ret = kstrtoul(arg + len, 10, ¶m);
- if (ret)
- goto inval;
+ ret = kstrtoul(arg + len, 10, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return ret;
+ }
- if ((ctx->flags & TPARG_FL_KERNEL) &&
- param > PARAM_MAX_STACK) {
- err = TP_ERR_BAD_STACK_NUM;
- goto inval;
- }
- code->op = FETCH_OP_STACK;
- code->param = (unsigned int)param;
- return 0;
- }
- goto inval;
+ if (!param || param > PARAM_MAX_STACK) {
+ trace_probe_log_err(ctx->offset, BAD_ARG_NUM);
+ return -EINVAL;
}
+ param--; /* argN starts from 1, but internal arg[N] starts from 0 */
- if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
- code->op = FETCH_OP_COMM;
- return 0;
+ if (tparg_is_function_entry(ctx->flags)) {
+ code->op = FETCH_OP_ARG;
+ code->param = (unsigned int)param;
+ /*
+ * The tracepoint probe will probe a stub function, and the
+ * first parameter of the stub is a dummy and should be ignored.
+ */
+ if (ctx->flags & TPARG_FL_TPOINT)
+ code->param++;
+ } else if (tparg_is_function_return(ctx->flags)) {
+ /* function entry argument access from return probe */
+ ret = __store_entry_arg(ctx->tp, param);
+ if (ret < 0) /* This error should be an internal error */
+ return ret;
+
+ code->op = FETCH_OP_EDATA;
+ code->offset = ret;
+ } else {
+ trace_probe_log_err(ctx->offset, NOFENTRY_ARGS);
+ return -EINVAL;
}
+ return 0;
+}
+#else
+static int parse_probe_var_arg(char *arg, int len, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+}
+#endif
- /* $current returns the address of the current task_struct. */
- if (str_has_prefix(arg, "current")) {
- /* $current is only supported by kernel probe. */
- if (!(ctx->flags & TPARG_FL_KERNEL)) {
- err = TP_ERR_BAD_VAR;
- goto inval;
- }
- arg += strlen("current");
- if (*arg == '-' && IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS))
- return parse_btf_arg(orig_arg, pcode, end, ctx);
+/* Parse $vars. @orig_arg points '$', which syncs to @ctx->offset */
+static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
+ struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+ char *arg = orig_arg + 1;
+ int len, ret;
- if (*arg != '\0')
- goto inval;
+ if (ctx->flags & TPARG_FL_TEVENT) {
+ ret = parse_trace_event(arg, code, ctx);
+ if (!ret)
+ return 0;
+ }
- code->op = FETCH_OP_CURRENT;
+ if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
+ code->op = FETCH_OP_COMM;
return 0;
}
-#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
- len = str_has_prefix(arg, "arg");
- if (len) {
- ret = kstrtoul(arg + len, 10, ¶m);
- if (ret)
- goto inval;
+ /* eprobe only support event fields or '$comm'. */
+ if (ctx->flags & TPARG_FL_TEVENT)
+ goto inval;
- if (!param || param > PARAM_MAX_STACK) {
- err = TP_ERR_BAD_ARG_NUM;
- goto inval;
- }
- param--; /* argN starts from 1, but internal arg[N] starts from 0 */
+ if (str_has_prefix(arg, "retval"))
+ return parse_probe_var_retval(orig_arg, arg, pcode, end, ctx);
- if (tparg_is_function_entry(ctx->flags)) {
- code->op = FETCH_OP_ARG;
- code->param = (unsigned int)param;
- /*
- * The tracepoint probe will probe a stub function, and the
- * first parameter of the stub is a dummy and should be ignored.
- */
- if (ctx->flags & TPARG_FL_TPOINT)
- code->param++;
- } else if (tparg_is_function_return(ctx->flags)) {
- /* function entry argument access from return probe */
- ret = __store_entry_arg(ctx->tp, param);
- if (ret < 0) /* This error should be an internal error */
- return ret;
+ len = str_has_prefix(arg, "stack");
+ if (len)
+ return parse_probe_var_stack(arg, len, code, ctx);
- code->op = FETCH_OP_EDATA;
- code->offset = ret;
- } else {
- err = TP_ERR_NOFENTRY_ARGS;
- goto inval;
- }
- return 0;
- }
-#endif
+ /* $current returns the address of the current task_struct. */
+ if (str_has_prefix(arg, "current"))
+ return parse_probe_var_current(orig_arg, arg, pcode, end, ctx);
+
+ len = str_has_prefix(arg, "arg");
+ if (len)
+ return parse_probe_var_arg(arg, len, code, ctx);
inval:
- __trace_probe_log_err(ctx->offset, err);
+ trace_probe_log_err(ctx->offset, BAD_VAR);
return -EINVAL;
}
^ permalink raw reply related
* [PATCH 2/5] tracing/probes: Refactor parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-13 7:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178392766002.3229912.15711868107734330750.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Decompose parse_probe_arg() by extracting register, memory/symbol,
dereference, immediate, and default BTF/CPU parsing handlers into
dedicated static helper functions (parse_probe_arg_register,
parse_probe_arg_mem_symbol, parse_probe_arg_deref, parse_probe_arg_imm,
and parse_probe_arg_default). This modularizes the recursive argument
parser and improves readability.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_probe.c | 300 ++++++++++++++++++++++++++------------------
1 file changed, 174 insertions(+), 126 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 8af97e29f096..eb9f7bd13c19 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1516,158 +1516,206 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
return 0;
}
-/* Recursive argument parser */
-static int
-parse_probe_arg(char *arg, const struct fetch_type *type,
- struct fetch_insn **pcode, struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_register(char *arg, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
+
+ if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
+ /* eprobe and fprobe do not handle registers */
+ trace_probe_log_err(ctx->offset, BAD_VAR);
+ return -EINVAL;
+ }
+ ret = regs_query_register_offset(arg + 1);
+ if (ret >= 0) {
+ code->op = FETCH_OP_REG;
+ code->param = (unsigned int)ret;
+ return 0;
+ }
+ trace_probe_log_err(ctx->offset, BAD_REG_NAME);
+ return -EINVAL;
+}
+
+static int parse_probe_arg_mem_symbol(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
unsigned long param;
+ long offset = 0;
+ int ret;
+
+ if (isdigit(arg[1])) {
+ ret = kstrtoul(arg + 1, 0, ¶m);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_MEM_ADDR);
+ return ret;
+ }
+ /* load address */
+ code->op = FETCH_OP_IMM;
+ code->immediate = param;
+ } else if (arg[1] == '+') {
+ /* Kernel probes do not support file offsets */
+ if (ctx->flags & TPARG_FL_KERNEL) {
+ trace_probe_log_err(ctx->offset, FILE_ON_KPROBE);
+ return -EINVAL;
+ }
+ ret = kstrtol(arg + 2, 0, &offset);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_FILE_OFFS);
+ return ret;
+ }
+
+ code->op = FETCH_OP_FOFFS;
+ code->immediate = (unsigned long)offset;
+ offset = 0;
+ } else {
+ /* uprobes don't support symbols */
+ if (!(ctx->flags & TPARG_FL_KERNEL)) {
+ trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
+ return -EINVAL;
+ }
+ /* Preserve symbol for updating */
+ code->op = FETCH_NOP_SYMBOL;
+ code->data = kstrdup(arg + 1, GFP_KERNEL);
+ if (!code->data)
+ return -ENOMEM;
+ if (++code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ code->op = FETCH_OP_IMM;
+ code->immediate = 0;
+ }
+ /* These are fetching from memory */
+ if (++code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ *pcode = code;
+ code->op = FETCH_OP_DEREF;
+ code->offset = offset;
+ return 0;
+}
+
+static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
int deref = FETCH_OP_DEREF;
long offset = 0;
char *tmp;
- int ret = 0;
+ int ret;
- switch (arg[0]) {
- case '$':
- ret = parse_probe_vars(arg, type, pcode, end, ctx);
- break;
+ if (arg[1] == 'u') {
+ deref = FETCH_OP_UDEREF;
+ arg[1] = arg[0];
+ arg++;
+ }
+ if (arg[0] == '+')
+ arg++; /* Skip '+', because kstrtol() rejects it. */
+ tmp = strchr(arg, '(');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset, DEREF_NEED_BRACE);
+ return -EINVAL;
+ }
+ *tmp = '\0';
+ ret = kstrtol(arg, 0, &offset);
+ if (ret) {
+ trace_probe_log_err(ctx->offset, BAD_DEREF_OFFS);
+ return ret;
+ }
+ ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
+ arg = tmp + 1;
+ return handle_dereference(arg, pcode, end, ctx, deref, offset);
+}
- case '%': /* named register */
- if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
- /* eprobe and fprobe do not handle registers */
- trace_probe_log_err(ctx->offset, BAD_VAR);
- break;
+static int parse_probe_arg_imm(char *arg, struct fetch_insn *code,
+ struct traceprobe_parse_context *ctx)
+{
+ char *tmp;
+ int ret;
+
+ if (arg[1] == '"') { /* Immediate string */
+ ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
+ if (ret)
+ return ret;
+ code->op = FETCH_OP_IMMSTR;
+ code->data = tmp;
+ } else {
+ ret = str_to_immediate(arg + 1, &code->immediate);
+ if (ret) {
+ trace_probe_log_err(ctx->offset + 1, BAD_IMM);
+ return ret;
}
- ret = regs_query_register_offset(arg + 1);
- if (ret >= 0) {
- code->op = FETCH_OP_REG;
- code->param = (unsigned int)ret;
- ret = 0;
- } else
- trace_probe_log_err(ctx->offset, BAD_REG_NAME);
- break;
+ code->op = FETCH_OP_IMM;
+ }
+ return 0;
+}
- case '@': /* memory, file-offset or symbol */
- if (isdigit(arg[1])) {
- ret = kstrtoul(arg + 1, 0, ¶m);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_MEM_ADDR);
- break;
- }
- /* load address */
- code->op = FETCH_OP_IMM;
- code->immediate = param;
- } else if (arg[1] == '+') {
- /* Kernel probes do not support file offsets */
- if (ctx->flags & TPARG_FL_KERNEL) {
- trace_probe_log_err(ctx->offset, FILE_ON_KPROBE);
- return -EINVAL;
- }
- ret = kstrtol(arg + 2, 0, &offset);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_FILE_OFFS);
- break;
- }
+static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
+ struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
- code->op = FETCH_OP_FOFFS;
- code->immediate = (unsigned long)offset; // imm64?
- offset = 0;
- } else {
- /* uprobes don't support symbols */
- if (!(ctx->flags & TPARG_FL_KERNEL)) {
- trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
- return -EINVAL;
- }
- /* Preserve symbol for updating */
- code->op = FETCH_NOP_SYMBOL;
- code->data = kstrdup(arg + 1, GFP_KERNEL);
- if (!code->data)
- return -ENOMEM;
- if (++code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
+ str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
+ return parse_this_cpu(arg, pcode, end, ctx);
+ }
+
+ if (isalpha(arg[0]) || arg[0] == '_') {
+ /* BTF variable or event field */
+ if (ctx->flags & TPARG_FL_TEVENT) {
+ ret = parse_trace_event(arg, *pcode, ctx);
+ if (ret < 0) {
+ trace_probe_log_err(ctx->offset, NO_EVENT_FIELD);
return -EINVAL;
}
- code->op = FETCH_OP_IMM;
- code->immediate = 0;
+ return 0;
}
- /* These are fetching from memory */
- if (++code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ if (!tparg_is_function_entry(ctx->flags) &&
+ !tparg_is_function_return(ctx->flags)) {
+ trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
return -EINVAL;
}
- *pcode = code;
- code->op = FETCH_OP_DEREF;
- code->offset = offset;
- break;
+ return parse_btf_arg(arg, pcode, end, ctx);
+ }
+ return 0;
+}
+
+/* Recursive argument parser */
+static int
+parse_probe_arg(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct fetch_insn *code = *pcode;
+ int ret;
+
+ switch (arg[0]) {
+ case '$':
+ ret = parse_probe_vars(arg, type, pcode, end, ctx);
+ break;
+ case '%': /* named register */
+ ret = parse_probe_arg_register(arg, code, ctx);
+ break;
+ case '@': /* memory, file-offset or symbol */
+ ret = parse_probe_arg_mem_symbol(arg, pcode, end, ctx);
+ break;
case '+': /* deref memory */
case '-':
- if (arg[1] == 'u') {
- deref = FETCH_OP_UDEREF;
- arg[1] = arg[0];
- arg++;
- }
- if (arg[0] == '+')
- arg++; /* Skip '+', because kstrtol() rejects it. */
- tmp = strchr(arg, '(');
- if (!tmp) {
- trace_probe_log_err(ctx->offset, DEREF_NEED_BRACE);
- return -EINVAL;
- }
- *tmp = '\0';
- ret = kstrtol(arg, 0, &offset);
- if (ret) {
- trace_probe_log_err(ctx->offset, BAD_DEREF_OFFS);
- break;
- }
- ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
- arg = tmp + 1;
- ret = handle_dereference(arg, pcode, end, ctx, deref, offset);
- if (ret < 0)
- return ret;
+ ret = parse_probe_arg_deref(arg, pcode, end, ctx);
break;
case '\\': /* Immediate value */
- if (arg[1] == '"') { /* Immediate string */
- ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
- if (ret)
- break;
- code->op = FETCH_OP_IMMSTR;
- code->data = tmp;
- } else {
- ret = str_to_immediate(arg + 1, &code->immediate);
- if (ret)
- trace_probe_log_err(ctx->offset + 1, BAD_IMM);
- else
- code->op = FETCH_OP_IMM;
- }
+ ret = parse_probe_arg_imm(arg, code, ctx);
break;
case '(':
ret = handle_typecast(arg, pcode, end, ctx);
break;
default:
- if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
- str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
- ret = parse_this_cpu(arg, pcode, end, ctx);
- } else if (isalpha(arg[0]) || arg[0] == '_') {
- /* BTF variable or event field*/
- if (ctx->flags & TPARG_FL_TEVENT) {
- ret = parse_trace_event(arg, *pcode, ctx);
- if (ret < 0) {
- trace_probe_log_err(ctx->offset,
- NO_EVENT_FIELD);
- return -EINVAL;
- }
- break;
- }
- if (!tparg_is_function_entry(ctx->flags) &&
- !tparg_is_function_return(ctx->flags)) {
- trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
- return -EINVAL;
- }
- ret = parse_btf_arg(arg, pcode, end, ctx);
- }
+ ret = parse_probe_arg_default(arg, pcode, end, ctx);
break;
}
if (!ret && code->op == FETCH_OP_NOP) {
^ permalink raw reply related
* [PATCH 3/5] tracing/probes: Sort ERRORS list in trace_probe.h alphabetically
From: Masami Hiramatsu (Google) @ 2026-07-13 7:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178392766002.3229912.15711868107734330750.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
Sort the C-macro ERRORS list alphabetically in trace_probe.h to make
it easier to find and maintain error entries.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_probe.h | 160 ++++++++++++++++++++++----------------------
1 file changed, 80 insertions(+), 80 deletions(-)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e6268a8dc378..e64e323244a5 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -512,95 +512,95 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
#undef ERRORS
#define ERRORS \
- C(FILE_NOT_FOUND, "Failed to find the given file"), \
- C(NO_REGULAR_FILE, "Not a regular file"), \
- C(BAD_REFCNT, "Invalid reference counter offset"), \
- C(REFCNT_OPEN_BRACE, "Reference counter brace is not closed"), \
- C(BAD_REFCNT_SUFFIX, "Reference counter has wrong suffix"), \
- C(BAD_UPROBE_OFFS, "Invalid uprobe offset"), \
- C(BAD_MAXACT_TYPE, "Maxactive is only for function exit"), \
- C(BAD_MAXACT, "Invalid maxactive number"), \
- C(MAXACT_TOO_BIG, "Maxactive is too big"), \
- C(BAD_PROBE_ADDR, "Invalid probed address or symbol"), \
- C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \
- C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
- C(NO_TRACEPOINT, "Tracepoint is not found"), \
- C(BAD_TP_NAME, "Invalid character in tracepoint name"),\
- C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
- C(NO_GROUP_NAME, "Group name is not specified"), \
- C(GROUP_TOO_LONG, "Group name is too long"), \
- C(BAD_GROUP_NAME, "Group name must follow the same rules as C identifiers"), \
- C(NO_EVENT_NAME, "Event name is not specified"), \
- C(EVENT_TOO_LONG, "Event name is too long"), \
- C(BAD_EVENT_NAME, "Event name must follow the same rules as C identifiers"), \
- C(EVENT_EXIST, "Given group/event name is already used by another event"), \
- C(RETVAL_ON_PROBE, "$retval is not available on probe"), \
- C(NO_RETVAL, "This function returns 'void' type"), \
- C(BAD_STACK_NUM, "Invalid stack number"), \
+ C(ARGIDX_2BIG, "$argN index is too big"), \
+ C(ARGS_2LONG, "$arg* failed because the argument list is too long"), \
+ C(ARG_NAME_TOO_LONG, "Argument name is too long"), \
+ C(ARG_TOO_LONG, "Argument expression is too long"), \
+ C(ARRAY_NO_CLOSE, "Array is not closed"), \
+ C(ARRAY_TOO_BIG, "Array number is too big"), \
+ C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
+ C(BAD_ARG_NAME, "Argument name must follow the same rules as C identifiers"), \
C(BAD_ARG_NUM, "Invalid argument number"), \
- C(BAD_VAR, "Invalid $-variable specified"), \
- C(BAD_REG_NAME, "Invalid register name"), \
- C(BAD_MEM_ADDR, "Invalid memory address"), \
- C(BAD_IMM, "Invalid immediate value"), \
- C(IMMSTR_NO_CLOSE, "String is not closed with '\"'"), \
- C(FILE_ON_KPROBE, "File offset is not available for kernel probes"), \
- C(BAD_FILE_OFFS, "Invalid file offset value"), \
- C(SYM_ON_UPROBE, "Symbol is not available with uprobe"), \
- C(TOO_MANY_OPS, "Dereference is too much nested"), \
- C(DEREF_NEED_BRACE, "Dereference needs a brace"), \
+ C(BAD_ARRAY_NUM, "Invalid array size"), \
+ C(BAD_ARRAY_SUFFIX, "Array has wrong suffix"), \
+ C(BAD_ATTACH_ARG, "Attached event does not have this field"), \
+ C(BAD_ATTACH_EVENT, "Attached event does not exist"), \
+ C(BAD_BITFIELD, "Invalid bitfield"), \
+ C(BAD_BTF_TID, "Failed to get BTF type info."), \
C(BAD_DEREF_OFFS, "Invalid dereference offset"), \
- C(DEREF_OPEN_BRACE, "Dereference brace is not closed"), \
- C(COMM_CANT_DEREF, "$comm can not be dereferenced"), \
+ C(BAD_EVENT_NAME, "Event name must follow the same rules as C identifiers"), \
C(BAD_FETCH_ARG, "Invalid fetch argument"), \
- C(ARRAY_NO_CLOSE, "Array is not closed"), \
- C(BAD_ARRAY_SUFFIX, "Array has wrong suffix"), \
- C(BAD_ARRAY_NUM, "Invalid array size"), \
- C(ARRAY_TOO_BIG, "Array number is too big"), \
- C(BAD_TYPE, "Unknown type is specified"), \
- C(BAD_STRING, "String accepts only memory argument"), \
+ C(BAD_FILE_OFFS, "Invalid file offset value"), \
+ C(BAD_GROUP_NAME, "Group name must follow the same rules as C identifiers"), \
+ C(BAD_HYPHEN, "Failed to parse single hyphen. Forgot '>'?"), \
+ C(BAD_IMM, "Invalid immediate value"), \
+ C(BAD_INSN_BNDRY, "Probe point is not an instruction boundary"), \
+ C(BAD_MAXACT, "Invalid maxactive number"), \
+ C(BAD_MAXACT_TYPE, "Maxactive is only for function exit"), \
+ C(BAD_MEM_ADDR, "Invalid memory address"), \
+ C(BAD_PROBE_ADDR, "Invalid probed address or symbol"), \
+ C(BAD_REFCNT, "Invalid reference counter offset"), \
+ C(BAD_REFCNT_SUFFIX, "Reference counter has wrong suffix"), \
+ C(BAD_REG_NAME, "Invalid register name"), \
+ C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
+ C(BAD_STACK_NUM, "Invalid stack number"), \
+ C(BAD_STRING, "String accepts only memory argument"), \
C(BAD_SYMSTRING, "Symbol String doesn't accept data/userdata"), \
- C(BAD_BITFIELD, "Invalid bitfield"), \
- C(ARG_NAME_TOO_LONG, "Argument name is too long"), \
- C(NO_ARG_NAME, "Argument name is not specified"), \
- C(BAD_ARG_NAME, "Argument name must follow the same rules as C identifiers"), \
- C(USED_ARG_NAME, "This argument name is already used"), \
- C(ARG_TOO_LONG, "Argument expression is too long"), \
+ C(BAD_TP_NAME, "Invalid character in tracepoint name"), \
+ C(BAD_TYPE, "Unknown type is specified"), \
+ C(BAD_TYPE4STR, "This type does not fit for string."), \
+ C(BAD_UPROBE_OFFS, "Invalid uprobe offset"), \
+ C(BAD_VAR, "Invalid $-variable specified"), \
+ C(BAD_VAR_ARGS, "$arg* must be an independent parameter without name etc."), \
+ C(COMM_CANT_DEREF, "$comm can not be dereferenced"), \
+ C(DEREF_NEED_BRACE, "Dereference needs a brace"), \
+ C(DEREF_OPEN_BRACE, "Dereference brace is not closed"), \
+ C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"), \
+ C(DIFF_PROBE_TYPE, "Probe type is different from existing probe"), \
+ C(DOUBLE_ARGS, "$arg* can be used only once in the parameters"), \
+ C(EVENT_EXIST, "Given group/event name is already used by another event"), \
+ C(EVENT_TOO_BIG, "Event too big (too many fields?)"), \
+ C(EVENT_TOO_LONG, "Event name is too long"), \
+ C(FAIL_REG_PROBE, "Failed to register probe event"), \
+ C(FILE_NOT_FOUND, "Failed to find the given file"), \
+ C(FILE_ON_KPROBE, "File offset is not available for kernel probes"), \
+ C(GROUP_TOO_LONG, "Group name is too long"), \
+ C(IMMSTR_NO_CLOSE, "String is not closed with '\"'"), \
+ C(MAXACT_TOO_BIG, "Maxactive is too big"), \
+ C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"), \
+ C(NOFENTRY_ARGS, "$arg* can be used only on function entry or exit"), \
+ C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \
+ C(NOSUP_BTFARG, "BTF is not available or not supported"), \
+ C(NOSUP_DAT_ARG, "Non pointer structure/union argument is not supported."), \
+ C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"), \
C(NO_ARG_BODY, "No argument expression"), \
- C(BAD_INSN_BNDRY, "Probe point is not an instruction boundary"),\
- C(FAIL_REG_PROBE, "Failed to register probe event"),\
- C(DIFF_PROBE_TYPE, "Probe type is different from existing probe"),\
- C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"),\
- C(SAME_PROBE, "There is already the exact same probe event"),\
- C(NO_EVENT_INFO, "This requires both group and event name to attach"),\
- C(BAD_ATTACH_EVENT, "Attached event does not exist"),\
- C(BAD_ATTACH_ARG, "Attached event does not have this field"),\
+ C(NO_ARG_NAME, "Argument name is not specified"), \
+ C(NO_BTFARG, "This variable is not found at this probe point"), \
+ C(NO_BTF_ENTRY, "No BTF entry for this probe point"), \
+ C(NO_BTF_FIELD, "This field is not found."), \
C(NO_EP_FILTER, "No filter rule after 'if'"), \
- C(NOSUP_BTFARG, "BTF is not available or not supported"), \
- C(NO_BTFARG, "This variable is not found at this probe point"),\
- C(NO_BTF_ENTRY, "No BTF entry for this probe point"), \
- C(BAD_VAR_ARGS, "$arg* must be an independent parameter without name etc."),\
- C(NOFENTRY_ARGS, "$arg* can be used only on function entry or exit"), \
- C(DOUBLE_ARGS, "$arg* can be used only once in the parameters"), \
- C(ARGS_2LONG, "$arg* failed because the argument list is too long"), \
- C(ARGIDX_2BIG, "$argN index is too big"), \
+ C(NO_EVENT_FIELD, "This event field is not found."), \
+ C(NO_EVENT_INFO, "This requires both group and event name to attach"), \
+ C(NO_EVENT_NAME, "Event name is not specified"), \
+ C(NO_GROUP_NAME, "Group name is not specified"), \
C(NO_PTR_STRCT, "This is not a pointer to union/structure."), \
- C(NOSUP_DAT_ARG, "Non pointer structure/union argument is not supported."),\
- C(BAD_HYPHEN, "Failed to parse single hyphen. Forgot '>'?"), \
- C(NO_EVENT_FIELD, "This event field is not found."), \
- C(NO_BTF_FIELD, "This field is not found."), \
- C(BAD_BTF_TID, "Failed to get BTF type info."),\
- C(BAD_TYPE4STR, "This type does not fit for string."),\
- C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"),\
- C(TOO_MANY_ARGS, "Too many arguments are specified"), \
+ C(NO_REGULAR_FILE, "Not a regular file"), \
+ C(NO_RETVAL, "This function returns 'void' type"), \
+ C(NO_TRACEPOINT, "Tracepoint is not found"), \
+ C(REFCNT_OPEN_BRACE, "Reference counter brace is not closed"), \
+ C(RETVAL_ON_PROBE, "$retval is not available on probe"), \
+ C(SAME_PROBE, "There is already the exact same probe event"), \
+ C(SYM_ON_UPROBE, "Symbol is not available with uprobe"), \
+ C(TOO_MANY_ARGS, "Too many arguments are specified"), \
C(TOO_MANY_EARGS, "Too many entry arguments specified"), \
- C(EVENT_TOO_BIG, "Event too big (too many fields?)"), \
- C(TYPECAST_NOT_EVENT, "Typecasts are only for eprobe fields"), \
+ C(TOO_MANY_NESTED, "Too many nested typecasts/dereferences"), \
+ C(TOO_MANY_OPS, "Dereference is too much nested"), \
+ C(TYPECAST_BAD_ARROW, "Typecast field option does not support -> operator"), \
+ C(TYPECAST_NOT_ALIGNED, "Typecast field option is not byte-aligned"), \
+ C(TYPECAST_NOT_EVENT, "Typecasts are only for eprobe fields"), \
C(TYPECAST_REQ_FIELD, "Typecast requires a field access"), \
- C(TOO_MANY_NESTED, "Too many nested typecasts/dereferences"), \
- C(TYPECAST_SYM_OFFSET, "@SYM+/-OFFSET with typecast needs parentheses"), \
- C(TYPECAST_NOT_ALIGNED, "Typecast field option is not byte-aligned"), \
- C(TYPECAST_BAD_ARROW, "Typecast field option does not support -> operator"), \
- C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"),
+ C(TYPECAST_SYM_OFFSET, "@SYM+/-OFFSET with typecast needs parentheses"), \
+ C(USED_ARG_NAME, "This argument name is already used"),
#undef C
#define C(a, b) TP_ERR_##a
^ permalink raw reply related
* [PATCH 4/5] tracing/probes: Extend max length of argument string
From: Masami Hiramatsu (Google) @ 2026-07-13 7:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178392766002.3229912.15711868107734330750.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
To support BTF argument parsing (such as accessing fields within nested
structures via typecasting), the maximum argument string length needs
to be extended. Extend MAX_ARGSTR_LEN from 63 to 255.
Since MAX_ARGSTR_LEN was previously reused to format command heads in
trace_*probe_match_command_head() functions, introduce a dedicated
MAX_COMMON_HEAD_LEN (63) macro for matching command heads and switch
these functions to use the new macro.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Make the MAX_ARGSTR_LEN to 255 instead of 256.
---
kernel/trace/trace_fprobe.c | 2 +-
kernel/trace/trace_kprobe.c | 2 +-
kernel/trace/trace_probe.h | 3 ++-
kernel/trace/trace_uprobe.c | 2 +-
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 2 +-
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 2 +-
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 2 +-
7 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 536781cd4c47..5638a90e61cc 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -238,7 +238,7 @@ static bool trace_fprobe_is_busy(struct dyn_event *ev)
static bool trace_fprobe_match_command_head(struct trace_fprobe *tf,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
if (!argc)
return true;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cfa807d8e760..cc24e992732c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -149,7 +149,7 @@ static bool trace_kprobe_is_busy(struct dyn_event *ev)
static bool trace_kprobe_match_command_head(struct trace_kprobe *tk,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
if (!argc)
return true;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e64e323244a5..e6d427910c4f 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -32,7 +32,8 @@
#include "trace_output.h"
#define MAX_TRACE_ARGS 128
-#define MAX_ARGSTR_LEN 63
+#define MAX_ARGSTR_LEN 255
+#define MAX_COMMON_HEAD_LEN 63
#define MAX_ARRAY_LEN 64
#define MAX_ARG_NAME_LEN 32
#define MAX_BTF_ARGS_LEN 128
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b2e264a4b96c..67bd8fd91e3e 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -281,7 +281,7 @@ static bool trace_uprobe_is_busy(struct dyn_event *ev)
static bool trace_uprobe_match_command_head(struct trace_uprobe *tu,
int argc, const char **argv)
{
- char buf[MAX_ARGSTR_LEN + 1];
+ char buf[MAX_COMMON_HEAD_LEN + 1];
int len;
if (!argc)
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
index e9d7e6919c7f..984ab94df213 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
@@ -75,7 +75,7 @@ check_error 'f vfs_read ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_L
check_error 'f vfs_read ^=@11' # NO_ARG_NAME
check_error 'f vfs_read ^var.1=@11' # BAD_ARG_NAME
check_error 'f vfs_read var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 'f vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 'f vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 'f vfs_read arg1=^' # NO_ARG_BODY
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
index ffe8ffef4027..2d0905b2c8b7 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
@@ -61,7 +61,7 @@ check_error 't kfree ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_LOG
check_error 't kfree ^=@11' # NO_ARG_NAME
check_error 't kfree ^var.1=@11' # BAD_ARG_NAME
check_error 't kfree var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 't kfree ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 't kfree ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 't kfree arg1=^' # NO_ARG_BODY
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index 21ce8414459f..d28f63b7e8a9 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -71,7 +71,7 @@ check_error 'p vfs_read ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_L
check_error 'p vfs_read ^=@11' # NO_ARG_NAME
check_error 'p vfs_read ^var.1=@11' # BAD_ARG_NAME
check_error 'p vfs_read var1=@11 ^var1=@12' # USED_ARG_NAME
-check_error 'p vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG
+check_error 'p vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))))))))))))))))))))))' # ARG_TOO_LONG
check_error 'p vfs_read arg1=^' # NO_ARG_BODY
# instruction boundary check is valid on x86 (at this moment)
^ permalink raw reply related
* [PATCH v1] tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer()
From: Fuad Tabba @ 2026-07-13 7:28 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, vdonnefort, will, tabba, linux-kernel
page_va[] is annotated __counted_by(nr_page_va), so nr_page_va must
cover an index before that element is accessed. The allocation loop
writes page_va[id] while nr_page_va is still id and increments it only
afterwards, so every write is one element past the declared count.
The store is out of bounds with respect to the annotation: a build with
CONFIG_UBSAN_BOUNDS on a toolchain that honours __counted_by
(clang >= 20.1, gcc >= 15.1) flags it as an array-index overflow.
Increment nr_page_va before writing the element it now covers. A failed
allocation then leaves the slot counted but NULL; the error path frees
it with free_page(0), which is a no-op.
Fixes: 96e43537af546 ("tracing: Introduce trace remotes")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
I found this while reviewing Vincent's series [1].
Applies on top of Vincent's "[PATCH v2 0/3] tracing/remotes: Fix leak in
trace_remote_alloc_buffer() error path" [2], which fixes the other
issues in this function.
[1] https://lore.kernel.org/all/20260710114819.2689386-1-vdonnefort@google.com/
[2] https://lore.kernel.org/all/20260709160017.1729517-1-vdonnefort@google.com/
kernel/trace/trace_remote.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
index 0f6ef5c36d84e..ef42d9c38b374 100644
--- a/kernel/trace/trace_remote.c
+++ b/kernel/trace/trace_remote.c
@@ -1004,11 +1004,10 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
desc->nr_cpus++;
for (id = 0; id < nr_pages; id++) {
+ rb_desc->nr_page_va++;
rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
if (!rb_desc->page_va[id])
goto err;
-
- rb_desc->nr_page_va++;
}
rb_desc = __next_ring_buffer_desc(rb_desc);
}
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
prerequisite-patch-id: 8bdd7b28f09e97cff4f5995196b83b377c961d8b
prerequisite-patch-id: 6781b7ce5156d229fd21a757ef0cd66dd76130cc
prerequisite-patch-id: 56bcbbdceddf4533770d48d1835d5b7e11f47d56
--
2.39.5
^ permalink raw reply related
* [PATCH 5/5] tracing/probes: Eliminate recursion in parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-13 7:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
In-Reply-To: <178392766002.3229912.15711868107734330750.stgit@devnote2>
From: Masami Hiramatsu <mhiramat@kernel.org>
To avoid potential stack overflows on limited kernel stacks, convert
parse_probe_arg() from a recursive function into a loop-based
implementation with a simple local state stack.
Since recursion is eliminated using a loop with a fixed-size
stack in the context, this restricts the dereference nesting
depth. The maximum nesting depth of dereferences is now restricted
to the same limit as typecasts (TRACEPROBE_MAX_NESTED_LEVEL, which
is 8) and reports the same TOO_MANY_NESTED error. Update ftrace
selftests to reflect this restriction and simplify the checks.
Note that this change slightly alters the behavior of nested
dereferencing in fetcharg. Previously, dereferencing without BTF
allowed for up to 14 levels of nesting, whereas dereferencing
with BTF was limited to 3 levels. With this change, the nesting
depth is now limited to 8 levels in both cases.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Clearly state that both derefernce nest and typecast nest are
limited by the same TRACEPROBE_MAX_NESTED_LEVEL.
- Extend TRACEPROBE_MAX_NESTED_LEVEL to 8.
- Fix testcase to check TOO_MANY_NESTED instead of TOO_MANY_OPS
(that should not be hit anymore)
---
kernel/trace/trace_probe.c | 334 ++++++++++++--------
kernel/trace/trace_probe.h | 33 ++
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 4
.../ftrace/test.d/dynevent/tprobe_syntax_errors.tc | 2
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 4
5 files changed, 240 insertions(+), 137 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eb9f7bd13c19..05e9bff16648 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -356,61 +356,11 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
struct fetch_insn **pcode, struct fetch_insn *end,
struct traceprobe_parse_context *ctx);
-/* handle dereference nested call */
-static inline int handle_dereference(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end, struct traceprobe_parse_context *ctx,
- int deref, long offset)
+static int parse_this_cpu(char *arg, struct traceprobe_parse_context *ctx)
{
- const struct fetch_type *type = find_fetch_type(NULL, ctx->flags);
- struct fetch_insn *code = *pcode;
- int cur_offs = ctx->offset;
- char *tmp;
- int ret;
-
- tmp = strrchr(arg, ')');
- if (!tmp) {
- trace_probe_log_err(ctx->offset + strlen(arg),
- DEREF_OPEN_BRACE);
- return -EINVAL;
- }
-
- *tmp = '\0';
- ret = parse_probe_arg(arg, type, &code, end, ctx);
- if (ret)
- return ret;
- ctx->offset = cur_offs;
- if (code->op == FETCH_OP_COMM || code->op == FETCH_OP_IMMSTR) {
- trace_probe_log_err(ctx->offset, COMM_CANT_DEREF);
- return -EINVAL;
- }
-
- /*
- * this_cpu_ptr(@SYM) does not use SYM value, but use SYM address.
- * So we overwrite the last FETCH_OP_DEREF with FETCH_OP_CPU_PTR.
- */
- if (!(deref == FETCH_OP_CPU_PTR && *arg == '@')) {
- code++;
- if (code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
- return -EINVAL;
- }
- }
- *pcode = code;
-
- code->op = deref;
- code->offset = offset;
- /* Reset the last type if used */
- ctx->last_type = NULL;
- return 0;
-}
-
-static int parse_this_cpu(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
-{
- struct fetch_insn *code;
bool is_ptr = false;
- int ret;
+ bool is_read = false;
+ char *tmp;
/*
* This is only for kernel probes, excluding eprobe, because per-cpu
@@ -428,23 +378,24 @@ static int parse_this_cpu(char *arg, struct fetch_insn **pcode,
} else if (str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
arg += THIS_CPU_READ_LEN;
ctx->offset += THIS_CPU_READ_LEN;
+ is_read = true;
} else
return -EINVAL;
- ret = handle_dereference(arg, pcode, end, ctx, FETCH_OP_CPU_PTR, 0);
- if (ret || is_ptr)
- return ret;
-
- /* this_cpu_read(VAR) -> +0(this_cpu_ptr(VAR)) */
- code = *pcode;
- code++;
- if (code == end) {
- trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ tmp = strrchr(arg, ')');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset + strlen(arg),
+ DEREF_OPEN_BRACE);
return -EINVAL;
}
- code->op = FETCH_OP_DEREF;
- code->offset = 0;
- *pcode = code;
+ *tmp = '\0';
+
+ ctx->stack[ctx->depth].type = STATE_DEREF;
+ ctx->stack[ctx->depth].deref.deref = FETCH_OP_CPU_PTR;
+ ctx->stack[ctx->depth].deref.offset = 0;
+ ctx->stack[ctx->depth].deref.cur_offs = ctx->offset;
+ ctx->stack[ctx->depth].deref.inner_arg = arg;
+ ctx->stack[ctx->depth].deref.is_cpu_read = is_read;
return 0;
}
@@ -1007,14 +958,12 @@ static char *find_matched_close_paren(char *s)
return NULL;
}
-static int handle_typecast(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int handle_typecast(char *arg, struct traceprobe_parse_context *ctx)
{
int orig_offset = ctx->offset;
char *close;
char *tmp;
- int ret;
+ char *fieldname;
if (!(tparg_is_event_probe(ctx->flags) ||
tparg_is_function_entry(ctx->flags) ||
@@ -1028,12 +977,6 @@ static int handle_typecast(char *arg, struct fetch_insn **pcode,
* For example: (STRUCT)VAR->FIELD and (STRUCT)(VAR)->FIELD are same.
* VAR is solved in the nested call.
*/
- ctx->nested_level++;
- if (ctx->nested_level > TRACEPROBE_MAX_NESTED_LEVEL) {
- trace_probe_log_err(ctx->offset, TOO_MANY_NESTED);
- return -E2BIG;
- }
-
tmp = strchr(arg, ')');
if (!tmp) {
trace_probe_log_err(ctx->offset + strlen(arg),
@@ -1103,30 +1046,19 @@ static int handle_typecast(char *arg, struct fetch_insn **pcode,
}
*close = '\0';
- /* We need to parse the nested one */
- ret = parse_probe_arg(tmp, find_fetch_type(NULL, ctx->flags),
- pcode, end, ctx);
- if (ret < 0)
- return ret;
- ctx->nested_level--;
- clear_struct_btf(ctx);
-
- /* Let tmp point the field name. */
+ /* Let fieldname point the field name. */
if (close[1] == '-')
- tmp = close + 3; /* Skip "->" after closing parenthesis */
+ fieldname = close + 3; /* Skip "->" after closing parenthesis */
else
- tmp = close + 2; /* Skip ">" after inner variable name */
-
- /* resolve the typecast struct name */
- ctx->offset = orig_offset + 1; /* for the '(' */
- ret = parse_btf_casttype(arg + 1, ctx);
- if (ret < 0)
- return ret;
-
- ctx->offset = orig_offset + tmp - arg;
- ret = parse_btf_field(tmp, ctx->last_struct, pcode, end, ctx);
- ctx->prefix_byteoffs = 0;
- return ret;
+ fieldname = close + 2; /* Skip ">" after inner variable name */
+
+ ctx->stack[ctx->depth].type = STATE_TYPECAST;
+ ctx->stack[ctx->depth].typecast.casttype = arg + 1;
+ ctx->stack[ctx->depth].typecast.fieldname = fieldname;
+ ctx->stack[ctx->depth].typecast.orig_offset = orig_offset;
+ ctx->stack[ctx->depth].typecast.field_offset_diff = fieldname - arg;
+ ctx->stack[ctx->depth].typecast.inner_arg = tmp;
+ return 0;
}
#else /* !CONFIG_PROBE_EVENTS_BTF_ARGS */
@@ -1171,9 +1103,20 @@ static int check_prepare_btf_string_fetch(char *typename,
return 0;
}
-static int handle_typecast(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
+static int parse_btf_casttype(char *casttype,
+ struct traceprobe_parse_context *ctx)
+{
+ return -EOPNOTSUPP;
+}
+
+static int parse_btf_field(char *fieldname, const struct btf_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
struct traceprobe_parse_context *ctx)
+{
+ return -EOPNOTSUPP;
+}
+
+static int handle_typecast(char *arg, struct traceprobe_parse_context *ctx)
{
trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
return -EOPNOTSUPP;
@@ -1598,9 +1541,7 @@ static int parse_probe_arg_mem_symbol(char *arg, struct fetch_insn **pcode,
return 0;
}
-static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
- struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_deref(char *arg, struct traceprobe_parse_context *ctx)
{
int deref = FETCH_OP_DEREF;
long offset = 0;
@@ -1627,7 +1568,22 @@ static int parse_probe_arg_deref(char *arg, struct fetch_insn **pcode,
}
ctx->offset += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
arg = tmp + 1;
- return handle_dereference(arg, pcode, end, ctx, deref, offset);
+
+ tmp = strrchr(arg, ')');
+ if (!tmp) {
+ trace_probe_log_err(ctx->offset + strlen(arg),
+ DEREF_OPEN_BRACE);
+ return -EINVAL;
+ }
+ *tmp = '\0';
+
+ ctx->stack[ctx->depth].type = STATE_DEREF;
+ ctx->stack[ctx->depth].deref.deref = deref;
+ ctx->stack[ctx->depth].deref.offset = offset;
+ ctx->stack[ctx->depth].deref.cur_offs = ctx->offset;
+ ctx->stack[ctx->depth].deref.inner_arg = arg;
+ ctx->stack[ctx->depth].deref.is_cpu_read = false;
+ return 0;
}
static int parse_probe_arg_imm(char *arg, struct fetch_insn *code,
@@ -1659,11 +1615,6 @@ static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
{
int ret;
- if (str_has_prefix(arg, THIS_CPU_PTR_PREFIX) ||
- str_has_prefix(arg, THIS_CPU_READ_PREFIX)) {
- return parse_this_cpu(arg, pcode, end, ctx);
- }
-
if (isalpha(arg[0]) || arg[0] == '_') {
/* BTF variable or event field */
if (ctx->flags & TPARG_FL_TEVENT) {
@@ -1685,11 +1636,56 @@ static int parse_probe_arg_default(char *arg, struct fetch_insn **pcode,
return 0;
}
-/* Recursive argument parser */
-static int
-parse_probe_arg(char *arg, const struct fetch_type *type,
- struct fetch_insn **pcode, struct fetch_insn *end,
- struct traceprobe_parse_context *ctx)
+static int parse_probe_arg_nested(char **parg, struct traceprobe_parse_context *ctx)
+{
+ char *arg = *parg;
+ int ret;
+
+ while (true) {
+ /* Determine if this is a nested argument */
+ if (arg[0] != '+' && arg[0] != '-' && arg[0] != '(' &&
+ !str_has_prefix(arg, THIS_CPU_PTR_PREFIX) &&
+ !str_has_prefix(arg, THIS_CPU_READ_PREFIX))
+ break;
+
+ /* If nested, check the maximum depth limit */
+ if (ctx->depth >= TRACEPROBE_MAX_NESTED_LEVEL) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_NESTED);
+ return -E2BIG;
+ }
+
+ /* Perform the actual parsing subroutine calls */
+ switch (arg[0]) {
+ case '+':
+ case '-':
+ ret = parse_probe_arg_deref(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].deref.inner_arg;
+ break;
+ case '(':
+ ret = handle_typecast(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].typecast.inner_arg;
+ break;
+ default:
+ ret = parse_this_cpu(arg, ctx);
+ if (ret)
+ return ret;
+ arg = ctx->stack[ctx->depth].deref.inner_arg;
+ break;
+ }
+ ctx->depth++;
+ }
+
+ *parg = arg;
+ return 0;
+}
+
+static int parse_probe_arg_leaf(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
{
struct fetch_insn *code = *pcode;
int ret;
@@ -1704,26 +1700,112 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
case '@': /* memory, file-offset or symbol */
ret = parse_probe_arg_mem_symbol(arg, pcode, end, ctx);
break;
- case '+': /* deref memory */
- case '-':
- ret = parse_probe_arg_deref(arg, pcode, end, ctx);
- break;
case '\\': /* Immediate value */
ret = parse_probe_arg_imm(arg, code, ctx);
break;
- case '(':
- ret = handle_typecast(arg, pcode, end, ctx);
- break;
default:
ret = parse_probe_arg_default(arg, pcode, end, ctx);
break;
}
- if (!ret && code->op == FETCH_OP_NOP) {
+
+ if (ret)
+ return ret;
+
+ if (code->op == FETCH_OP_NOP) {
/* Parsed, but do not find fetch method */
trace_probe_log_err(ctx->offset, BAD_FETCH_ARG);
- ret = -EINVAL;
+ return -EINVAL;
}
- return ret;
+
+ return 0;
+}
+
+static int unwind_parse_states(struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ struct parse_state *state;
+ struct fetch_insn *code;
+ int ret;
+
+ while (ctx->depth > 0) {
+ ctx->depth--;
+ state = &ctx->stack[ctx->depth];
+
+ if (state->type == STATE_DEREF) {
+ code = *pcode;
+ ctx->offset = state->deref.cur_offs;
+ if (code->op == FETCH_OP_COMM || code->op == FETCH_OP_IMMSTR) {
+ trace_probe_log_err(ctx->offset, COMM_CANT_DEREF);
+ return -EINVAL;
+ }
+
+ if (!(state->deref.deref == FETCH_OP_CPU_PTR &&
+ *state->deref.inner_arg == '@')) {
+ code++;
+ if (code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ }
+ *pcode = code;
+
+ code->op = state->deref.deref;
+ code->offset = state->deref.offset;
+ ctx->last_type = NULL;
+
+ if (state->deref.is_cpu_read) {
+ code = *pcode;
+ code++;
+ if (code == end) {
+ trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+ return -EINVAL;
+ }
+ code->op = FETCH_OP_DEREF;
+ code->offset = 0;
+ *pcode = code;
+ }
+ } else if (state->type == STATE_TYPECAST) {
+ clear_struct_btf(ctx);
+
+ /* resolve the typecast struct name */
+ ctx->offset = state->typecast.orig_offset + 1; /* for the '(' */
+ ret = parse_btf_casttype(state->typecast.casttype, ctx);
+ if (ret < 0)
+ return ret;
+
+ ctx->offset = state->typecast.orig_offset +
+ state->typecast.field_offset_diff;
+ ret = parse_btf_field(state->typecast.fieldname,
+ ctx->last_struct, pcode,
+ end, ctx);
+ ctx->prefix_byteoffs = 0;
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Loop-based (non-recursive) argument parser */
+static int
+parse_probe_arg(char *arg, const struct fetch_type *type,
+ struct fetch_insn **pcode, struct fetch_insn *end,
+ struct traceprobe_parse_context *ctx)
+{
+ int ret;
+
+ ctx->depth = 0;
+
+ ret = parse_probe_arg_nested(&arg, ctx);
+ if (ret)
+ return ret;
+
+ ret = parse_probe_arg_leaf(arg, type, pcode, end, ctx);
+ if (ret)
+ return ret;
+
+ return unwind_parse_states(pcode, end, ctx);
}
/* Bitfield type needs to be parsed into a fetch function */
@@ -1984,12 +2066,6 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
ctx);
if (ret < 0)
goto fail;
- /* nested_level must be 0 here, otherwise there is a bug. */
- if (WARN_ON_ONCE(ctx->nested_level)) {
- ret = -EINVAL;
- goto fail;
- }
-
/* Update storing type if BTF is available */
if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) &&
ctx->last_type) {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e6d427910c4f..ebdc706e7cb6 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -437,6 +437,34 @@ static inline bool tparg_is_event_probe(unsigned int flags)
return !!(flags & TPARG_FL_TEVENT);
}
+/* Each typecast consumes nested level. So the max number of typecast is 8. */
+#define TRACEPROBE_MAX_NESTED_LEVEL 8
+
+enum parse_state_type {
+ STATE_DEREF,
+ STATE_TYPECAST,
+};
+
+struct parse_state {
+ int type;
+ union {
+ struct {
+ int deref;
+ long offset;
+ int cur_offs;
+ char *inner_arg;
+ bool is_cpu_read;
+ } deref;
+ struct {
+ char *casttype;
+ char *fieldname;
+ int orig_offset;
+ int field_offset_diff;
+ char *inner_arg;
+ } typecast;
+ };
+};
+
struct traceprobe_parse_context {
struct trace_event_call *event;
/* BTF related parameters */
@@ -453,12 +481,11 @@ struct traceprobe_parse_context {
struct trace_probe *tp;
unsigned int flags;
int offset;
- int nested_level;
int prefix_byteoffs; /* The byte offset of the prefix field of typecast */
+ struct parse_state stack[TRACEPROBE_MAX_NESTED_LEVEL + 1];
+ int depth;
};
-/* Each typecast consumes nested level. So the max number of typecast is 3. */
-#define TRACEPROBE_MAX_NESTED_LEVEL 3
extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
const char *argv,
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
index 984ab94df213..384209968325 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc
@@ -60,7 +60,7 @@ check_error 'f vfs_read ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 'f vfs_read +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 'f vfs_read +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 'f vfs_read +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 'f vfs_read +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 'f vfs_read +0(@11):u8[^10a]' # BAD_ARRAY_NUM
@@ -114,7 +114,7 @@ check_error 'f vfs_read file^-.foo' # BAD_HYPHEN
check_error 'f vfs_read ^file:string' # BAD_TYPE4STR
if grep -qF "[(structname" README ; then
check_error 'f vfs_read arg1=(task_struct)file^' # TYPECAST_REQ_FIELD
-check_error 'f vfs_read arg1=(a)((b)((c)(^(d)file->d)->c)->b)->a' # TOO_MANY_NESTED
+check_error 'f vfs_read arg1=(a)((b)((c)((d)((e)((f)((g)((h)(^(i)file->i)->h)->g)->f)->e)->d)->c)->b)->a' # TOO_MANY_NESTED
check_error 'f vfs_read arg1=(task_struct,^in_execve)file->comm' # TYPECAST_NOT_ALIGNED
check_error 'f vfs_read arg1=(task_struct,^foo_bar)file->pid' # NO_BTF_FIELD
check_error 'f vfs_read arg1=(^task_struct1234)file->pid' # NO_PTR_STRCT
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
index 2d0905b2c8b7..72b8652df9ba 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/tprobe_syntax_errors.tc
@@ -46,7 +46,7 @@ check_error 't kfree ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 't kfree +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 't kfree +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 't kfree +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 't kfree +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 't kfree +0(@11):u8[^10a]' # BAD_ARRAY_NUM
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index d28f63b7e8a9..b0e6b80ccb01 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -56,7 +56,7 @@ check_error 'p vfs_read ^&1' # BAD_FETCH_ARG
# We've introduced this limitation with array support
if grep -q ' <type>\\\[<array-size>\\\]' README; then
-check_error 'p vfs_read +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS?
+check_error 'p vfs_read +0(+0(+0(+0(+0(+0(+0(+0(^+0(@0)))))))))' # TOO_MANY_NESTED
check_error 'p vfs_read +0(@11):u8[10^' # ARRAY_NO_CLOSE
check_error 'p vfs_read +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX
check_error 'p vfs_read +0(@11):u8[^10a]' # BAD_ARRAY_NUM
@@ -117,7 +117,7 @@ check_error 'p kfree ^$arg10' # NO_BTFARG (exceed the number of parameters)
check_error 'r kfree ^$retval' # NO_RETVAL
if grep -qF "[(structname" README ; then
check_error 'p vfs_read arg1=(task_struct)file^' # TYPECAST_REQ_FIELD
-check_error 'p vfs_read arg1=(a)((b)((c)(^(d)file->d)->c)->b)->a' # TOO_MANY_NESTED
+check_error 'p vfs_read arg1=(a)((b)((c)((d)((e)((f)((g)((h)(^(i)file->i)->h)->g)->f)->e)->d)->c)->b)->a' # TOO_MANY_NESTED
check_error 'p vfs_read arg1=(task_struct,^in_execve)file->comm' # TYPECAST_NOT_ALIGNED
check_error 'p vfs_read arg1=(task_struct,^foo_bar)file->pid' # NO_BTF_FIELD
check_error 'p vfs_read arg1=(^task_struct1234)file->pid' # NO_PTR_STRCT
^ permalink raw reply related
* Re: [PATCH v1] tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer()
From: Vincent Donnefort @ 2026-07-13 8:21 UTC (permalink / raw)
To: Fuad Tabba
Cc: rostedt, mhiramat, linux-trace-kernel, mathieu.desnoyers, will,
tabba, linux-kernel
In-Reply-To: <20260713072823.2668323-1-fuad.tabba@linux.dev>
On Mon, Jul 13, 2026 at 08:28:23AM +0100, Fuad Tabba wrote:
> page_va[] is annotated __counted_by(nr_page_va), so nr_page_va must
> cover an index before that element is accessed. The allocation loop
> writes page_va[id] while nr_page_va is still id and increments it only
> afterwards, so every write is one element past the declared count.
>
> The store is out of bounds with respect to the annotation: a build with
> CONFIG_UBSAN_BOUNDS on a toolchain that honours __counted_by
> (clang >= 20.1, gcc >= 15.1) flags it as an array-index overflow.
>
> Increment nr_page_va before writing the element it now covers. A failed
> allocation then leaves the slot counted but NULL; the error path frees
> it with free_page(0), which is a no-op.
>
> Fixes: 96e43537af546 ("tracing: Introduce trace remotes")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Tested-by: Vincent Donnefort <vdonnefort@google.com>
> ---
> I found this while reviewing Vincent's series [1].
>
> Applies on top of Vincent's "[PATCH v2 0/3] tracing/remotes: Fix leak in
> trace_remote_alloc_buffer() error path" [2], which fixes the other
> issues in this function.
>
> [1] https://lore.kernel.org/all/20260710114819.2689386-1-vdonnefort@google.com/
> [2] https://lore.kernel.org/all/20260709160017.1729517-1-vdonnefort@google.com/
>
> kernel/trace/trace_remote.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 0f6ef5c36d84e..ef42d9c38b374 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
> @@ -1004,11 +1004,10 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
> desc->nr_cpus++;
>
> for (id = 0; id < nr_pages; id++) {
> + rb_desc->nr_page_va++;
> rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
> if (!rb_desc->page_va[id])
> goto err;
> -
> - rb_desc->nr_page_va++;
> }
> rb_desc = __next_ring_buffer_desc(rb_desc);
> }
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> prerequisite-patch-id: 8bdd7b28f09e97cff4f5995196b83b377c961d8b
> prerequisite-patch-id: 6781b7ce5156d229fd21a757ef0cd66dd76130cc
> prerequisite-patch-id: 56bcbbdceddf4533770d48d1835d5b7e11f47d56
> --
> 2.39.5
>
^ permalink raw reply
* Re: [PATCH v2 4/4] mm/mlock: migrate folios out of CMA when mlocking a range
From: Wandun @ 2026-07-13 10:01 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <ak0OZKzapUefmA3v@lucifer>
On 7/7/26 22:54, Lorenzo Stoakes wrote:
> On Tue, Jul 07, 2026 at 08:59:25PM +0800, Wandun Chen wrote:
>> From: Wandun Chen <chenwandun@lixiang.com>
>>
>> The region covered by mlock[all] may contain CMA pages. cma_alloc installs
>> migration entries in the page table, if a memory access occurs at this
>> point, it must wait for the migration to complete, which may cause
>> latency spikes on the RT kernels.
>>
>> Try to move the migration cost into the mlock[all] caller, which is
>> typically a setup path. So reduce the chance of latency spikes on RT
>> kernels by migrating the currently mapped CMA pages out of CMA region.
>
> 'reduce the chances of latency' so do you have any data to back this invasive
> change or not?
>
> And for RT, but nothing in here at all checks for RT? You're using this
> compaction sysctl as an RT check somehow? That's gross.
sysctl_compact_unevictable_allowed is set to zero by default in RT kernels.
Also, sysctl_compact_unevictable_allowed can also be set to 1 in the RT
kernel, though this will produce a warning.
When sysctl_compact_unevictable_allowed is set to 1, my understanding is
that the mlock region should not contain CMA pages; otherwise, migration
entries will appear during CMA migration, and have to wait for the
migration to complete.
>
> This doesn't feel like the right solution.
>
>>
>> Suggested-by: Frank van der Linden <fvdl@google.com>
>> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
>> Link: https://lore.kernel.org/all/CAPTztWZpnX1j8-7yeppVUsxE=O9hbVeqricDjZt8_pnN7a-kBQ@mail.gmail.com/#t
>> ---
>> mm/mlock.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 118 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/mlock.c b/mm/mlock.c
>> index ac65de40b22b..f56c685533f5 100644
>> --- a/mm/mlock.c
>> +++ b/mm/mlock.c
>> @@ -25,6 +25,7 @@
>> #include <linux/memcontrol.h>
>> #include <linux/mm_inline.h>
>> #include <linux/secretmem.h>
>> +#include <linux/migrate.h>
>> #include <linux/compaction.h>
>>
>> #include "internal.h"
>> @@ -428,6 +429,119 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
>> return 0;
>> }
>>
>> +#ifdef CONFIG_CMA
>
> Ugh yuck. This is horrible. Why are we polluting mlock.c with CMA stuff?
Will move to the relevant CMA file, if the CMA migration approach is acceptable.
>
> Also no comment?
>
>> +static int mlock_collect_migratable_pte_range(pmd_t *pmd, unsigned long addr,
>> + unsigned long end, struct mm_walk *walk)
>> +{
>
> You've literally copy/pasted mlock_pte_range(), this is disgusting.
>
> Please don't copy/paste code like this, this isn't PHP, it's the kernel, it's
> completely unacceptable.
>
>> + struct vm_area_struct *vma = walk->vma;
>> + struct list_head *folio_list = walk->private;
>> + spinlock_t *ptl;
>> + pte_t *start_pte, *pte;
>> + pte_t ptent;
>> + struct folio *folio;
>> + unsigned int step = 1;
>> +
>> + if (!(vma->vm_flags & VM_LOCKED))
>
> Once again vma_test(vma, VMA_LOCKED_BIT) please.
>
> But also, again (since you copy/pasted), why? You're literally mlocking here...
>
> You going for that 'mlocking already mlocked ranges' sweet spot or am I missing
> something?
Will remove this check in next version.
>
>> + return 0;
>> +
>> + ptl = pmd_trans_huge_lock(pmd, vma);
>> + if (ptl) {
>> + if (!pmd_present(*pmd)) {
>> + if (unlikely(softleaf_is_migration(softleaf_from_pmd(*pmd)))) {
>
> OK so you're not actually checking for CMA here at all, you're just doing the
> migration wait stuff... again here? What?
This is to prevent CMA pages from being migrated by cma_alloc at this point,
with migration entries already installed. If there are multiple CMA regions
in the system, after the wait completes the page could still be a CMA page,
so wait and recheck again.
>
>> + spin_unlock(ptl);
>> + pmd_migration_entry_wait(vma->vm_mm, pmd);
>> + walk->action = ACTION_AGAIN;
>> + return 0;
>> + }
>> + goto out;
>> + }
>> + if (is_huge_zero_pmd(*pmd))
>> + goto out;
>> + folio = pmd_folio(*pmd);
>> + if (folio_is_zone_device(folio))
>> + goto out;
>> + if (is_migrate_cma_page(&folio->page))
>
> Err isn't this per-page, and you just checked that this is a huge folio, and
> you're only checking the first page? That seems wrong?
Not wrong. IIUC there are two cases:
If folio_order(folio) < pageblock_order, the folio fits entirely within a
single pageblock, so the first page is sufficient.
If folio_order(folio) >= pageblock_order, __free_one_page() prevents any
buddy merge across a CMA/non-CMA boundary.
>
>> + isolate_folio_to_list(folio, folio_list);
>
> Then you just take the whole folio?
>
> It's really horrible that you're burying this in copy/pasted code from the rest.
>
>> + goto out;
>> + }
>> +
>> + start_pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>> + if (!start_pte) {
>> + walk->action = ACTION_AGAIN;
>> + return 0;
>> + }
>> +
>> + for (pte = start_pte; addr != end; pte += step, addr += step * PAGE_SIZE) {
>> + step = 1;
>> + ptent = ptep_get(pte);
>> + if (!pte_present(ptent)) {
>> + if (unlikely(softleaf_is_migration(softleaf_from_pte(ptent)))) {
>> + pte_unmap_unlock(start_pte, ptl);
>> + migration_entry_wait(vma->vm_mm, pmd, addr);
>
> BTW I also wonder if we have guaranteed forward progress here if say a migration
> happened to migrate back and forth again across a range...?
In theory, it is possible to keep waiting here repeatedly:
if there are multiple CMA regions, a cma_alloc() from CMA region A installs a
migration entry; once that migration completes, the page lands in CMA region B;
then a cma_alloc() from CMA region B installs another migration entry, and so on,
this function keep seeing a migration entry indefinitely. I suspect this is an
extremely rare scenario in practice, but I will add a retry limit in a follow-up
to guard against it.
Pages migrated out by mlock_migrate_cma_range() itself will not land in CMA
(allocated via alloc_migration_target with GFP_HIGHUSER which avoids CMA),
so this function always make forward progress.
>
>> + walk->action = ACTION_AGAIN;
>> + return 0;
>> + }
>
> Again, since you copy/pasted, same comment - this is disgusting.
>
> And again I'm confused why you're waiting here again after you did it previously
> due to the previous commit that already does it?
>
> And why for a non-CMA range, if somebody happens to set CONFIG_CMA they now get
> this done twice?
The previous commit prevents page migration triggered by kcompactd, but cannot
prevent page migration triggered by cma_alloc().
compact_unevictable_allowed only prevents kcompactd-driven migration.
For an RT application, migration triggered by kcompactd is outside the
application's own behavior and can impact real-time latency. However,
cma_alloc() does not check compact_unevictable_allowed, so CMA-driven
migration will still migrate pages in the mlocked range, even if the RT
application has already called mlock().
The idea of this patch is to migrate CMA pages out of the locked range
at mlock() time. After that, even if cma_alloc() is called later, it
will no longer affect the mlocked range.
Perhaps I should describe this more thoroughly in the commit message, will
add this info in next version.
>
>> + continue;
>> + }
>> + folio = vm_normal_folio(vma, addr, ptent);
>> + if (!folio || folio_is_zone_device(folio))
>> + continue;
>> + step = folio_mlock_step(folio, pte, addr, end);
>> + if (is_migrate_cma_page(&folio->page))
>> + continue;
>
> You mean ! surely?
>
> Why are you inverting this vs. the above? you replied to the patch so maybe you
> correct this there.
>
>> + isolate_folio_to_list(folio, folio_list);
>
> And again you bury the actual point of this in one easily missed line and zero
> comments.
I will add some comments in the next version.
>
> No.
>
>> + }
>> + pte_unmap(start_pte);
>> +out:
>> + spin_unlock(ptl);
>> + cond_resched();
>> + return 0;
>> +}
>
> Yeah this is just completely unacceptable. You have to find a way to deduplicate
> code. Copy/paste code dumps are not ok.
Will refactor it in the next version to share the logic with mlock_pte_range().
>
> But at the same time, you're dumping CMA crap in core mm mlock code which is
> horrible, you've also dumped some migration code so you're really mixing things
> up here horribly, and it doesn't seem justified?
>
As mentioned earlier, the migration entry handling here is intentional, we need to
wait for any in-flight migration to complete before checking whether the page is
CMA, otherwise we might miss CMA pages that are temporarily hidden behind a
migration entry.
>> +
>> +static const struct mm_walk_ops mlock_collect_migratable_ops = {
>> + .pmd_entry = mlock_collect_migratable_pte_range,
>> + .walk_lock = PGWALK_RDLOCK,
>> +};
>> +
>> +static void mlock_migrate_cma_range(unsigned long start, unsigned long len)
>> +{
>> + struct mm_struct *mm = current->mm;
>> + unsigned long end = start + len;
>> + LIST_HEAD(folio_list);
>> + struct migration_target_control mtc = {
>> + .nid = NUMA_NO_NODE,
>> + .gfp_mask = GFP_HIGHUSER | __GFP_NOWARN,
>> + .reason = MR_SYSCALL,
>> + };
>> +
>> + if (compaction_allow_unevictable())
>> + return;
>
> Again you're assuming compaction for some reason. Why?
If compact_unevictable_allowed is true, it means the system allows kcompactd
to migrate pages in mlocked ranges, implying that the latency impact of such
migration is acceptable. In that case, there is no need to proactively migrate
CMA pages out of the mlocked range either.
>
> It feels like you're just gating specific behaviour for your workload on this
> flag and assuming that's ok.
>
>> +
>> + lru_cache_disable();
>
> What, why?
IIUC if folios have not been drained to the LRU, the isolation will fail.
>
>> +
>> + if (mmap_read_lock_killable(mm))
>> + goto out;
>
> OK so you got a fatal signal and you don't bother telling anybody about it and
> just skip migration?...
Oh, here should return -EINTR, will fix in next version.
>
>> +
>
> Weird whitespace...
>
>> + walk_page_range(mm, start, end, &mlock_collect_migratable_ops,
>> + &folio_list);
>> + mmap_read_unlock(mm);
>> +
>> + if (list_empty(&folio_list))
>> + goto out;
>> +
>> + if (migrate_pages(&folio_list, alloc_migration_target, NULL,
>> + (unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL, NULL))
>> + putback_movable_pages(&folio_list);
>> +out:
>> + lru_cache_enable();
>> +}
>> +#else
>> +static inline void mlock_migrate_cma_range(unsigned long start,
>
> inline in a .c file? Why? Drop it.
Got it.
>
>> + unsigned long len)
>> +{
>> +}
>> +#endif /* CONFIG_CMA */
>> +
>> /*
>> * mlock_vma_pages_range() - mlock any pages already in the range,
>> * or munlock all pages in the range.
>> @@ -678,6 +792,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
>> error = __mm_populate(start, len, 0);
>> if (error)
>> return __mlock_posix_error_return(error);
>> + mlock_migrate_cma_range(start, len);
>
> Err, unconditionally?
In mlock_migrate_cma_range(), the decision of whether to actually perform the
migration is gated by compact_unevictable_allowed.
>
>> return 0;
>> }
>>
>> @@ -790,8 +905,10 @@ SYSCALL_DEFINE1(mlockall, int, flags)
>> capable(CAP_IPC_LOCK))
>> ret = apply_mlockall_flags(flags);
>> mmap_write_unlock(current->mm);
>> - if (!ret && (flags & MCL_CURRENT))
>> + if (!ret && (flags & MCL_CURRENT)) {
>> mm_populate(0, TASK_SIZE);
>> + mlock_migrate_cma_range(0, TASK_SIZE);
>
> Err what? Why are you doing this? You're forcing a wait on migration of
> literally everything across the whole of the process whether or not they're CMA
> ranges, but as a one time thing?
mm_populate(0, TASK_SIZE) operates on the entire process address space, so
mlock_migrate_cma_range() needs to scan the same range to find and migrate
any CMA pages.
We are aware that this operation can be time-consuming. On RT systems,
mlockall() is typically called during the application's initialization
phase, so the cost is paid upfront at setup time rather than at runtime.
The goal is to ensure that there are no latency spikes during the real-time
execution phase.
>
>
>> + }
>>
>> return ret;
>> }
>> --
>> 2.43.0
>>
>
> In general this feels like the wrong solution for a specific workload that
> sticks horrible stuff in core mm and I don't really love it :)
>
> Thanks, Lorenzo
Thanks,
Wandun
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Mark Brown @ 2026-07-13 10:46 UTC (permalink / raw)
To: Praveen Talari
Cc: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-arm-msm,
linux-trace-kernel, linux-spi, Mukesh Kumar Savaliya,
Konrad Dybcio
In-Reply-To: <4bcaa245-2127-45aa-bf05-48e275c0129c@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]
On Sat, Jul 11, 2026 at 08:21:02AM +0530, Praveen Talari wrote:
> On 11-07-2026 02:56, Mark Brown wrote:
> > On Sat, Jul 11, 2026 at 12:18:42AM +0530, Praveen Talari wrote:
> > > +#include <trace/events/qcom_geni_se.h>
> > Should this be in rivers/soc/qcom/qcom-geni-se.c (and the first patch?)
> > - that way if another driver starts using them we won't multiply define
> > the tracepoints.
> Yes, you are correct but
> If this header in drivers/soc/qcom/qcom-geni-se.c, how it will be access
> trace API in other drivers like i2c, spi and uart?
The header needs to be in the users but the CREATE_ define that you cut
needs to be in only one place to actually create the tracepoints.
> > > @@ -986,10 +997,13 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
> > > writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
> > > dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n",
> > > mas->tx_rem_bytes, mas->cur_bits_per_word);
> > > + trace_geni_se_regs(se);
> > SE_GENI_TX_WATERMARK_REG is one of the registers in the tracepoint,
> > perhaps trace before we write to clear it?
> it will be captured in m_irq register so it is not required to capture.
Are you sure that won't lead to user confusion?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/1] psi: Introduce in-kernel PSI auto monitor feature
From: Johannes Weiner @ 2026-07-13 11:59 UTC (permalink / raw)
To: Pintu Kumar Agarwal
Cc: linux-kernel, linux-trace-kernel, surenb, rostedt, mhiramat,
peterz, mathieu.desnoyers, mingo, juri.lelli, vincent.guittot,
dietmar.eggemann, bsegall, mgorman, vschneid, kprateek.nayak,
pintu.ping, nathan, ojeda, nsc, gary, tglx, thomas.weissschuh,
aliceryhl, dianders, linux.amoon, rdunlap, akpm, shuah
In-Reply-To: <20260702171606.527077-1-pintu.agarwal@oss.qualcomm.com>
On Thu, Jul 02, 2026 at 10:46:05PM +0530, Pintu Kumar Agarwal wrote:
> Hi all,
>
> This RFC introduces an in-kernel PSI auto monitor aimed at improving
> root-cause visibility for resource pressure events in Linux systems.
>
> Motivation:
>
> PSI already provides an excellent mechanism to detect CPU, memory and
> I/O pressure and includes trigger-based notifications via pollable
> interfaces. However, it deliberately avoids attributing pressure to
> individual tasks.
Why is this necessary?
> In real-world systems, this creates a gap: when a PSI trigger fires,
> users still need to determine *which tasks caused the stall* by combining
> multiple tools (top, meminfo, vmstat, perf, tracing, etc.), often after
> the event has already passed.
I've never found myself needing to identify which specific task was
stalled. Tasks competing over a shared resource are interdependent.
Consider this scenario: Task A, B, C are allocating memory and
creating pressure together; B randomly becomes the sucker to hit
direct reclaim and making room for the other too as well. Why is it
meaningful to know that B stalled? It caused the resource contention
no more than the other two.
Then A and C refault: A is fast, but C happens to hit when the flash
drive is running garbage collection. Why is it meaningful to know C?
Again, C is no more the culprit in that situation than the others.
The meaningful thing you can say is that the domain as a whole is
under pressure. Who exactly becomes the lightning rod is noise.
That said, if you do need it, why not use delayacct? It already tracks
the reclaim, swap, thrashing time and events that also go into psi on
a per-task basis. It's not sampled, either.
^ permalink raw reply
* [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races
From: Tengda Wu @ 2026-07-13 13:46 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mark Rutland, Mathieu Desnoyers, linux-trace-kernel, linux-kernel,
Tengda Wu
The trace_parser structure is allocated and initialized when a trace
file is opened, and is subsequently used in the write handler to parse
user input. If userspace opens a trace file descriptor and shares it
across multiple threads, concurrent write calls will race on the
parser's internal state, specifically the idx, cont, and buffer fields,
leading to corrupted input or undefined behavior.
Fix this by embedding a mutex directly in struct trace_parser. The mutex
is initialized in trace_parser_get_init() and destroyed in
trace_parser_put(). All write-side users that access parser state
(trace_get_user() followed by checking trace_parser_loaded() /
trace_parser_cont() against the buffer) now hold the mutex across the
full critical section, avoiding any TOCTOU gap between the parse and the
subsequent consumption of parser->buffer.
Affected write paths:
- ftrace_graph_write / ftrace_graph_release
- ftrace_regex_write / ftrace_regex_release
Fixes: e704eff3ff51 ("ftrace: Have set_graph_function handle multiple functions in one write")
Fixes: 689fd8b65d66 ("tracing: trace parser support for function and graph")
Cc: stable@vger.kernel.org
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
kernel/trace/ftrace.c | 7 +++++++
kernel/trace/trace.c | 2 ++
kernel/trace/trace.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index f93e34dd2328..ef47e5659283 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
/* iter->hash is a local copy, so we don't need regex_lock */
parser = &iter->parser;
+
+ guard(mutex)(&parser->lock);
read = trace_get_user(parser, ubuf, cnt, ppos);
if (read >= 0 && trace_parser_loaded(parser) &&
@@ -6984,12 +6986,14 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
iter = file->private_data;
parser = &iter->parser;
+ mutex_lock(&parser->lock);
if (trace_parser_loaded(parser)) {
int enable = !(iter->flags & FTRACE_ITER_NOTRACE);
ftrace_process_regex(iter, parser->buffer,
parser->idx, enable);
}
+ mutex_unlock(&parser->lock);
trace_parser_put(parser);
@@ -7321,10 +7325,12 @@ ftrace_graph_release(struct inode *inode, struct file *file)
parser = &fgd->parser;
+ mutex_lock(&parser->lock);
if (trace_parser_loaded((parser))) {
ret = ftrace_graph_set_hash(fgd->new_hash,
parser->buffer);
}
+ mutex_unlock(&parser->lock);
trace_parser_put(parser);
@@ -7437,6 +7443,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
parser = &fgd->parser;
+ guard(mutex)(&parser->lock);
read = trace_get_user(parser, ubuf, cnt, ppos);
if (read >= 0 && trace_parser_loaded(parser) &&
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1146b83b711a..fbf5f220d580 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1100,6 +1100,7 @@ int trace_parser_get_init(struct trace_parser *parser, int size)
return 1;
parser->size = size;
+ mutex_init(&parser->lock);
return 0;
}
@@ -1108,6 +1109,7 @@ int trace_parser_get_init(struct trace_parser *parser, int size)
*/
void trace_parser_put(struct trace_parser *parser)
{
+ mutex_destroy(&parser->lock);
kfree(parser->buffer);
parser->buffer = NULL;
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2537c33ddd49..f643842d2ffd 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1387,6 +1387,7 @@ struct trace_parser {
char *buffer;
unsigned idx;
unsigned size;
+ struct mutex lock;
};
static inline bool trace_parser_loaded(struct trace_parser *parser)
--
2.34.1
^ permalink raw reply related
* [PATCH] rtla/timerlat_top: Fix on-threshold actions firing on signal
From: Tomas Glozar @ 2026-07-13 14:10 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar
Cc: John Kacur, Luis Goncalves, Crystal Wood, Costa Shulyupin,
Wander Lairson Costa, LKML, linux-trace-kernel, Attila Fazekas
A bug was reported when rtla-timerlat-top tool performs on-threshold
actions, even though no threshold was hit. This is reproduced even if no
threshold is set at all:
$ rtla timerlat top -q -c 0 --on-threshold shell,command='echo BAD'
BAD
Timer Latency
...
The bug is due to incorrect logic in timerlat_top_bpf_main_loop().
The loop uses timerlat_bpf_wait(), the return values of which are:
- > 0 (number of ringbuffer entries): at least 1 CPU hit threshold
- = 0: time out
- < 0: wait was interrupted by a signal
Commit 3138df6f0cd0 ("rtla/timerlat: Exit top main loop on any non-zero
wait_retval") changed the condition for "threshold hit" from
"wait_reval == 1" (exactly 1 CPU hit threshold) to "wait_retval != 0",
to fix a race where multiple CPUs hit the threshold at the same time.
That also made it incorrectly include a signal (< 0), coming from either
duration expired (SIGALRM) or user interrupt (SIGINT).
Check for wait_retval greater than zero in the if condition to cover all
return values correctly.
Fixes: 3138df6f0cd0 ("rtla/timerlat: Exit top main loop on any non-zero wait_retval")
Reported-by: Attila Fazekas <afazekas@redhat.com>
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
tools/tracing/rtla/src/timerlat_top.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index 18e1071a2e242..6206a0a565ad3 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -536,7 +536,7 @@ timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
if (!params->quiet)
timerlat_print_stats(tool);
- if (wait_retval != 0) {
+ if (wait_retval > 0) {
/* Stopping requested by tracer */
retval = common_threshold_handler(tool);
if (retval)
--
2.55.0
^ permalink raw reply related
* Re: [PATCH] ufs: core: tracing: Do not dereference pointers in TP_printk()
From: Steven Rostedt @ 2026-07-13 14:13 UTC (permalink / raw)
To: Martin K. Petersen
Cc: LKML, Linux Trace Kernel, linux-scsi, Masami Hiramatsu,
Mathieu Desnoyers, Alim Akhtar, Avri Altman, Bart Van Assche,
James Bottomley, Peter Wang
In-Reply-To: <178390967056.3399387.17096580780727607387.b4-ty@oracle.com>
On Sun, 12 Jul 2026 22:32:37 -0400
"Martin K. Petersen" <martin.petersen@oracle.com> wrote:
> Applied to 7.2/scsi-fixes, thanks!
>
> [1/1] ufs: core: tracing: Do not dereference pointers in TP_printk()
> https://git.kernel.org/mkp/scsi/c/46aea2c64e11
Ah, I should have waited for you. I had the reviewed by from Peter and
Bart, and thought they were the maintainers of the code so I pushed it to
Linus already.
I see now they are listed as reviewers. Sorry about that.
-- Steve
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Steven Rostedt @ 2026-07-13 14:23 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
minki.jang, hajun.sung
In-Reply-To: <20260713121659.6c8549331474337511c4f442@kernel.org>
On Mon, 13 Jul 2026 12:16:59 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > I also kept this around in case it was needed. Looks like it may be needed.
>
> Hmm, it's not clear whether this is the result of a scheduling storm
> caused by kernel threads. But maybe worth to try.
I don't see what else it could be.
>
> Jeongho, can you try this patch? Or/and you maybe also try using
> mlock the LMKD process memory which is passed to trace_marker.
Yes, Jeongho, can you please test this patch and also see if mlocking the
memory used by trace_marker would help too?
-- Steve
^ permalink raw reply
* Re: [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Mark Rutland @ 2026-07-13 16:47 UTC (permalink / raw)
To: Jose Fernandez (Anthropic)
Cc: Steven Rostedt, Masami Hiramatsu, Catalin Marinas, Will Deacon,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Puranjay Mohan, Clayton Craft, Xu Kuohai, linux-kernel,
linux-trace-kernel, linux-arm-kernel, llvm, bpf, Florent Revest,
Xu Kuohai, Ben Cressey
In-Reply-To: <alD7Rr659mGVznQp@linux.dev>
On Fri, Jul 10, 2026 at 02:07:01PM +0000, Jose Fernandez (Anthropic) wrote:
> Hi all,
>
> Just a gentle ping on this series. It has Reviewed-by from Puranjay
> Mohan, Acked-by from Xu Kuohai on both patches, and Tested-by from
> Clayton Craft and Nathan Chancellor, with no outstanding comments since
> Jun 11.
>
> Could you please consider picking this up for the next cycle?
Sorry, this is on my queue to look at, but I have been busy elsewhere
over the last few weeks.
I'll get to it shortly.
Mark.
^ permalink raw reply
* Re: [PATCH v2 05/33] mm/rmap: update mm/interval_tree.c comments
From: Vlastimil Babka (SUSE) @ 2026-07-13 17:48 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-5-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> Update the file comment to clarify that both file-backed and anonymous
> interval trees are provided, referencing the relevant data types for
> clarity.
>
> Also add comments to indicate which parts of the file apply to each.
>
> While we're here, convert the VM_BUG_ON_VMA() to VM_WARN_ON_ONCE_VMA().
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> mm/interval_tree.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/mm/interval_tree.c b/mm/interval_tree.c
> index 344d1f5946c7..2d50bc6228c4 100644
> --- a/mm/interval_tree.c
> +++ b/mm/interval_tree.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> - * mm/interval_tree.c - interval tree for mapping->i_mmap
> + * mm/interval_tree.c - interval tree for address_space->i_mmap and
> + * anon_vma->rb_root
> *
> * Copyright (C) 2012, Michel Lespinasse <walken@google.com>
> */
> @@ -10,6 +11,8 @@
> #include <linux/rmap.h>
> #include <linux/interval_tree_generic.h>
>
> +/* File-backed interval tree (address_space->i_mmap) */
> +
> INTERVAL_TREE_DEFINE(struct vm_area_struct, shared.rb,
> unsigned long, shared.rb_subtree_last,
> vma_start_pgoff, vma_last_pgoff, /* empty */, vma_interval_tree)
> @@ -23,7 +26,7 @@ void vma_interval_tree_insert_after(struct vm_area_struct *node,
> struct vm_area_struct *parent;
> unsigned long last = vma_last_pgoff(node);
>
> - VM_BUG_ON_VMA(vma_start_pgoff(node) != vma_start_pgoff(prev), node);
> + VM_WARN_ON_ONCE_VMA(vma_start_pgoff(node) != vma_start_pgoff(prev), node);
>
> if (!prev->shared.rb.rb_right) {
> parent = prev;
> @@ -48,6 +51,8 @@ void vma_interval_tree_insert_after(struct vm_area_struct *node,
> &vma_interval_tree_augment);
> }
>
> +/* Anonymous interval tree (anon_vma->rb_root) */
> +
> static inline unsigned long avc_start_pgoff(struct anon_vma_chain *avc)
> {
> return vma_start_pgoff(avc->vma);
>
^ permalink raw reply
* Re: [PATCH v2 06/33] mm/rmap: parameterise vma_interval_tree_*() by address_space
From: Vlastimil Babka (SUSE) @ 2026-07-13 17:56 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-6-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> The file-backed mapping interval tree functions vma_interval_tree_*()
> accept a raw rb_root_cached pointer to determine the tree in which they are
> operating.
>
> However, in each case, this is always associated with an address_space data
> type.
>
> So simply pass a pointer to that instead to simplify the code, and more
> clearly differentiate between these operations and those concerning
> anonymous mappings.
>
> While we're here, make the generated interval tree functions static as they
> do not need to be used externally (any previously existing external users
> have now been removed).
>
> We also rename VMA parameters from 'node' to 'vma' as calling this a node
> is simply confusing, update the input index types to pgoff_t since they
> reference page offsets and rename the parameters to pgoff_start and
> pgoff_last.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Nice.
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 07/33] mm/rmap: elide unnecessary static inline's in interval_tree.c
From: Vlastimil Babka (SUSE) @ 2026-07-13 17:59 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-7-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> It's not necessary to declare these functions static inline as they are
> contained within a single compilation unit.
>
> This makes the anonymous interval tree code consistent with the newly
> updated file-backed interval tree code.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 08/33] mm/rmap: rename vma_interval_tree_*() to mapping_rmap_tree_*()
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:03 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-8-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> The family of vma_interval_tree_() functions manipulate the
> address_space (which, of course, is generally referred to as 'mapping')
> reverse mapping, but are named the 'VMA' interval tree.
>
> VMAs may be mapped by an anon_vma, an address_space, or both. Therefore
> calling the mapping interval tree a 'VMA' interval tree is rather
> confusing.
>
> This is also inconsistent with the anon_vma_interval_tree_*() functions
> which explicitly reference the rmap object to which they pertain.
>
> Rename the vma_interval_tree_*() functions to mapping_rmap_tree_*() to
> correct this.
>
> We will rename the anon rmap functions similarly in a subsequent patch.
>
> No functional change intended.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 09/33] mm/rmap: parameterise anon_vma_interval_tree_*() by anon_vma
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:04 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-9-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> Similar to what we did with mapping_rmap_tree*(), let's declare
> anon_vma_interval_tree*() in terms of anon_vma rather than rb_root_cached.
>
> In each case the rb tree referenced is &anon_vma->rb_root, so just pass
> anon_vma and the functions can figure this out themselves.
>
> Also update the VMA userland tests to reflect the change.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 10/33] mm/rmap: rename anon_vma_interval_tree_*() params and use pgoff_t
From: Vlastimil Babka (SUSE) @ 2026-07-13 18:05 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-10-2a5aa403d977@kernel.org>
On 7/10/26 22:16, Lorenzo Stoakes wrote:
> Rename parameters used by anon_vma_interval_tree_*() functions: 'node' to
> 'avc', 'start/first' to 'pgoff_start', and 'last' to 'pgoff_last' to make
> clear what is being passed.
>
> Also, express page offsets in terms of pgoff_t to be consistent.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ 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