linux-kernel - Re: [PATCH 02/26] perf: Introduce mmap3 version of mmap event

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: <CABPqkBSKpZFbYWrQueS9cFDkGJPb3NdA1HTPh0+XN_+qvLpdLw@mail.gmail.com>
Date:   Mon, 14 Sep 2020 10:26:05 -0700
From:   Stephane Eranian <eranian@...gle.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Michael Petlan <mpetlan@...hat.com>,
        Song Liu <songliubraving@...com>,
        "Frank Ch. Eigler" <fche@...hat.com>,
        Ian Rogers <irogers@...gle.com>,
        Alexey Budankov <alexey.budankov@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH 02/26] perf: Introduce mmap3 version of mmap event

On Mon, Sep 14, 2020 at 2:08 AM <peterz@...radead.org> wrote:
>
> On Sun, Sep 13, 2020 at 11:41:00PM -0700, Stephane Eranian wrote:
> > On Sun, Sep 13, 2020 at 2:03 PM Jiri Olsa <jolsa@...nel.org> wrote:
> > what happens if I set mmap3 and mmap2?
> >
> > I think using mmap3 for every mmap may be overkill as you add useless
> > 20 bytes to an mmap record.
> > I am not sure if your code handles the case where mmap3 is not needed
> > because there is no buildid, e.g, anonymous memory.
> > It seems to me you've written the patch in such a way that if the user
> > tool supports mmap3, then it supersedes mmap2, and thus
> > you need all the fields of mmap2. But if could be more interesting to
> > return either MMAP2 or MMAP3 depending on tool support
> > and type of mmap, that would certainly save 20 bytes on any anon mmap.
> > But maybe that logic is already in your patch and I missed it.
>
> That, and what if you don't want any of that buildid nonsense at all? I
> always kill that because it makes perf pointlessly slow and has
> absolutely no upsides for me.
>
I have seen situations where the perf tool takes a visibly significant
amount of time (many seconds) to inject the buildids at the end of the
collection
of perf record (same if using perf inject -b). That is because it
needs to go through all the records in the perf.data to find MMAP
records and then read
the buildids from the filesystem. This has caused some problems in our
environment. Having the kernel add the buildid to *relevant* mmaps
would avoid
a lot of that penalty, by avoiding having to parse the perf.data file
and leveraging the fact that the buildid may be in memory already.
Although my concern on
this has to do with large pages and the impact they have on alignment
of sections in memory.  I think Ian can comment better on this.

I think this patch series is useful if it can demonstrate a speedup
during recording (perf record or perf record | perf inject -b). But it
needs to be
optimized to minimize the volume of useless info returned. And Jiri
needs to decide if MMAP3 is a replacement of MMAP2, or a different
kind of record
targeted at ELF images only in which case some of the fields may be
removed. My tendency would be to go for the latter.