[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aOsJQ3gMcg5oKy94@gmail.com>
Date: Sun, 12 Oct 2025 10:49:55 +0900
From: Ryan Chung <seokwoo.chung130@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>, g@...il.com
Cc: rostedt@...dmis.org, mathieu.desnoyers@...icios.com, shuah@...nel.org,
hca@...ux.ibm.com, corbet@....net,
linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 0/5] tracing: fprobe: list-style filters,
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@...il.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@...nel.org>
Best regards,
Ryan Chung
Powered by blists - more mailing lists