Re: [PATCH] perf inject: correct event attribute sizes

Arnaldo Carvalho de Melo
2021-01-15T19:48:49Z

Em Wed, Dec 23, 2020 at 11:11:05PM +0100, Jiri Olsa escreveu:
> On Tue, Dec 15, 2020 at 10:58:16PM -0800, Denis Nikitin wrote:
> > When perf inject reads a perf.data file from an older version of perf,
> > it writes event attributes into the output with the original size field,
> > but lays them out as if they had the size currently used. Readers see
> > a corrupt file. Update the size field to match the layout.
> >
> > Signed-off-by: Al Grant <al.grant@foss.arm.com>
> > Signed-off-by: Denis Nikitin <denik@chromium.org>
> > ---
> > tools/perf/util/header.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index be850e9f8852..0d95981df8dd 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -3331,6 +3331,14 @@ int perf_session__write_header(struct perf_session *session,
> > attr_offset = lseek(ff.fd, 0, SEEK_CUR);
> >
> > evlist__for_each_entry(evlist, evsel) {
> > + if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
> > + /*
> > + * We are likely in "perf inject" and have read
> > + * from an older file. Update attr size so that
> > + * reader gets the right offset to the ids.
> > + */
> > + evsel->core.attr.size = sizeof(evsel->core.attr);
> > + }
>
> seems ok, just wondering if it would be better
> to fix it in perf_event__process_attr instead
>
> where we know where the data is coming from,
> here we could cover for some unexpected case
> of having different attr.size?
>
> anyway both would be probably equal, I'm ok
> with either way
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>

Thanks, applied.

- Arnaldo