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

[Google]

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>