linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] tracing: Cleanup uprobe/eprobe events
@ 2025-08-13 14:29 Masami Hiramatsu (Google)
  2025-08-13 14:29 ` [PATCH 1/4] tracing: probes: Use __free() for trace_probe_log Masami Hiramatsu (Google)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2025-08-13 14:29 UTC (permalink / raw)
  To: Steven Rostedt, Mathieu Desnoyers
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel

Hi,

This series is just for cleanup ugly gotos in uprobe/eprobe events
using __free(). Also, this allocates the traceprobe_parse_context
outside of the argument parsing loop for efficiency.

Thank you,

---

Masami Hiramatsu (Google) (4):
      tracing: probes: Use __free() for trace_probe_log
      tracing: eprobe: Cleanup eprobe event using __free()
      tracing: uprobes: Cleanup __trace_uprobe_create() with __free()
      tracing: uprobe: eprobes: Allocate traceprobe_parse_context per probe


 kernel/trace/trace_eprobe.c |  108 ++++++++++++++++++-------------------------
 kernel/trace/trace_probe.c  |    3 +
 kernel/trace/trace_probe.h  |    4 +-
 kernel/trace/trace_uprobe.c |   82 +++++++++++++--------------------
 4 files changed, 81 insertions(+), 116 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] tracing: probes: Use __free() for trace_probe_log
  2025-08-13 14:29 [PATCH 0/4] tracing: Cleanup uprobe/eprobe events Masami Hiramatsu (Google)
@ 2025-08-13 14:29 ` Masami Hiramatsu (Google)
  2025-08-13 14:29 ` [PATCH 2/4] tracing: eprobe: Cleanup eprobe event using __free() Masami Hiramatsu (Google)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2025-08-13 14:29 UTC (permalink / raw)
  To: Steven Rostedt, Mathieu Desnoyers
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Use __free() for trace_probe_log_clear() to cleanup error log interface.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_eprobe.c |    5 ++---
 kernel/trace/trace_probe.c  |    3 ++-
 kernel/trace/trace_probe.h  |    4 +++-
 kernel/trace/trace_uprobe.c |    6 ++----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index a1d402124836..aaba765d54cf 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -874,6 +874,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	 * Fetch args (no space):
 	 *  <name>=$<field>[:TYPE]
 	 */
+	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = EPROBE_EVENT_SYSTEM;
 	const char *sys_event = NULL, *sys_name = NULL;
 	struct trace_event_call *event_call;
@@ -887,7 +888,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	if (argc < 2 || argv[0][0] != 'e')
 		return -ECANCELED;
 
-	trace_probe_log_init("event_probe", argc, argv);
+	trlog = trace_probe_log_init("event_probe", argc, argv);
 
 	event = strchr(&argv[0][1], ':');
 	if (event) {
@@ -987,7 +988,6 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 			goto error;
 		}
 	}
-	trace_probe_log_clear();
 	return ret;
 
 mem_error:
@@ -996,7 +996,6 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 parse_error:
 	ret = -EINVAL;
 error:
-	trace_probe_log_clear();
 	trace_event_probe_cleanup(ep);
 	return ret;
 }
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 5cbdc423afeb..5b92376a58fc 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -156,7 +156,7 @@ static const struct fetch_type *find_fetch_type(const char *type, unsigned long
 static struct trace_probe_log trace_probe_log;
 extern struct mutex dyn_event_ops_mutex;
 
-void trace_probe_log_init(const char *subsystem, int argc, const char **argv)
+const char *trace_probe_log_init(const char *subsystem, int argc, const char **argv)
 {
 	lockdep_assert_held(&dyn_event_ops_mutex);
 
@@ -164,6 +164,7 @@ void trace_probe_log_init(const char *subsystem, int argc, const char **argv)
 	trace_probe_log.argc = argc;
 	trace_probe_log.argv = argv;
 	trace_probe_log.index = 0;
+	return subsystem;
 }
 
 void trace_probe_log_clear(void)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 842383fbc03b..76bf2dee8071 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -573,11 +573,13 @@ struct trace_probe_log {
 	int		index;
 };
 
-void trace_probe_log_init(const char *subsystem, int argc, const char **argv);
+const char *trace_probe_log_init(const char *subsystem, int argc, const char **argv);
 void trace_probe_log_set_index(int index);
 void trace_probe_log_clear(void);
 void __trace_probe_log_err(int offset, int err);
 
+DEFINE_FREE(trace_probe_log_clear, const char *, if (_T) trace_probe_log_clear())
+
 #define trace_probe_log_err(offs, err)	\
 	__trace_probe_log_err(offs, TP_ERR_##err)
 
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 8b0bcc0d8f41..722316b3dc16 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -539,6 +539,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
  */
 static int __trace_uprobe_create(int argc, const char **argv)
 {
+	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
 	char *arg, *filename, *rctr, *rctr_end, *tmp;
 	unsigned long offset, ref_ctr_offset;
@@ -565,7 +566,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	if (argc < 2)
 		return -ECANCELED;
 
-	trace_probe_log_init("trace_uprobe", argc, argv);
+	trlog = trace_probe_log_init("trace_uprobe", argc, argv);
 
 	if (argc - 2 > MAX_TRACE_ARGS) {
 		trace_probe_log_set_index(2);
@@ -597,7 +598,6 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	if (ret) {
 		trace_probe_log_err(0, FILE_NOT_FOUND);
 		kfree(filename);
-		trace_probe_log_clear();
 		return ret;
 	}
 	if (!d_is_reg(path.dentry)) {
@@ -728,14 +728,12 @@ static int __trace_uprobe_create(int argc, const char **argv)
 error:
 	free_trace_uprobe(tu);
 out:
-	trace_probe_log_clear();
 	return ret;
 
 fail_mem:
 	ret = -ENOMEM;
 
 fail_address_parse:
-	trace_probe_log_clear();
 	path_put(&path);
 	kfree(filename);
 


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] tracing: eprobe: Cleanup eprobe event using __free()
  2025-08-13 14:29 [PATCH 0/4] tracing: Cleanup uprobe/eprobe events Masami Hiramatsu (Google)
  2025-08-13 14:29 ` [PATCH 1/4] tracing: probes: Use __free() for trace_probe_log Masami Hiramatsu (Google)
