All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Andres Freund <andres@anarazel.de>
Cc: linux-kernel@vger.kernel.org,
	Stephane Eranian <eranian@google.com>,
	acme@kernel.org, jolsa@redhat.com, mingo@elte.hu,
	anton@ozlabs.org, namhyung@kernel.org,
	Stephane Eranian <eranian@gmail.com>
Subject: Re: perf/jit doesn't cope well with mprotect() to jit containing pages
Date: Thu, 26 Jan 2017 23:15:08 +0100	[thread overview]
Message-ID: <20170126221508.GF6536@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20161212084903.GZ3124@twins.programming.kicks-ass.net>

On Mon, Dec 12, 2016 at 09:49:03AM +0100, Peter Zijlstra wrote:

> > BTW, it's also a bit weird that those MMAP2 records triggered by
> > mprotect/mmap, have prot set to 0...
> 
> Yes, mprotect() does: vma->vm_flags = newflags; before calling
> perf_event_mmap(vma); which then looks at VM_{READ,WRITE,EXEC} bits in
> that word to generate the prot value.
> 
> So that being 0 is a bit weird.

OK, that was a silly bug :/

With that fixed, the following proglet:

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <malloc.h>
#include <stdlib.h>
#include <errno.h>
#include <sys/mman.h>

void main(void)
{
	void *ptr[5];
	int i;

	for (i=0; i<5; i++) {
		ptr[i] = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
		*(char *)ptr[i] = '\0';
		mprotect(ptr[i], 4096, PROT_EXEC);
	}
}


Gives:

root@ivb-ep:~# perf record -d ./mprot
...
root@ivb-ep:~# perf report -D | grep MMAP2
...
43805378481 0xc60 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec0862000(0x1000) @ 0x7f6ec0862000 00:00 0 0]: --xp //anon
43805381755 0xcc0 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec0861000(0x1000) @ 0x7f6ec0861000 00:00 0 0]: rw-p //anon
43805392374 0xd20 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec0861000(0x2000) @ 0x7f6ec0861000 00:00 0 0]: --xp //anon
43805395093 0xd80 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec0860000(0x1000) @ 0x7f6ec0860000 00:00 0 0]: rw-p //anon
43805402842 0xde0 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec0860000(0x3000) @ 0x7f6ec0860000 00:00 0 0]: --xp //anon
43805405388 0xe40 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec085f000(0x1000) @ 0x7f6ec085f000 00:00 0 0]: rw-p //anon
43805412605 0xea0 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec085f000(0x4000) @ 0x7f6ec085f000 00:00 0 0]: --xp //anon
43805415134 0xf00 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec085e000(0x1000) @ 0x7f6ec085e000 00:00 0 0]: rw-p //anon
43805422225 0xf60 [0x60]: PERF_RECORD_MMAP2 2073/2073: [0x7f6ec085e000(0x5000) @ 0x7f6ec085e000 00:00 0 0]: --xp //anon

If you omit the mmap_data=1 thing, it looks like:

123087941779 0x500 [0x60]: PERF_RECORD_MMAP2 2084/2084: [0x7fca5a8bd000(0x1000) @ 0x7fca5a8bd000 00:00 0 0]: --xp //anon
123087953825 0x560 [0x60]: PERF_RECORD_MMAP2 2084/2084: [0x7fca5a8bc000(0x2000) @ 0x7fca5a8bc000 00:00 0 0]: --xp //anon
123087962944 0x5c0 [0x60]: PERF_RECORD_MMAP2 2084/2084: [0x7fca5a8bb000(0x3000) @ 0x7fca5a8bb000 00:00 0 0]: --xp //anon
123087971135 0x620 [0x60]: PERF_RECORD_MMAP2 2084/2084: [0x7fca5a8ba000(0x4000) @ 0x7fca5a8ba000 00:00 0 0]: --xp //anon
123087979360 0x680 [0x60]: PERF_RECORD_MMAP2 2084/2084: [0x7fca5a8b9000(0x5000) @ 0x7fca5a8b9000 00:00 0 0]: --xp //anon

---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 896aa53..e67c5ba 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6660,6 +6683,27 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 	char *buf = NULL;
 	char *name;
 
