Re: [PATCH v2] perf probe: Clear all structure fields in clear_perf_{probe, trace}_event()

From: Google
Date: Mon Nov 25 2024 - 01:14:48 EST


On Thu, 14 Nov 2024 18:54:51 +0800
Li Huafei <lihuafei1@xxxxxxxxxx> wrote:

> I added two probe events:
>
> # perf probe -f -a schedule+8
> Added new event:
> probe:schedule (on schedule+8)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:schedule -aR sleep 1
>
> # perf probe -f -a schedule+20
> Added new event:
> probe:schedule_1 (on schedule+20)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:schedule_1 -aR sleep 1
>
> However, 'perf probe -l' shows the same offset:
>
> # perf probe -l
> probe:schedule (on schedule+8@kernel/sched/core.c)
> probe:schedule_1 (on schedule+8@kernel/sched/core.c)
>
> __show_perf_probe_events() does not clean up the 'pev' content when
> parsing the rawlist. If the 'pev->offset' is not set while processing
> the next probe event string, the offset value of the previous event will
> be used. After adding debug information, it was found that indeed there
> was line number information when processing 'probe:schedule_1', so the
> offset was not set and used the offset from 'probe:schedule'.
>
> To fix this, clear all the fields of the structures in
> clear_perf_{probe, trace}_event(). not just the allocated fields.
>

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks!

> Fixes: d8f9da240495 ("perf tools: Use zfree() where applicable")
> Signed-off-by: Li Huafei <lihuafei1@xxxxxxxxxx>
> ---
> Changes in v2:
> - Do the cleanup in clear_perf_{probe, trace}_event().
> - Refine the subject and the commit log.
>
> v1: https://lore.kernel.org/lkml/20241108181909.3515716-1-lihuafei1@xxxxxxxxxx/
> ---
> tools/perf/util/probe-event.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a17c9b8a7a79..47903bb56fc6 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2376,8 +2376,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
> field = next;
> }
> }
> - pev->nargs = 0;
> zfree(&pev->args);
> + memset(pev, 0, sizeof(*pev));
> }
>
> #define strdup_or_goto(str, label) \
> @@ -2475,7 +2475,7 @@ void clear_probe_trace_event(struct probe_trace_event *tev)
> }
> }
> zfree(&tev->args);
> - tev->nargs = 0;
> + memset(tev, 0, sizeof(*tev));
> }
>
> struct kprobe_blacklist_node {
> --
> 2.25.1
>


--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>