[PATCH v3 0/5] tracing: fprobe: list-style filters,

[PATCH v3 0/5] tracing: fprobe: list-style filters,

[Ryan Chung]

This series aims to extend fprobe with list-style filters and a clear
entry/exist qualifier. Users can now specify a comma-separated symbol
list with ! exclusions, and use a spec-level suffix to select probe
type:

- funcA*, !funcAB, funcC -> entry probes
- funcA*, !funcAB, funcC:entry -> explicit entry
- funcA*, !funcAB, funcC:exit -> return/exit across the whole list

For compatibility, %return remains supported for single, literal
symbols. When a list or wildcard is used, an explicit [GROUP/EVENT is
required and autogeneration is disabled. Autogen names are kept for
single-symbol specs, with wildcard sanitization. For list/wildcard forms
we set ctx->funcname = NULL so BTF lookups are not attempted.

The series moves parsing to the parse path, documents the new syntax,
and adds selftests that accept valid list cases and reject empty tokens,
stray commas, and %return mixed with lists or wildcards. Selftests also
verify enable/disable flow and that entry+exit on the same set do not
double-count attached functions.

Help wanted: This is my first time contributing ftrace selftests. I
would appreciate comments and recommendations on test structure and
coverage.

Basic coverage is included, but this likely needs broader testing across
architectures. Feedback and additional test ideas are welcome.

Changes since v2:
- Introduce spec-level: :entry/:exit; reject %return with
  lists/wildcards
- Require explict [GROUP/]EVENT for list/wildcard; keep autogen only for
  single literal.
- Sanitize autogen names for single-symbol wildcards
- Set ctx->funcname = NULL for list/wildcard to bypass BTF
- Move list parsing out of __register_trace_fprobe() and into the parse
  path
- Update docs and tracefs README and add dynevent selftests for
  accept/reject and enable/disable flow

Link: https://lore.kernel.org/lkml/20250904103219.f4937968362bfff1ecd3f004@kernel.org/

Ryan Chung (5):
  docs: tracing: fprobe: document list filters and :entry/:exit
  tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard
  tracing: fprobe: support comma-separated symbols and :entry/:exit
  selftests/ftrace: dynevent: add reject cases for list/:entry/:exit
  selftests/ftrace: dynevent: add reject cases

 Documentation/trace/fprobetrace.rst           |  27 +-
 kernel/trace/trace.c                          |   3 +-
 kernel/trace/trace_fprobe.c                   | 247 ++++++++++++++----
 .../test.d/dynevent/add_remove_fprobe.tc      | 121 +++++++++
 .../test.d/dynevent/fprobe_syntax_errors.tc   |  13 +
 5 files changed, 349 insertions(+), 62 deletions(-)

-- 
2.43.0

[PATCH v3 1/5] docs: tracing: fprobe: document list filters and :entry/:exit

[Ryan Chung]

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

diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
index b4c2ca3d02c1..629e2d7402bd 100644
--- a/Documentation/trace/fprobetrace.rst
+++ b/Documentation/trace/fprobetrace.rst
@@ -25,21 +25,36 @@ Synopsis of fprobe-events
 -------------------------
 ::
 
-  f[:[GRP1/][EVENT1]] SYM [FETCHARGS]                       : Probe on function entry
-  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]     : Probe on function exit
-  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]                : Probe on tracepoint
+  # fprobe (function entry/exit)
+  f[:[GRP1/][EVENT1]] SYM_OR_LIST[:entry|:exit] [FETCHARGS]
+
+  # legacy single-symbol exit
+  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]
+
+  # Probe on tracepoint
+  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]
 
  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".
+ EVENT1         : Event name for fprobe. If omitted,
+                  - 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.
  MAXACTIVE      : Maximum number of instances of the specified function that
                   can be probed simultaneously, or 0 for the default value
                   as defined in Documentation/trace/fprobe.rst
-
+ SYM_OR_LIST    : Either a single symbol, or a comma-separated list of
+                  include/exclude patterns:
+                  - Tokens are matched as symbols; wildcards may be used.
+                  - Tokens prefixed with '!' are exclusions.
+                  - Examples:
+                        foo             # single literal (entry)
+                        foo:exit        # single literal exit
+                        foo%return      # legacy single-symbol exit
  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

Re: [PATCH v3 1/5] docs: tracing: fprobe: document list filters and :entry/:exit

[Masami Hiramatsu]

On Sun,  5 Oct 2025 08:46:55 +0900
Ryan Chung <seokwoo.chung130@gmail.com> wrote:

> Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> ---
>  Documentation/trace/fprobetrace.rst | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> index b4c2ca3d02c1..629e2d7402bd 100644
> --- a/Documentation/trace/fprobetrace.rst
> +++ b/Documentation/trace/fprobetrace.rst
> @@ -25,21 +25,36 @@ Synopsis of fprobe-events
>  -------------------------
>  ::
>  
> -  f[:[GRP1/][EVENT1]] SYM [FETCHARGS]                       : Probe on function entry
> -  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]     : Probe on function exit
> -  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]                : Probe on tracepoint
> +  # fprobe (function entry/exit)
> +  f[:[GRP1/][EVENT1]] SYM_OR_LIST[:entry|:exit] [FETCHARGS]
> +
> +  # legacy single-symbol exit
> +  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]
> +
> +  # Probe on tracepoint
> +  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]
>  
>   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".
> + EVENT1         : Event name for fprobe. If omitted,
> +                  - 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.
>   MAXACTIVE      : Maximum number of instances of the specified function that
>                    can be probed simultaneously, or 0 for the default value
>                    as defined in Documentation/trace/fprobe.rst
> -
> + SYM_OR_LIST    : Either a single symbol, or a comma-separated list of
> +                  include/exclude patterns:
> +                  - Tokens are matched as symbols; wildcards may be used.
> +                  - Tokens prefixed with '!' are exclusions.
> +                  - Examples:
> +                        foo             # single literal (entry)
> +                        foo:exit        # single literal exit
> +                        foo%return      # legacy single-symbol exit

So you can explain it in syntax formats:

Single function (including wildcard):

  f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS]

Multiple functions:

  f[:[GRP1/]EVENT3 SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS]

Where,
 - 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).

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>

Re: [PATCH v3 1/5] docs: tracing: fprobe: document list filters and :entry/:exit

[Ryan Chung]

On Wed, Oct 08, 2025 at 10:06:11AM +0900, Masami Hiramatsu wrote:
> On Sun,  5 Oct 2025 08:46:55 +0900
> Ryan Chung <seokwoo.chung130@gmail.com> wrote:
> 
> > Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> > ---
> >  Documentation/trace/fprobetrace.rst | 27 +++++++++++++++++++++------
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> > index b4c2ca3d02c1..629e2d7402bd 100644
> > --- a/Documentation/trace/fprobetrace.rst
> > +++ b/Documentation/trace/fprobetrace.rst
> > @@ -25,21 +25,36 @@ Synopsis of fprobe-events
> >  -------------------------
> >  ::
> >  
> > -  f[:[GRP1/][EVENT1]] SYM [FETCHARGS]                       : Probe on function entry
> > -  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]     : Probe on function exit
> > -  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]                : Probe on tracepoint
> > +  # fprobe (function entry/exit)
> > +  f[:[GRP1/][EVENT1]] SYM_OR_LIST[:entry|:exit] [FETCHARGS]
> > +
> > +  # legacy single-symbol exit
> > +  f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS]
> > +
> > +  # Probe on tracepoint
> > +  t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS]
> >  
> >   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".
> > + EVENT1         : Event name for fprobe. If omitted,
> > +                  - 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.
> >   MAXACTIVE      : Maximum number of instances of the specified function that
> >                    can be probed simultaneously, or 0 for the default value
> >                    as defined in Documentation/trace/fprobe.rst
> > -
> > + SYM_OR_LIST    : Either a single symbol, or a comma-separated list of
> > +                  include/exclude patterns:
> > +                  - Tokens are matched as symbols; wildcards may be used.
> > +                  - Tokens prefixed with '!' are exclusions.
> > +                  - Examples:
> > +                        foo             # single literal (entry)
> > +                        foo:exit        # single literal exit
> > +                        foo%return      # legacy single-symbol exit
> 
> So you can explain it in syntax formats:
> 
> Single function (including wildcard):
> 
>   f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS]
> 
> Multiple functions:
> 
>   f[:[GRP1/]EVENT3 SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS]
> 
> Where,
>  - 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).
> 
> 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>o

Hi Masami. Thank you for your coments. I will fold this into v4 and make
sure examples and naming rules match the behavior. Thanks for the
guidance.

Best regards,
Ryan Chung

[PATCH v3 2/5] tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard

[Ryan Chung]

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b3c94fbaf002..ac0d3acc337e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5524,7 +5524,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
-- 
2.43.0

Re: [PATCH v3 2/5] tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard

[Masami Hiramatsu]

This should be a part of [3/5], because when bisecting, the test will check the
README file and check the feature.

Thank you,

On Sun,  5 Oct 2025 08:46:56 +0900
Ryan Chung <seokwoo.chung130@gmail.com> wrote:

> Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> ---
>  kernel/trace/trace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b3c94fbaf002..ac0d3acc337e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5524,7 +5524,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
> -- 
> 2.43.0
> 


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

Re: [PATCH v3 2/5] tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard

[Ryan Chung]

