Linux Trace Kernel
 help / color / mirror / Atom feed
* 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

* [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

* [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 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 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 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, &param);
+		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, &param);
-			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 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, &param);
+		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, &param);
-			if (ret)
-				goto inval;
+	ret = kstrtoul(arg + len, 10, &param);
+	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, &param);
-		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 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

* 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 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: [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

* Re: [BUG] tracing: Too many tries to read user space
From: Masami Hiramatsu @ 2026-07-13  3:16 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
	minki.jang, hajun.sung
In-Reply-To: <20260710083357.49e05ff6@gandalf.local.home>

On Fri, 10 Jul 2026 08:33:57 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 10 Jul 2026 12:22:31 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Hm, in my view, this warning indicates that the circuit breaker has
> > triggered correctly, so that is not a bug. Under the heavy memory
> > pressure and low-memory situation, the page can be reclaimed soon
> > after it is copied.
> 
> So you are saying that every time the copy_from_user() is executed, the
> page is reclaimed? And this causes a schedule?
> 
> Now, I did have a version that used sched_switch and only incremented the
> counter when a non-kernel thread was scheduled in. Then the test would
> check if the counter increased by 2 or more. As an increase by 1 meant that
> only kernel threads scheduled in which would not corrupt the buffer. The 1
> increment was the current task scheduling back.
> 
> This is based on that work (I'm glad I save old versions in my git tree :-)
> 
> Funny, the comments were from the original change I did back in August of
> 2025, which mentions kernel threads scheduling in to handle the fault.
> 
> 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.

Jeongho, can you try this patch? Or/and you maybe also try using
mlock the LMKD process memory which is passed to trace_marker.

Thank you, 

> 
> -- Steve
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 18710c190c92..19354fe2fca1 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -53,6 +53,8 @@
>  #include <linux/io.h> /* vmap_page_range() */
>  #include <linux/fs_context.h>
>  
> +#include <trace/events/sched.h>
> +
>  #include <asm/setup.h> /* COMMAND_LINE_SIZE */
>  
>  #include "trace.h"
> @@ -5984,6 +5986,32 @@ struct trace_user_buf {
>  static DEFINE_MUTEX(trace_user_buffer_mutex);
>  static struct trace_user_buf_info *trace_user_buffer;
>  
> +static DEFINE_PER_CPU(unsigned long, sched_switch_cnt);
> +
> +/*
> + * The per CPU buffer trace_user_buffer is written to optimstically.
> + * The counter sched_switch_cnt is taken, preemption is enabled,
> + * the copying of the user space memory is placed into the trace_user_buffer,
> + * Preeption is re-enabled and the count is read again. If the count is greater
> + * than one from its previous reading, it means that another user space
> + * task scheduled in and the buffer is unreliable for use.
> + */
> +static void
> +probe_sched_switch(void *ignore, bool preempt,
> +		   struct task_struct *prev, struct task_struct *next,
> +		   unsigned int prev_state)
> +{
> +	/*
> +	 * The buffer can only be corrupted by another user space task.
> +	 * Ignore kernel tasks that may be scheduled in order to process
> +	 * the faulting memory.
> +	 */
> +	if (!is_user_task(next))
> +		return;
> +
> +	this_cpu_inc(sched_switch_cnt);
> +}
> +
>  /**
>   * trace_user_fault_destroy - free up allocated memory of a trace user buffer
>   * @tinfo: The descriptor to free up
> @@ -6003,6 +6031,8 @@ void trace_user_fault_destroy(struct trace_user_buf_info *tinfo)
>  		kfree(buf);
>  	}
>  	free_percpu(tinfo->tbuf);
> +
> +	unregister_trace_sched_switch(probe_sched_switch, NULL);
>  }
>  
>  static int user_fault_buffer_enable(struct trace_user_buf_info *tinfo, size_t size)
> @@ -6053,11 +6083,17 @@ static int user_buffer_init(struct trace_user_buf_info **tinfo, size_t size)
>  
>  	lockdep_assert_held(&trace_user_buffer_mutex);
>  
> +	ret = register_trace_sched_switch(probe_sched_switch, NULL);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!*tinfo) {
>  		alloc = true;
>  		*tinfo = kzalloc_obj(**tinfo);
> -		if (!*tinfo)
> +		if (!*tinfo) {
> +			unregister_trace_sched_switch(probe_sched_switch, NULL);
>  			return -ENOMEM;
> +		}
>  	}
>  
>  	ret = user_fault_buffer_enable(*tinfo, size);
> @@ -6241,7 +6277,7 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
>  			return NULL;
>  
>  		/* Read the current CPU context switch counter */
> -		cnt = nr_context_switches_cpu(cpu);
> +		cnt = this_cpu_read(sched_switch_cnt);
>  
>  		/*
>  		 * Preemption is going to be enabled, but this task must
> @@ -6272,12 +6308,19 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
>  			return NULL;
>  
>  		/*
> -		 * Preemption is disabled again, now check the per CPU context
> -		 * switch counter. If it doesn't match, then another user space
> -		 * process may have schedule in and corrupted our buffer. In that
> -		 * case the copying must be retried.
> +		 * Preemption is disabled again, now check the sched_switch_cnt.
> +		 * If it increased by two or more, then another user space process
> +		 * may have schedule in and corrupted our buffer. In that case
> +		 * the copying must be retried.
> +		 *
> +		 * Note, if this task was scheduled out and only kernel threads
> +		 * were scheduled in (maybe to process the fault), then the
> +		 * counter would increment again when this task scheduled in.
> +		 * If this task scheduled out and another user task scheduled
> +		 * in, this task would still need to be scheduled back in and
> +		 * the counter would increment by at least two.
>  		 */
> -	} while (nr_context_switches_cpu(cpu) != cnt);
> +	} while (this_cpu_read(sched_switch_cnt) > cnt + 1);
>  
>  	return buffer;
>  }


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

^ permalink raw reply

* Re: [BUG] tracing: Too many tries to read user space
From: Masami Hiramatsu @ 2026-07-13  2:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
	minki.jang, hajun.sung
In-Reply-To: <20260710074605.33802554@gandalf.local.home>

