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: <CAJpZYjXwUz7x1XUF7AzgYR6PZo_igrwK9BkxGx_3N0pCs1YRvw@mail.gmail.com>
Date: Mon, 17 Mar 2025 14:45:39 -0700
From: Chun-Tse Shao <ctshao@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
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 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.

-CT

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