On Wed, Oct 08, 2025 at 09:53:16AM +0900, Masami Hiramatsu wrote:
> This should be a part of [3/5], because when bisecting, the test will check the
> README file and check the feature.
> 
> Thank you,
> 

Ok. I will fold the readme_msg change in [3/5] (the patch that
introduces :entry|:exit and keeps %return for single-symbol input) so
the tracefs README matches the feature during bisection and for tests.

> On Sun,  5 Oct 2025 08:46:56 +0900
> Ryan Chung <seokwoo.chung130@gmail.com> wrote:
> 
> > Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> > ---
> >  kernel/trace/trace.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index b3c94fbaf002..ac0d3acc337e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -5524,7 +5524,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
> > -- 
> > 2.43.0
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit

[Ryan Chung]

Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
---
 kernel/trace/trace_fprobe.c | 247 ++++++++++++++++++++++++++++--------
 1 file changed, 192 insertions(+), 55 deletions(-)

diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index b36ade43d4b3..ec5b6e1c1a1b 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -191,6 +191,9 @@ struct trace_fprobe {
 	bool			tprobe;
 	struct tracepoint_user	*tuser;
 	struct trace_probe	tp;
+	char			*filter;
+	char			*nofilter;
+	bool			list_mode;
 };
 
 static bool is_trace_fprobe(struct dyn_event *ev)
@@ -203,14 +206,10 @@ static struct trace_fprobe *to_trace_fprobe(struct dyn_event *ev)
 	return container_of(ev, struct trace_fprobe, devent);
 }
 
-/**
- * for_each_trace_fprobe - iterate over the trace_fprobe list
- * @pos:	the struct trace_fprobe * for each entry
- * @dpos:	the struct dyn_event * to use as a loop cursor
- */
-#define for_each_trace_fprobe(pos, dpos)	\
-	for_each_dyn_event(dpos)		\
-		if (is_trace_fprobe(dpos) && (pos = to_trace_fprobe(dpos)))
+static struct trace_fprobe *trace_fprobe_from_dyn(struct dyn_event *ev)
+{
+	return is_trace_fprobe(ev) ? to_trace_fprobe(ev) : NULL;
+}
 
 static bool trace_fprobe_is_return(struct trace_fprobe *tf)
 {
@@ -227,6 +226,109 @@ static const char *trace_fprobe_symbol(struct trace_fprobe *tf)
 	return tf->symbol ? tf->symbol : "unknown";
 }
 
+static bool has_wildcard(const char *s)
+{
+	return s && (strchr(s, '*') || strchr(s, '?'));
+}
+
+static int parse_fprobe_spec(const char *in, bool is_tracepoint,
+		char **base, bool *is_return, bool *list_mode,
+		char **filter, char **nofilter)
+{
+	const char *p;
+	char *work = NULL;
+	char *b = NULL, *f = NULL, *nf = NULL;
+	bool legacy_ret = false;
+	bool list = false;
+	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) {
+		if (strchr(in, ',') || strchr(in, ':'))
+			return -EINVAL;
+		if (strstr(in, "%return"))
+			return -EINVAL;
+		for (p = in; *p; p++)
+			if (!isalnum(*p) && *p != '_')
+				return -EINVAL;
+		b = kstrdup(in, GFP_KERNEL);
+		if (!b)
+			return -ENOMEM;
+		*base = b;
+		return 0;
+	}
+
+	work = kstrdup(in, GFP_KERNEL);
+	if (!work)
+		return -ENOMEM;
+
+	p = strstr(work, "%return");
+	if (p) {
+		if (!strcmp(p, ":exit")) {
+			*is_return = true;
+			*p = '\0';
+		} else if (!strcmp(p, ":entry")) {
+			*p = '\0';
+		} else {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
+	list = !!strchr(work, ',') || has_wildcard(work);
+	if (legacy_ret)
+		*is_return = true;
+
+	b = kstrdup(work, GFP_KERNEL);
+	if (!b) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (list) {
+		char *tmp = b, *tok;
+		size_t fsz = strlen(b) + 1, nfsz = strlen(b) + 1;
+
+		f = kzalloc(fsz, GFP_KERNEL);
+		nf = kzalloc(nfsz, GFP_KERNEL);
+		if (!f || !nf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		while ((tok = strsep(&tmp, ",")) != NULL) {
+			char *dst;
+			bool neg = (*tok == '!');
+
+			if (*tok == '\0')
+				continue;
+			if (neg)
+				tok++;
+			dst = neg ? nf : f;
+			if (dst[0] != '\0')
+				strcat(dst, ",");
+			strcat(dst, tok);
+		}
+		*list_mode = true;
+	}
+
+	*base = b; b = NULL;
+	*filter = f; f = NULL;
+	*nofilter = nf; nf = NULL;
+
+out:
+	kfree(work);
+	kfree(b);
+	kfree(f);
+	kfree(nf);
+	return ret;
+}
+
 static bool trace_fprobe_is_busy(struct dyn_event *ev)
 {
 	struct trace_fprobe *tf = to_trace_fprobe(ev);
@@ -556,13 +658,17 @@ 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);
 	}
 }
 
 /* Since alloc_trace_fprobe() can return error, check the pointer is ERR too. */
-DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *, if (!IS_ERR_OR_NULL(_T)) free_trace_fprobe(_T))
+DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *,
+	if (!IS_ERR_OR_NULL(_T))
+		free_trace_fprobe(_T))
 
 /*
  * Allocate new trace_probe and initialize it (including fprobe).
@@ -605,10 +711,16 @@ static struct trace_fprobe *find_trace_fprobe(const char *event,
 	struct dyn_event *pos;
 	struct trace_fprobe *tf;
 
-	for_each_trace_fprobe(tf, pos)
+	list_for_each_entry(pos, &dyn_event_list, list) {
+		tf = trace_fprobe_from_dyn(pos);
+		if (!tf)
+			continue;
+
 		if (strcmp(trace_probe_name(&tf->tp), event) == 0 &&
 		    strcmp(trace_probe_group_name(&tf->tp), group) == 0)
 			return tf;
+	}
+
 	return NULL;
 }
 
@@ -835,7 +947,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);
 }
 
@@ -1114,7 +1231,11 @@ static int __tprobe_event_module_cb(struct notifier_block *self,
 		return NOTIFY_DONE;
 
 	mutex_lock(&event_mutex);
-	for_each_trace_fprobe(tf, pos) {
+	list_for_each_entry(pos, &dyn_event_list, list) {
+		tf = trace_fprobe_from_dyn(pos);
+		if (!tf)
+			continue;
+
 		/* Skip fprobe and disabled tprobe events. */
 		if (!trace_fprobe_is_tracepoint(tf) || !tf->tuser)
 			continue;
@@ -1155,55 +1276,35 @@ static int parse_symbol_and_return(int argc, const char *argv[],
 				   char **symbol, bool *is_return,
 				   bool is_tracepoint)
 {
-	char *tmp = strchr(argv[1], '%');
-	int i;
-
-	if (tmp) {
-		int len = tmp - argv[1];
-
-		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)
-		return -ENOMEM;
-
-	if (*is_return)
-		return 0;
+	int i, ret;
+	bool list_mode = false;
+	char *filter = NULL; *nofilter = NULL;
 
-	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;
-			return -EINVAL;
-		}
-	}
+	ret = parse_fprobe_spec(argv[1], is_tracepoint, symbol, is_return,
+			&list_mode, &filter, &nofilter);
+	if (ret)
+		return ret;
 
-	/* If there is $retval, this should be a return fprobe. */
 	for (i = 2; i < argc; i++) {
-		tmp = strstr(argv[i], "$retval");
+		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);
 				kfree(*symbol);
 				*symbol = NULL;
+				kfree(filter);
+				kfree(nofilter);
 				return -EINVAL;
 			}
 			*is_return = true;
 			break;
 		}
 	}
+
+	kfree(filter);
+	kfree(nofilter);
 	return 0;
 }
 
@@ -1247,6 +1348,11 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 	int i, new_argc = 0, ret = 0;
 	bool is_tracepoint = false;
 	bool is_return = false;
+	bool list_mode = false;
+
+	char *parsed_filter __free(kfree) = NULL;
+	char *parsed_nofilter __free(kfree) = NULL;
+	bool has_wild = false;
 
 	if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
 		return -ECANCELED;
@@ -1267,8 +1373,9 @@ 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;
 
@@ -1283,10 +1390,16 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
 			return -EINVAL;
 	}
 
-	if (!event) {
-		ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
-		if (!ebuf)
-			return -ENOMEM;
+		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;
+		}
 		/* Make a new event name */
 		if (is_tracepoint)
 			snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s%s",
@@ -1319,7 +1432,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);
@@ -1353,6 +1467,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);
@@ -1439,8 +1568,16 @@ 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)) {
+		if (tf->list_mode) {
+			if (trace_fprobe_is_return(tf))
+				seq_puts(m, ":exit");
+		} else {
+			if (trace_fprobe_is_return(tf))
+				seq_puts(m, "%return");
+		}
+	}
 
 	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

Re: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit

[Masami Hiramatsu]

On Sun,  5 Oct 2025 08:46:57 +0900
Ryan Chung <seokwoo.chung130@gmail.com> wrote:

Please describe what this patch adds, for what reason.

> Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> ---
>  kernel/trace/trace_fprobe.c | 247 ++++++++++++++++++++++++++++--------
>  1 file changed, 192 insertions(+), 55 deletions(-)
> 
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index b36ade43d4b3..ec5b6e1c1a1b 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -191,6 +191,9 @@ struct trace_fprobe {
>  	bool			tprobe;
>  	struct tracepoint_user	*tuser;
>  	struct trace_probe	tp;
> +	char			*filter;
> +	char			*nofilter;
> +	bool			list_mode;
>  };
>  
>  static bool is_trace_fprobe(struct dyn_event *ev)
> @@ -203,14 +206,10 @@ static struct trace_fprobe *to_trace_fprobe(struct dyn_event *ev)
>  	return container_of(ev, struct trace_fprobe, devent);
>  }
>  
> -/**
> - * for_each_trace_fprobe - iterate over the trace_fprobe list
> - * @pos:	the struct trace_fprobe * for each entry
> - * @dpos:	the struct dyn_event * to use as a loop cursor
> - */
> -#define for_each_trace_fprobe(pos, dpos)	\
> -	for_each_dyn_event(dpos)		\
> -		if (is_trace_fprobe(dpos) && (pos = to_trace_fprobe(dpos)))

Why remove this? This is for finding all fprobes.

> +static struct trace_fprobe *trace_fprobe_from_dyn(struct dyn_event *ev)
> +{
> +	return is_trace_fprobe(ev) ? to_trace_fprobe(ev) : NULL;
> +}
>  
>  static bool trace_fprobe_is_return(struct trace_fprobe *tf)
>  {
> @@ -227,6 +226,109 @@ static const char *trace_fprobe_symbol(struct trace_fprobe *tf)
>  	return tf->symbol ? tf->symbol : "unknown";
>  }
>  
> +static bool has_wildcard(const char *s)
> +{
> +	return s && (strchr(s, '*') || strchr(s, '?'));
> +}
> +
> +static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> +		char **base, bool *is_return, bool *list_mode,
> +		char **filter, char **nofilter)
> +{
> +	const char *p;
> +	char *work = NULL;
> +	char *b = NULL, *f = NULL, *nf = NULL;

See below (out: label)

> +	bool legacy_ret = false;
> +	bool list = false;
> +	int ret = 0;

nit: sort local variable by line length. (longer to shorter)

> +
> +	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) {
> +		if (strchr(in, ',') || strchr(in, ':'))
> +			return -EINVAL;
> +		if (strstr(in, "%return"))
> +			return -EINVAL;

It seems below loop checks all above cases.

> +		for (p = in; *p; p++)
> +			if (!isalnum(*p) && *p != '_')
> +				return -EINVAL;

This only allows that the @in must be a symbol name.

> +		b = kstrdup(in, GFP_KERNEL);
> +		if (!b)
> +			return -ENOMEM;
> +		*base = b;
> +		return 0;
> +	}
> +
> +	work = kstrdup(in, GFP_KERNEL);
> +	if (!work)
> +		return -ENOMEM;
> +
> +	p = strstr(work, "%return");

Note that strstr does not care it ends with given string.

> +	if (p) {
> +		if (!strcmp(p, ":exit")) {
> +			*is_return = true;
> +			*p = '\0';
> +		} else if (!strcmp(p, ":entry")) {
> +			*p = '\0';
> +		} else {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +	}
> +
> +	list = !!strchr(work, ',') || has_wildcard(work);

Wildcard is OK for legacy.

> +	if (legacy_ret)
> +		*is_return = true;
> +
> +	b = kstrdup(work, GFP_KERNEL);
> +	if (!b) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	if (list) {
> +		char *tmp = b, *tok;
> +		size_t fsz = strlen(b) + 1, nfsz = strlen(b) + 1;

size_t fsz, nfsz;

fsz = nfsz = strlen(b) + 1;

> +
> +		f = kzalloc(fsz, GFP_KERNEL);
> +		nf = kzalloc(nfsz, GFP_KERNEL);
> +		if (!f || !nf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +
> +		while ((tok = strsep(&tmp, ",")) != NULL) {
> +			char *dst;
> +			bool neg = (*tok == '!');
> +
> +			if (*tok == '\0')
> +				continue;
> +			if (neg)
> +				tok++;
> +			dst = neg ? nf : f;
> +			if (dst[0] != '\0')
> +				strcat(dst, ",");
> +			strcat(dst, tok);
> +		}
> +		*list_mode = true;
> +	}
> +
> +	*base = b; b = NULL;
> +	*filter = f; f = NULL;
> +	*nofilter = nf; nf = NULL;
> +
> +out:
> +	kfree(work);
> +	kfree(b);
> +	kfree(f);
> +	kfree(nf);

Instead of using goto only for kfree(), use __free(kfree)
to clean those up automatically.

> +	return ret;
> +}
> +
>  static bool trace_fprobe_is_busy(struct dyn_event *ev)
>  {
>  	struct trace_fprobe *tf = to_trace_fprobe(ev);
> @@ -556,13 +658,17 @@ 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);
>  	}
>  }
>  
>  /* Since alloc_trace_fprobe() can return error, check the pointer is ERR too. */
> -DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *, if (!IS_ERR_OR_NULL(_T)) free_trace_fprobe(_T))
> +DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *,
> +	if (!IS_ERR_OR_NULL(_T))
> +		free_trace_fprobe(_T))

OK, it looks good to clean up. But please do it separated patch.
Do not touch if it is not related to your change.

>  
>  /*
>   * Allocate new trace_probe and initialize it (including fprobe).
> @@ -605,10 +711,16 @@ static struct trace_fprobe *find_trace_fprobe(const char *event,
>  	struct dyn_event *pos;
>  	struct trace_fprobe *tf;
>  
> -	for_each_trace_fprobe(tf, pos)
> +	list_for_each_entry(pos, &dyn_event_list, list) {
> +		tf = trace_fprobe_from_dyn(pos);
> +		if (!tf)
> +			continue;
> +
>  		if (strcmp(trace_probe_name(&tf->tp), event) == 0 &&
>  		    strcmp(trace_probe_group_name(&tf->tp), group) == 0)
>  			return tf;
> +	}
> +

Ditto and there is no need to change.

>  	return NULL;
>  }
>  
> @@ -835,7 +947,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);
>  }
>  
> @@ -1114,7 +1231,11 @@ static int __tprobe_event_module_cb(struct notifier_block *self,
>  		return NOTIFY_DONE;
>  
>  	mutex_lock(&event_mutex);
> -	for_each_trace_fprobe(tf, pos) {
> +	list_for_each_entry(pos, &dyn_event_list, list) {
> +		tf = trace_fprobe_from_dyn(pos);
> +		if (!tf)
> +			continue;
> +
>  		/* Skip fprobe and disabled tprobe events. */
>  		if (!trace_fprobe_is_tracepoint(tf) || !tf->tuser)
>  			continue;
> @@ -1155,55 +1276,35 @@ static int parse_symbol_and_return(int argc, const char *argv[],
>  				   char **symbol, bool *is_return,
>  				   bool is_tracepoint)
>  {
> -	char *tmp = strchr(argv[1], '%');
> -	int i;
> -
> -	if (tmp) {
> -		int len = tmp - argv[1];
> -
> -		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)
> -		return -ENOMEM;
> -
> -	if (*is_return)
> -		return 0;
> +	int i, ret;
> +	bool list_mode = false;
> +	char *filter = NULL; *nofilter = NULL;

Sort it as other functions. longer line to shorter.

>  
> -	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;
> -			return -EINVAL;
> -		}
> -	}
> +	ret = parse_fprobe_spec(argv[1], is_tracepoint, symbol, is_return,
> +			&list_mode, &filter, &nofilter);
> +	if (ret)
> +		return ret;
>  
> -	/* If there is $retval, this should be a return fprobe. */
>  	for (i = 2; i < argc; i++) {
> -		tmp = strstr(argv[i], "$retval");
> +		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);
>  				kfree(*symbol);
>  				*symbol = NULL;
> +				kfree(filter);
> +				kfree(nofilter);
>  				return -EINVAL;
>  			}
>  			*is_return = true;
>  			break;
>  		}
>  	}
> +
> +	kfree(filter);
> +	kfree(nofilter);
>  	return 0;
>  }
>  
> @@ -1247,6 +1348,11 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  	int i, new_argc = 0, ret = 0;
>  	bool is_tracepoint = false;
>  	bool is_return = false;
> +	bool list_mode = false;
> +

Do not split local variable definitions with empty lines.

> +	char *parsed_filter __free(kfree) = NULL;
> +	char *parsed_nofilter __free(kfree) = NULL;
> +	bool has_wild = false;

Please sort.

>  
>  	if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
>  		return -ECANCELED;
> @@ -1267,8 +1373,9 @@ 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);

Hmm, if so, where is the parse_symbol_and_return() called?
I think you can pick the $retval search loop from the 
parse_symbol_and_return() for updating is_return (or make
it failure if is_tracepoint == true).

>  	if (ret < 0)
>  		return -EINVAL;
>  
> @@ -1283,10 +1390,16 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>  			return -EINVAL;
>  	}
>  
> -	if (!event) {
> -		ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> -		if (!ebuf)
> -			return -ENOMEM;
> +		if (!event) {
> +		/*
> +		 * Event name rules:
> +		 * - For list/wildcard: require explicit [GROUP/]EVENT
> +		 * - For single literal: autogenerate symbol__entry/symbol__exit
> +		 */

nit: to avoid confusing, comment should be indented as same as the
code. Or, put the comment right before the `if`.

> +			if (list_mode || has_wildcard(symbol)) {
> +				trace_probe_log_err(0, NO_GROUP_NAME);
> +			return -EINVAL;
> +		}
>  		/* Make a new event name */
>  		if (is_tracepoint)
>  			snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s%s",
> @@ -1319,7 +1432,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);
> @@ -1353,6 +1467,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;
> +			}
> +		}