On Fri, 10 Jul 2026 07:46:05 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 10 Jul 2026 12:22:31 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > However, this seems a bit strange that we only checks the CPU-wide context
> > switching in the loop. Instead, can we introduce a per-cpu sequence counter
> > to per-cpu buffer, and check it? 
> 
> I originally tried this but found a situation that it fails:
> 
>   tbuf->sequence = 0;
> 
> 	Task 1				Task 2
> 	------				------
> 
>    tbuf->sequence++;
>    seq = tbuf->sequence; (seq = 1)
> 
>    preempt_enable();
> 
>    [schedule] ---------------------->
> 
> 				  tbuf->sequence++;
> 				  seq = tbuf->sequence; (seq = 2);
> 
> 				  preempt_enable();
> 
> 				  copy_from_user(buffer);
> 
> 	     <--------------------[schedule]
> 
>    copy_from_user(buffer);
> 
>    *** BUFFER NOW CORRUPTED ***
> 
>    [schedule] ---------------------->
> 
> 				  preempt_disable();
> 
> 				} while (tubf->sequence != seq); // tbuf->sequence == seq !!!!
> 
> 
> This is why we use a CPU wide counter.

Ah, indeed. As similar to seqlock, maybe we need to increment sequence
number after the process so that we can find the buffer is used some
other process. Or, just use a mutex to wait. But anyway, that will be
make it slower?

BTW, it is just an idea, but can we split fast path (per-cpu buffer)
and slow path (allocate an intermadiate buffer) for the page which
can cause page fault ?

Thanks,

> 
> -- Steve
>   
> 


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

^ permalink raw reply

* Re: [PATCH] ufs: core: tracing: Do not dereference pointers in TP_printk()
From: Martin K. Petersen @ 2026-07-13  2:32 UTC (permalink / raw)
  To: LKML, Linux Trace Kernel, linux-scsi, Steven Rostedt
  Cc: Martin K . Petersen, Masami Hiramatsu, Mathieu Desnoyers,
	Alim Akhtar, Avri Altman, Bart Van Assche, James Bottomley,
	Peter Wang
In-Reply-To: <20260630185412.283c26c5@gandalf.local.home>

On Tue, 30 Jun 2026 18:54:12 -0400, Steven Rostedt wrote:

> The trace events in drivers/ufs/core/ufs_trace.h were converted to take a
> pointer to the hba structure as an argument for the tracepoint and then in
> TP_printk() the printing of the dev_name from the ring buffer was
> converted to using the dev dereferenced pointer from the hba saved
> pointer.
> 
> This is not allowed as the TP_printk() is executed at the time the trace
> event is read from /sys/kernel/tracing/trace file. That can happen
> literally, seconds, minutes, hours, weeks, days, or even months later!
> There is no guarantee that the hba pointer will still exist by the time it
> is dereferenced when the "trace" file is read.
> 
> [...]

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

-- 
Martin K. Petersen

^ permalink raw reply

* Re: [PATCH] tracing: fprobe: Remove __packed from generic __fprobe_header
From: Masami Hiramatsu @ 2026-07-13  2:23 UTC (permalink / raw)
  To: Markus Schneider-Pargmann
  Cc: Steven Rostedt, Mathieu Desnoyers, Heiko Carstens, linux-kernel,
	linux-trace-kernel
In-Reply-To: <DJU105LESLBJ.1O3UZRY325T2M@baylibre.com>

On Thu, 09 Jul 2026 14:06:32 +0200
"Markus Schneider-Pargmann" <msp@baylibre.com> wrote:

> Hi Masami,
> 
> On Mon Jun 22, 2026 at 6:56 AM CEST, Masami Hiramatsu wrote:
> > On Wed, 10 Jun 2026 11:20:24 +0200
> > "Markus Schneider-Pargmann" <msp@baylibre.com> wrote:
> >
> >> Hi Masami,
> >> 
> >> On Wed Jun 10, 2026 at 10:17 AM CEST, Masami Hiramatsu wrote:
> >> > Hi Markus,
> >> >
> >> > Thanks for ping me.
> >> >
> >> > On Tue, 28 Apr 2026 10:30:29 +0200
> >> > "Markus Schneider-Pargmann (The Capable Hub)" <msp@baylibre.com> wrote:
> >> >
> >> >> fp pointer and unsigned long have the same size on all relevant
> >> >> architectures that build Linux. Furthermore this struct is only used in
> >> >> architectures that do not set ARCH_DEFINE_ENCODE_FPROBE_HEADER which is
> >> >> set only for 64bit architectures (apart from LoongArch).
> >> >> 
> >> >> Both fields are aligned on these architectures so the struct with
> >> >> __packed and without it are the same.
> >> >> 
> >> >> Remove the __packed as it is unnecessary.
> >> >> 
> >> >> Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")
> >> >
> >> > NOTE: This is not a Fix, but just cleanup or minor update. Or, you have
> >> > any problem with this __packed attribute?
> >> 
> >> Thanks, yes it is not fixing a bug, I can remove this.
> >> 
> >> >
> >> > Unless there is no problem (or any concern), I would like to keep this
> >> > as it is.
> >> 
> >> There is currently no problem with __packed in the upstream kernel. I
> >> just thought this would be a good cleanup to remove the unnecessary
> >> attribute. I am working on CHERI architectures where pointers have
> >> capabilities. __packed breaks these capability tags and therefore
> >> doesn't work on CHERI. When looking into why this struct has a __packed
> >> attribute I didn't see a reason, so I thought this would be a good patch
> >> for upstream as well even though CHERI is not yet relevant for upstream
> >> linux.
> >
> > Hi Markus,
> >
> > Ah, OK. CHERI makes pointers to non-long value. Are you sure
> > removing __packed makes fprobe working with CHERI? If so,
> > please describe it. Then I can pick it.
> 
> Sorry for the late response.
> 
> No, this patch alone doesn't make fprobe work with CHERI. __packed is
> problematic for CHERI because it sets the alignment to 1 which forces
> the compiler to do unaligned access operations even if the
> struct/pointer is aligned. Unaligned operations do not keep the CHERI
> capabilities. Without __packed the compiler knows everything is aligned
> and can use aligned operations and keep the CHERI capabilities. CHERI
> hardware does not support unaligned pointer capability access.
> 
> On its own it doesn't help CHERI, there are more patches that are more
> CHERI specific that are needed for fprobe to work on CHERI. I just
> thought it would be a great standalone patch for upstream.

