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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9iY7HFebiSaWZJQ@x1>
Date: Mon, 17 Mar 2025 18:49:32 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Chun-Tse Shao <ctshao@...gle.com>
Cc: Namhyung Kim <namhyung@...nel.org>, linux-kernel@...r.kernel.org,
	peterz@...radead.org, mingo@...hat.com, mark.rutland@....com,
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
	irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com, terrelln@...com, leo.yan@....com,
	james.clark@...aro.org, christophe.leroy@...roup.eu,
	ben.gainey@....com, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v2 1/2] perf record: Add 8-byte aligned event type
 PERF_RECORD_COMPRESSED2

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@...nel.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@...lbox perf-tools-next]$ perf.old script -i /tmp/perf.data.ck8
> > 0xbf0 [0x250]: failed to process type: 83 [Invalid argument]
> > ⬢ [acme@...lbox 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