Re: [PATCH 1/5] tracing: probe: Allocate traceprobe_parse_context from heap
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-07-18 16:58:29
Also in:
lkml
On Fri, 18 Jul 2025 20:34:08 +0900 "Masami Hiramatsu (Google)" [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index 854e5668f5ee..7bc4c84464e4 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h@@ -10,6 +10,7 @@ * Author: Srikar Dronamraju */ +#include <linux/cleanup.h> #include <linux/seq_file.h> #include <linux/slab.h> #include <linux/smp.h>
Nit, but let's keep the "upside-down x-mas tree" format: #include <linux/seq_file.h> #include <linux/cleanup.h> #include <linux/slab.h> #include <linux/smp.h>
quoted hunk ↗ jump to hunk
@@ -438,6 +439,14 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg); * this MUST be called for clean up the context and return a resource. */ void traceprobe_finish_parse(struct traceprobe_parse_context *ctx); +static inline void traceprobe_free_parse_ctx(struct traceprobe_parse_context *ctx) +{ + traceprobe_finish_parse(ctx); + kfree(ctx); +} + +DEFINE_FREE(traceprobe_parse_context, struct traceprobe_parse_context *, + if (!IS_ERR_OR_NULL(_T)) traceprobe_free_parse_ctx(_T))
ctx will either be allocated or NULL, I think the above could be: if (_T) traceprobe_free_parse_ctx(_T))
quoted hunk ↗ jump to hunk
extern int traceprobe_split_symbol_offset(char *symbol, long *offset); int traceprobe_parse_event_name(const char **pevent, const char **pgroup,diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index f95a2c3d5b1b..1fd479718d03 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c@@ -537,6 +537,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu) */ static int __trace_uprobe_create(int argc, const char **argv) { + struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL; struct trace_uprobe *tu; const char *event = NULL, *group = UPROBE_EVENT_SYSTEM; char *arg, *filename, *rctr, *rctr_end, *tmp;@@ -693,15 +694,17 @@ static int __trace_uprobe_create(int argc, const char **argv) tu->path = path; tu->filename = filename; + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) { + ret = -ENOMEM; + goto error; + } + ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER; + /* parse arguments */ for (i = 0; i < argc; i++) { - struct traceprobe_parse_context ctx = { - .flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER, - }; - trace_probe_log_set_index(i + 2); - ret = traceprobe_parse_probe_arg(&tu->tp, i, argv[i], &ctx); - traceprobe_finish_parse(&ctx); + ret = traceprobe_parse_probe_arg(&tu->tp, i, argv[i], ctx);
Doesn't this change the semantics a bit? Before this change, traceprobe_finish_parse(&ctx) is called for every iteration of the loop. Now we only do it when it exits the function. -- Steve
if (ret) goto error; }