OK, anyway, I would like to pick your patch but without fixes tag.
It may make no differences, so should be treated as a cleanup patch.
Let me pick it to probes/for-next.

Thank you,

> 
> Best
> Markus


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

^ permalink raw reply

* Re: [PATCH 4/5] tracing/probes: Extend max length of argument string
From: Masami Hiramatsu @ 2026-07-13  2:16 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Steven Rostedt, Shuah Khan, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, linux-kselftest
In-Reply-To: <178386880215.3174487.982264935084717241.stgit@devnote2>

On Mon, 13 Jul 2026 00:06:42 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> 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 256.
> 
> 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>

Sashiko asked to extend other macros, 
> Does this code also need to increase MAX_BTF_ARGS_LEN and MAX_DENTRY_ARGS_LEN?

I don't agree this. MAX_BTF_ARGS_LEN is only for expanding "$args" into
each parameter names. The other arguments are kept on the same memory.

MAX_DENTRY_ARGS_LEN is already 256, so I think if that is too long,
it should be rejected.
(But MAX_ARGSTR_LEN should be 255.)

BTW, both macro names are a bit complicated because those are just
buffer size, not max length of something.

Thank you,

> ---
>  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..e6aee800a7d9 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		256
> +#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)
> 


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

^ permalink raw reply

* [PATCH] tracing: Defer trigger private data frees past the grace period
From: David Carlier @ 2026-07-12 16:10 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-trace-kernel, linux-kernel,
	David Carlier

Commit 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
event_trigger_data") made trigger_data_free() defer the kfree() of the
event_trigger_data to a kthread that runs tracepoint_synchronize_unregister()
before freeing. The .free callbacks that own satellite data kept freeing it
synchronously right after calling trigger_data_free(), relying on the
synchronize that used to run inline.

With that synchronize now deferred, event_hist_trigger_free(),
event_enable_trigger_free() and event_hist_trigger_named_free() free
hist_data, enable_data and cmd_ops while a concurrent tracepoint handler can
still dereference them through the list_del_rcu()'d trigger, causing a
use-after-free.

Add an optional free_private() callback to event_trigger_data, invoked by
the free kthread after the grace period, and move the satellite frees into
it. The event_mutex-requiring bookkeeping (remove_hist_vars(),
unregister_field_var_hists()) stays synchronous; only the handler-visible
memory free is deferred.

Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/trace/trace.h                |  1 +
 kernel/trace/trace_events_hist.c    | 19 +++++++++++++------
 kernel/trace/trace_events_trigger.c | 18 +++++++++++++++---
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..f04598337060 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1941,6 +1941,7 @@ struct event_trigger_data {
 	struct list_head		named_list;
 	struct event_trigger_data	*named_data;
 	struct llist_node		llist;
+	void				(*free_private)(struct event_trigger_data *data);
 };
 
 /* Avoid typos */
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 82ce492ab268..bc696e4bd695 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6335,6 +6335,16 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
 	}
 }
 
+static void hist_trigger_free_private(struct event_trigger_data *data)
+{
+	destroy_hist_data(data->private_data);
+}
+
+static void hist_trigger_named_free_private(struct event_trigger_data *data)
+{
+	kfree(data->cmd_ops);
+}
+
 static void event_hist_trigger_free(struct event_trigger_data *data)
 {
 	struct hist_trigger_data *hist_data = data->private_data;
@@ -6347,13 +6357,12 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
 		if (data->name)
 			del_named_trigger(data);
 
-		trigger_data_free(data);
-
 		remove_hist_vars(hist_data);
 
 		unregister_field_var_hists(hist_data);
 
-		destroy_hist_data(hist_data);
+		data->free_private = hist_trigger_free_private;
+		trigger_data_free(data);
 	}
 	free_hist_pad();
 }
@@ -6384,11 +6393,9 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
 
 	data->ref--;
 	if (!data->ref) {
-		struct event_command *cmd_ops = data->cmd_ops;
-
 		del_named_trigger(data);
+		data->free_private = hist_trigger_named_free_private;
 		trigger_data_free(data);
-		kfree(cmd_ops);
 	}
 }
 
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..27c54da041b7 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -38,6 +38,13 @@ static void trigger_create_kthread_locked(void)
 	}
 }
 
+static void trigger_data_free_one(struct event_trigger_data * data)
+{
+	if (data->free_private)
+		data->free_private(data);
+	kfree(data);
+}
+
 static void trigger_data_free_queued_locked(void)
 {
 	struct event_trigger_data *data, *tmp;
@@ -52,7 +59,7 @@ static void trigger_data_free_queued_locked(void)
 	tracepoint_synchronize_unregister();
 
 	llist_for_each_entry_safe(data, tmp, llnodes, llist)
-		kfree(data);
+		trigger_data_free_one(data);
 }
 
 /* Bulk garbage collection of event_trigger_data elements */
@@ -75,7 +82,7 @@ static int trigger_kthread_fn(void *ignore)
 		tracepoint_synchronize_unregister();
 
 		llist_for_each_entry_safe(data, tmp, llnodes, llist)
-			kfree(data);
+			trigger_data_free_one(data);
 	}
 
 	return 0;
@@ -1717,6 +1724,11 @@ int event_enable_trigger_print(struct seq_file *m,
 	return 0;
 }
 
+static void enable_trigger_free_private(struct event_trigger_data *data)
+{
+	kfree(data->private_data);
+}
+
 void event_enable_trigger_free(struct event_trigger_data *data)
 {
 	struct enable_trigger_data *enable_data = data->private_data;
@@ -1729,8 +1741,8 @@ void event_enable_trigger_free(struct event_trigger_data *data)
 		/* Remove the SOFT_MODE flag */
 		trace_event_enable_disable(enable_data->file, 0, 1);
 		trace_event_put_ref(enable_data->file->event_call);
+		data->free_private = enable_trigger_free_private;
 		trigger_data_free(data);
-		kfree(enable_data);
 	}
 }
 
-- 
2.53.0


^ permalink raw reply related

* [PATCH 5/5] tracing/probes: Eliminate recursion in parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest
In-Reply-To: <178386876526.3174487.5142283230157728964.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 tiny local state stack.