Odd indentation. Please fix.

> +
>  	/* parse arguments */
>  	for (i = 0; i < argc; i++) {
>  		trace_probe_log_set_index(i + 2);
> @@ -1439,8 +1568,16 @@ 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)) {
> +		if (tf->list_mode) {
> +			if (trace_fprobe_is_return(tf))
> +				seq_puts(m, ":exit");

In both cases, we can use ":exit" suffix. This means we will
accept legacy "%return" for backward compatibility, but
shows ":exit" always.

> +		} else {
> +			if (trace_fprobe_is_return(tf))
> +				seq_puts(m, "%return");
> +		}
> +	}
>  
>  	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
> 

Thank you,


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

Re: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit

[Ryan Chung]

Hi. Thanks for the quick review and comments. Here are my responses to
the remaining points.

On Wed, Oct 08, 2025 at 07:09:37PM +0900, Masami Hiramatsu wrote:
> On Sun,  5 Oct 2025 08:46:57 +0900
> Ryan Chung <seokwoo.chung130@gmail.com> wrote:
> 
> Please describe what this patch adds, for what reason.
> 

This is my mistake; I forgot to do so. I will make sure to include it
next time.

> > Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
> > ---
> >  kernel/trace/trace_fprobe.c | 247 ++++++++++++++++++++++++++++--------
> >  1 file changed, 192 insertions(+), 55 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> > index b36ade43d4b3..ec5b6e1c1a1b 100644
> > --- a/kernel/trace/trace_fprobe.c
> > +++ b/kernel/trace/trace_fprobe.c
> > @@ -191,6 +191,9 @@ struct trace_fprobe {
> >  	bool			tprobe;
> >  	struct tracepoint_user	*tuser;
> >  	struct trace_probe	tp;
> > +	char			*filter;
> > +	char			*nofilter;
> > +	bool			list_mode;
> >  };
> >  
> >  static bool is_trace_fprobe(struct dyn_event *ev)
> > @@ -203,14 +206,10 @@ static struct trace_fprobe *to_trace_fprobe(struct dyn_event *ev)
> >  	return container_of(ev, struct trace_fprobe, devent);
> >  }
> >  
> > -/**
> > - * for_each_trace_fprobe - iterate over the trace_fprobe list
> > - * @pos:	the struct trace_fprobe * for each entry
> > - * @dpos:	the struct dyn_event * to use as a loop cursor
> > - */
> > -#define for_each_trace_fprobe(pos, dpos)	\
> > -	for_each_dyn_event(dpos)		\
> > -		if (is_trace_fprobe(dpos) && (pos = to_trace_fprobe(dpos)))
> 
> Why remove this? This is for finding all fprobes.
> 

I will revert this and keep for_each_trace_fprobe as is. 

> > +static struct trace_fprobe *trace_fprobe_from_dyn(struct dyn_event *ev)
> > +{
> > +	return is_trace_fprobe(ev) ? to_trace_fprobe(ev) : NULL;
> > +}
> >  
> >  static bool trace_fprobe_is_return(struct trace_fprobe *tf)
> >  {
> > @@ -227,6 +226,109 @@ static const char *trace_fprobe_symbol(struct trace_fprobe *tf)
> >  	return tf->symbol ? tf->symbol : "unknown";
> >  }
> >  
> > +static bool has_wildcard(const char *s)
> > +{
> > +	return s && (strchr(s, '*') || strchr(s, '?'));
> > +}
> > +
> > +static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> > +		char **base, bool *is_return, bool *list_mode,
> > +		char **filter, char **nofilter)
> > +{
> > +	const char *p;
> > +	char *work = NULL;
> > +	char *b = NULL, *f = NULL, *nf = NULL;
> 
> See below (out: label)
> 

I will switch those temporaries to __free(kfree) and drop the goto that
existed only to kfree. This addresses the cleanup pattern comment.

> > +	bool legacy_ret = false;
> > +	bool list = false;
> > +	int ret = 0;
> 
> nit: sort local variable by line length. (longer to shorter)
> 

Ok. I will sort locals longest -> shortest and fix a few initializations
(char *filter = NULL, char *nofilter = Null;).

> > +
> > +	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) {
> > +		if (strchr(in, ',') || strchr(in, ':'))
> > +			return -EINVAL;
> > +		if (strstr(in, "%return"))
> > +			return -EINVAL;
> 
> It seems below loop checks all above cases.
> 

I will remove the redundant pre-checks and rely on the validation loop,
with precise rules. 

> > +		for (p = in; *p; p++)
> > +			if (!isalnum(*p) && *p != '_')
> > +				return -EINVAL;
> 
> This only allows that the @in must be a symbol name.
> 

Just to clarify: should tracepoint arguments support the subsystem:event
format (e.g., "sched:sched_switch"), or should they remain restricted to
simple symbol names only? The current validation enforces
symbol-name-only, but I wanted to confirm this is the intended behavior
before next version.

> > +		b = kstrdup(in, GFP_KERNEL);
> > +		if (!b)
> > +			return -ENOMEM;
> > +		*base = b;
> > +		return 0;
> > +	}
> > +
> > +	work = kstrdup(in, GFP_KERNEL);
> > +	if (!work)
> > +		return -ENOMEM;
> > +
> > +	p = strstr(work, "%return");
> 
> Note that strstr does not care it ends with given string.
> 

Good catch. I will replace it with explicit end-of-string checks so we
accept only a single terminal suffix: %return, :entry, or :exit.
Partial/embedded matches will be rejected.

> > +	if (p) {
> > +		if (!strcmp(p, ":exit")) {
> > +			*is_return = true;
> > +			*p = '\0';
> > +		} else if (!strcmp(p, ":entry")) {
> > +			*p = '\0';
> > +		} else {
> > +			ret = -EINVAL;
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	list = !!strchr(work, ',') || has_wildcard(work);
> 
> Wildcard is OK for legacy.
> 

I will keep the wildcard acceptance for the legacy string, and treat
presence of "," or wildcard as "list mode" that builds filter/nofilter
for register_fprobe(); otherwise it remains single-symbol legacy.

> > +	if (legacy_ret)
> > +		*is_return = true;
> > +
> > +	b = kstrdup(work, GFP_KERNEL);
> > +	if (!b) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	if (list) {
> > +		char *tmp = b, *tok;
> > +		size_t fsz = strlen(b) + 1, nfsz = strlen(b) + 1;
> 
> size_t fsz, nfsz;
> 
> fsz = nfsz = strlen(b) + 1;
> 

I will adopt the above style.

> > +
> > +		f = kzalloc(fsz, GFP_KERNEL);
> > +		nf = kzalloc(nfsz, GFP_KERNEL);
> > +		if (!f || !nf) {
> > +			ret = -ENOMEM;
> > +			goto out;
> > +		}
> > +
> > +		while ((tok = strsep(&tmp, ",")) != NULL) {
> > +			char *dst;
> > +			bool neg = (*tok == '!');
> > +
> > +			if (*tok == '\0')
> > +				continue;
> > +			if (neg)
> > +				tok++;
> > +			dst = neg ? nf : f;
> > +			if (dst[0] != '\0')
> > +				strcat(dst, ",");
> > +			strcat(dst, tok);
> > +		}
> > +		*list_mode = true;
> > +	}
> > +
> > +	*base = b; b = NULL;
> > +	*filter = f; f = NULL;
> > +	*nofilter = nf; nf = NULL;
> > +
> > +out:
> > +	kfree(work);
> > +	kfree(b);
> > +	kfree(f);
> > +	kfree(nf);
> 
> Instead of using goto only for kfree(), use __free(kfree)
> to clean those up automatically.
> 

Ok. As mentioned above, I will convert all such temporaries to
__free(kfree) and remove the goto cleanup.

> > +	return ret;
> > +}
> > +
> >  static bool trace_fprobe_is_busy(struct dyn_event *ev)
> >  {
> >  	struct trace_fprobe *tf = to_trace_fprobe(ev);
> > @@ -556,13 +658,17 @@ 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);
> >  	}
> >  }
> >  
> >  /* Since alloc_trace_fprobe() can return error, check the pointer is ERR too. */
> > -DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *, if (!IS_ERR_OR_NULL(_T)) free_trace_fprobe(_T))
> > +DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *,
> > +	if (!IS_ERR_OR_NULL(_T))
> > +		free_trace_fprobe(_T))
> 
> OK, it looks good to clean up. But please do it separated patch.
> Do not touch if it is not related to your change.
> 

Do you want this to be in a separate series or for this patch series?

