Re: [EXTERNAL] Re: [PATCH v4] perf inject --jit: Remove //anon mmap events
From: Ian Rogers
Date: Wed May 27 2020 - 17:00:13 EST
On Wed, May 27, 2020 at 12:27 PM Steve MacLean
> >> ** Implemented solution
> >> This patch solves the issue by removing // anon mmap events for any
> >> process which has a valid jit-<pid>.dump file.
> >> It tracks on a per process basis to handle the case where some running
> >> apps support jit-<pid>.dump, but some only support perf-<pid>.map.
> >> It adds new assumptions:
> >> * // anon mmap events are only required for perf-<pid>.map support.
> >> * An app that uses jit-<pid>.dump, no longer needs perf-<pid>.map
> >> support. It assumes that any perf-<pid>.map info is inferior.
> >> *** Details
> >> Use thread->priv to store whether a jitdump file has been processed
> >> During "perf inject --jit", discard "//anon*" mmap events for any pid
> >> which has sucessfully processed a jitdump file.
> > Thanks Steve this is an important fix! As //anon could be for malloc or other uses, should the stripping behavior be behind a flag?
> > Ian
> I hadn't anticipated a need to preserve the //anon mmap events when profiling JIT generated code.
> As far as I know mmap events are captured by perf only for mapping code to symbols. File mappings are kept
> by the change. Only // anon mappings are stripped. (Only for processes which emitted jitdump files.)
> And these are stripped only during the `perf inject --jit` step. I believe the // Anon mapping are only
> generally useful for mapping JIT code.
> I suppose if someone was trying to count mmap events it might be confusing, but `perf inject --jit` creates
> synthetic mmap file events which would also make this scenario confusing.
> I personally don't see a good reason to add a flag. I also don't see a simple way either. Not running `perf inject --jit`
> would preserve existing behavior w/o jitdump support. Without stripping the anon events jitdump support is painfully
Agreed that things are broken. In general only executable mappings are
held onto by perf, so it could be I'm over worrying about //anon
stripping breaking around memory allocations. We have some other use
cases for //anon at Google but they aren't impacted by jitdump. We
have also been trying to migrate jit caches to using memfd_create,
which has the same problem that this patch fixes for //anon. Fixing
memfd_create is a separate issue to //anon. I'll try to get a repro
for Java that demonstrates the problem and then add a Tested-by.