Define struct parse_state to hold the parsing contexts for nested
dereference and typecast operations. Modify parse_probe_arg_deref(),
parse_this_cpu(), and handle_typecast() to return parsing state details
rather than calling parse_probe_arg() recursively. parse_probe_arg()
then processes nested nodes by pushing onto the parse_state array,
parsing the leaf node, and unwinding/evaluating the array entries
in LIFO order.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_probe.c                         |  337 ++++++++++++--------
 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, 243 insertions(+), 137 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 2b8cc4cf5109..345fa06f6980 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;
@@ -1609,9 +1552,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;
@@ -1638,7 +1579,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,
@@ -1670,11 +1626,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) {
@@ -1696,11 +1647,59 @@ 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) {
+			if (arg[0] == '(')
+				trace_probe_log_err(ctx->offset, TOO_MANY_NESTED);
+			else
+				trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
+			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;
@@ -1715,26 +1714,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 */
@@ -1995,12 +2080,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 e6aee800a7d9..f0958707db8e 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 4. */
+#define TRACEPROBE_MAX_NESTED_LEVEL 4
+
+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..a48ad751b615 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(+0(+0(+0(@0))))))))))))))'	# TOO_MANY_OPS?
 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)file->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..d871ba0b7110 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(+0(+0(+0(@0))))))))))))))'	# TOO_MANY_OPS?
 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..c93d5f2612fd 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(+0(+0(+0(@0))))))))))))))'	# TOO_MANY_OPS?
 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)file->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

* [PATCH 4/5] tracing/probes: Extend max length of argument string
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest
In-Reply-To: <178386876526.3174487.5142283230157728964.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 256.

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>
---
 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..e6aee800a7d9 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		256
+#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 3/5] tracing/probes: Sort ERRORS list in trace_probe.h alphabetically
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest
In-Reply-To: <178386876526.3174487.5142283230157728964.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 2/5] tracing/probes: Refactor parse_probe_arg()
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest
In-Reply-To: <178386876526.3174487.5142283230157728964.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 b762336ebf03..2b8cc4cf5109 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1527,158 +1527,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, &param);
+		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, &param);
-			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 1/5] tracing/probes: Refactor parse_probe_vars()
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest
In-Reply-To: <178386876526.3174487.5142283230157728964.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>
---
 kernel/trace/trace_probe.c |  248 +++++++++++++++++++++++++++-----------------
 1 file changed, 151 insertions(+), 97 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 18c212122344..b762336ebf03 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1319,67 +1319,147 @@ 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,