> >  
> >  /*
> >   * Allocate new trace_probe and initialize it (including fprobe).
> > @@ -605,10 +711,16 @@ static struct trace_fprobe *find_trace_fprobe(const char *event,
> >  	struct dyn_event *pos;
> >  	struct trace_fprobe *tf;
> >  
> > -	for_each_trace_fprobe(tf, pos)
> > +	list_for_each_entry(pos, &dyn_event_list, list) {
> > +		tf = trace_fprobe_from_dyn(pos);
> > +		if (!tf)
> > +			continue;
> > +
> >  		if (strcmp(trace_probe_name(&tf->tp), event) == 0 &&
> >  		    strcmp(trace_probe_group_name(&tf->tp), group) == 0)
> >  			return tf;
> > +	}
> > +
> 
> Ditto and there is no need to change.
> 

Ok. I will revert those sites to the existing macro-based iteration.

> >  	return NULL;
> >  }
> >  
> > @@ -835,7 +947,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);
> >  }
> >  
> > @@ -1114,7 +1231,11 @@ static int __tprobe_event_module_cb(struct notifier_block *self,
> >  		return NOTIFY_DONE;
> >  
> >  	mutex_lock(&event_mutex);
> > -	for_each_trace_fprobe(tf, pos) {
> > +	list_for_each_entry(pos, &dyn_event_list, list) {
> > +		tf = trace_fprobe_from_dyn(pos);
> > +		if (!tf)
> > +			continue;
> > +
> >  		/* Skip fprobe and disabled tprobe events. */
> >  		if (!trace_fprobe_is_tracepoint(tf) || !tf->tuser)
> >  			continue;
> > @@ -1155,55 +1276,35 @@ static int parse_symbol_and_return(int argc, const char *argv[],
> >  				   char **symbol, bool *is_return,
> >  				   bool is_tracepoint)
> >  {
> > -	char *tmp = strchr(argv[1], '%');
> > -	int i;
> > -
> > -	if (tmp) {
> > -		int len = tmp - argv[1];
> > -
> > -		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)
> > -		return -ENOMEM;
> > -
> > -	if (*is_return)
> > -		return 0;
> > +	int i, ret;
> > +	bool list_mode = false;
> > +	char *filter = NULL; *nofilter = NULL;
> 
> Sort it as other functions. longer line to shorter.
> 

I did not know this. I will fix the ordering (and see next item about
the function's role).

> >  
> > -	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;
> > -			return -EINVAL;
> > -		}
> > -	}
> > +	ret = parse_fprobe_spec(argv[1], is_tracepoint, symbol, is_return,
> > +			&list_mode, &filter, &nofilter);
> > +	if (ret)
> > +		return ret;
> >  
> > -	/* If there is $retval, this should be a return fprobe. */
> >  	for (i = 2; i < argc; i++) {
> > -		tmp = strstr(argv[i], "$retval");
> > +		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);
> >  				kfree(*symbol);
> >  				*symbol = NULL;
> > +				kfree(filter);
> > +				kfree(nofilter);
> >  				return -EINVAL;
> >  			}
> >  			*is_return = true;
> >  			break;
> >  		}
> >  	}
> > +
> > +	kfree(filter);
> > +	kfree(nofilter);
> >  	return 0;
> >  }
> >  
> > @@ -1247,6 +1348,11 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
> >  	int i, new_argc = 0, ret = 0;
> >  	bool is_tracepoint = false;
> >  	bool is_return = false;
> > +	bool list_mode = false;
> > +
> 
> Do not split local variable definitions with empty lines.
> 

I will collapse those blocks.

> > +	char *parsed_filter __free(kfree) = NULL;
> > +	char *parsed_nofilter __free(kfree) = NULL;
> > +	bool has_wild = false;
> 
> Please sort.
> 

I will sort and group them, so no empty line splits.

> >  
> >  	if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
> >  		return -ECANCELED;
> > @@ -1267,8 +1373,9 @@ 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);
> 
> Hmm, if so, where is the parse_symbol_and_return() called?
> I think you can pick the $retval search loop from the 
> parse_symbol_and_return() for updating is_return (or make
> it failure if is_tracepoint == true).
> 

Makes sense. I will fold the $retval scan into the new parser so there's
a single source of truth. $retval will remain rejected for tracepoints
with a proper error index. parse_symbol_and_return() can then be removed
or turned into a thin wrapper if still referenced.

> >  	if (ret < 0)
> >  		return -EINVAL;
> >  
> > @@ -1283,10 +1390,16 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
> >  			return -EINVAL;
> >  	}
> >  
> > -	if (!event) {
> > -		ebuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
> > -		if (!ebuf)
> > -			return -ENOMEM;
> > +		if (!event) {
> > +		/*
> > +		 * Event name rules:
> > +		 * - For list/wildcard: require explicit [GROUP/]EVENT
> > +		 * - For single literal: autogenerate symbol__entry/symbol__exit
> > +		 */
> 
> nit: to avoid confusing, comment should be indented as same as the
> code. Or, put the comment right before the `if`.
> 

I will move the comment above the if and align indentation. 

> > +			if (list_mode || has_wildcard(symbol)) {
> > +				trace_probe_log_err(0, NO_GROUP_NAME);
> > +			return -EINVAL;
> > +		}
> >  		/* Make a new event name */
> >  		if (is_tracepoint)
> >  			snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s%s",
> > @@ -1319,7 +1432,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);
> > @@ -1353,6 +1467,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;
> > +			}
> > +		}
> 
> Odd indentation. Please fix.
> 

My mistake. I will fix the indentation here.

> > +
> >  	/* parse arguments */
> >  	for (i = 0; i < argc; i++) {
> >  		trace_probe_log_set_index(i + 2);
> > @@ -1439,8 +1568,16 @@ 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)) {
> > +		if (tf->list_mode) {
> > +			if (trace_fprobe_is_return(tf))
> > +				seq_puts(m, ":exit");
> 
> In both cases, we can use ":exit" suffix. This means we will
> accept legacy "%return" for backward compatibility, but
> shows ":exit" always.
> 

I will make show always print :exit for return probes, regardless of the
input form, and never print %return.

> > +		} else {
> > +			if (trace_fprobe_is_return(tf))
> > +				seq_puts(m, "%return");
> > +		}
> > +	}
> >  
> >  	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
> > 
> 
> Thank you,
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit

[kernel test robot]

Hi Ryan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.17]
[also build test WARNING on linus/master next-20251010]
[cannot apply to trace/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chung/docs-tracing-fprobe-document-list-filters-and-entry-exit/20251010-111713
base:   v6.17
patch link:    https://lore.kernel.org/r/20251004235001.133111-4-seokwoo.chung130%40gmail.com
patch subject: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit
config: x86_64-randconfig-073-20251010 (https://download.01.org/0day-ci/archive/20251010/202510102214.7msIkpAr-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510102214.7msIkpAr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510102214.7msIkpAr-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/trace/trace_fprobe.c:274:7: error: read-only variable is not assignable
     274 |                         *p = '\0';
         |                         ~~ ^
   kernel/trace/trace_fprobe.c:276:7: error: read-only variable is not assignable
     276 |                         *p = '\0';
         |                         ~~ ^
   kernel/trace/trace_fprobe.c:1281:24: error: use of undeclared identifier 'nofilter'; did you mean 'filter'?
    1281 |         char *filter = NULL; *nofilter = NULL;
         |                               ^~~~~~~~
         |                               filter
   kernel/trace/trace_fprobe.c:1281:8: note: 'filter' declared here
    1281 |         char *filter = NULL; *nofilter = NULL;
         |               ^
   kernel/trace/trace_fprobe.c:1284:26: error: use of undeclared identifier 'nofilter'; did you mean 'filter'?
    1284 |                         &list_mode, &filter, &nofilter);
         |                                               ^~~~~~~~
         |                                               filter
   kernel/trace/trace_fprobe.c:1281:8: note: 'filter' declared here
    1281 |         char *filter = NULL; *nofilter = NULL;
         |               ^
   kernel/trace/trace_fprobe.c:1298:11: error: use of undeclared identifier 'nofilter'; did you mean 'filter'?
    1298 |                                 kfree(nofilter);
         |                                       ^~~~~~~~
         |                                       filter
   kernel/trace/trace_fprobe.c:1281:8: note: 'filter' declared here
    1281 |         char *filter = NULL; *nofilter = NULL;
         |               ^
   kernel/trace/trace_fprobe.c:1307:8: error: use of undeclared identifier 'nofilter'; did you mean 'filter'?
    1307 |         kfree(nofilter);
         |               ^~~~~~~~
         |               filter
   kernel/trace/trace_fprobe.c:1281:8: note: 'filter' declared here
    1281 |         char *filter = NULL; *nofilter = NULL;
         |               ^
>> kernel/trace/trace_fprobe.c:1355:7: warning: unused variable 'has_wild' [-Wunused-variable]
    1355 |         bool has_wild = false;
         |              ^~~~~~~~
   1 warning and 6 errors generated.


vim +/has_wild +1355 kernel/trace/trace_fprobe.c

  1274	
  1275	static int parse_symbol_and_return(int argc, const char *argv[],
  1276					   char **symbol, bool *is_return,
  1277					   bool is_tracepoint)
  1278	{
  1279		int i, ret;
  1280		bool list_mode = false;
  1281		char *filter = NULL; *nofilter = NULL;
  1282	
  1283		ret = parse_fprobe_spec(argv[1], is_tracepoint, symbol, is_return,
  1284				&list_mode, &filter, &nofilter);
  1285		if (ret)
  1286			return ret;
  1287	
  1288		for (i = 2; i < argc; i++) {
  1289			char *tmp = strstr(argv[i], "$retval");
  1290	
  1291			if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') {
  1292				if (is_tracepoint) {
  1293					trace_probe_log_set_index(i);
  1294					trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE);
  1295					kfree(*symbol);
  1296					*symbol = NULL;
  1297					kfree(filter);
  1298					kfree(nofilter);
  1299					return -EINVAL;
  1300				}
  1301				*is_return = true;
  1302				break;
  1303			}
  1304		}
  1305	
  1306		kfree(filter);
