public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes
@ 2026-02-05 13:58 Seokwoo Chung (Ryan)
  2026-02-05 13:58 ` [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit Seokwoo Chung (Ryan)
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
  To: mhiramat
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest,
	Seokwoo Chung (Ryan)

Extend the fprobe event interface to accept comma-separated symbol lists
with ! exclusion prefix, and :entry/:exit suffixes as an alternative to
%return.  Single-symbol probes retain full backward compatibility with
%return.

Example usage:
  f:mygroup/myevent vfs_read,!vfs_write,vfs_open:entry
  f:mygroup/myexit  vfs_read,vfs_open:exit

Changes since v5:
  - Fix missing closing brace in the empty-token check that caused a
    build error.
  - Remove redundant strchr/strstr checks for tracepoint validation;
    the character validation loop already rejects ',', ':', and '%'.
  - Add trace_probe_log_err() to the tracepoint character validation
    loop so users see what went wrong in tracefs/error_log (reviewer
    feedback from Masami Hiramatsu).
  - Remove unnecessary braces around single-statement if per kernel
    coding style (reviewer feedback).
  - Extract list parsing into parse_fprobe_list() to keep
    parse_fprobe_spec() focused (reviewer feedback).
  - New patch 2/4: add glob_match_comma_list() in kernel/trace/fprobe.c
    so register_fprobe() correctly handles comma-separated filter
    strings.  Without this, enabling a list-mode fprobe event failed
    with "Could not enable event" because glob_match() does not
    understand commas.
  - Reorder: documentation patch now comes after all code changes.
  - Updated selftest commit message to note that existing tests
    (add_remove_fprobe.tc, fprobe_syntax_errors.tc,
    add_remove_fprobe_repeat.tc) report UNSUPPORTED because their
    "requires" lines still check for the old %return syntax in README.
    Their requires lines need updating in a follow-up patch.

Tested in QEMU/KVM but I am not too confident if I configured correctly and
would like to ask for further testing. 

Seokwoo Chung (Ryan) (4):
  tracing/fprobe: Support comma-separated symbols and :entry/:exit
  fprobe: Support comma-separated filters in register_fprobe()
  docs: tracing/fprobe: Document list filters and :entry/:exit
  selftests/ftrace: Add accept cases for fprobe list syntax

 Documentation/trace/fprobetrace.rst           |  17 +-
 kernel/trace/fprobe.c                         |  30 ++-
 kernel/trace/trace.c                          |   3 +-
 kernel/trace/trace_fprobe.c                   | 219 ++++++++++++++----
 .../ftrace/test.d/dynevent/fprobe_list.tc     |  92 ++++++++
 5 files changed, 308 insertions(+), 53 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc

-- 
2.43.0


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

* [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit
  2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
@ 2026-02-05 13:58 ` Seokwoo Chung (Ryan)
  2026-03-24  1:50   ` Masami Hiramatsu
  2026-02-05 13:58 ` [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe() Seokwoo Chung (Ryan)
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
  To: mhiramat
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest,
	Seokwoo Chung (Ryan)

Extend the fprobe event interface to support:
  - Comma-separated symbol lists: "func1,func2,func3"
  - Exclusion prefix: "func1,!func2,func3"
  - Explicit :entry and :exit suffixes (replacing %return for lists)

Single-symbol probes retain backward compatibility with %return.

The list parsing is factored into a dedicated parse_fprobe_list()
helper that splits comma-separated input into filter (included) and
nofilter (excluded) strings.  Tracepoint validation now reports the
error position via trace_probe_log_err() so users can see what went
wrong in tracefs/error_log.

Changes since v5:
  - Fix missing closing brace in the empty-token check that caused a
    build error.
  - Remove redundant strchr/strstr checks for tracepoint validation
    (the character validation loop already rejects ',', ':', and '%').
  - Add trace_probe_log_err() to the tracepoint character validation
    loop per reviewer feedback.
  - Remove unnecessary braces around single-statement if per kernel
    coding style.
  - Extract list parsing into parse_fprobe_list() per reviewer feedback
    to keep parse_fprobe_spec() focused.

Update tracefs/README to reflect the new syntax.

Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
 kernel/trace/trace.c        |   3 +-
 kernel/trace/trace_fprobe.c | 219 ++++++++++++++++++++++++++++--------
 2 files changed, 174 insertions(+), 48 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8bd4ec08fb36..649a6e6021b4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5578,7 +5578,8 @@ static const char readme_msg[] =
 	"\t           r[maxactive][:[<group>/][<event>]] <place> [<args>]\n"
 #endif
 #ifdef CONFIG_FPROBE_EVENTS
-	"\t           f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
+	"\t           f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]\n"
+	"\t		(single symbols still accept %return)\n"
 	"\t           t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
 #endif
 #ifdef CONFIG_HIST_TRIGGERS
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 262c0556e4af..f8846cd1d020 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -187,11 +187,14 @@ DEFINE_FREE(tuser_put, struct tracepoint_user *,
  */
 struct trace_fprobe {
 	struct dyn_event	devent;
+	char			*filter;
 	struct fprobe		fp;
+	bool			list_mode;
+	char			*nofilter;
 	const char		*symbol;
+	struct trace_probe	tp;
 	bool			tprobe;
 	struct tracepoint_user	*tuser;
-	struct trace_probe	tp;
 };
 
 static bool is_trace_fprobe(struct dyn_event *ev)
@@ -559,6 +562,8 @@ static void free_trace_fprobe(struct trace_fprobe *tf)
 		trace_probe_cleanup(&tf->tp);
 		if (tf->tuser)
 			tracepoint_user_put(tf->tuser);
+		kfree(tf->filter);
+		kfree(tf->nofilter);
 		kfree(tf->symbol);
 		kfree(tf);
 	}
@@ -838,7 +843,12 @@ static int __register_trace_fprobe(struct trace_fprobe *tf)
 	if (trace_fprobe_is_tracepoint(tf))
 		return __regsiter_tracepoint_fprobe(tf);
 
-	/* TODO: handle filter, nofilter or symbol list */
+	/* Registration path:
+	 * - list_mode: pass filter/nofilter
+	 * - single: pass symbol only (legacy)
+	 */
+	if (tf->list_mode)
+		return register_fprobe(&tf->fp, tf->filter, tf->nofilter);
 	return register_fprobe(&tf->fp, tf->symbol, NULL);
 }
 
@@ -1154,60 +1164,131 @@ static struct notifier_block tprobe_event_module_nb = {
 };
 #endif /* CONFIG_MODULES */
 
-static int parse_symbol_and_return(int argc, const char *argv[],
-				   char **symbol, bool *is_return,
-				   bool is_tracepoint)
+static bool has_wildcard(const char *s)
 {
-	char *tmp = strchr(argv[1], '%');
-	int i;
+	return s && (strchr(s, '*') || strchr(s, '?'));
+}
 
