linux-kernel - RE: [EXTERNAL] Re: [PATCH v4] perf inject --jit: Remove //anon mmap events

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]

Date:   Fri, 29 May 2020 18:48:51 +0000
From:   Steve MacLean <Steve.MacLean@...rosoft.com>
To:     Ian Rogers <irogers@...gle.com>
CC:     Steve MacLean <steve.maclean@...ux.microsoft.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Stephane Eranian <eranian@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH v4] perf inject --jit: Remove //anon mmap
 events

> I've been trying variants of:
>
> Before:
>/tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar dacapo-9.12-bach.jar jython /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l
> 578
> /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l
> 6
> 
> After:
> /tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar dacapo-9.12-bach.jar jython /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l
> 589
> /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l
> 60
>
> So maybe the jit cache isn't behaving the way that the patch cures, the uptick in unknowns appears consistent in my testing though. I expect user error, is there an obvious explanation I'm missing?

That is certainly disturbing. I don't see an obvious user error, but I haven't played with java jitdump

Couple of ideas come to mind.

+ When I was drafting .NET jitdump support, I looked briefly at the java agent.  My impression was the java agent support wasn't thread safe.  The jitdump file format requires that certain records are collocated to simplify the reader.  I wasn't sure how that was being accomplished by the writer if JAVA's JIT was multithreaded (like CoreCLR JIT is).

+ The way I implemented jitdump on CoreCLR was to hook into the existing system to write perf-map files.  So I am pretty confident there is a 1:1 correspondence of perf-map records with jitdump records.  Is it possible that Java choose to only emit interesting jitdump records?  Perhaps skipping thunks/stubs/trampolines?  

+ There are still some `unknown` records in CoreCLR, but I don't believe there is an increase.  I am pretty sure some of our stubs are not being generated.  They were present in our before case too.

+ Your methodology would be more repeatable if you record once, and analyze multiple times.  record, report, inject / report, inject / report

+ I was focusing on eliminating duplicate symbols for the same address.  So having the module name in the report allowed me to see if the symbol was from jitdump or from perf-map.  In the before case I could see duplicate symbols for the same address.  This was how the problem was first observed.

+ I would be more interested in looking at the diff of the reports.  Possibly sorted by address.  Probably with zero context.

If I were to guess, I would think Java choose to optimize jitdump and only record interesting code.

So if we need to support that scenario the approach of this patch wouldn't work.

We would need to use a different approach.  Ideas...
+ Keep anon mappings from overwriting jitdump mappings.  Introduce a weak add map method, that would only fill empty/anon.
+ Move the anon mapping timestamp to the beginning of time, so they are processed first
+ Fix the kernel perf map events
+ Keep the anon mappings in a separate fall back map

I kind of like the add weak map approach.