Re: [PATCH v2 1/2] perf record: Add 8-byte aligned event type PERF_RECORD_COMPRESSED2

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Mon, Mar 17, 2025 at 02:45:39PM -0700, Chun-Tse Shao wrote:
> On Mon, Mar 17, 2025 at 1:35 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Mon, Mar 17, 2025 at 09:46:40AM -0700, Namhyung Kim wrote:
> > > On Mon, Mar 17, 2025 at 01:17:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > On Mon, Mar 17, 2025 at 12:52:09PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Checking the discussion and the patch.
> > > >
> > > > My first impression yesterday when I saw this on the smartphone was: how
> > > > will an old perf binary handle the new PERF_RECORD_COMPRESSED2? Will it
> > > > ignore it while emitting a warning, since it can be skipped and then
> > > > what we will get a partial view?
> > > >
> > > > Having some session output showing how an older perf binary handles
> > > > PERF_RECORD_COMPRESS2 would be informative.
> > >
> > > I think it'll show the below warning:
> > >
> > >   <offset> [<size>]: failed to process type: 83
> >
> > Right that is what I got:
> >
> > ⬢ [acme@toolbox perf-tools-next]$ perf.old script -i /tmp/perf.data.ck8
> > 0xbf0 [0x250]: failed to process type: 83 [Invalid argument]
> > ⬢ [acme@toolbox perf-tools-next]$
> >
> > I think we should change that to something more informative, like:
> >
> > 0xbf0 [0x250]: failed to process unknown type 83, please update perf.
> >
> > And then does it stop at that record it doesn't grok?
> >
> >         if ((skip = perf_session__process_event(session, event, head, "pipe")) < 0) {
> >                 pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
> >                        head, event->header.size, event->header.type);
> >                 err = -EINVAL;
> >                 goto out_err;
> >         }
> >
> >         head += size;
> >
> > So we're stopping there.
> >
> > Maybe we can just warn and skip?
> 
> Thank you Arnaldo, it is a good suggestion and I will work on this later.

Thank you for considering that, really appreciated!

perf deals with so much stuff and code flux that all the help that we
can get is what is needed for it to continue to be relevant and useful.

After all what is the point of a tool that produces bad results? :-)

- Arnaldo

> -CT
> 
> >
> > Anyway, the series as is seems ok.
> >
> > I'll test a bit more and send my Tested-by
> >
> > - Arnaldo