> 1307		kfree(nofilter);
  1308		return 0;
  1309	}
  1310	
  1311	static int trace_fprobe_create_internal(int argc, const char *argv[],
  1312						struct traceprobe_parse_context *ctx)
  1313	{
  1314		/*
  1315		 * Argument syntax:
  1316		 *  - Add fentry probe:
  1317		 *      f[:[GRP/][EVENT]] [MOD:]KSYM [FETCHARGS]
  1318		 *  - Add fexit probe:
  1319		 *      f[N][:[GRP/][EVENT]] [MOD:]KSYM%return [FETCHARGS]
  1320		 *  - Add tracepoint probe:
  1321		 *      t[:[GRP/][EVENT]] TRACEPOINT [FETCHARGS]
  1322		 *
  1323		 * Fetch args:
  1324		 *  $retval	: fetch return value
  1325		 *  $stack	: fetch stack address
  1326		 *  $stackN	: fetch Nth entry of stack (N:0-)
  1327		 *  $argN	: fetch Nth argument (N:1-)
  1328		 *  $comm       : fetch current task comm
  1329		 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
  1330		 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
  1331		 * Dereferencing memory fetch:
  1332		 *  +|-offs(ARG) : fetch memory at ARG +|- offs address.
  1333		 * Alias name of args:
  1334		 *  NAME=FETCHARG : set NAME as alias of FETCHARG.
  1335		 * Type of args:
  1336		 *  FETCHARG:TYPE : use TYPE instead of unsigned long.
  1337		 */
  1338		struct trace_fprobe *tf __free(free_trace_fprobe) = NULL;
  1339		const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
  1340		struct module *mod __free(module_put) = NULL;
  1341		const char **new_argv __free(kfree) = NULL;
  1342		char *symbol __free(kfree) = NULL;
  1343		char *ebuf __free(kfree) = NULL;
  1344		char *gbuf __free(kfree) = NULL;
  1345		char *sbuf __free(kfree) = NULL;
  1346		char *abuf __free(kfree) = NULL;
  1347		char *dbuf __free(kfree) = NULL;
  1348		int i, new_argc = 0, ret = 0;
  1349		bool is_tracepoint = false;
  1350		bool is_return = false;
  1351		bool list_mode = false;
  1352	
  1353		char *parsed_filter __free(kfree) = NULL;
  1354		char *parsed_nofilter __free(kfree) = NULL;
> 1355		bool has_wild = false;
  1356	
  1357		if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
  1358			return -ECANCELED;
  1359	
  1360		if (argv[0][0] == 't') {
  1361			is_tracepoint = true;
  1362			group = TRACEPOINT_EVENT_SYSTEM;
  1363		}
  1364	
  1365		if (argv[0][1] != '\0') {
  1366			if (argv[0][1] != ':') {
  1367				trace_probe_log_set_index(0);
  1368				trace_probe_log_err(1, BAD_MAXACT);
  1369				return -EINVAL;
  1370			}
  1371			event = &argv[0][2];
  1372		}
  1373	
  1374		trace_probe_log_set_index(1);
  1375	
  1376		/* Parse spec early (single vs list, suffix, base symbol) */
  1377		ret = parse_fprobe_spec(argv[1], is_tracepoint, &symbol, &is_return,
  1378				&list_mode, &parsed_filter, &parsed_nofilter);
  1379		if (ret < 0)
  1380			return -EINVAL;
  1381	
  1382		trace_probe_log_set_index(0);
  1383		if (event) {
  1384			gbuf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
  1385			if (!gbuf)
  1386				return -ENOMEM;
  1387			ret = traceprobe_parse_event_name(&event, &group, gbuf,
  1388							  event - argv[0]);
  1389			if (ret)
  1390				return -EINVAL;
  1391		}
  1392	
  1393			if (!event) {
  1394			/*
  1395			 * Event name rules:
  1396			 * - For list/wildcard: require explicit [GROUP/]EVENT
  1397			 * - For single literal: autogenerate symbol__entry/symbol__exit
  1398			 */
  1399				if (list_mode || has_wildcard(symbol)) {
  1400					trace_probe_log_err(0, NO_GROUP_NAME);
  1401				return -EINVAL;
  1402			}
  1403			/* Make a new event name */
  1404			if (is_tracepoint)
  1405				snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s%s",
  1406					 isdigit(*symbol) ? "_" : "", symbol);
  1407			else
  1408				snprintf(ebuf, MAX_EVENT_NAME_LEN, "%s__%s", symbol,
  1409					 is_return ? "exit" : "entry");
  1410			sanitize_event_name(ebuf);
  1411			event = ebuf;
  1412		}
  1413	
  1414		if (is_return)
  1415			ctx->flags |= TPARG_FL_RETURN;
  1416		else
  1417			ctx->flags |= TPARG_FL_FENTRY;
  1418	
  1419		ctx->funcname = NULL;
  1420		if (is_tracepoint) {
  1421			/* Get tracepoint and lock its module until the end of the registration. */
  1422			struct tracepoint *tpoint;
  1423	
  1424			ctx->flags |= TPARG_FL_TPOINT;
  1425			mod = NULL;
  1426			tpoint = find_tracepoint(symbol, &mod);
  1427			if (tpoint) {
  1428				sbuf = kmalloc(KSYM_NAME_LEN, GFP_KERNEL);
  1429				if (!sbuf)
  1430					return -ENOMEM;
  1431				ctx->funcname = kallsyms_lookup((unsigned long)tpoint->probestub,
  1432								NULL, NULL, NULL, sbuf);
  1433			}
  1434		}
  1435	
  1436		if (!list_mode && !has_wildcard(symbol) && !is_tracepoint)
  1437			ctx->funcname = symbol;
  1438	
  1439		abuf = kmalloc(MAX_BTF_ARGS_LEN, GFP_KERNEL);
  1440		if (!abuf)
  1441			return -ENOMEM;
  1442		argc -= 2; argv += 2;
  1443		new_argv = traceprobe_expand_meta_args(argc, argv, &new_argc,
  1444						       abuf, MAX_BTF_ARGS_LEN, ctx);
  1445		if (IS_ERR(new_argv))
  1446			return PTR_ERR(new_argv);
  1447		if (new_argv) {
  1448			argc = new_argc;
  1449			argv = new_argv;
  1450		}
  1451		if (argc > MAX_TRACE_ARGS) {
  1452			trace_probe_log_set_index(2);
  1453			trace_probe_log_err(0, TOO_MANY_ARGS);
  1454			return -E2BIG;
  1455		}
  1456	
  1457		ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
  1458		if (ret)
  1459			return ret;
  1460	
  1461		/* setup a probe */
  1462		tf = alloc_trace_fprobe(group, event, symbol, argc, is_return, is_tracepoint);
  1463		if (IS_ERR(tf)) {
  1464			ret = PTR_ERR(tf);
  1465			/* This must return -ENOMEM, else there is a bug */
  1466			WARN_ON_ONCE(ret != -ENOMEM);
  1467			return ret;
  1468		}
  1469	
  1470		/* carry list parsing result into tf */
  1471		if (!is_tracepoint) {
  1472			tf->list_mode = list_mode;
  1473				if (parsed_filter) {
  1474					tf->filter = kstrdup(parsed_filter, GFP_KERNEL);
  1475					if (!tf->filter)
  1476						return -ENOMEM;
  1477				}
  1478				if (parsed_nofilter) {
  1479					tf->nofilter = kstrdup(parsed_nofilter, GFP_KERNEL);
  1480					if (!tf->nofilter)
  1481						return -ENOMEM;
  1482				}
  1483			}
  1484	
  1485		/* parse arguments */
  1486		for (i = 0; i < argc; i++) {
  1487			trace_probe_log_set_index(i + 2);
  1488			ctx->offset = 0;
  1489			ret = traceprobe_parse_probe_arg(&tf->tp, i, argv[i], ctx);
  1490			if (ret)
  1491				return ret;	/* This can be -ENOMEM */
  1492		}
  1493	
  1494		if (is_return && tf->tp.entry_arg) {
  1495			tf->fp.entry_handler = trace_fprobe_entry_handler;
  1496			tf->fp.entry_data_size = traceprobe_get_entry_data_size(&tf->tp);
  1497			if (ALIGN(tf->fp.entry_data_size, sizeof(long)) > MAX_FPROBE_DATA_SIZE) {
  1498				trace_probe_log_set_index(2);
  1499				trace_probe_log_err(0, TOO_MANY_EARGS);
  1500				return -E2BIG;
  1501			}
  1502		}
  1503	
  1504		ret = traceprobe_set_print_fmt(&tf->tp,
  1505				is_return ? PROBE_PRINT_RETURN : PROBE_PRINT_NORMAL);
  1506		if (ret < 0)
  1507			return ret;
  1508	
  1509		ret = register_trace_fprobe_event(tf);
  1510		if (ret) {
  1511			trace_probe_log_set_index(1);
  1512			if (ret == -EILSEQ)
  1513				trace_probe_log_err(0, BAD_INSN_BNDRY);
  1514			else if (ret == -ENOENT)
  1515				trace_probe_log_err(0, BAD_PROBE_ADDR);
  1516			else if (ret != -ENOMEM && ret != -EEXIST)
  1517				trace_probe_log_err(0, FAIL_REG_PROBE);
  1518			return -EINVAL;
  1519		}
  1520	
  1521		/* 'tf' is successfully registered. To avoid freeing, assign NULL. */
  1522		tf = NULL;
  1523	
  1524		return 0;
  1525	}
  1526	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit

