Thread (17 messages) 17 messages, 2 authors, 2025-07-19

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;
 	}
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help