+	if (vma->vm_flags & VM_READ)
+		prot |= PROT_READ;
+	if (vma->vm_flags & VM_WRITE)
+		prot |= PROT_WRITE;
+	if (vma->vm_flags & VM_EXEC)
+		prot |= PROT_EXEC;
+
+	if (vma->vm_flags & VM_MAYSHARE)
+		flags = MAP_SHARED;
+	else
+		flags = MAP_PRIVATE;
+
+	if (vma->vm_flags & VM_DENYWRITE)
+		flags |= MAP_DENYWRITE;
+	if (vma->vm_flags & VM_MAYEXEC)
+		flags |= MAP_EXECUTABLE;
+	if (vma->vm_flags & VM_LOCKED)
+		flags |= MAP_LOCKED;
+	if (vma->vm_flags & VM_HUGETLB)
+		flags |= MAP_HUGETLB;
+
 	if (file) {
 		struct inode *inode;
 		dev_t dev;
@@ -6686,27 +6730,6 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 		maj = MAJOR(dev);
 		min = MINOR(dev);
 
-		if (vma->vm_flags & VM_READ)
-			prot |= PROT_READ;
-		if (vma->vm_flags & VM_WRITE)
-			prot |= PROT_WRITE;
-		if (vma->vm_flags & VM_EXEC)
-			prot |= PROT_EXEC;
-
-		if (vma->vm_flags & VM_MAYSHARE)
-			flags = MAP_SHARED;
-		else
-			flags = MAP_PRIVATE;
-
-		if (vma->vm_flags & VM_DENYWRITE)
-			flags |= MAP_DENYWRITE;
-		if (vma->vm_flags & VM_MAYEXEC)
-			flags |= MAP_EXECUTABLE;
-		if (vma->vm_flags & VM_LOCKED)
-			flags |= MAP_LOCKED;
-		if (vma->vm_flags & VM_HUGETLB)
-			flags |= MAP_HUGETLB;
-
 		goto got_name;
 	} else {
 		if (vma->vm_ops && vma->vm_ops->name) {

  parent reply	other threads:[~2017-01-26 22:55 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-10  5:02 perf/jit doesn't cope well with mprotect() to jit containing pages Andres Freund
2016-12-12  8:49 ` Peter Zijlstra
2016-12-12  9:01   ` Andres Freund
2016-12-12  9:28     ` Peter Zijlstra
2017-01-26  1:25       ` Andres Freund
2017-01-26 22:15   ` Peter Zijlstra [this message]
2017-01-26 23:04     ` Andres Freund
2017-01-30 11:52     ` [tip:perf/core] perf/core: Fix PERF_RECORD_MMAP2 prot/flags for anonymous memory tip-bot for Peter Zijlstra
2017-01-26 20:32 ` perf/jit doesn't cope well with mprotect() to jit containing pages Stephane Eranian
2017-01-26 21:00   ` Andres Freund
2017-01-26 21:17     ` Stephane Eranian
2017-01-26 21:22       ` Andres Freund
2017-01-26 21:34         ` Stephane Eranian
2017-01-26 21:51           ` Andres Freund
2017-01-26 22:19     ` Peter Zijlstra
2017-01-26 22:26       ` Stephane Eranian
2017-01-26 22:38         ` Andres Freund
2017-01-26 22:51           ` Stephane Eranian
2017-01-26 23:09             ` Andres Freund
2017-01-26 23:16               ` Stephane Eranian
2017-01-27 13:07                 ` Peter Zijlstra
2017-01-27 15:43                   ` Arnaldo Carvalho de Melo
2017-01-27 17:35                     ` Stephane Eranian
2017-01-27 17:38                     ` [PATCH] handle munmap records in tools/perf was: " Arnaldo Carvalho de Melo
2017-01-27 17:46                       ` Stephane Eranian
2017-01-27 18:05                         ` Arnaldo Carvalho de Melo
2017-01-27 18:10                           ` Stephane Eranian
2017-01-27 19:18                             ` Arnaldo Carvalho de Melo
2017-01-27 19:26                               ` Stephane Eranian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170126221508.GF6536@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=andres@anarazel.de \
    --cc=anton@ozlabs.org \
    --cc=eranian@gmail.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.