-	if (tmp) {
-		int len = tmp - argv[1];
+static int parse_fprobe_list(char *b, char **filter, char **nofilter)
+{
+	char *f __free(kfree) = NULL;
+	char *nf __free(kfree) = NULL;
+	char *tmp = b, *tok;
+	size_t sz;
 
-		if (!is_tracepoint && !strcmp(tmp, "%return")) {
-			*is_return = true;
-		} else {
-			trace_probe_log_err(len, BAD_ADDR_SUFFIX);
-			return -EINVAL;
-		}
-		*symbol = kmemdup_nul(argv[1], len, GFP_KERNEL);
-	} else
-		*symbol = kstrdup(argv[1], GFP_KERNEL);
-	if (!*symbol)
+	sz = strlen(b) + 1;
+
+	f = kzalloc(sz, GFP_KERNEL);
+	nf = kzalloc(sz, GFP_KERNEL);
+	if (!f || !nf)
 		return -ENOMEM;
 
-	if (*is_return)
-		return 0;
+	while ((tok = strsep(&tmp, ",")) != NULL) {
+		char *dst;
+		bool neg = (*tok == '!');
 
-	if (is_tracepoint) {
-		tmp = *symbol;
-		while (*tmp && (isalnum(*tmp) || *tmp == '_'))
-			tmp++;
-		if (*tmp) {
-			/* find a wrong character. */
-			trace_probe_log_err(tmp - *symbol, BAD_TP_NAME);
-			kfree(*symbol);
-			*symbol = NULL;
+		if (*tok == '\0') {
+			trace_probe_log_err(tmp - b - 1, BAD_TP_NAME);
 			return -EINVAL;
 		}
+
+		if (neg)
+			tok++;
+		dst = neg ? nf : f;
+		if (dst[0] != '\0')
+			strcat(dst, ",");
+		strcat(dst, tok);
 	}
 
-	/* If there is $retval, this should be a return fprobe. */
-	for (i = 2; i < argc; i++) {
-		tmp = strstr(argv[i], "$retval");
-		if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
-			if (is_tracepoint) {
-				trace_probe_log_set_index(i);
-				trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
-				kfree(*symbol);
-				*symbol = NULL;
+	*filter = no_free_ptr(f);
+	*nofilter = no_free_ptr(nf);
+
+	return 0;
+}
+
+static int parse_fprobe_spec(const char *in, bool is_tracepoint,
+		char **base, bool *is_return, bool *list_mode,
+		char **filter, char **nofilter)
+{
+	char *work __free(kfree) = NULL;
+	char *b __free(kfree) = NULL;
+	char *f __free(kfree) = NULL;
+	char *nf __free(kfree) = NULL;
+	bool legacy_ret = false;
+	bool list = false;
+	const char *p;
+	int ret = 0;
+
+	if (!in || !base || !is_return || !list_mode || !filter || !nofilter)
+		return -EINVAL;
+
+	*base = NULL; *filter = NULL; *nofilter = NULL;
+	*is_return = false; *list_mode = false;
+
+	if (is_tracepoint) {
+		for (p = in; *p; p++)
+			if (!isalnum(*p) && *p != '_') {
+				trace_probe_log_err(p - in, BAD_TP_NAME);
+				return -EINVAL;
+			}
+		b = kstrdup(in, GFP_KERNEL);
+		if (!b)
+			return -ENOMEM;
+		*base = no_free_ptr(b);
+		return 0;
+	}
+
+	work = kstrdup(in, GFP_KERNEL);
+	if (!work)
+		return -ENOMEM;
+
+	p = strstr(work, "%return");
+	if (p && p[7] == '\0') {
+		*is_return = true;
+		legacy_ret = true;
+		*(char *)p = '\0';
+	} else {
+		/*
+		 * If "symbol:entry" or "symbol:exit" is given, it is new
+		 * style probe.
+		 */
+		p = strrchr(work, ':');
+		if (p) {
+			if (!strcmp(p, ":exit")) {
+				*is_return = true;
+				*(char *)p = '\0';
+			} else if (!strcmp(p, ":entry")) {
+				*(char *)p = '\0';
+			} else {
 				return -EINVAL;
 			}
-			*is_return = true;
-			break;
 		}
 	}
-	return 0;
+
+	list = !!strchr(work, ',');
+	
+	if (list && legacy_ret)
+		return -EINVAL;
+
+	if (legacy_ret)
+		*is_return = true;
+
+	b = kstrdup(work, GFP_KERNEL);
+	if (!b)
+		return -ENOMEM;
+
+	if (list) {
+		ret = parse_fprobe_list(b, &f, &nf);
+		if (ret)
+			return ret;
+		*list_mode = true;
+	}
+
+	*base = no_free_ptr(b);
+	*filter = no_free_ptr(f);
+	*nofilter = no_free_ptr(nf);
+
+	return ret;
 }
 
 static int trace_fprobe_create_internal(int argc, const char *argv[],
@@ -1241,6 +1322,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 	const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
 	struct module *mod __free(module_put) = NULL;
 	const char **new_argv __free(kfree) = NULL;
+	char *parsed_nofilter __free(kfree) = NULL;
+	char *parsed_filter __free(kfree) = NULL;
 	char *symbol __free(kfree) = NULL;
 	char *ebuf __free(kfree) = NULL;
 	char *gbuf __free(kfree) = NULL;
@@ -1249,6 +1332,7 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 	char *dbuf __free(kfree) = NULL;
 	int i, new_argc = 0, ret = 0;
 	bool is_tracepoint = false;
+	bool list_mode = false;
 	bool is_return = false;
 
 	if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
@@ -1270,11 +1354,26 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 
 	trace_probe_log_set_index(1);
 
-	/* a symbol(or tracepoint) must be specified */
-	ret = parse_symbol_and_return(argc, argv, &symbol, &is_return, is_tracepoint);
+	/* Parse spec early (single vs list, suffix, base symbol) */
+	ret = parse_fprobe_spec(argv[1], is_tracepoint, &symbol, &is_return,
+			&list_mode, &parsed_filter, &parsed_nofilter);
 	if (ret < 0)
 		return -EINVAL;
 
+	for (i = 2; i < argc; i++) {
+		char *tmp = strstr(argv[i], "$retval");
+
+		if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
+			if (is_tracepoint) {
+				trace_probe_log_set_index(i);
+				trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
+				return -EINVAL;
+			}
+			is_return = true;
+			break;
+		}
+	}
+
 	trace_probe_log_set_index(0);
 	if (event) {
 		gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
@@ -1287,6 +1386,15 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 	}
 
 	if (!event) {
+		/*
+		 * Event name rules:
+		 * - For list/wildcard: require explicit [GROUP/]EVENT
+		 * - For single literal: autogenerate symbol__entry/symbol__exit
+		 */
+		if (list_mode || has_wildcard(symbol)) {
+			trace_probe_log_err(0, NO_GROUP_NAME);
+			return -EINVAL;
+		}
 		ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
 		if (!ebuf)
 			return -ENOMEM;
@@ -1322,7 +1430,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 							NULL, NULL, NULL, sbuf);
 		}
 	}
-	if (!ctx->funcname)
+
+	if (!list_mode && !has_wildcard(symbol) && !is_tracepoint)
 		ctx->funcname = symbol;
 
 	abuf = kmalloc(MAX_BTF_ARGS_LEN, GFP_KERNEL);
@@ -1356,6 +1465,21 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 		return ret;
 	}
 
+	/* carry list parsing result into tf */
+	if (!is_tracepoint) {
+		tf->list_mode = list_mode;
+		if (parsed_filter) {
+			tf->filter = kstrdup(parsed_filter, GFP_KERNEL);
+			if (!tf->filter)
+				return -ENOMEM;
+		}
+		if (parsed_nofilter) {
+			tf->nofilter = kstrdup(parsed_nofilter, GFP_KERNEL);
+			if (!tf->nofilter)
+				return -ENOMEM;
+		}
+	}
+
 	/* parse arguments */
 	for (i = 0; i < argc; i++) {
 		trace_probe_log_set_index(i + 2);
@@ -1442,8 +1566,9 @@ static int trace_fprobe_show(struct seq_file *m, struct dyn_event *ev)
 	seq_printf(m, ":%s/%s", trace_probe_group_name(&tf->tp),
 				trace_probe_name(&tf->tp));
 
-	seq_printf(m, " %s%s", trace_fprobe_symbol(tf),
-			       trace_fprobe_is_return(tf) ? "%return" : "");
+	seq_printf(m, " %s", trace_fprobe_symbol(tf));
+	if (!trace_fprobe_is_tracepoint(tf) && trace_fprobe_is_return(tf))
+		seq_puts(m, ":exit");
 
 	for (i = 0; i < tf->tp.nr_args; i++)
 		seq_printf(m, " %s=%s", tf->tp.args[i].name, tf->tp.args[i].comm);
-- 
2.43.0


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

* [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe()
  2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
  2026-02-05 13:58 ` [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit Seokwoo Chung (Ryan)
@ 2026-02-05 13:58 ` Seokwoo Chung (Ryan)
  2026-03-24  1:59   ` Masami Hiramatsu
  2026-02-05 13:58 ` [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit Seokwoo Chung (Ryan)
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
  To: mhiramat
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest,
	Seokwoo Chung (Ryan)

register_fprobe() passes its filter and notfilter strings directly to
glob_match(), which only understands shell-style globs (*, ?, [...]).
Comma-separated symbol lists such as "vfs_read,vfs_open" never match
any symbol because no kernel symbol contains a comma.

Add glob_match_comma_list() that splits the filter on commas and
checks each entry individually with glob_match().  The existing
single-pattern fast path is preserved (no commas means the loop
executes exactly once).

This is required by the comma-separated fprobe list syntax introduced
in the preceding patch; without it, enabling a list-mode fprobe event
fails with "Could not enable event".

Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
 kernel/trace/fprobe.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 1188eefef07c..2acd24b80d04 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -672,12 +672,38 @@ struct filter_match_data {
 	struct module **mods;
 };
 
+/*
+ * Check if @name matches any comma-separated glob pattern in @list.
+ * If @list contains no commas, this is equivalent to glob_match().
+ */
+static bool glob_match_comma_list(const char *list, const char *name)
+{
+	const char *cur = list;
+
+	while (*cur) {
+		const char *sep = strchr(cur, ',');
+		int len = sep ? sep - cur : strlen(cur);
+		char pat[KSYM_NAME_LEN];
+
+		if (len > 0 && len < KSYM_NAME_LEN) {
+			memcpy(pat, cur, len);
+			pat[len] = '\0';
+			if (glob_match(pat, name))
+				return true;
+		}
+		if (!sep)
+			break;
+		cur = sep + 1;
+	}
+	return false;
+}
+
 static int filter_match_callback(void *data, const char *name, unsigned long addr)
 {
 	struct filter_match_data *match = data;
 
-	if (!glob_match(match->filter, name) ||
-	    (match->notfilter && glob_match(match->notfilter, name)))
+	if (!glob_match_comma_list(match->filter, name) ||
+	    (match->notfilter && glob_match_comma_list(match->notfilter, name)))
 		return 0;
 
 	if (!ftrace_location(addr))
-- 
2.43.0


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

* [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit
  2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
  2026-02-05 13:58 ` [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit Seokwoo Chung (Ryan)
  2026-02-05 13:58 ` [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe() Seokwoo Chung (Ryan)
@ 2026-02-05 13:58 ` Seokwoo Chung (Ryan)
  2026-03-24  4:07   ` Masami Hiramatsu
  2026-02-05 13:58 ` [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax Seokwoo Chung (Ryan)
  2026-03-24  1:51 ` [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Masami Hiramatsu
  4 siblings, 1 reply; 10+ messages in thread
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
  To: mhiramat
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest,
	Seokwoo Chung (Ryan)

Update fprobe event documentation to describe comma-separated symbol lists,
exclusions, and explicit suffixes.

Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
 Documentation/trace/fprobetrace.rst | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
index b4c2ca3d02c1..bbcfd57f0005 100644
--- a/Documentation/trace/fprobetrace.rst
+++ b/Documentation/trace/fprobetrace.rst
@@ -25,14 +25,18 @@ Synopsis of fprobe-events
 -------------------------
 ::
 
-  f[:[GRP1/][EVENT1]] SYM [FETCHARGS]                       : Probe on function entry
-  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]     : Probe on function exit
+  f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS]		    : Single function
+  f[:[GRP1/][EVENT1]] SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS] :Multiple
+  function
   t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]                : Probe on tracepoint
 
  GRP1           : Group name for fprobe. If omitted, use "fprobes" for it.
  GRP2           : Group name for tprobe. If omitted, use "tracepoints" for it.
  EVENT1         : Event name for fprobe. If omitted, the event name is
-                  "SYM__entry" or "SYM__exit".
+		  - For a single literal symbol, the event name is
+		    "SYM__entry" or "SYM__exit".
+		  - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is
+		    required; otherwise the parser rejects it.
  EVENT2         : Event name for tprobe. If omitted, the event name is
                   the same as "TRACEPOINT", but if the "TRACEPOINT" starts
                   with a digit character, "_TRACEPOINT" is used.
@@ -40,6 +44,13 @@ Synopsis of fprobe-events
                   can be probed simultaneously, or 0 for the default value
                   as defined in Documentation/trace/fprobe.rst
 
+ SYM		: Function name or comma-separated list of symbols.
+		  - SYM prefixed with "!" are exclusions.
+		  - ":entry" suffix means it probes entry of given symbols
+		    (default)
+		  - ":exit" suffix means it probes exit of given symbols.
+		  - "%return" suffix means it probes exit of SYM (single
+		    symbol).
  FETCHARGS      : Arguments. Each probe can have up to 128 args.
   ARG           : Fetch "ARG" function argument using BTF (only for function
                   entry or tracepoint.) (\*1)
-- 
2.43.0


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

* [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax
  2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
                   ` (2 preceding siblings ...)
  2026-02-05 13:58 ` [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit Seokwoo Chung (Ryan)
@ 2026-02-05 13:58 ` Seokwoo Chung (Ryan)
  2026-03-24  4:12   ` Masami Hiramatsu
  2026-03-24  1:51 ` [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Masami Hiramatsu
  4 siblings, 1 reply; 10+ messages in thread
From: Seokwoo Chung (Ryan) @ 2026-02-05 13:58 UTC (permalink / raw)
  To: mhiramat
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest,
	Seokwoo Chung (Ryan)

Add fprobe_list.tc to test the comma-separated symbol list syntax
with :entry/:exit suffixes.  Three scenarios are covered:

  1. List with default (entry) behavior and ! exclusion
  2. List with explicit :entry suffix
  3. List with :exit suffix for return probes

Each test verifies that the correct functions appear in
enabled_functions and that excluded (!) symbols are absent.

Note: The existing tests add_remove_fprobe.tc, fprobe_syntax_errors.tc,
and add_remove_fprobe_repeat.tc check their "requires" line against the
tracefs README for the old "%return" syntax pattern.  Since the README
now documents ":entry|:exit" instead, these tests report UNSUPPORTED.
Their "requires" lines need updating in a follow-up patch.

Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
---
 .../ftrace/test.d/dynevent/fprobe_list.tc     | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
new file mode 100644
index 000000000000..45e57c6f487d
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
@@ -0,0 +1,92 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Fprobe event list syntax and :entry/:exit suffixes
+# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]":README
+
+# Setup symbols to test. These are common kernel functions.
+PLACE=vfs_read
+PLACE2=vfs_write
+PLACE3=vfs_open
+
+echo 0 > events/enable
+echo > dynamic_events
+
+# Get baseline count of enabled functions (should be 0 if clean, but be safe)
+if [ -f enabled_functions ]; then
+	ocnt=`cat enabled_functions | wc -l`
+else
+	ocnt=0
+fi
+
+# Test 1: List default (entry) with exclusion
+# Target: Trace vfs_read and vfs_open, but EXCLUDE vfs_write
+echo "f:test/list_entry $PLACE,!$PLACE2,$PLACE3" >> dynamic_events
+grep -q "test/list_entry" dynamic_events
+test -d events/test/list_entry
+
+echo 1 > events/test/list_entry/enable
+
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+# Check count (Baseline + 2 new functions)
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+	exit_fail
+fi
+
+# Cleanup Test 1
+echo 0 > events/test/list_entry/enable
+echo "-:test/list_entry" >> dynamic_events
+! grep -q "test/list_entry" dynamic_events
+
+# Count should return to baseline
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $ocnt ]; then
+	exit_fail
+fi
+
+# Test 2: List with explicit :entry suffix
+# (Should behave exactly like Test 1)
+echo "f:test/list_entry_exp $PLACE,!$PLACE2,$PLACE3:entry" >> dynamic_events
+grep -q "test/list_entry_exp" dynamic_events
+test -d events/test/list_entry_exp
+
+echo 1 > events/test/list_entry_exp/enable
+
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+	exit_fail
+fi
+
+# Cleanup Test 2
+echo 0 > events/test/list_entry_exp/enable
+echo "-:test/list_entry_exp" >> dynamic_events
+
+# Test 3: List with :exit suffix
+echo "f:test/list_exit $PLACE,!$PLACE2,$PLACE3:exit" >> dynamic_events
+grep -q "test/list_exit" dynamic_events
+test -d events/test/list_exit
+
+echo 1 > events/test/list_exit/enable
+
+# Even for return probes, enabled_functions lists the attached symbols
+grep -q "$PLACE" enabled_functions
+grep -q "$PLACE3" enabled_functions
+! grep -q "$PLACE2" enabled_functions
+
+cnt=`cat enabled_functions | wc -l`
+if [ $cnt -ne $((ocnt + 2)) ]; then
+	exit_fail
+fi
+
+# Cleanup Test 3
+echo 0 > events/test/list_exit/enable
+echo "-:test/list_exit" >> dynamic_events
+
+clear_trace
-- 
2.43.0


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

* Re: [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit
  2026-02-05 13:58 ` [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit Seokwoo Chung (Ryan)
@ 2026-03-24  1:50   ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2026-03-24  1:50 UTC (permalink / raw)
  To: Seokwoo Chung (Ryan)
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest

On Thu,  5 Feb 2026 08:58:39 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:

> Extend the fprobe event interface to support:
>   - Comma-separated symbol lists: "func1,func2,func3"
>   - Exclusion prefix: "func1,!func2,func3"
>   - Explicit :entry and :exit suffixes (replacing %return for lists)
> 
> Single-symbol probes retain backward compatibility with %return.
> 
> The list parsing is factored into a dedicated parse_fprobe_list()
> helper that splits comma-separated input into filter (included) and
> nofilter (excluded) strings.  Tracepoint validation now reports the
> error position via trace_probe_log_err() so users can see what went
> wrong in tracefs/error_log.
> 
> Changes since v5:
>   - Fix missing closing brace in the empty-token check that caused a
>     build error.
>   - Remove redundant strchr/strstr checks for tracepoint validation
>     (the character validation loop already rejects ',', ':', and '%').
>   - Add trace_probe_log_err() to the tracepoint character validation
>     loop per reviewer feedback.
>   - Remove unnecessary braces around single-statement if per kernel
>     coding style.
>   - Extract list parsing into parse_fprobe_list() per reviewer feedback
>     to keep parse_fprobe_spec() focused.


Thanks for updating! I have some comments below.

> 
> Update tracefs/README to reflect the new syntax.
> 
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
>  kernel/trace/trace.c        |   3 +-
>  kernel/trace/trace_fprobe.c | 219 ++++++++++++++++++++++++++++--------
>  2 files changed, 174 insertions(+), 48 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8bd4ec08fb36..649a6e6021b4 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5578,7 +5578,8 @@ static const char readme_msg[] =
>  	"\t           r[maxactive][:[<group>/][<event>]] <place> [<args>]\n"
>  #endif
>  #ifdef CONFIG_FPROBE_EVENTS
> -	"\t           f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
> +	"\t           f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]\n"
> +	"\t		(single symbols still accept %return)\n"
>  	"\t           t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
>  #endif
>  #ifdef CONFIG_HIST_TRIGGERS
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 262c0556e4af..f8846cd1d020 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -187,11 +187,14 @@ DEFINE_FREE(tuser_put, struct tracepoint_user *,
>   */
>  struct trace_fprobe {
>  	struct dyn_event	devent;
> +	char			*filter;

Could you move this filter to the previous line of nofilter?

>  	struct fprobe		fp;
> +	bool			list_mode;


This list_mode seems an alias of (filter || nofilter). In this case, 
we don't need trace_fprobe::list_mode. Please remove this field.


> +	char			*nofilter;

>  	const char		*symbol;
> +	struct trace_probe	tp;

tp must be the last field. Please do not move.

>  	bool			tprobe;
>  	struct tracepoint_user	*tuser;
> -	struct trace_probe	tp;
>  };
>  
>  static bool is_trace_fprobe(struct dyn_event *ev)
> @@ -559,6 +562,8 @@ static void free_trace_fprobe(struct trace_fprobe *tf)
>  		trace_probe_cleanup(&tf->tp);
>  		if (tf->tuser)
>  			tracepoint_user_put(tf->tuser);
> +		kfree(tf->filter);
> +		kfree(tf->nofilter);
>  		kfree(tf->symbol);
>  		kfree(tf);
>  	}
> @@ -838,7 +843,12 @@ static int __register_trace_fprobe(struct trace_fprobe *tf)
>  	if (trace_fprobe_is_tracepoint(tf))
>  		return __regsiter_tracepoint_fprobe(tf);
>  
> -	/* TODO: handle filter, nofilter or symbol list */
> +	/* Registration path:
> +	 * - list_mode: pass filter/nofilter
> +	 * - single: pass symbol only (legacy)
> +	 */
> +	if (tf->list_mode)
> +		return register_fprobe(&tf->fp, tf->filter, tf->nofilter);
>  	return register_fprobe(&tf->fp, tf->symbol, NULL);
>  }
>  
> @@ -1154,60 +1164,131 @@ static struct notifier_block tprobe_event_module_nb = {
>  };
>  #endif /* CONFIG_MODULES */
>  
> -static int parse_symbol_and_return(int argc, const char *argv[],
> -				   char **symbol, bool *is_return,
> -				   bool is_tracepoint)
> +static bool has_wildcard(const char *s)
>  {
> -	char *tmp = strchr(argv[1], '%');
> -	int i;
> +	return s && (strchr(s, '*') || strchr(s, '?'));
> +}
>  
> -	if (tmp) {
> -		int len = tmp - argv[1];
> +static int parse_fprobe_list(char *b, char **filter, char **nofilter)
> +{
> +	char *f __free(kfree) = NULL;
> +	char *nf __free(kfree) = NULL;
> +	char *tmp = b, *tok;
> +	size_t sz;
>  
> -		if (!is_tracepoint && !strcmp(tmp, "%return")) {
> -			*is_return = true;
> -		} else {
> -			trace_probe_log_err(len, BAD_ADDR_SUFFIX);
> -			return -EINVAL;
> -		}
> -		*symbol = kmemdup_nul(argv[1], len, GFP_KERNEL);
> -	} else
> -		*symbol = kstrdup(argv[1], GFP_KERNEL);
> -	if (!*symbol)
> +	sz = strlen(b) + 1;
> +
> +	f = kzalloc(sz, GFP_KERNEL);
> +	nf = kzalloc(sz, GFP_KERNEL);
> +	if (!f || !nf)
>  		return -ENOMEM;
>  
> -	if (*is_return)
> -		return 0;
> +	while ((tok = strsep(&tmp, ",")) != NULL) {
> +		char *dst;
> +		bool neg = (*tok == '!');
>  
> -	if (is_tracepoint) {
> -		tmp = *symbol;
> -		while (*tmp && (isalnum(*tmp) || *tmp == '_'))
> -			tmp++;
> -		if (*tmp) {
> -			/* find a wrong character. */
> -			trace_probe_log_err(tmp - *symbol, BAD_TP_NAME);
> -			kfree(*symbol);
> -			*symbol = NULL;
> +		if (*tok == '\0') {
> +			trace_probe_log_err(tmp - b - 1, BAD_TP_NAME);
>  			return -EINVAL;
>  		}
> +
> +		if (neg)
> +			tok++;
> +		dst = neg ? nf : f;
> +		if (dst[0] != '\0')
> +			strcat(dst, ",");
> +		strcat(dst, tok);
>  	}
>  
> -	/* If there is $retval, this should be a return fprobe. */
> -	for (i = 2; i < argc; i++) {
> -		tmp = strstr(argv[i], "$retval");
> -		if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
> -			if (is_tracepoint) {
> -				trace_probe_log_set_index(i);
> -				trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
> -				kfree(*symbol);
> -				*symbol = NULL;
> +	*filter = no_free_ptr(f);
> +	*nofilter = no_free_ptr(nf);
> +
> +	return 0;
> +}
> +
> +static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> +		char **base, bool *is_return, bool *list_mode,
> +		char **filter, char **nofilter)
> +{
> +	char *work __free(kfree) = NULL;
> +	char *b __free(kfree) = NULL;
> +	char *f __free(kfree) = NULL;
> +	char *nf __free(kfree) = NULL;
> +	bool legacy_ret = false;
> +	bool list = false;
> +	const char *p;
> +	int ret = 0;
> +
> +	if (!in || !base || !is_return || !list_mode || !filter || !nofilter)
> +		return -EINVAL;
> +
> +	*base = NULL; *filter = NULL; *nofilter = NULL;
> +	*is_return = false; *list_mode = false;
> +
> +	if (is_tracepoint) {
> +		for (p = in; *p; p++)
> +			if (!isalnum(*p) && *p != '_') {
> +				trace_probe_log_err(p - in, BAD_TP_NAME);
> +				return -EINVAL;
> +			}
> +		b = kstrdup(in, GFP_KERNEL);
> +		if (!b)
> +			return -ENOMEM;
> +		*base = no_free_ptr(b);
> +		return 0;
> +	}
> +
> +	work = kstrdup(in, GFP_KERNEL);
> +	if (!work)
> +		return -ENOMEM;
> +
> +	p = strstr(work, "%return");
> +	if (p && p[7] == '\0') {
> +		*is_return = true;
> +		legacy_ret = true;
> +		*(char *)p = '\0';
> +	} else {
> +		/*
> +		 * If "symbol:entry" or "symbol:exit" is given, it is new
> +		 * style probe.
> +		 */
> +		p = strrchr(work, ':');
> +		if (p) {
> +			if (!strcmp(p, ":exit")) {
> +				*is_return = true;
> +				*(char *)p = '\0';
> +			} else if (!strcmp(p, ":entry")) {
> +				*(char *)p = '\0';
> +			} else {
>  				return -EINVAL;
>  			}
> -			*is_return = true;
> -			break;
>  		}
>  	}
> -	return 0;
> +
> +	list = !!strchr(work, ',');
> +	

Here is a needless whitespace (tab) above line.

> +	if (list && legacy_ret)
> +		return -EINVAL;
> +
> +	if (legacy_ret)
> +		*is_return = true;
> +
> +	b = kstrdup(work, GFP_KERNEL);
> +	if (!b)
> +		return -ENOMEM;
> +
> +	if (list) {
> +		ret = parse_fprobe_list(b, &f, &nf);
> +		if (ret)
> +			return ret;
> +		*list_mode = true;
> +	}
> +
> +	*base = no_free_ptr(b);
> +	*filter = no_free_ptr(f);
> +	*nofilter = no_free_ptr(nf);
> +
> +	return ret;
>  }
>  
>  static int trace_fprobe_create_internal(int argc, const char *argv[],
> @@ -1241,6 +1322,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  	const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
>  	struct module *mod __free(module_put) = NULL;
>  	const char **new_argv __free(kfree) = NULL;
> +	char *parsed_nofilter __free(kfree) = NULL;
> +	char *parsed_filter __free(kfree) = NULL;

nit: we should make those as filter/nofilter simply.

>  	char *symbol __free(kfree) = NULL;
>  	char *ebuf __free(kfree) = NULL;
>  	char *gbuf __free(kfree) = NULL;
> @@ -1249,6 +1332,7 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  	char *dbuf __free(kfree) = NULL;
>  	int i, new_argc = 0, ret = 0;
>  	bool is_tracepoint = false;
> +	bool list_mode = false;

Then, we don't need this list_mode. we can replace it with "(filter || nofilter)".

>  	bool is_return = false;
>  
>  	if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
> @@ -1270,11 +1354,26 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  
>  	trace_probe_log_set_index(1);
>  
> -	/* a symbol(or tracepoint) must be specified */
> -	ret = parse_symbol_and_return(argc, argv, &symbol, &is_return, is_tracepoint);
> +	/* Parse spec early (single vs list, suffix, base symbol) */
> +	ret = parse_fprobe_spec(argv[1], is_tracepoint, &symbol, &is_return,
> +			&list_mode, &parsed_filter, &parsed_nofilter);
>  	if (ret < 0)
>  		return -EINVAL;
>  
> +	for (i = 2; i < argc; i++) {
> +		char *tmp = strstr(argv[i], "$retval");
> +
> +		if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
> +			if (is_tracepoint) {
> +				trace_probe_log_set_index(i);
> +				trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
> +				return -EINVAL;
> +			}
> +			is_return = true;
> +			break;
> +		}
> +	}

Please do this $retval check in parse_fprobe_spec() as parse_symbol_and_return() did.

> +
>  	trace_probe_log_set_index(0);
>  	if (event) {
>  		gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> @@ -1287,6 +1386,15 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  	}
>  
>  	if (!event) {
> +		/*
> +		 * Event name rules:
> +		 * - For list/wildcard: require explicit [GROUP/]EVENT
> +		 * - For single literal: autogenerate symbol__entry/symbol__exit
> +		 */
> +		if (list_mode || has_wildcard(symbol)) {
> +			trace_probe_log_err(0, NO_GROUP_NAME);

 NO_EVENT_NAME error?

> +			return -EINVAL;
> +		}
>  		ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
>  		if (!ebuf)
>  			return -ENOMEM;
> @@ -1322,7 +1430,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  							NULL, NULL, NULL, sbuf);
>  		}
>  	}
> -	if (!ctx->funcname)
> +
> +	if (!list_mode && !has_wildcard(symbol) && !is_tracepoint)
>  		ctx->funcname = symbol;
>  
>  	abuf = kmalloc(MAX_BTF_ARGS_LEN, GFP_KERNEL);
> @@ -1356,6 +1465,21 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  		return ret;
>  	}
>  
> +	/* carry list parsing result into tf */
> +	if (!is_tracepoint) {
> +		tf->list_mode = list_mode;
> +		if (parsed_filter) {
> +			tf->filter = kstrdup(parsed_filter, GFP_KERNEL);
> +			if (!tf->filter)
> +				return -ENOMEM;
> +		}
> +		if (parsed_nofilter) {
> +			tf->nofilter = kstrdup(parsed_nofilter, GFP_KERNEL);
> +			if (!tf->nofilter)
> +				return -ENOMEM;
> +		}
> +	}

Also, it is natural to do this inside alloc_trace_fprobe(). Can you pass
filter and nofilter to alloc_trace_fprobe()?
(and just make those NULL instead of using kstrdup()?)

Thank you,


> +
>  	/* parse arguments */
>  	for (i = 0; i < argc; i++) {
>  		trace_probe_log_set_index(i + 2);
> @@ -1442,8 +1566,9 @@ static int trace_fprobe_show(struct seq_file *m, struct dyn_event *ev)
>  	seq_printf(m, ":%s/%s", trace_probe_group_name(&tf->tp),
>  				trace_probe_name(&tf->tp));
>  
> -	seq_printf(m, " %s%s", trace_fprobe_symbol(tf),
> -			       trace_fprobe_is_return(tf) ? "%return" : "");
> +	seq_printf(m, " %s", trace_fprobe_symbol(tf));
> +	if (!trace_fprobe_is_tracepoint(tf) && trace_fprobe_is_return(tf))
> +		seq_puts(m, ":exit");
>  
>  	for (i = 0; i < tf->tp.nr_args; i++)
>  		seq_printf(m, " %s=%s", tf->tp.args[i].name, tf->tp.args[i].comm);
> -- 
> 2.43.0
> 
> 


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

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

* Re: [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes
  2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
                   ` (3 preceding siblings ...)
  2026-02-05 13:58 ` [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax Seokwoo Chung (Ryan)
@ 2026-03-24  1:51 ` Masami Hiramatsu
  4 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2026-03-24  1:51 UTC (permalink / raw)
  To: Seokwoo Chung (Ryan)
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest

Hi,

Sorry, I completely missed this series. Let me review it.

Thank you,

On Thu,  5 Feb 2026 08:58:38 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:

> Extend the fprobe event interface to accept comma-separated symbol lists
> with ! exclusion prefix, and :entry/:exit suffixes as an alternative to
> %return.  Single-symbol probes retain full backward compatibility with
> %return.
> 
> Example usage:
>   f:mygroup/myevent vfs_read,!vfs_write,vfs_open:entry
>   f:mygroup/myexit  vfs_read,vfs_open:exit
> 
> Changes since v5:
>   - Fix missing closing brace in the empty-token check that caused a
>     build error.
>   - Remove redundant strchr/strstr checks for tracepoint validation;
>     the character validation loop already rejects ',', ':', and '%'.
>   - Add trace_probe_log_err() to the tracepoint character validation
>     loop so users see what went wrong in tracefs/error_log (reviewer
>     feedback from Masami Hiramatsu).
>   - Remove unnecessary braces around single-statement if per kernel
>     coding style (reviewer feedback).
>   - Extract list parsing into parse_fprobe_list() to keep
>     parse_fprobe_spec() focused (reviewer feedback).
>   - New patch 2/4: add glob_match_comma_list() in kernel/trace/fprobe.c
>     so register_fprobe() correctly handles comma-separated filter
>     strings.  Without this, enabling a list-mode fprobe event failed
>     with "Could not enable event" because glob_match() does not
>     understand commas.
>   - Reorder: documentation patch now comes after all code changes.
>   - Updated selftest commit message to note that existing tests
>     (add_remove_fprobe.tc, fprobe_syntax_errors.tc,
>     add_remove_fprobe_repeat.tc) report UNSUPPORTED because their
>     "requires" lines still check for the old %return syntax in README.
>     Their requires lines need updating in a follow-up patch.
> 
> Tested in QEMU/KVM but I am not too confident if I configured correctly and
> would like to ask for further testing. 
> 
> Seokwoo Chung (Ryan) (4):
>   tracing/fprobe: Support comma-separated symbols and :entry/:exit
>   fprobe: Support comma-separated filters in register_fprobe()
>   docs: tracing/fprobe: Document list filters and :entry/:exit
>   selftests/ftrace: Add accept cases for fprobe list syntax
> 
>  Documentation/trace/fprobetrace.rst           |  17 +-
>  kernel/trace/fprobe.c                         |  30 ++-
>  kernel/trace/trace.c                          |   3 +-
>  kernel/trace/trace_fprobe.c                   | 219 ++++++++++++++----
>  .../ftrace/test.d/dynevent/fprobe_list.tc     |  92 ++++++++
>  5 files changed, 308 insertions(+), 53 deletions(-)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> 
> -- 
> 2.43.0
> 
> 


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

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

* Re: [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe()
  2026-02-05 13:58 ` [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe() Seokwoo Chung (Ryan)
@ 2026-03-24  1:59   ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2026-03-24  1:59 UTC (permalink / raw)
  To: Seokwoo Chung (Ryan)
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest

On Thu,  5 Feb 2026 08:58:40 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:

> register_fprobe() passes its filter and notfilter strings directly to
> glob_match(), which only understands shell-style globs (*, ?, [...]).
> Comma-separated symbol lists such as "vfs_read,vfs_open" never match
> any symbol because no kernel symbol contains a comma.
> 
> Add glob_match_comma_list() that splits the filter on commas and
> checks each entry individually with glob_match().  The existing
> single-pattern fast path is preserved (no commas means the loop
> executes exactly once).
> 
> This is required by the comma-separated fprobe list syntax introduced
> in the preceding patch; without it, enabling a list-mode fprobe event
> fails with "Could not enable event".

OK, in this case, you should reorder patch this as the first one.
Please make this [1/4] and remove this requirement explanation paragraph.
The patch itself looks good to me.

Thank you,

> 
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
>  kernel/trace/fprobe.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 1188eefef07c..2acd24b80d04 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -672,12 +672,38 @@ struct filter_match_data {
>  	struct module **mods;
>  };
>  
> +/*
> + * Check if @name matches any comma-separated glob pattern in @list.
> + * If @list contains no commas, this is equivalent to glob_match().
> + */
> +static bool glob_match_comma_list(const char *list, const char *name)
> +{
> +	const char *cur = list;
> +
> +	while (*cur) {
> +		const char *sep = strchr(cur, ',');
> +		int len = sep ? sep - cur : strlen(cur);
> +		char pat[KSYM_NAME_LEN];
> +
> +		if (len > 0 && len < KSYM_NAME_LEN) {
> +			memcpy(pat, cur, len);
> +			pat[len] = '\0';
> +			if (glob_match(pat, name))
> +				return true;
> +		}
> +		if (!sep)
> +			break;
> +		cur = sep + 1;
> +	}
> +	return false;
> +}
> +
>  static int filter_match_callback(void *data, const char *name, unsigned long addr)
>  {
>  	struct filter_match_data *match = data;
>  
> -	if (!glob_match(match->filter, name) ||
> -	    (match->notfilter && glob_match(match->notfilter, name)))
> +	if (!glob_match_comma_list(match->filter, name) ||
> +	    (match->notfilter && glob_match_comma_list(match->notfilter, name)))
>  		return 0;
>  
>  	if (!ftrace_location(addr))
> -- 
> 2.43.0
> 
> 


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

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

* Re: [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit
  2026-02-05 13:58 ` [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit Seokwoo Chung (Ryan)
@ 2026-03-24  4:07   ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2026-03-24  4:07 UTC (permalink / raw)
  To: Seokwoo Chung (Ryan)
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest

On Thu,  5 Feb 2026 08:58:41 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:

> Update fprobe event documentation to describe comma-separated symbol lists,
> exclusions, and explicit suffixes.
> 
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
>  Documentation/trace/fprobetrace.rst | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> index b4c2ca3d02c1..bbcfd57f0005 100644
> --- a/Documentation/trace/fprobetrace.rst
> +++ b/Documentation/trace/fprobetrace.rst
> @@ -25,14 +25,18 @@ Synopsis of fprobe-events
>  -------------------------
>  ::
>  
> -  f[:[GRP1/][EVENT1]] SYM [FETCHARGS]                       : Probe on function entry
> -  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]     : Probe on function exit
> +  f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS]		    : Single function

We also accept wildcard pattern instead of SYM.

  f[:[GRP1/][EVENT1]] (SYM|PATTERN)[%return] [FETCHARGS]	 : Single target

> +  f[:[GRP1/][EVENT1]] SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS] :Multiple

Hmm if this is for multiple function, we can not omit event name, so

  f:[GRP1/]EVNET1 SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS]   : Multiple function

> +  function

Also, if you put this in the next line, please indent it.

>    t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]                : Probe on tracepoint
>  
>   GRP1           : Group name for fprobe. If omitted, use "fprobes" for it.
>   GRP2           : Group name for tprobe. If omitted, use "tracepoints" for it.
>   EVENT1         : Event name for fprobe. If omitted, the event name is
> -                  "SYM__entry" or "SYM__exit".
> +		  - For a single literal symbol, the event name is
> +		    "SYM__entry" or "SYM__exit".
> +		  - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is

an explicit EVENT1 is required. (group name can be omitted)

> +		    required; otherwise the parser rejects it.
>   EVENT2         : Event name for tprobe. If omitted, the event name is
>                    the same as "TRACEPOINT", but if the "TRACEPOINT" starts
>                    with a digit character, "_TRACEPOINT" is used.
> @@ -40,6 +44,13 @@ Synopsis of fprobe-events
>                    can be probed simultaneously, or 0 for the default value
>                    as defined in Documentation/trace/fprobe.rst
>  
> + SYM		: Function name or comma-separated list of symbols.

Doesn't SYM still be a function name? In above synopsis, SYM is an entry of
comma-separated list.

> +		  - SYM prefixed with "!" are exclusions.
> +		  - ":entry" suffix means it probes entry of given symbols
> +		    (default)
> +		  - ":exit" suffix means it probes exit of given symbols.
> +		  - "%return" suffix means it probes exit of SYM (single
> +		    symbol).


And we need PATTERN here.

  PATTERN     : Function name pattern with wildcards (You can use "*" or "?").

Thank you,

>   FETCHARGS      : Arguments. Each probe can have up to 128 args.
>    ARG           : Fetch "ARG" function argument using BTF (only for function
>                    entry or tracepoint.) (\*1)
> -- 
> 2.43.0
> 
> 


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

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

* Re: [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax
  2026-02-05 13:58 ` [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax Seokwoo Chung (Ryan)
@ 2026-03-24  4:12   ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2026-03-24  4:12 UTC (permalink / raw)
  To: Seokwoo Chung (Ryan)
  Cc: rostedt, corbet, shuah, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest

On Thu,  5 Feb 2026 08:58:42 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:

> Add fprobe_list.tc to test the comma-separated symbol list syntax
> with :entry/:exit suffixes.  Three scenarios are covered:
> 
>   1. List with default (entry) behavior and ! exclusion
>   2. List with explicit :entry suffix
>   3. List with :exit suffix for return probes


Could you also add wildcard pattern test?

> 
> Each test verifies that the correct functions appear in
> enabled_functions and that excluded (!) symbols are absent.
> 
> Note: The existing tests add_remove_fprobe.tc, fprobe_syntax_errors.tc,
> and add_remove_fprobe_repeat.tc check their "requires" line against the
> tracefs README for the old "%return" syntax pattern.  Since the README
> now documents ":entry|:exit" instead, these tests report UNSUPPORTED.
> Their "requires" lines need updating in a follow-up patch.

This means you'll break the selftest. please fix those test first.
(This fix must be done before "tracing/fprobe: Support comma-separated 
symbols and :entry/:exit" so that we can safely bisect it.)

Thank you,


> 
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
>  .../ftrace/test.d/dynevent/fprobe_list.tc     | 92 +++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> new file mode 100644
> index 000000000000..45e57c6f487d
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> @@ -0,0 +1,92 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: Fprobe event list syntax and :entry/:exit suffixes
> +# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]":README
> +
> +# Setup symbols to test. These are common kernel functions.
> +PLACE=vfs_read
> +PLACE2=vfs_write
> +PLACE3=vfs_open
> +
> +echo 0 > events/enable
> +echo > dynamic_events
> +
> +# Get baseline count of enabled functions (should be 0 if clean, but be safe)
> +if [ -f enabled_functions ]; then
> +	ocnt=`cat enabled_functions | wc -l`
> +else
> +	ocnt=0
> +fi
> +
> +# Test 1: List default (entry) with exclusion
> +# Target: Trace vfs_read and vfs_open, but EXCLUDE vfs_write
> +echo "f:test/list_entry $PLACE,!$PLACE2,$PLACE3" >> dynamic_events
> +grep -q "test/list_entry" dynamic_events
> +test -d events/test/list_entry
> +
> +echo 1 > events/test/list_entry/enable
> +
> +grep -q "$PLACE" enabled_functions
> +grep -q "$PLACE3" enabled_functions
> +! grep -q "$PLACE2" enabled_functions
> +
> +# Check count (Baseline + 2 new functions)
> +cnt=`cat enabled_functions | wc -l`
> +if [ $cnt -ne $((ocnt + 2)) ]; then
> +	exit_fail
> +fi
> +
> +# Cleanup Test 1
> +echo 0 > events/test/list_entry/enable
> +echo "-:test/list_entry" >> dynamic_events
> +! grep -q "test/list_entry" dynamic_events
> +
> +# Count should return to baseline
> +cnt=`cat enabled_functions | wc -l`
> +if [ $cnt -ne $ocnt ]; then
> +	exit_fail
> +fi
> +
> +# Test 2: List with explicit :entry suffix
> +# (Should behave exactly like Test 1)
> +echo "f:test/list_entry_exp $PLACE,!$PLACE2,$PLACE3:entry" >> dynamic_events
> +grep -q "test/list_entry_exp" dynamic_events
> +test -d events/test/list_entry_exp
> +
> +echo 1 > events/test/list_entry_exp/enable
> +
> +grep -q "$PLACE" enabled_functions
> +grep -q "$PLACE3" enabled_functions
> +! grep -q "$PLACE2" enabled_functions
> +
> +cnt=`cat enabled_functions | wc -l`
> +if [ $cnt -ne $((ocnt + 2)) ]; then
> +	exit_fail
> +fi
> +
> +# Cleanup Test 2
> +echo 0 > events/test/list_entry_exp/enable
> +echo "-:test/list_entry_exp" >> dynamic_events
> +
> +# Test 3: List with :exit suffix
> +echo "f:test/list_exit $PLACE,!$PLACE2,$PLACE3:exit" >> dynamic_events
> +grep -q "test/list_exit" dynamic_events
> +test -d events/test/list_exit
> +
> +echo 1 > events/test/list_exit/enable
> +
> +# Even for return probes, enabled_functions lists the attached symbols
> +grep -q "$PLACE" enabled_functions
> +grep -q "$PLACE3" enabled_functions
> +! grep -q "$PLACE2" enabled_functions
> +
> +cnt=`cat enabled_functions | wc -l`
> +if [ $cnt -ne $((ocnt + 2)) ]; then
> +	exit_fail
> +fi
> +
> +# Cleanup Test 3
> +echo 0 > events/test/list_exit/enable
> +echo "-:test/list_exit" >> dynamic_events
> +
> +clear_trace
> -- 
> 2.43.0
> 


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

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

end of thread, other threads:[~2026-03-24  4:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-05 13:58 [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Seokwoo Chung (Ryan)
2026-02-05 13:58 ` [PATCH v6 1/4] tracing/fprobe: Support comma-separated symbols and :entry/:exit Seokwoo Chung (Ryan)
2026-03-24  1:50   ` Masami Hiramatsu
2026-02-05 13:58 ` [PATCH v6 2/4] fprobe: Support comma-separated filters in register_fprobe() Seokwoo Chung (Ryan)
2026-03-24  1:59   ` Masami Hiramatsu
2026-02-05 13:58 ` [PATCH v6 3/4] docs: tracing/fprobe: Document list filters and :entry/:exit Seokwoo Chung (Ryan)
2026-03-24  4:07   ` Masami Hiramatsu
2026-02-05 13:58 ` [PATCH v6 4/4] selftests/ftrace: Add accept cases for fprobe list syntax Seokwoo Chung (Ryan)
2026-03-24  4:12   ` Masami Hiramatsu
2026-03-24  1:51 ` [PATCH v6 0/4] tracing/fprobe: Support comma-separated symbol lists and :entry/:exit suffixes Masami Hiramatsu

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