[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191001085511.GB30823@krava>
Date: Tue, 1 Oct 2019 10:55:11 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: Steve MacLean <Steve.MacLean@...rosoft.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Namhyung Kim <namhyung@...nel.org>,
Eric Saint-Etienne <eric.saint.etienne@...cle.com>,
John Keeping <john@...anate.com>,
Andi Kleen <ak@...ux.intel.com>,
Song Liu <songliubraving@...com>,
Davidlohr Bueso <dave@...olabs.net>,
Leo Yan <leo.yan@...aro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Brian Robbins <brianrob@...rosoft.com>,
Tom McDonald <Thomas.McDonald@...rosoft.com>,
John Salem <josalem@...rosoft.com>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 3/4 RESEND] perf inject --jit: Remove //anon mmap events
On Mon, Sep 30, 2019 at 09:00:01PM +0000, Steve MacLean wrote:
> 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 collocated 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 superseded by the
> jitdump jit-xxx.dump file.
>
> Further the '//anon*' mmap events are only required for the legacy
> perf-xxx.map mapping.
>
> Summary:
>
> Add rbtree to track which pids have successfully injected a jitdump file.
>
> During "perf inject --jit", discard "//anon*" mmap events for any pid which
> has successfully 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.
>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Stephane Eranian <eranian@...gle.com>
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Steve MacLean <Steve.MacLean@...rosoft.com>
> ---
> tools/perf/builtin-inject.c | 4 +--
> tools/perf/util/jitdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index c14f40b8..4c921e0 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -261,7 +261,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) {
> @@ -299,7 +299,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 22d09c4..6a1563f 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -751,6 +751,59 @@ jit_detect(char *mmap_name, pid_t pid)
> return 0;
> }
>
> +struct pid_rbtree
> +{
> + struct rb_node node;
> + pid_t pid;
> +};
> +
> +static void jit_add_pid(struct rb_root *root, pid_t pid)
> +{
> + struct rb_node **new = &(root->rb_node), *parent = NULL;
> + struct pid_rbtree* data = NULL;
> +
> + /* Figure out where to put new node */
> + while (*new) {
> + struct pid_rbtree *this = container_of(*new, struct pid_rbtree, node);
> + pid_t nodePid = this->pid;
looks like Andi is right, I'm still getting malformed patch error
the patch has extra characters '=20' and broken lines, like:
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -751,6 +751,59 @@ jit_detect(char *mmap_name, pid_t pid)
return 0;
}
=20
+struct pid_rbtree
+{
+ struct rb_node node;
+ pid_t pid;
+};
+
+static void jit_add_pid(struct rb_root *root, pid_t pid)
+{
+ struct rb_node **new =3D &(root->rb_node), *parent =3D NULL;
+ struct pid_rbtree* data =3D NULL;
+
+ /* Figure out where to put new node */
+ while (*new) {
+ struct pid_rbtree *this =3D container_of(*new, struct pid_r=
btree, node);
+ pid_t nodePid =3D this->pid;
jirka
Powered by blists - more mailing lists