@ 2025-08-13 14:29 ` Masami Hiramatsu (Google)
  2025-08-13 14:30 ` [PATCH 3/4] tracing: uprobes: Cleanup __trace_uprobe_create() with __free() Masami Hiramatsu (Google)
  2025-08-13 14:30 ` [PATCH 4/4] tracing: uprobe: eprobes: Allocate traceprobe_parse_context per probe Masami Hiramatsu (Google)
  3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2025-08-13 14:29 UTC (permalink / raw)
  To: Steven Rostedt, Mathieu Desnoyers
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Use __free(trace_event_probe_cleanup) to remove unneeded gotos and
cleanup the last part of trace_eprobe_parse_filter().

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_eprobe.c |   71 ++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 41 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index aaba765d54cf..f7a1ff509d7e 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -61,6 +61,9 @@ static void trace_event_probe_cleanup(struct trace_eprobe *ep)
 	kfree(ep);
 }
 
+DEFINE_FREE(trace_event_probe_cleanup, struct trace_eprobe *,
+		if (!IS_ERR_OR_NULL(_T)) trace_event_probe_cleanup(_T))
+
 static struct trace_eprobe *to_trace_eprobe(struct dyn_event *ev)
 {
 	return container_of(ev, struct trace_eprobe, devent);
@@ -197,10 +200,10 @@ static struct trace_eprobe *alloc_event_probe(const char *group,
 					      struct trace_event_call *event,
 					      int nargs)
 {
-	struct trace_eprobe *ep;
+	struct trace_eprobe *ep __free(trace_event_probe_cleanup) = NULL;
 	const char *event_name;
 	const char *sys_name;
-	int ret = -ENOMEM;
+	int ret;
 
 	if (!event)
 		return ERR_PTR(-ENODEV);
@@ -211,25 +214,22 @@ static struct trace_eprobe *alloc_event_probe(const char *group,
 	ep = kzalloc(struct_size(ep, tp.args, nargs), GFP_KERNEL);
 	if (!ep) {
 		trace_event_put_ref(event);
-		goto error;
+		return ERR_PTR(-ENOMEM);
 	}
 	ep->event = event;
 	ep->event_name = kstrdup(event_name, GFP_KERNEL);
 	if (!ep->event_name)
-		goto error;
+		return ERR_PTR(-ENOMEM);
 	ep->event_system = kstrdup(sys_name, GFP_KERNEL);
 	if (!ep->event_system)
-		goto error;
+		return ERR_PTR(-ENOMEM);
 
 	ret = trace_probe_init(&ep->tp, this_event, group, false, nargs);
 	if (ret < 0)
-		goto error;
+		return ERR_PTR(ret);
 
 	dyn_event_init(&ep->devent, &eprobe_dyn_event_ops);
-	return ep;
-error:
-	trace_event_probe_cleanup(ep);
-	return ERR_PTR(ret);
+	return_ptr(ep);
 }
 
 static int eprobe_event_define_fields(struct trace_event_call *event_call)
@@ -856,13 +856,10 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
 	ret = create_event_filter(top_trace_array(), ep->event, ep->filter_str,
 				  true, &dummy);
 	free_event_filter(dummy);
-	if (ret)
-		goto error;
-
-	return 0;
-error:
-	kfree(ep->filter_str);
-	ep->filter_str = NULL;
+	if (ret) {
+		kfree(ep->filter_str);
+		ep->filter_str = NULL;
+	}
 	return ret;
 }
 
@@ -874,6 +871,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	 * Fetch args (no space):
 	 *  <name>=$<field>[:TYPE]
 	 */
+	struct trace_eprobe *ep __free(trace_event_probe_cleanup) = NULL;
 	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = EPROBE_EVENT_SYSTEM;
 	const char *sys_event = NULL, *sys_name = NULL;
@@ -881,7 +879,6 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	char *buf1 __free(kfree) = NULL;
 	char *buf2 __free(kfree) = NULL;
 	char *gbuf __free(kfree) = NULL;
-	struct trace_eprobe *ep = NULL;
 	int ret = 0, filter_idx = 0;
 	int i, filter_cnt;
 
@@ -894,12 +891,12 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	if (event) {
 		gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
 		if (!gbuf)
-			goto mem_error;
+			return -ENOMEM;
 		event++;
 		ret = traceprobe_parse_event_name(&event, &group, gbuf,
 						  event - argv[0]);
 		if (ret)
-			goto parse_error;
+			return -EINVAL;
 	}
 
 	trace_probe_log_set_index(1);
@@ -907,18 +904,18 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 
 	buf2 = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
 	if (!buf2)
-		goto mem_error;
+		return -ENOMEM;
 
 	ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0);
 	if (ret || !sys_event || !sys_name) {
 		trace_probe_log_err(0, NO_EVENT_INFO);
-		goto parse_error;
+		return -EINVAL;
 	}
 
 	if (!event) {
 		buf1 = kstrdup(sys_event, GFP_KERNEL);
 		if (!buf1)
-			goto mem_error;
+			return -ENOMEM;
 		event = buf1;
 	}
 
@@ -934,8 +931,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	if (argc - 2 > MAX_TRACE_ARGS) {
 		trace_probe_log_set_index(2);
 		trace_probe_log_err(0, TOO_MANY_ARGS);
-		ret = -E2BIG;
-		goto error;
+		return -E2BIG;
 	}
 
 	scoped_guard(mutex, &event_mutex) {
@@ -949,15 +945,14 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 			trace_probe_log_err(0, BAD_ATTACH_EVENT);
 		/* This must return -ENOMEM or missing event, else there is a bug */
 		WARN_ON_ONCE(ret != -ENOMEM && ret != -ENODEV);
-		ep = NULL;
-		goto error;
+		return ret;
 	}
 
 	if (filter_idx) {
 		trace_probe_log_set_index(filter_idx);
 		ret = trace_eprobe_parse_filter(ep, filter_cnt, argv + filter_idx);
 		if (ret)
-			goto parse_error;
+			return -EINVAL;
 	} else
 		ep->filter_str = NULL;
 
@@ -967,11 +962,12 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 		trace_probe_log_set_index(i + 2);
 		ret = trace_eprobe_tp_update_arg(ep, argv, i);
 		if (ret)
-			goto error;
+			return ret;
 	}
 	ret = traceprobe_set_print_fmt(&ep->tp, PROBE_PRINT_EVENT);
 	if (ret < 0)
-		goto error;
+		return ret;
+
 	init_trace_eprobe_call(ep);
 	scoped_guard(mutex, &event_mutex) {
 		ret = trace_probe_register_event_call(&ep->tp);
@@ -980,24 +976,17 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 				trace_probe_log_set_index(0);
 				trace_probe_log_err(0, EVENT_EXIST);
 			}
-			goto error;
+			return ret;
 		}
 		ret = dyn_event_add(&ep->devent, &ep->tp.event->call);
 		if (ret < 0) {
 			trace_probe_unregister_event_call(&ep->tp);
-			goto error;
+			return ret;
 		}
+		/* To avoid freeing registered eprobe event, clear ep. */
+		ep = NULL;
 	}
 	return ret;
-
-mem_error:
-	ret = -ENOMEM;
-	goto error;
-parse_error:
-	ret = -EINVAL;
-error:
-	trace_event_probe_cleanup(ep);
-	return ret;
 }
 
 /*


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] tracing: uprobes: Cleanup __trace_uprobe_create() with __free()
  2025-08-13 14:29 [PATCH 0/4] tracing: Cleanup uprobe/eprobe events Masami Hiramatsu (Google)
  2025-08-13 14:29 ` [PATCH 1/4] tracing: probes: Use __free() for trace_probe_log Masami Hiramatsu (Google)
  2025-08-13 14:29 ` [PATCH 2/4] tracing: eprobe: Cleanup eprobe event using __free() Masami Hiramatsu (Google)
@ 2025-08-13 14:30 ` Masami Hiramatsu (Google)
  2025-08-13 14:30 ` [PATCH 4/4] tracing: uprobe: eprobes: Allocate traceprobe_parse_context per probe Masami Hiramatsu (Google)
  3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2025-08-13 14:30 UTC (permalink / raw)
  To: Steven Rostedt, Mathieu Desnoyers
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Use __free() to cleanup ugly gotos in __trace_uprobe_create().

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_uprobe.c |   68 ++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 42 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 722316b3dc16..9c628dab3dc6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -533,22 +533,25 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
 	return ret;
 }
 
+DEFINE_FREE(free_trace_uprobe, struct trace_uprobe *, if (_T) free_trace_uprobe(_T))
+
 /*
  * Argument syntax:
  *  - Add uprobe: p|r[:[GRP/][EVENT]] PATH:OFFSET[%return][(REF)] [FETCHARGS]
  */
 static int __trace_uprobe_create(int argc, const char **argv)
 {
+	struct trace_uprobe *tu __free(free_trace_uprobe) = NULL;
 	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
-	char *arg, *filename, *rctr, *rctr_end, *tmp;
+	struct path path __free(path_put) = {};
 	unsigned long offset, ref_ctr_offset;
+	char *filename __free(kfree) = NULL;
+	char *arg, *rctr, *rctr_end, *tmp;
 	char *gbuf __free(kfree) = NULL;
 	char *buf __free(kfree) = NULL;
 	enum probe_print_type ptype;
-	struct trace_uprobe *tu;
 	bool is_return = false;
-	struct path path;
 	int i, ret;
 
 	ref_ctr_offset = 0;
@@ -586,10 +589,8 @@ static int __trace_uprobe_create(int argc, const char **argv)
 
 	/* Find the last occurrence, in case the path contains ':' too. */
 	arg = strrchr(filename, ':');
-	if (!arg || !isdigit(arg[1])) {
-		kfree(filename);
+	if (!arg || !isdigit(arg[1]))
 		return -ECANCELED;
-	}
 
 	trace_probe_log_set_index(1);	/* filename is the 2nd argument */
 
@@ -597,13 +598,11 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	ret = kern_path(filename, LOOKUP_FOLLOW, &path);
 	if (ret) {
 		trace_probe_log_err(0, FILE_NOT_FOUND);
-		kfree(filename);
 		return ret;
 	}
 	if (!d_is_reg(path.dentry)) {
 		trace_probe_log_err(0, NO_REGULAR_FILE);
-		ret = -EINVAL;
-		goto fail_address_parse;
+		return -EINVAL;
 	}
 
 	/* Parse reference counter offset if specified. */
@@ -611,16 +610,14 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	if (rctr) {
 		rctr_end = strchr(rctr, ')');
 		if (!rctr_end) {
-			ret = -EINVAL;
 			rctr_end = rctr + strlen(rctr);
 			trace_probe_log_err(rctr_end - filename,
 					    REFCNT_OPEN_BRACE);
-			goto fail_address_parse;
+			return -EINVAL;
 		} else if (rctr_end[1] != '\0') {
-			ret = -EINVAL;
 			trace_probe_log_err(rctr_end + 1 - filename,
 					    BAD_REFCNT_SUFFIX);
-			goto fail_address_parse;
+			return -EINVAL;
 		}
 
 		*rctr++ = '\0';
@@ -628,7 +625,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 		ret = kstrtoul(rctr, 0, &ref_ctr_offset);
 		if (ret) {
 			trace_probe_log_err(rctr - filename, BAD_REFCNT);
-			goto fail_address_parse;
+			return ret;
 		}
 	}
 
@@ -640,8 +637,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 			is_return = true;
 		} else {
 			trace_probe_log_err(tmp - filename, BAD_ADDR_SUFFIX);
-			ret = -EINVAL;
-			goto fail_address_parse;
+			return -EINVAL;
 		}
 	}
 
@@ -649,7 +645,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	ret = kstrtoul(arg, 0, &offset);
 	if (ret) {
 		trace_probe_log_err(arg - filename, BAD_UPROBE_OFFS);
-		goto fail_address_parse;
+		return ret;
 	}
 
 	/* setup a probe */
@@ -657,12 +653,12 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	if (event) {
 		gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
 		if (!gbuf)
-			goto fail_mem;
+			return -ENOMEM;
 
 		ret = traceprobe_parse_event_name(&event, &group, gbuf,
 						  event - argv[0]);
 		if (ret)
-			goto fail_address_parse;
+			return ret;
 	}
 
 	if (!event) {
@@ -671,7 +667,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 
 		tail = kstrdup(kbasename(filename), GFP_KERNEL);
 		if (!tail)
-			goto fail_mem;
+			return -ENOMEM;
 
 		ptr = strpbrk(tail, ".-_");
 		if (ptr)
@@ -679,7 +675,7 @@ static int __trace_uprobe_create(int argc, const char **argv)
 
 		buf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
 		if (!buf)
-			goto fail_mem;
+			return -ENOMEM;
 		snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_0x%lx", 'p', tail, offset);
 		event = buf;
 		kfree(tail);
@@ -693,49 +689,37 @@ static int __trace_uprobe_create(int argc, const char **argv)
 		ret = PTR_ERR(tu);
 		/* This must return -ENOMEM otherwise there is a bug */
 		WARN_ON_ONCE(ret != -ENOMEM);
-		goto fail_address_parse;
+		return ret;
 	}
 	tu->offset = offset;
 	tu->ref_ctr_offset = ref_ctr_offset;
 	tu->path = path;
-	tu->filename = filename;
+	/* Clear @path so that it will not freed by path_put() */
+	memset(&path, 0, sizeof(path));
+	tu->filename = no_free_ptr(filename);
 
 	/* parse arguments */
 	for (i = 0; i < argc; i++) {
 		struct traceprobe_parse_context *ctx __free(traceprobe_parse_context)
 			= kzalloc(sizeof(*ctx), GFP_KERNEL);
 
-		if (!ctx) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (!ctx)
+			return -ENOMEM;
 		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);
 		if (ret)
-			goto error;
+			return ret;
 	}
 
 	ptype = is_ret_probe(tu) ? PROBE_PRINT_RETURN : PROBE_PRINT_NORMAL;
 	ret = traceprobe_set_print_fmt(&tu->tp, ptype);
 	if (ret < 0)
-		goto error;
+		return ret;
 
 	ret = register_trace_uprobe(tu);
 	if (!ret)
-		goto out;
-
-error:
-	free_trace_uprobe(tu);
-out:
-	return ret;
-
-fail_mem:
-	ret = -ENOMEM;
-
-fail_address_parse:
-	path_put(&path);
-	kfree(filename);
+		tu = NULL;
 
 	return ret;
 }


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] tracing: uprobe: eprobes: Allocate traceprobe_parse_context per probe
  2025-08-13 14:29 [PATCH 0/4] tracing: Cleanup uprobe/eprobe events Masami Hiramatsu (Google)
                   ` (2 preceding siblings ...)
  2025-08-13 14:30 ` [PATCH 3/4] tracing: uprobes: Cleanup __trace_uprobe_create() with __free() Masami Hiramatsu (Google)
@ 2025-08-13 14:30 ` Masami Hiramatsu (Google)
  3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2025-08-13 14:30 UTC (permalink / raw)
  To: Steven Rostedt, Mathieu Desnoyers
  Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Since traceprobe_parse_context is reusable among a probe arguments,