[kernel test robot]

Hi Ryan,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.17]
[also build test ERROR on linus/master next-20251010]
[cannot apply to trace/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ryan-Chung/docs-tracing-fprobe-document-list-filters-and-entry-exit/20251010-111713
base:   v6.17
patch link:    https://lore.kernel.org/r/20251004235001.133111-4-seokwoo.chung130%40gmail.com
patch subject: [PATCH v3 3/5] tracing: fprobe: support comma-separated symbols and :entry/:exit
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20251010/202510102331.y36ENO9m-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510102331.y36ENO9m-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510102331.y36ENO9m-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   kernel/trace/trace_fprobe.c: In function 'parse_fprobe_spec':
>> kernel/trace/trace_fprobe.c:274:28: error: assignment of read-only location '*p'
     274 |                         *p = '\0';
         |                            ^
   kernel/trace/trace_fprobe.c:276:28: error: assignment of read-only location '*p'
     276 |                         *p = '\0';
         |                            ^
   kernel/trace/trace_fprobe.c: In function 'parse_symbol_and_return':
>> kernel/trace/trace_fprobe.c:1281:31: error: 'nofilter' undeclared (first use in this function); did you mean 'filter'?
    1281 |         char *filter = NULL; *nofilter = NULL;
         |                               ^~~~~~~~
         |                               filter
   kernel/trace/trace_fprobe.c:1281:31: note: each undeclared identifier is reported only once for each function it appears in
   kernel/trace/trace_fprobe.c: In function 'trace_fprobe_create_internal':
   kernel/trace/trace_fprobe.c:1355:14: warning: unused variable 'has_wild' [-Wunused-variable]
    1355 |         bool has_wild = false;
         |              ^~~~~~~~
   kernel/trace/trace_fprobe.c: At top level:
>> kernel/trace/trace_fprobe.c:1275:12: warning: 'parse_symbol_and_return' defined but not used [-Wunused-function]
    1275 | static int parse_symbol_and_return(int argc, const char *argv[],
         |            ^~~~~~~~~~~~~~~~~~~~~~~


vim +274 kernel/trace/trace_fprobe.c

   233	
   234	static int parse_fprobe_spec(const char *in, bool is_tracepoint,
   235			char **base, bool *is_return, bool *list_mode,
   236			char **filter, char **nofilter)
   237	{
   238		const char *p;
   239		char *work = NULL;
   240		char *b = NULL, *f = NULL, *nf = NULL;
   241		bool legacy_ret = false;
   242		bool list = false;
   243		int ret = 0;
   244	
   245		if (!in || !base || !is_return || !list_mode || !filter || !nofilter)
   246			return -EINVAL;
   247	
   248		*base = NULL; *filter = NULL; *nofilter = NULL;
   249		*is_return = false; *list_mode = false;
   250	
   251		if (is_tracepoint) {
   252			if (strchr(in, ',') || strchr(in, ':'))
   253				return -EINVAL;
   254			if (strstr(in, "%return"))
   255				return -EINVAL;
   256			for (p = in; *p; p++)
   257				if (!isalnum(*p) && *p != '_')
   258					return -EINVAL;
   259			b = kstrdup(in, GFP_KERNEL);
   260			if (!b)
   261				return -ENOMEM;
   262			*base = b;
   263			return 0;
   264		}
   265	
   266		work = kstrdup(in, GFP_KERNEL);
   267		if (!work)
   268			return -ENOMEM;
   269	
   270		p = strstr(work, "%return");
   271		if (p) {
   272			if (!strcmp(p, ":exit")) {
   273				*is_return = true;
 > 274				*p = '\0';
   275			} else if (!strcmp(p, ":entry")) {
   276				*p = '\0';
   277			} else {
   278				ret = -EINVAL;
   279				goto out;
   280			}
   281		}
   282	
   283		list = !!strchr(work, ',') || has_wildcard(work);
   284		if (legacy_ret)
   285			*is_return = true;
   286	
   287		b = kstrdup(work, GFP_KERNEL);
   288		if (!b) {
   289			ret = -ENOMEM;
   290			goto out;
   291		}
   292	
   293		if (list) {
   294			char *tmp = b, *tok;
   295			size_t fsz = strlen(b) + 1, nfsz = strlen(b) + 1;
   296	
   297			f = kzalloc(fsz, GFP_KERNEL);
   298			nf = kzalloc(nfsz, GFP_KERNEL);
   299			if (!f || !nf) {
   300				ret = -ENOMEM;
   301				goto out;
   302			}
   303	
   304			while ((tok = strsep(&tmp, ",")) != NULL) {
   305				char *dst;
   306				bool neg = (*tok == '!');
   307	
   308				if (*tok == '\0')
   309					continue;
   310				if (neg)
   311					tok++;
   312				dst = neg ? nf : f;
   313				if (dst[0] != '\0')
   314					strcat(dst, ",");
   315				strcat(dst, tok);
   316			}
   317			*list_mode = true;
   318		}
   319	
   320		*base = b; b = NULL;
   321		*filter = f; f = NULL;
   322		*nofilter = nf; nf = NULL;
   323	
   324	out:
   325		kfree(work);
   326		kfree(b);
   327		kfree(f);
   328		kfree(nf);
   329		return ret;
   330	}
   331	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3 4/5] selftests/ftrace: dynevent: add reject cases for list/:entry/:exit

[Ryan Chung]

Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
---
 .../test.d/dynevent/add_remove_fprobe.tc      | 121 ++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
index 2506f464811b..d5761d31217c 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
@@ -2,6 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # description: Generic dynamic event - add/remove fprobe events
 # requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[%return] [<args>]":README
+# Note: list-style specs and :entry/:exit may be unavailable on older kernels.
+# These tests auto-skip at runtime if the list form is rejected by tracefs.
 
 echo 0 > events/enable
 echo > dynamic_events
@@ -89,4 +91,123 @@ if [ $cnt -ne $ocnt ]; then
 	exit_fail
 fi
 
+# ---- New accept cases for list syntax with :entry/:exit and !-exclusions ----
+if echo "f:test/__list_check $PLACE,$PLACE3" >> dynamic_events 2> /dev/null; then
+	# Clean the probe added by the guard
+	echo "-:test/__list_check" >> dynamic_events
+
+	# List default (entry) with exclusion, explicit group/event
+	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
+	# Should attach to PLACE and PLACE3, but not PLACE2
+	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
+
+	# Disable and remove; count should be back to baseline
+	echo 0 > events/test/list_entry/enable
+	echo "-:test/list_entry" >> dynamic_events
+	! grep -q "test/list_entry" dynamic_events
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $ocnt ]; then
+		exit_fail
+	fi
+
+	# List with explicit :entry suffix (same behavior as default)
+	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
+
+	echo 0 > events/test/list_entry_exp/enable
+	echo "-:test/list_entry_exp" >> dynamic_events
+	! grep -q "test/list_entry_exp" dynamic_events
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $ocnt ]; then
+		exit_fail
+	fi
+
+	# List with :exit suffix across the same set
+	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
+	# On return probes, enabled_functions still reflects attached functions.
+	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
+
+	echo 0 > events/test/list_exit/enable
+	echo "-:test/list_exit" >> dynamic_events
+	! grep -q "test/list_exit" dynamic_events
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $ocnt ]; then
+		exit_fail
+	fi
+
+	# Enabling entry and exit together does not double-count functions
+	echo "f:test/list_both_e $PLACE,!$PLACE2,$PLACE3" >> dynamic_events
+	echo "f:test/list_both_x $PLACE,!$PLACE2,$PLACE3:exit" >> dynamic_events
+	grep -q "test/list_both_e" dynamic_events
+	grep -q "test/list_both_x" dynamic_events
+	test -d events/test/list_both_e
+	test -d events/test/list_both_x
+
+	echo 1 > events/test/list_both_e/enable
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $((ocnt + 2)) ]; then
+		exit_fail
+	fi
+
+	# Enabling :exit for the same set should keep the count the same
+	echo 1 > events/test/list_both_x/enable
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $((ocnt + 2)) ]; then
+		exit_fail
+	fi
+
+	# Disable one; count should remain (the other still holds the attach)
+	echo 0 > events/test/list_both_e/enable
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $((ocnt + 2)) ]; then
+		exit_fail
+	fi
+
+	# Disable the other; count returns to baseline
+	echo 0 > events/test/list_both_x/enable
+	cnt=`cat enabled_functions | wc -l`
+	if [ $cnt -ne $ocnt ]; then
+		exit_fail
+	fi
+
+	# Remove both definitions
+	echo "-:test/list_both_e" >> dynamic_events
+	echo "-:test/list_both_x" >> dynamic_events
+	! grep -q "test/list_both_e" dynamic_events
+	! grep -q "test/list_both_x" dynamic_events
+else
+	# List-form not supported; skip silently
+	:
+fi
+
 clear_trace
-- 
2.43.0

