linux-kernel - Re: [PATCH v1 5/8] perf header: Allow attributes to be written after data

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] [day] [month] [year] [list]

Message-ID: <CAM9d7cg0WpMnRja1z0a+R2BN8TicgB=Y4EDt=95383gFaqQKZA@mail.gmail.com>
Date: Fri, 30 Aug 2024 09:04:48 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	Nick Terrell <terrelln@...com>, Yanteng Si <siyanteng@...ngson.cn>, 
	Yicong Yang <yangyicong@...ilicon.com>, James Clark <james.clark@...aro.org>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/8] perf header: Allow attributes to be written after data

On Thu, Aug 29, 2024 at 10:03 PM Ian Rogers <irogers@...gle.com> wrote:
>
> On Thu, Aug 29, 2024 at 9:39 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > Maybe I'm too naive but can we skip header updates on pipe data?  I'm
> > curious if this makes sense..
> >
> > Thanks,
> > Namhyung
> >
> >
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a7c859db2e15..b36f84f29295 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2341,6 +2341,9 @@ int cmd_inject(int argc, const char **argv)
> >         if (ret)
> >                 goto out_delete;
> >
> > +       if (data.is_pipe)
> > +               inject.is_pipe = true;
> > +
>
> I'm not sure what you are saying. We can't know definitively if the
> input is a pipe style file or pipe until the header is read, which is
> part of session__new and something we pass whether we want to repipe
> the header or not. So we've made a decision or not to repipe but
> opening the header may change the decision that was already made. As
> you say we can opportunistically just copy/repipe the header if we
> know the input and output types match, but:
> 1) generating the header isn't that much work,
> 2) if the header needs to change for extra attributes, such as with
> some of the auxiliary flags, then the repiped header was no good
> anyway.
> Trying to keep header repiping alive for inject, the only use, is
> weird given all the gotchas. I think it is simpler to open, know what
> we're dealing with, then generate the output header accordingly -
> possibly synthesizing events for the attributes in the case of file to
> pipe.

I'm ok with removing repipe in session__new.  What I want is
not to overwrite the file header for a data file containing pipe
header.  In your example, 'perf record -o- > a.data' should have
the pipe header in a.data.  Then b.data from perf inject should
have the pipe header as well, right?  Then we don't need to
worry about the rewrite IIUC.

Thanks,
Namhyung