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

[Arnaldo Carvalho de Melo]

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@xxxxxxxxxxxx>
> > Signed-off-by: Denis Nikitin <denik@xxxxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx>

Thanks, applied.

- Arnaldo