[PATCH v3 5/5] selftests/ftrace: dynevent: add reject cases

[Ryan Chung]

Signed-off-by: Ryan Chung <seokwoo.chung130@gmail.com>
---
 .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc  | 13 +++++++++++++
 1 file changed, 13 insertions(+)

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 fee479295e2f..720c0047c0ff 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
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # description: Fprobe event parser error log check
 # requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[%return] [<args>]":README
+# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]":README
 
 check_error() { # command-with-error-pos-by-^
     ftrace_errlog_check 'trace_fprobe' "$1" 'dynamic_events'
@@ -95,6 +96,18 @@ fi
 # %return suffix errors
 check_error 'f vfs_read^%hoge'		# BAD_ADDR_SUFFIX
 
+# New list/wildcard syntax errors
+if grep -q: ":exit" README; then
+check_error 'f ^vfs_read, do_sys_open'	# LIST_NEEDS_EVENT
+check_error 'f ^vfs_read,do_sys_open'	# LIST_NEEDS_EVENT
+check_error 'f:dyn/ret_forbid vfs_*^%return'	# WILDCARD_WITH_RETURN
+check_error 'f:dyn/ret_forbid vfs_read,do_sys_open^%return'	# LIST_WITH_RETURN
+check_error 'f:dyn/list_bad ^,vfs_read'		# LEADING_COMMA
+check_error 'f:dyn/list_bad vfs_read,^'		# TRAILING_COMMA
+check_error 'f:dyn/list_bad vfs_read,^,do_sys_open'	# EMPTY_TOKEN
+check_error 'f:dyn/mixed vfs_read%return^:exit'		# MIXED_SUFFIX
+
+
 # BTF arguments errors
 if grep -q "<argname>" README; then
 check_error 'f vfs_read args=^$arg*'		# BAD_VAR_ARGS
-- 
2.43.0

Re: [PATCH v3 0/5] tracing: fprobe: list-style filters,

[Masami Hiramatsu]

Hi Ryan,

Thanks for update!

On Sun,  5 Oct 2025 08:46:54 +0900
Ryan Chung <seokwoo.chung130@gmail.com> wrote:

> This series aims to extend fprobe with list-style filters and a clear
> entry/exist qualifier. Users can now specify a comma-separated symbol
> list with ! exclusions, and use a spec-level suffix to select probe
> type:
> 
> - funcA*, !funcAB, funcC -> entry probes
> - funcA*, !funcAB, funcC:entry -> explicit entry
> - funcA*, !funcAB, funcC:exit -> return/exit across the whole list


Just a note, it should not accept spaces in the list. The space
is the highest level delimiter. I hope actual implementation
does not accept spaces. So something like:

 "funcA*,!funcAB,funcC"
 "funcA*,!funcAB,funcC:entry"
 "funcA*,!funcAB,funcC:exit"


> 
> For compatibility, %return remains supported for single, literal
> symbols. When a list or wildcard is used, an explicit [GROUP/EVENT is
> required and autogeneration is disabled. Autogen names are kept for
> single-symbol specs, with wildcard sanitization. For list/wildcard forms
> we set ctx->funcname = NULL so BTF lookups are not attempted.

OK. So "funcA*%return" and "funcA,funcB%return" will fail.

> 
> The series moves parsing to the parse path, documents the new syntax,
> and adds selftests that accept valid list cases and reject empty tokens,
> stray commas, and %return mixed with lists or wildcards. Selftests also
> verify enable/disable flow and that entry+exit on the same set do not
> double-count attached functions.

Thanks for adding selftests and document, that is important to maintain
features.

> 
> Help wanted: This is my first time contributing ftrace selftests. I
> would appreciate comments and recommendations on test structure and
> coverage.

OK, let me review it.

Thanks,


> 
> Basic coverage is included, but this likely needs broader testing across
> architectures. Feedback and additional test ideas are welcome.
> 
> Changes since v2:
> - Introduce spec-level: :entry/:exit; reject %return with
>   lists/wildcards
> - Require explict [GROUP/]EVENT for list/wildcard; keep autogen only for
>   single literal.
> - Sanitize autogen names for single-symbol wildcards
> - Set ctx->funcname = NULL for list/wildcard to bypass BTF
> - Move list parsing out of __register_trace_fprobe() and into the parse
>   path
> - Update docs and tracefs README and add dynevent selftests for
>   accept/reject and enable/disable flow
> 
> Link: https://lore.kernel.org/lkml/20250904103219.f4937968362bfff1ecd3f004@kernel.org/
> 
> Ryan Chung (5):
>   docs: tracing: fprobe: document list filters and :entry/:exit
>   tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard
>   tracing: fprobe: support comma-separated symbols and :entry/:exit
>   selftests/ftrace: dynevent: add reject cases for list/:entry/:exit
>   selftests/ftrace: dynevent: add reject cases
> 
>  Documentation/trace/fprobetrace.rst           |  27 +-
>  kernel/trace/trace.c                          |   3 +-
>  kernel/trace/trace_fprobe.c                   | 247 ++++++++++++++----
>  .../test.d/dynevent/add_remove_fprobe.tc      | 121 +++++++++
>  .../test.d/dynevent/fprobe_syntax_errors.tc   |  13 +
>  5 files changed, 349 insertions(+), 62 deletions(-)
> 
> -- 
> 2.43.0
> 


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

Re: [PATCH v3 0/5] tracing: fprobe: list-style filters,

[Ryan Chung]

Hi. Thanks for the review.

On Wed, Oct 08, 2025 at 09:51:11AM +0900, Masami Hiramatsu wrote:
> Hi Ryan,
> 
> Thanks for update!
> 
> On Sun,  5 Oct 2025 08:46:54 +0900
> Ryan Chung <seokwoo.chung130@gmail.com> wrote:
> 
> > This series aims to extend fprobe with list-style filters and a clear
> > entry/exist qualifier. Users can now specify a comma-separated symbol
> > list with ! exclusions, and use a spec-level suffix to select probe
> > type:
> > 
> > - funcA*, !funcAB, funcC -> entry probes
> > - funcA*, !funcAB, funcC:entry -> explicit entry
> > - funcA*, !funcAB, funcC:exit -> return/exit across the whole list
> 
> 
> Just a note, it should not accept spaces in the list. The space
> is the highest level delimiter. I hope actual implementation
> does not accept spaces. So something like:
> 
>  "funcA*,!funcAB,funcC"
>  "funcA*,!funcAB,funcC:entry"
>  "funcA*,!funcAB,funcC:exit"
> 
> 
I see. I will adjust the code so that the parser reject any whitespace.

> > 
> > For compatibility, %return remains supported for single, literal
> > symbols. When a list or wildcard is used, an explicit [GROUP/EVENT is
> > required and autogeneration is disabled. Autogen names are kept for
> > single-symbol specs, with wildcard sanitization. For list/wildcard forms
> > we set ctx->funcname = NULL so BTF lookups are not attempted.
> 
> OK. So "funcA*%return" and "funcA,funcB%return" will fail.
> 

Yes. %return is only accepted for a single literal symbol.

> > 
> > The series moves parsing to the parse path, documents the new syntax,
> > and adds selftests that accept valid list cases and reject empty tokens,
> > stray commas, and %return mixed with lists or wildcards. Selftests also
> > verify enable/disable flow and that entry+exit on the same set do not
> > double-count attached functions.
> 
> Thanks for adding selftests and document, that is important to maintain
> features.
> 
> > 
> > Help wanted: This is my first time contributing ftrace selftests. I
> > would appreciate comments and recommendations on test structure and
> > coverage.
> 
> OK, let me review it.
> 
> Thanks,
> 

Thank you.

> 
> > 
> > Basic coverage is included, but this likely needs broader testing across
> > architectures. Feedback and additional test ideas are welcome.
> > 
> > Changes since v2:
> > - Introduce spec-level: :entry/:exit; reject %return with
> >   lists/wildcards
> > - Require explict [GROUP/]EVENT for list/wildcard; keep autogen only for
> >   single literal.
> > - Sanitize autogen names for single-symbol wildcards
> > - Set ctx->funcname = NULL for list/wildcard to bypass BTF
> > - Move list parsing out of __register_trace_fprobe() and into the parse
> >   path
> > - Update docs and tracefs README and add dynevent selftests for
> >   accept/reject and enable/disable flow
> > 
> > Link: https://lore.kernel.org/lkml/20250904103219.f4937968362bfff1ecd3f004@kernel.org/
> > 
> > Ryan Chung (5):
> >   docs: tracing: fprobe: document list filters and :entry/:exit
> >   tracing: fprobe: require explicit [GROUP/]EVENT for list/wildcard
> >   tracing: fprobe: support comma-separated symbols and :entry/:exit
> >   selftests/ftrace: dynevent: add reject cases for list/:entry/:exit
> >   selftests/ftrace: dynevent: add reject cases
> > 
> >  Documentation/trace/fprobetrace.rst           |  27 +-
> >  kernel/trace/trace.c                          |   3 +-
> >  kernel/trace/trace_fprobe.c                   | 247 ++++++++++++++----
> >  .../test.d/dynevent/add_remove_fprobe.tc      | 121 +++++++++
> >  .../test.d/dynevent/fprobe_syntax_errors.tc   |  13 +
> >  5 files changed, 349 insertions(+), 62 deletions(-)
> > 
> > -- 
> > 2.43.0
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Best regards,
Ryan Chung