Re: [PATCH 3/4] perf inject --jit: Remove //anon mmap events
On Wed, Oct 02, 2019 at 01:58:21PM -0700, Steve MacLean wrote:
> From: Steve MacLean
>
> 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 superseded by the
> jitdump jit-xxx.dump file.
>
> Further the '//anon*' mmap events are only required for the legacy
> perf-xxx.map mapping.
it's been some time I saw the code, Stephane?
I added some comments below
>
> 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
> 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
> 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
> Cc: Ingo Molnar
> Cc: Arnaldo Carvalho de Melo
> Cc: Mark Rutland
> Cc: Alexander Shishkin
> Cc: Jiri Olsa
> Cc: Namhyung Kim
> Cc: Stephane Eranian
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Steve MacLean
> ---
> 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 @@ static int jit_repipe_debug_info(struct jit_buf_desc
> *jd, union jr_entry *jr)
> 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;
we don't use the parenthesis like that
> + 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);
there's rb_entry macro for this
> + pid_t nodePid = this->pid;
> +
> + parent = *new;
> + if (pid < nodePid)
> + new = &((*new)->rb_left);
> + else if (pid > nodePid)
> + new = &((*new)->rb_right);
> + else
> + return;
> + }
> +
> + data = malloc(sizeof(struct pid_rbtree));
plz check every allocation
anyway, I wonder if you could just use thread::priv flag for that, like:
thread = machine__findnew_thread(machine, pid, pid);
if (!thread)
bad
(int
[PATCH 3/4] perf inject --jit: Remove //anon mmap events
From: Steve MacLean
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 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
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
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
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Mark Rutland
Cc: Alexander Shishkin
Cc: Jiri Olsa
Cc: Namhyung Kim
Cc: Stephane Eranian
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Steve MacLean
---
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 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd,
union jr_entry *jr)
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;
+
+ parent = *new;
+ if (pid < nodePid)
+ new = &((*new)->rb_left);
+ else if (pid > nodePid)
+ new = &((*new)->rb_right);
+ else
+ return;
+ }
+
+ data = malloc(sizeof(struct pid_rbtree));
+ data->pid = pid;
+
+ /* Add new node and rebalance tree. */
+ rb_link_node(&data->node, parent, new);
+ rb_insert_color(&data->node, root);
+
+ return;
+}
+
+static bool jit_has_pid(struct rb_root *root, pid_t pid)
+{
+ struct rb_node *node = root->rb_node;
+
+ while (node) {
+ struct pid_rbtree *this = container_of(node, struct pid_rbtree,
node);
+ pid_t nodePid = this->pid;
+
+ if (pid < nodePid)
+ node = node->rb_left;
+ else if (pid > nodePid)
+ n
Re: [PATCH 3/4] perf inject --jit: Remove //anon mmap events
On Mon, Sep 30, 2019 at 08:49:00PM +, Steve MacLean wrote: > SNIP > > > I can't apply this one: > > > patching file builtin-inject.c > > Hunk #1 FAILED at 263. > > 1 out of 1 hunk FAILED -- saving rejects to file builtin-inject.c.rej > > I assume this is because I based my patches on the wrong tip. > > > patching file util/jitdump.c > > patch: malformed patch at line 236: btree, node); > > This doesn't make sense to me. The patch doesn't try to inject near line > 236. There aren't 236 lines in the e-mail Most likely your mail client did line wrap. See Documentation/process/email-clients.rst -Andi
RE: [PATCH 3/4] perf inject --jit: Remove //anon mmap events
SNIP > I can't apply this one: > patching file builtin-inject.c > Hunk #1 FAILED at 263. > 1 out of 1 hunk FAILED -- saving rejects to file builtin-inject.c.rej I assume this is because I based my patches on the wrong tip. > patching file util/jitdump.c > patch: malformed patch at line 236: btree, node); This doesn't make sense to me. The patch doesn't try to inject near line 236. There aren't 236 lines in the e-mail Looking at the MAINTAINERS file, it looks like I should have based my changes on: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core I'll rebase the patch and resend.
Re: [PATCH 3/4] perf inject --jit: Remove //anon mmap events
On Sat, Sep 28, 2019 at 01:45:36AM +, 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 co-located 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 > 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 > 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. I can't apply this one: patching file builtin-inject.c Hunk #1 FAILED at 263. 1 out of 1 hunk FAILED -- saving rejects to file builtin-inject.c.rej patching file util/jitdump.c patch: malformed patch at line 236: btree, node); jirka
[PATCH 3/4] perf inject --jit: Remove //anon mmap events
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 co-located 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
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
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
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Mark Rutland
Cc: Alexander Shishkin
Cc: Jiri Olsa
Cc: Namhyung Kim
Cc: Stephane Eranian
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Steve MacLean
---
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 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..6d891d1 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -749,6 +749,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;
+
+ parent = *new;
+ if (pid < nodePid)
+ new = &((*new)->rb_left);
+ else if (pid > nodePid)
+ new = &((*new)->rb_right);
+ else
+ return;
+ }
+
+ data = malloc(sizeof(struct pid_rbtree));
+ data->pid = pid;
+
+ /* Add new node and rebalance tree. */
+ rb_link_node(&data->node, parent, new);
+ rb_insert_color(&data->node, root);
+
+ return;
+}
+
+static bool jit_has_pid(struct rb_root *root, pid_t pid)
+{
+ struct rb_node *node = root->rb_node;
+
+ while (node) {
+ struct pid_rbtree *this = container_of(node, struct pid_rbtree,
node);
+ pid_t nodePid = this->pid;
+
+ if (pid < nodePid)
+ node = node->rb_left;
+ else if (pid > nodePid)
+ node = node->rb_right;
+ else
+