it is more efficient to allocate it outside of the loop for parsing
probe argument as kprobe and fprobe events do.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_eprobe.c |   32 ++++++++++++--------------------
 kernel/trace/trace_uprobe.c |   12 ++++++------
 2 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index f7a1ff509d7e..d58d8702a327 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -801,25 +801,6 @@ find_and_get_event(const char *system, const char *event_name)
 	return NULL;
 }
 
-static int trace_eprobe_tp_update_arg(struct trace_eprobe *ep, const char *argv[], int i)
-{
-	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
-	int ret;
-
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
-	if (!ctx)
-		return -ENOMEM;
-	ctx->event = ep->event;
-	ctx->flags = TPARG_FL_KERNEL | TPARG_FL_TEVENT;
-
-	ret = traceprobe_parse_probe_arg(&ep->tp, i, argv[i], ctx);
-	/* Handle symbols "@" */
-	if (!ret)
-		ret = traceprobe_update_arg(&ep->tp.args[i]);
-
-	return ret;
-}
-
 static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const char *argv[])
 {
 	struct event_filter *dummy = NULL;
@@ -871,6 +852,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	 * Fetch args (no space):
 	 *  <name>=$<field>[:TYPE]
 	 */
+	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
 	struct trace_eprobe *ep __free(trace_event_probe_cleanup) = NULL;
 	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = EPROBE_EVENT_SYSTEM;
@@ -956,11 +938,21 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	} else
 		ep->filter_str = NULL;
 
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->event = ep->event;
+	ctx->flags = TPARG_FL_KERNEL | TPARG_FL_TEVENT;
+
 	argc -= 2; argv += 2;
 	/* parse arguments */
 	for (i = 0; i < argc; i++) {
 		trace_probe_log_set_index(i + 2);
-		ret = trace_eprobe_tp_update_arg(ep, argv, i);
+
+		ret = traceprobe_parse_probe_arg(&ep->tp, i, argv[i], ctx);
+		/* Handle symbols "@" */
+		if (!ret)
+			ret = traceprobe_update_arg(&ep->tp.args[i]);
 		if (ret)
 			return ret;
 	}
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 9c628dab3dc6..8f9b95cee786 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -541,6 +541,7 @@ DEFINE_FREE(free_trace_uprobe, struct trace_uprobe *, if (_T) free_trace_uprobe(
  */
 static int __trace_uprobe_create(int argc, const char **argv)
 {
+	struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
 	struct trace_uprobe *tu __free(free_trace_uprobe) = NULL;
 	const char *trlog __free(trace_probe_log_clear) = NULL;
 	const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
@@ -698,14 +699,13 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	memset(&path, 0, sizeof(path));
 	tu->filename = no_free_ptr(filename);
 
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->flags = (is_return ? TPARG_FL_RETURN : 0) | TPARG_FL_USER;
+
 	/* parse arguments */
 	for (i = 0; i < argc; i++) {
-		struct traceprobe_parse_context *ctx __free(traceprobe_parse_context)
-			= kzalloc(sizeof(*ctx), GFP_KERNEL);
-
-		if (!ctx)
-			return -ENOMEM;
-		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);
 		if (ret)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-13 14:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-13 14:29 [PATCH 0/4] tracing: Cleanup uprobe/eprobe events Masami Hiramatsu (Google)
2025-08-13 14:29 ` [PATCH 1/4] tracing: probes: Use __free() for trace_probe_log Masami Hiramatsu (Google)
2025-08-13 14:29 ` [PATCH 2/4] tracing: eprobe: Cleanup eprobe event using __free() Masami Hiramatsu (Google)
2025-08-13 14:30 ` [PATCH 3/4] tracing: uprobes: Cleanup __trace_uprobe_create() with __free() Masami Hiramatsu (Google)
2025-08-13 14:30 ` [PATCH 4/4] tracing: uprobe: eprobes: Allocate traceprobe_parse_context per probe Masami Hiramatsu (Google)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).