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

[Ryan Chung <seokwoo.chung130@gmail.com>]

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