On Wed, 14 Jan 2026 17:13:38 -0500
"Seokwoo Chung (Ryan)" <[email protected]> wrote:

Missing change log. The subject is the "what" the patch is doing, the
change log needs the "why".

> Signed-off-by: Seokwoo Chung (Ryan) <[email protected]>
> ---
>  kernel/trace/trace.c        |  4 +--
>  kernel/trace/trace_fprobe.c | 49 ++++++++++++++++++++-----------------
>  kernel/trace/trace_probe.h  |  2 ++
>  3 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 10cdcc7b194e..73b59d47dfe7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5578,8 +5578,8 @@ static const char readme_msg[] =
>       "\t           r[maxactive][:[<group>/][<event>]] <place> [<args>]\n"
>  #endif
>  #ifdef CONFIG_FPROBE_EVENTS
> -     "\t           f[:[<group>/][<event>]] <func-name>[:entry|:exit] 
> [<args>]\n"
> -     "\t             (single symbols still accept %return)\n"
> +     "\t           f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
> +     "\t           f[:[<group>/][<event>]] <func-list>[:entry|:exit] 
> [<args>]\n" 
>       "\t           t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
>  #endif
>  #ifdef CONFIG_HIST_TRIGGERS
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 5a2a41eea603..1663c341ddb4 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -187,16 +187,23 @@ DEFINE_FREE(tuser_put, struct tracepoint_user *,
>   */
>  struct trace_fprobe {
>       struct dyn_event        devent;
> -     char                    *filter;
> +
>       struct fprobe           fp;
> -     bool                    list_mode;
> -     char                    *nofilter;
>       const char              *symbol;
> -     struct trace_probe      tp;
> +     char                    *filter;
> +     char                    *nofilter;
> +
>       bool                    tprobe;
>       struct tracepoint_user  *tuser;
> +
> +     struct trace_probe tp;

Why is this being updated? Nothing in the change log says why?

>  };
>  
> +static inline bool trace_fprobe_has_list(const struct trace_fprobe *tf)
> +{
> +     return tf->filter || tf->nofilter;
> +}
> +
>  static bool is_trace_fprobe(struct dyn_event *ev)
>  {
>       return ev->ops == &trace_fprobe_ops;
> @@ -847,7 +854,7 @@ static int __register_trace_fprobe(struct trace_fprobe 
> *tf)
>        * - list_mode: pass filter/nofilter
>        * - single: pass symbol only (legacy)
>        */
> -     if (tf->list_mode)
> +     if (trace_fprobe_has_list(tf))
>               return register_fprobe(&tf->fp, tf->filter, tf->nofilter);
>       return register_fprobe(&tf->fp, tf->symbol, NULL);
>  }
> @@ -1188,11 +1195,18 @@ static int parse_fprobe_spec(const char *in, bool 
> is_tracepoint,
>       *base = NULL; *filter = NULL; *nofilter = NULL;
>       *is_return = false; *list_mode = false;
>  
> -     if (is_tracepoint) {
> +     if (is_tracepoint) 
> +     {
>               if (strchr(in, ',') || strchr(in, ':'))
> +             {
> +                     trace_probe_log_err(0, BAD_TP_NAME);
>                       return -EINVAL;
> +             }
>               if (strstr(in, "%return"))
> +             {
> +                     trace_probe_log_err(p - in, BAD_TP_NAME);
>                       return -EINVAL;
> +             }
>               for (p = in; *p; p++)
>                       if (!isalnum(*p) && *p != '_')
>                               return -EINVAL;
> @@ -1225,6 +1239,7 @@ static int parse_fprobe_spec(const char *in, bool 
> is_tracepoint,
>                       } else if (!strcmp(p, ":entry")) {
>                               *(char *)p = '\0';
>                       } else {
> +                             trace_probe_log_err(p - work, BAD_LIST_SUFFIX);
>                               return -EINVAL;
>                       }
>               }
> @@ -1233,6 +1248,7 @@ static int parse_fprobe_spec(const char *in, bool 
> is_tracepoint,
>       list = !!strchr(work, ',');
>       
>       if (list && legacy_ret) {
> +             trace_probe_log_err(p - work, BAD_LEGACY_RET);
>               return -EINVAL;
>       }
>  
> @@ -1245,12 +1261,9 @@ static int parse_fprobe_spec(const char *in, bool 
> is_tracepoint,
>  
>       if (list) {
>               char *tmp = b, *tok;
> -             size_t fsz, nfsz;
>  
> -             fsz = nfsz = strlen(b) + 1;
> -
> -             f = kzalloc(fsz, GFP_KERNEL);
> -             nf = kzalloc(nfsz, GFP_KERNEL);
> +             f = kzalloc(strlen(b) + 1, GFP_KERNEL);
> +             nf = kzalloc(strlen(b) + 1, GFP_KERNEL);
>               if (!f || !nf)
>                       return -ENOMEM;
>  
> @@ -1261,6 +1274,7 @@ static int parse_fprobe_spec(const char *in, bool 
> is_tracepoint,
>                       if (*tok == '\0') {
>                               trace_probe_log_err(tmp - b - 1, BAD_TP_NAME);
>                               return -EINVAL;
> +                     }
>  
>                       if (neg)
>                               tok++;
> @@ -1455,17 +1469,8 @@ static int trace_fprobe_create_internal(int argc, 
> const char *argv[],
>  
>       /* 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;
> -             }
> +             tf->filter = no_free_ptr(parsed_filter);
> +             tf->nofilter = no_free_ptr(parsed_nofilter);
>       }
>  
>       /* parse arguments */
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 9fc56c937130..b8b0544eb7cd 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -494,9 +494,11 @@ extern int traceprobe_define_arg_fields(struct 
> trace_event_call *event_call,
>       C(BAD_PROBE_ADDR,       "Invalid probed address or symbol"),    \
>       C(NON_UNIQ_SYMBOL,      "The symbol is not unique"),            \
>       C(BAD_RETPROBE,         "Retprobe address must be an function entry"), \
> +     C(BAD_LEGACY_RET,       "Legacy %return not allowed with list"), \
>       C(NO_TRACEPOINT,        "Tracepoint is not found"),             \
>       C(BAD_TP_NAME,          "Invalid character in tracepoint name"),\
>       C(BAD_ADDR_SUFFIX,      "Invalid probed address suffix"), \
> +     C(BAD_LIST_SUFFIX,      "Bad list suffix"), \
>       C(NO_GROUP_NAME,        "Group name is not specified"),         \
>       C(GROUP_TOO_LONG,       "Group name is too long"),              \
>       C(BAD_GROUP_NAME,       "Group name must follow the same rules as C 
> identifiers"), \

This definitely needs discussion on why this is needed.

-- Steve


Reply via email to