Messages in this thread | | | Date | Wed, 11 Dec 2019 13:44:34 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v3] perf inject --jit: Remove //anon mmap events |
| |
On Thu, Oct 31, 2019 at 01:30:36PM -0700, Steve MacLean wrote: > From: Steve MacLean <Steve.MacLean@Microsoft.com> > > While a JIT is jitting code it will eventually need to commit more pages and > change these pages to executable permissions. > > Typically the JIT will want these colocated to minimize branch displacements. > > The kernel will coalesce these anonymous mapping with identical permissions > before sending an MMAP event for the new pages. This means the mmap event for > the new pages will include the older pages. > > These anonymous mmap events will obscure the jitdump injected pseudo events. > This means that the jitdump generated symbols, machine code, debugging info, > and unwind info will no longer be used. > > Observations: > > When a process emits a jit dump marker and a jitdump file, the perf-xxx.map > file represents inferior information which has been superceded by the > jitdump jit-xxx.dump file. > > Further the '//anon*' mmap events are only required for the legacy > perf-xxx.map mapping. > > When attaching to an existing process, the synthetic anon map events are > given a time stamp of -1. These should not obscure the jitdump events which > have an actual time. > > Summary: > > 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. > > Committer testing: > > // jitdump case > perf record <app with jitdump> > perf inject --jit --input perf.data --output perfjit.data > > // verify mmap "//anon" events present initially > perf script --input perf.data --show-mmap-events | grep '//anon' > // verify mmap "//anon" events removed > perf script --input perfjit.data --show-mmap-events | grep '//anon' > > // no jitdump case > perf record <app without jitdump> > perf inject --jit --input perf.data --output perfjit.data > > // verify mmap "//anon" events present initially > perf script --input perf.data --show-mmap-events | grep '//anon' > // verify mmap "//anon" events not removed > perf script --input perfjit.data --show-mmap-events | grep '//anon' > > Repro: > > This issue was discovered while testing the initial CoreCLR jitdump > implementation. https://github.com/dotnet/coreclr/pull/26897. >
Stephane, are you ok with this fix?
thanks, jirka
> Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Stephane Eranian <eranian@google.com> > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Steve MacLean <Steve.MacLean@Microsoft.com> > --- > tools/perf/builtin-inject.c | 4 ++-- > tools/perf/util/jitdump.c | 31 ++++++++++++++++++++++++++++++- > 2 files changed, 32 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c > index 372ecb3..0f38862 100644 > --- a/tools/perf/builtin-inject.c > +++ b/tools/perf/builtin-inject.c > @@ -263,7 +263,7 @@ static int perf_event__jit_repipe_mmap(struct perf_tool *tool, > * if jit marker, then inject jit mmaps and generate ELF images > */ > ret = jit_process(inject->session, &inject->output, machine, > - event->mmap.filename, sample->pid, &n); > + event->mmap.filename, event->mmap.pid, &n); > if (ret < 0) > return ret; > if (ret) { > @@ -301,7 +301,7 @@ static int perf_event__jit_repipe_mmap2(struct perf_tool *tool, > * if jit marker, then inject jit mmaps and generate ELF images > */ > ret = jit_process(inject->session, &inject->output, machine, > - event->mmap2.filename, sample->pid, &n); > + event->mmap2.filename, event->mmap2.pid, &n); > if (ret < 0) > return ret; > if (ret) { > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c > index e3ccb0c..d18596e 100644 > --- a/tools/perf/util/jitdump.c > +++ b/tools/perf/util/jitdump.c > @@ -26,6 +26,7 @@ > #include "jit.h" > #include "jitdump.h" > #include "genelf.h" > +#include "thread.h" > > #include <linux/ctype.h> > #include <linux/zalloc.h> > @@ -749,6 +750,28 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr) > return 0; > } > > +static void jit_add_pid(struct machine *machine, pid_t pid) > +{ > + struct thread *thread = machine__findnew_thread(machine, pid, pid); > + > + if (!thread) { > + pr_err("%s: thread %d not found or created\n", __func__, pid); > + return; > + } > + > + thread->priv = (void *)1; > +} > + > +static bool jit_has_pid(struct machine *machine, pid_t pid) > +{ > + struct thread *thread = machine__find_thread(machine, pid, pid); > + > + if (!thread) > + return 0; > + > + return (bool)thread->priv; > +} > + > int > jit_process(struct perf_session *session, > struct perf_data *output, > @@ -764,8 +787,13 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr) > /* > * first, detect marker mmap (i.e., the jitdump mmap) > */ > - if (jit_detect(filename, pid)) > + if (jit_detect(filename, pid)) { > + // Strip //anon* mmaps if we processed a jitdump for this pid > + if (jit_has_pid(machine, pid) && (strncmp(filename, "//anon", 6) == 0)) > + return 1; > + > return 0; > + } > > memset(&jd, 0, sizeof(jd)); > > @@ -784,6 +812,7 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr) > > ret = jit_inject(&jd, filename); > if (!ret) { > + jit_add_pid(machine, pid); > *nbytes = jd.bytes_written; > ret = 1; > } > -- > 1.8.3.1 >
| |