+				  int *err)
 {
 	struct fetch_insn *code = *pcode;
-	int err = TP_ERR_BAD_VAR;
-	char *arg = orig_arg + 1;
+
+	if (!(ctx->flags & TPARG_FL_RETURN)) {
+		*err = TP_ERR_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,
+				 int *err)
+{
 	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 (!(ctx->flags & TPARG_FL_KERNEL) ||
-		    !IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS)) {
-			code->op = FETCH_OP_RETVAL;
-			return 0;
+	if (isdigit(arg[len])) {
+		ret = kstrtoul(arg + len, 10, &param);
+		if (ret)
+			return ret;
+
+		if ((ctx->flags & TPARG_FL_KERNEL) &&
+		    param > PARAM_MAX_STACK) {
+			*err = TP_ERR_BAD_STACK_NUM;
+			return -EINVAL;
 		}
+		code->op = FETCH_OP_STACK;
+		code->param = (unsigned int)param;
+		return 0;
+	}
+
+	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,
+				   int *err)
+{
+	struct fetch_insn *code = *pcode;
+
+	/* $current is only supported by kernel probe. */
+	if (!(ctx->flags & TPARG_FL_KERNEL)) {
+		*err = TP_ERR_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')
+		return -EINVAL;
+
+	code->op = FETCH_OP_CURRENT;
+	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,
+			       int *err)
+{
+	unsigned long param;
+	int ret;
+
+	ret = kstrtoul(arg + len, 10, &param);
+	if (ret)
+		return ret;
+
+	if (!param || param > PARAM_MAX_STACK) {
+		*err = TP_ERR_BAD_ARG_NUM;
+		return -EINVAL;
 	}
+	param--; /* argN starts from 1, but internal arg[N] starts from 0 */
 
-	len = str_has_prefix(arg, "stack");
-	if (len) {
+	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;
 
-		if (arg[len] == '\0') {
-			code->op = FETCH_OP_STACKP;
-			return 0;
-		}
+		code->op = FETCH_OP_EDATA;
+		code->offset = ret;
+	} else {
+		*err = TP_ERR_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,
+			       int *err)
+{
+	*err = TP_ERR_BAD_VAR;
+	return -EINVAL;
+}
+#endif
 
-		if (isdigit(arg[len])) {
-			ret = kstrtoul(arg + len, 10, &param);
-			if (ret)
-				goto inval;
+/* 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 err = TP_ERR_BAD_VAR;
+	int ret = -EINVAL;
+	int len;
 
-			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;
+	if (ctx->flags & TPARG_FL_TEVENT) {
+		ret = parse_trace_event(arg, code, ctx);
+		if (!ret)
 			return 0;
-		}
-		goto inval;
 	}
 
 	if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
@@ -1387,65 +1467,39 @@ static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
 		return 0;
 	}
 
-	/* $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);
-
-		if (*arg != '\0')
-			goto inval;
+	/* eprobe only support event fields or '$comm'. */
+	if (ctx->flags & TPARG_FL_TEVENT)
+		goto end;
 
-		code->op = FETCH_OP_CURRENT;
-		return 0;
+	if (str_has_prefix(arg, "retval")) {
+		ret = parse_probe_var_retval(orig_arg, arg, pcode, end, ctx, &err);
+		goto end;
 	}
 
-#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
-	len = str_has_prefix(arg, "arg");
+	len = str_has_prefix(arg, "stack");
 	if (len) {
-		ret = kstrtoul(arg + len, 10, &param);
-		if (ret)
-			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 */
+		ret = parse_probe_var_stack(arg, len, code, ctx, &err);
+		goto end;
+	}
 
-		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;
+	/* $current returns the address of the current task_struct. */
+	if (str_has_prefix(arg, "current")) {
+		ret = parse_probe_var_current(orig_arg, arg, pcode, end, ctx, &err);
+		goto end;
+	}
 
-			code->op = FETCH_OP_EDATA;
-			code->offset = ret;
-		} else {
-			err = TP_ERR_NOFENTRY_ARGS;
-			goto inval;
-		}
-		return 0;
+	len = str_has_prefix(arg, "arg");
+	if (len) {
+		ret = parse_probe_var_arg(arg, len, code, ctx, &err);
+		goto end;
 	}
-#endif
 
-inval:
-	__trace_probe_log_err(ctx->offset, err);
-	return -EINVAL;
+end:
+	if (ret < 0) {
+		__trace_probe_log_err(ctx->offset, err);
+		return ret;
+	}
+	return 0;
 }
 
 static int str_to_immediate(char *str, unsigned long *imm)


^ permalink raw reply related

* [PATCH 0/5] tracing/probes: Clean up and refactor argument parser
From: Masami Hiramatsu (Google) @ 2026-07-12 15:06 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	linux-kselftest

Hi,

This series refactors and cleans up the fetcharg parser in the
trace_probe. 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.

Previously, parse_probe_arg() recursively called itself when parsing
nested dereferences and typecasts. Since the kernel stack is limited,
deep recursion is undesirable. The [5/5] eliminates the recursion
by converting the nested argument parsing into a loop-based
implementation utilizing a parse state stack. To prepare it, the
[1/5] and [2/5] cleanup (decompose) the parser functions.

Additionally, this series increases the maximum nesting depth from 3
to 4, extends the argument length limit from 63 to 256 and updates the
associated ftrace syntax error selftests.

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 256 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 4 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                         |  843 ++++++++++++--------
 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, 635 insertions(+), 426 deletions(-)

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

^ permalink raw reply

* Re: [PATCH v2 1/2] soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
From: Bjorn Andersson @ 2026-07-11 15:54 UTC (permalink / raw)
  To: Praveen Talari
  Cc: Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Mark Brown, linux-kernel, linux-arm-msm,
	linux-trace-kernel, linux-spi
In-Reply-To: <20260711-add-tracepoints-for-se-reg-dump-v2-1-ca1e9ba62359@oss.qualcomm.com>

On Sat, Jul 11, 2026 at 12:18:41AM +0530, Praveen Talari wrote:
> Add a new trace event header for the Qualcomm GENI Serial Engine (SE)
> framework providing a geni_se_regs tracepoint. This tracepoint
> captures a comprehensive snapshot of the GENI SE hardware state in a
> single trace record, making it possible to correlate register values at
> a precise point in time without multiple sequential reads.

As stated in [a], start your commit message by describing the problem
you're trying to solve, the reader shouldn't have to read the whole
commit message to get an idea about why the change is needed.

[a] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

> 
> The trace event records the following register groups:
> 
>  - Main/secondary command and IRQ status (M_CMD0, S_CMD0, M/S_IRQ_STATUS)
>  - Engine status, IOS, and command control/error registers
>  - TX/RX FIFO status and watermark registers (including RFR watermark)
>  - M/S GP length registers
>  - DMA TX/RX IRQ, enable, length, pointer, attribute, and burst registers
>  - DMA interface enable, general config, QSB trans config, and debug
>  - M/S IRQ enable, GSI event enable, and top-level SE IRQ enable
>  - Serial master/slave clock config, general config, output control,
>    clock control RO, FIFO interface disable, and FW multilock MSA
>  - Clock select register
> 
> Having all these registers captured atomically in a single ftrace record
> allows drivers built on top of the GENI SE framework (serial, SPI, I2C)
> to invoke this tracepoint on error paths and reconstruct the full engine
> state during post-mortem analysis without instrumenting each driver
> separately.

I'm not rejecting the patch, but isn't this exactly why we have e.g.
DCC? To allow grabbing a set of registers in adverse events - without
having to write debug code like this in each driver?

Regards,
Bjorn

> 
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
>  include/linux/soc/qcom/geni-se.h    |  38 +++++++++
>  include/trace/events/qcom_geni_se.h | 157 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 195 insertions(+)
> 
> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index c5e6ab85df09..8c08c1917374 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -81,13 +81,16 @@ struct geni_se {
>  };
>  
>  /* Common SE registers */
> +#define GENI_GENERAL_CFG		0x10
>  #define GENI_FORCE_DEFAULT_REG		0x20
>  #define GENI_OUTPUT_CTRL		0x24
>  #define SE_GENI_STATUS			0x40
>  #define GENI_SER_M_CLK_CFG		0x48
>  #define GENI_SER_S_CLK_CFG		0x4c
> +#define GENI_CLK_CTRL_RO		0x60
>  #define GENI_IF_DISABLE_RO		0x64
>  #define GENI_FW_REVISION_RO		0x68
> +#define GENI_FW_MULTILOCK_MSA_RO	0x74
>  #define SE_GENI_CLK_SEL			0x7c
>  #define SE_GENI_CFG_SEQ_START		0x84
>  #define SE_GENI_DMA_MODE_EN		0x258
> @@ -98,6 +101,8 @@ struct geni_se {
>  #define SE_GENI_M_IRQ_CLEAR		0x618
>  #define SE_GENI_M_IRQ_EN_SET		0x61c
>  #define SE_GENI_M_IRQ_EN_CLEAR		0x620
> +#define M_CMD_ERR_STATUS		0x624
> +#define M_FW_ERR_STATUS			0x628
>  #define SE_GENI_S_CMD0			0x630
>  #define SE_GENI_S_CMD_CTRL_REG		0x634
>  #define SE_GENI_S_IRQ_STATUS		0x640
> @@ -115,15 +120,42 @@ struct geni_se {
>  #define SE_GENI_IOS			0x908
>  #define SE_GENI_M_GP_LENGTH		0x910
>  #define SE_GENI_S_GP_LENGTH		0x914
> +/* TX DMA registers */
> +#define SE_DMA_TX_PTR_L			0xc30
> +#define SE_DMA_TX_PTR_H			0xc34
> +#define SE_DMA_TX_ATTR			0xc38
> +#define SE_DMA_TX_LEN			0xc3c
>  #define SE_DMA_TX_IRQ_STAT		0xc40
>  #define SE_DMA_TX_IRQ_CLR		0xc44
> +#define SE_DMA_TX_IRQ_EN		0xc48
> +#define SE_DMA_TX_IRQ_EN_SET		0xc4c
> +#define SE_DMA_TX_IRQ_EN_CLR		0xc50
> +#define SE_DMA_TX_LEN_IN		0xc54
>  #define SE_DMA_TX_FSM_RST		0xc58
> +#define SE_DMA_TX_MAX_BURST		0xc5c
> +/* RX DMA registers */
> +#define SE_DMA_RX_PTR_L			0xd30
> +#define SE_DMA_RX_PTR_H			0xd34
> +#define SE_DMA_RX_ATTR			0xd38
> +#define SE_DMA_RX_LEN			0xd3c
>  #define SE_DMA_RX_IRQ_STAT		0xd40
>  #define SE_DMA_RX_IRQ_CLR		0xd44
> +#define SE_DMA_RX_IRQ_EN		0xd48
> +#define SE_DMA_RX_IRQ_EN_SET		0xd4c
> +#define SE_DMA_RX_IRQ_EN_CLR		0xd50
>  #define SE_DMA_RX_LEN_IN		0xd54
>  #define SE_DMA_RX_FSM_RST		0xd58
> +#define SE_DMA_RX_MAX_BURST		0xd5c
> +/* DMA general / debug registers */
> +#define SE_GSI_EVENT_EN			0xe18
> +#define SE_IRQ_EN			0xe1c
> +#define DMA_IF_EN_RO			0xe20
>  #define SE_HW_PARAM_0			0xe24
>  #define SE_HW_PARAM_1			0xe28
> +#define DMA_GENERAL_CFG			0xe30
> +#define SE_DMA_QSB_TRANS_CFG		0xe38
> +#define SE_DMA_DEBUG_REG0		0xe40
> +#define SE_DMA_IF_EN			0x2004
>  
>  /* GENI_FORCE_DEFAULT_REG fields */
>  #define FORCE_DEFAULT	BIT(0)
> @@ -269,6 +301,12 @@ struct geni_se {
>  #define RX_GENI_GP_IRQ_EXT		GENMASK(13, 12)
>  #define RX_GENI_CANCEL_IRQ		BIT(14)
>  
> +/* SE_DMA_DEBUG_REG0 fields */
> +#define DMA_TX_ACTIVE			BIT(0)
> +#define DMA_RX_ACTIVE			BIT(1)
> +#define DMA_TX_STATE			GENMASK(7, 4)
> +#define DMA_RX_STATE			GENMASK(11, 8)
> +
>  /* SE_HW_PARAM_0 fields */
>  #define TX_FIFO_WIDTH_MSK		GENMASK(29, 24)
>  #define TX_FIFO_WIDTH_SHFT		24
> diff --git a/include/trace/events/qcom_geni_se.h b/include/trace/events/qcom_geni_se.h
> new file mode 100644
> index 000000000000..4a6e1ba2d147
> --- /dev/null
> +++ b/include/trace/events/qcom_geni_se.h
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM qcom_geni_se
> +
> +#if !defined(_TRACE_QCOM_GENI_SE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_QCOM_GENI_SE_H
> +
> +#include <linux/io.h>
> +#include <linux/tracepoint.h>
> +#include <linux/soc/qcom/geni-se.h>
> +
> +TRACE_EVENT(geni_se_regs,
> +	    TP_PROTO(struct geni_se *se),
> +
> +	    TP_ARGS(se),
> +
> +	    TP_STRUCT__entry(__string(geni_se_name,		dev_name(se->dev))
> +		__field(u32,	geni_se_m_cmd0)
> +		__field(u32,	geni_se_m_irq_status)
> +		__field(u32,	geni_se_s_cmd0)
> +		__field(u32,	geni_se_s_irq_status)
> +		__field(u32,	geni_se_status)
> +		__field(u32,	geni_se_ios)
> +		__field(u32,	geni_se_m_cmd_ctrl)
> +		__field(u32,	geni_se_m_cmd_err)
> +		__field(u32,	geni_se_m_fw_err)
> +		__field(u32,	geni_se_tx_fifo_status)
> +		__field(u32,	geni_se_rx_fifo_status)
> +		__field(u32,	geni_se_tx_watermark)
> +		__field(u32,	geni_se_rx_watermark)
> +		__field(u32,	geni_se_rx_watermark_rfr)
> +		__field(u32,	geni_se_m_gp_length)
> +		__field(u32,	geni_se_s_gp_length)
> +		__field(u32,	geni_se_dma_tx_irq)
> +		__field(u32,	geni_se_dma_rx_irq)
> +		__field(u32,	geni_se_dma_tx_irq_en)
> +		__field(u32,	geni_se_dma_rx_irq_en)
> +		__field(u32,	geni_se_dma_rx_len)
> +		__field(u32,	geni_se_dma_rx_len_in)
> +		__field(u32,	geni_se_dma_tx_len)
> +		__field(u32,	geni_se_dma_tx_len_in)
> +		__field(u32,	geni_se_dma_tx_ptr_l)
> +		__field(u32,	geni_se_dma_tx_ptr_h)
> +		__field(u32,	geni_se_dma_rx_ptr_l)
> +		__field(u32,	geni_se_dma_rx_ptr_h)
> +		__field(u32,	geni_se_dma_tx_attr)
> +		__field(u32,	geni_se_dma_tx_max_burst)
> +		__field(u32,	geni_se_dma_rx_attr)
> +		__field(u32,	geni_se_dma_rx_max_burst)
> +		__field(u32,	geni_se_dma_if_en)
> +		__field(u32,	geni_se_dma_if_en_ro)
> +		__field(u32,	geni_se_dma_general_cfg)
> +		__field(u32,	geni_se_dma_qsb_trans_cfg)
> +		__field(u32,	geni_se_dma_dbg)
> +		__field(u32,	geni_se_m_irq_en)
> +		__field(u32,	geni_se_s_irq_en)
> +		__field(u32,	geni_se_gsi_event_en)
> +		__field(u32,	geni_se_irq_en)
> +		__field(u32,	geni_se_ser_m_clk_cfg)
> +		__field(u32,	geni_se_ser_s_clk_cfg)
> +		__field(u32,	geni_se_general_cfg)
> +		__field(u32,	geni_se_output_ctrl)
> +		__field(u32,	geni_se_clk_ctrl_ro)
> +		__field(u32,	geni_se_fifo_if_disable)
> +		__field(u32,	geni_se_fw_multilock_msa)
> +		__field(u32,	geni_se_clk_sel)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(geni_se_name);
> +		__entry->geni_se_m_cmd0		  = readl(se->base + SE_GENI_M_CMD0);
> +		__entry->geni_se_m_irq_status	  = readl(se->base + SE_GENI_M_IRQ_STATUS);
> +		__entry->geni_se_s_cmd0		  = readl(se->base + SE_GENI_S_CMD0);
> +		__entry->geni_se_s_irq_status	  = readl(se->base + SE_GENI_S_IRQ_STATUS);
> +		__entry->geni_se_status		  = readl(se->base + SE_GENI_STATUS);
> +		__entry->geni_se_ios		  = readl(se->base + SE_GENI_IOS);
> +		__entry->geni_se_m_cmd_ctrl	  = readl(se->base + SE_GENI_M_CMD_CTRL_REG);
> +		__entry->geni_se_m_cmd_err	  = readl(se->base + M_CMD_ERR_STATUS);
> +		__entry->geni_se_m_fw_err	  = readl(se->base + M_FW_ERR_STATUS);
> +		__entry->geni_se_tx_fifo_status	  = readl(se->base + SE_GENI_TX_FIFO_STATUS);
> +		__entry->geni_se_rx_fifo_status	  = readl(se->base + SE_GENI_RX_FIFO_STATUS);
> +		__entry->geni_se_tx_watermark	  = readl(se->base + SE_GENI_TX_WATERMARK_REG);
> +		__entry->geni_se_rx_watermark	  = readl(se->base + SE_GENI_RX_WATERMARK_REG);
> +		__entry->geni_se_rx_watermark_rfr = readl(se->base + SE_GENI_RX_RFR_WATERMARK_REG);
> +		__entry->geni_se_m_gp_length	  = readl(se->base + SE_GENI_M_GP_LENGTH);
> +		__entry->geni_se_s_gp_length	  = readl(se->base + SE_GENI_S_GP_LENGTH);
> +		__entry->geni_se_dma_tx_irq	  = readl(se->base + SE_DMA_TX_IRQ_STAT);
> +		__entry->geni_se_dma_rx_irq	  = readl(se->base + SE_DMA_RX_IRQ_STAT);
> +		__entry->geni_se_dma_tx_irq_en	  = readl(se->base + SE_DMA_TX_IRQ_EN);
> +		__entry->geni_se_dma_rx_irq_en	  = readl(se->base + SE_DMA_RX_IRQ_EN);
> +		__entry->geni_se_dma_rx_len	  = readl(se->base + SE_DMA_RX_LEN);
> +		__entry->geni_se_dma_rx_len_in	  = readl(se->base + SE_DMA_RX_LEN_IN);
> +		__entry->geni_se_dma_tx_len	  = readl(se->base + SE_DMA_TX_LEN);
> +		__entry->geni_se_dma_tx_len_in	  = readl(se->base + SE_DMA_TX_LEN_IN);
> +		__entry->geni_se_dma_tx_ptr_l	  = readl(se->base + SE_DMA_TX_PTR_L);
> +		__entry->geni_se_dma_tx_ptr_h	  = readl(se->base + SE_DMA_TX_PTR_H);
> +		__entry->geni_se_dma_rx_ptr_l	  = readl(se->base + SE_DMA_RX_PTR_L);
> +		__entry->geni_se_dma_rx_ptr_h	  = readl(se->base + SE_DMA_RX_PTR_H);
> +		__entry->geni_se_dma_tx_attr	  = readl(se->base + SE_DMA_TX_ATTR);
> +		__entry->geni_se_dma_tx_max_burst = readl(se->base + SE_DMA_TX_MAX_BURST);
> +		__entry->geni_se_dma_rx_attr	  = readl(se->base + SE_DMA_RX_ATTR);
> +		__entry->geni_se_dma_rx_max_burst = readl(se->base + SE_DMA_RX_MAX_BURST);
> +		__entry->geni_se_dma_if_en	  = readl(se->base + SE_DMA_IF_EN);
> +		__entry->geni_se_dma_if_en_ro	  = readl(se->base + DMA_IF_EN_RO);
> +		__entry->geni_se_dma_general_cfg  = readl(se->base + DMA_GENERAL_CFG);
> +		__entry->geni_se_dma_qsb_trans_cfg = readl(se->base + SE_DMA_QSB_TRANS_CFG);
> +		__entry->geni_se_dma_dbg	  = readl(se->base + SE_DMA_DEBUG_REG0);
> +		__entry->geni_se_m_irq_en	  = readl(se->base + SE_GENI_M_IRQ_EN);
> +		__entry->geni_se_s_irq_en	  = readl(se->base + SE_GENI_S_IRQ_EN);
> +		__entry->geni_se_gsi_event_en	  = readl(se->base + SE_GSI_EVENT_EN);
> +		__entry->geni_se_irq_en		  = readl(se->base + SE_IRQ_EN);
> +		__entry->geni_se_ser_m_clk_cfg	  = readl(se->base + GENI_SER_M_CLK_CFG);
> +		__entry->geni_se_ser_s_clk_cfg	  = readl(se->base + GENI_SER_S_CLK_CFG);
> +		__entry->geni_se_general_cfg	  = readl(se->base + GENI_GENERAL_CFG);
> +		__entry->geni_se_output_ctrl	  = readl(se->base + GENI_OUTPUT_CTRL);
> +		__entry->geni_se_clk_ctrl_ro	  = readl(se->base + GENI_CLK_CTRL_RO);
> +		__entry->geni_se_fifo_if_disable  = readl(se->base + GENI_IF_DISABLE_RO);
> +		__entry->geni_se_fw_multilock_msa = readl(se->base + GENI_FW_MULTILOCK_MSA_RO);
> +		__entry->geni_se_clk_sel	  = readl(se->base + SE_GENI_CLK_SEL);
> +	    ),
> +
> +	    TP_printk("%s: m_cmd0=0x%08x m_irq_status=0x%08x s_cmd0=0x%08x s_irq_status=0x%08x geni_status=0x%08x geni_ios=0x%08x m_cmd_ctrl=0x%08x m_cmd_err=0x%08x m_fw_err=0x%08x tx_fifo_sts=0x%08x rx_fifo_sts=0x%08x tx_watermark=0x%08x rx_watermark=0x%08x rx_watermark_rfr=0x%08x m_gp_length=0x%08x s_gp_length=0x%08x dma_tx_irq=0x%08x dma_rx_irq=0x%08x dma_tx_irq_en=0x%08x dma_rx_irq_en=0x%08x dma_rx_len=0x%08x dma_rx_len_in=0x%08x dma_tx_len=0x%08x dma_tx_len_in=0x%08x dma_tx_ptr_l=0x%08x dma_tx_ptr_h=0x%08x dma_rx_ptr_l=0x%08x dma_rx_ptr_h=0x%08x dma_tx_attr=0x%08x dma_tx_max_burst=0x%08x dma_rx_attr=0x%08x dma_rx_max_burst=0x%08x dma_if_en=0x%08x dma_if_en_ro=0x%08x dma_general_cfg=0x%08x dma_qsb_trans_cfg=0x%08x dma_dbg=0x%08x m_irq_en=0x%08x s_irq_en=0x%08x gsi_event_en=0x%08x se_irq_en=0x%08x ser_m_clk_cfg=0x%08x ser_s_clk_cfg=0x%08x general_cfg=0x%08x output_ctrl=0x%08x clk_ctrl_ro=0x%08x fifo_if_dis=0x%08x fw_multilock_msa=0x%08x clk_sel=0x%08x",
> +		      __get_str(geni_se_name),
> +		      __entry->geni_se_m_cmd0, __entry->geni_se_m_irq_status,
> +		      __entry->geni_se_s_cmd0, __entry->geni_se_s_irq_status,
> +		      __entry->geni_se_status, __entry->geni_se_ios,
> +		      __entry->geni_se_m_cmd_ctrl,
> +		      __entry->geni_se_m_cmd_err, __entry->geni_se_m_fw_err,
> +		      __entry->geni_se_tx_fifo_status, __entry->geni_se_rx_fifo_status,
> +		      __entry->geni_se_tx_watermark, __entry->geni_se_rx_watermark,
> +		      __entry->geni_se_rx_watermark_rfr,
> +		      __entry->geni_se_m_gp_length, __entry->geni_se_s_gp_length,
> +		      __entry->geni_se_dma_tx_irq, __entry->geni_se_dma_rx_irq,
> +		      __entry->geni_se_dma_tx_irq_en, __entry->geni_se_dma_rx_irq_en,
> +		      __entry->geni_se_dma_rx_len, __entry->geni_se_dma_rx_len_in,
> +		      __entry->geni_se_dma_tx_len, __entry->geni_se_dma_tx_len_in,
> +		      __entry->geni_se_dma_tx_ptr_l, __entry->geni_se_dma_tx_ptr_h,
> +		      __entry->geni_se_dma_rx_ptr_l, __entry->geni_se_dma_rx_ptr_h,
> +		      __entry->geni_se_dma_tx_attr, __entry->geni_se_dma_tx_max_burst,
> +		      __entry->geni_se_dma_rx_attr, __entry->geni_se_dma_rx_max_burst,
> +		      __entry->geni_se_dma_if_en, __entry->geni_se_dma_if_en_ro,
> +		      __entry->geni_se_dma_general_cfg, __entry->geni_se_dma_qsb_trans_cfg,
> +		      __entry->geni_se_dma_dbg,
> +		      __entry->geni_se_m_irq_en, __entry->geni_se_s_irq_en,
> +		      __entry->geni_se_gsi_event_en, __entry->geni_se_irq_en,
> +		      __entry->geni_se_ser_m_clk_cfg, __entry->geni_se_ser_s_clk_cfg,
> +		      __entry->geni_se_general_cfg, __entry->geni_se_output_ctrl,
> +		      __entry->geni_se_clk_ctrl_ro, __entry->geni_se_fifo_if_disable,
> +		      __entry->geni_se_fw_multilock_msa, __entry->geni_se_clk_sel)
> +);
> +
> +#endif /* _TRACE_QCOM_GENI_SE_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> 
> -- 
> 2.34.1
> 

^ permalink raw reply

* Re: [PATCH bpf-next 1/2] srcu: Add lock guard for srcu_fast_updown flavor
From: Oleg Nesterov @ 2026-07-11 13:02 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Lai Jiangshan, Paul E. McKenney, Josh Triplett, Masami Hiramatsu,
	Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Alexei Starovoitov, Andrii Nakryiko, Steven Rostedt,
	Mathieu Desnoyers, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, rcu, linux-kernel,
	linux-trace-kernel, linux-perf-users, bpf
In-Reply-To: <20260706172744.3920417-2-puranjay@kernel.org>

On 07/06, Puranjay Mohan wrote:
>
> Add a guard(srcu_fast_updown) definition for scoped
> SRCU-fast-updown read-side critical sections, following the
> existing pattern of guard(srcu) and guard(srcu_fast).
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
>  include/linux/srcu.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index a54ce9e808b92..72c86d3b23f2e 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -638,4 +638,11 @@ DEFINE_LOCK_GUARD_1(srcu_fast_notrace, struct srcu_struct,
>  DECLARE_LOCK_GUARD_1_ATTRS(srcu_fast_notrace, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
>  #define class_srcu_fast_notrace_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_fast_notrace, _T)
>
> +DEFINE_LOCK_GUARD_1(srcu_fast_updown, struct srcu_struct,
> +		    _T->scp = srcu_read_lock_fast_updown(_T->lock),
> +		    srcu_read_unlock_fast_updown(_T->lock, _T->scp),
> +		    struct srcu_ctr __percpu *scp)
> +DECLARE_LOCK_GUARD_1_ATTRS(srcu_fast_updown, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
> +#define class_srcu_fast_updown_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_fast_updown, _T)

Looks "obviously correct" to me. FWIW,

Reviewed-by: Oleg Nesterov <oleg@redhat.com>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox