perf: Add munmap callback

Quoted 5 lines

kernel/events/core.c +5 -0

+void perf_event_munmap(void)
+{
+	struct perf_cpu_context *cpuctx;
+	unsigned long flags;
+	struct pmu *pmu;

kernel/events/core.c static void perf_pmu_sched_task(struct task_struct *prev, · line 3194

Quoted 3 lines

kernel/events/core.c +3 -0

+
+	local_irq_save(flags);
+	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {

Would be good have a fast path here that checks for the list being empty without disabling the interrupts. munmap can be somewhat hot. I think it's ok to make it slower with perf running, but we shouldn't impact it without perf.

Em Wed, Oct 24, 2018 at 09:23:34AM -0700, Andi Kleen escreveu:

Quoted 5 lines

Quoted 5 lines

kernel/events/core.c +5 -0

+void perf_event_munmap(void)
+{
+	struct perf_cpu_context *cpuctx;
+	unsigned long flags;
+	struct pmu *pmu;

Quoted 8 lines

kernel/events/core.c static void perf_pmu_sched_task(struct task_struct *prev, · line 3194

Quoted 3 lines

kernel/events/core.c +3 -0

+
+	local_irq_save(flags);
+	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {

Would be good have a fast path here that checks for the list being empty without disabling the interrupts. munmap can be somewhat hot. I think it's ok to make it slower with perf running, but we shouldn't impact it without perf.

Right, look at how its counterpart, perf_event_mmap() works:

void perf_event_mmap(struct vm_area_struct *vma)

{
         struct perf_mmap_event mmap_event;

         if (!atomic_read(&nr_mmap_events))
                 return;

<SNIP>
}

Quoted 9 lines

void perf_event_mmap(struct vm_area_struct *vma)

{
         struct perf_mmap_event mmap_event;

         if (!atomic_read(&nr_mmap_events))
                 return;

<SNIP>
}

Thanks. I'll add the nr_mmap_events check in V2.

Quoted 11 lines

Quoted 9 lines

void perf_event_mmap(struct vm_area_struct *vma)

{
         struct perf_mmap_event mmap_event;

         if (!atomic_read(&nr_mmap_events))
                 return;

<SNIP>
}

Thanks. I'll add the nr_mmap_events check in V2.

No, that's the wrong check here. The PEBS flush is independent of mmap events being requested.

If anything would need to check for any PEBS events active, which would need a new counter. I think the easiest is to just check if this_cpu_ptr(&sched_cb_list)
is empty, which should be good enough.

Quoted 21 lines

Em Wed, Oct 24, 2018 at 09:23:34AM -0700, Andi Kleen escreveu:

Quoted 5 lines

Quoted 5 lines

kernel/events/core.c +5 -0

+void perf_event_munmap(void)
+{
+	struct perf_cpu_context *cpuctx;
+	unsigned long flags;
+	struct pmu *pmu;

Quoted 8 lines

kernel/events/core.c static void perf_pmu_sched_task(struct task_struct *prev, · line 3194

Quoted 3 lines

kernel/events/core.c +3 -0

+
+	local_irq_save(flags);
+	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {

Would be good have a fast path here that checks for the list being empty without disabling the interrupts. munmap can be somewhat hot. I think it's ok to make it slower with perf running, but we shouldn't impact it without perf.

Right, look at how its counterpart, perf_event_mmap() works:

void perf_event_mmap(struct vm_area_struct *vma)

{
         struct perf_mmap_event mmap_event;

Quoted 5 lines

         if (!atomic_read(&nr_mmap_events))
                 return;

<SNIP>
}

Thanks. I'll add the nr_mmap_events check in V2.

From: Kan Liang <[email protected]>

To calculate the physical address, perf needs to walk the pages tables. The related mapping may has already been removed from pages table in some cases (e.g. large PEBS). The virtual address recorded in the first PEBS records may already be unmapped before draining PEBS buffers.

Add a munmap callback to notify the PMU of any unmapping, which only be invoked when the munmap is implemented.

Signed-off-by Kan Liang <[email protected]>

---
 include/linux/perf_event.h |  3 +++
 kernel/events/core.c       | 25 +++++++++++++++++++++++++
 mm/mmap.c                  |  1 +
 3 files changed, 29 insertions(+)

include/linux/perf_event.h +3 -0

index 53c500f0ca79..7f0a9258ce1f 100644
@@ -400,6 +400,7 @@ struct pmu {
         */
        void (*sched_task)              (struct perf_event_context *ctx,
                                        bool sched_in);
+       void (*munmap)                  (void);
        /*
         * PMU specific data size
         */

@@ -1113,6 +1114,7 @@ static inline void perf_event_task_sched_out(struct task_struct *prev,
 }
 
 extern void perf_event_mmap(struct vm_area_struct *vma);
+extern void perf_event_munmap(void);
 extern struct perf_guest_info_callbacks *perf_guest_cbs;
 extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
 extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);

@@ -1333,6 +1335,7 @@ static inline int perf_unregister_guest_info_callbacks
 (struct perf_guest_info_callbacks *callbacks)                          { return 0; }
 
 static inline void perf_event_mmap(struct vm_area_struct *vma)         { }
+static inline void perf_event_munmap(void)                             { }
 static inline void perf_event_exec(void)                               { }
 static inline void perf_event_comm(struct task_struct *tsk, bool exec) { }
 static inline void perf_event_namespaces(struct task_struct *tsk)      { }

kernel/events/core.c +25 -0

index 5a97f34bc14c..00338d6fbed7 100644
@@ -3184,6 +3184,31 @@ static void perf_pmu_sched_task(struct task_struct *prev,
        }
 }
 
+void perf_event_munmap(void)
+{
+       struct perf_cpu_context *cpuctx;
+       unsigned long flags;
+       struct pmu *pmu;
+
+       local_irq_save(flags);
+       list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
+               pmu = cpuctx->ctx.pmu;
+
+               if (!pmu->munmap)
+                       continue;
+
+               perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+               perf_pmu_disable(pmu);
+
+               pmu->munmap();
+
+               perf_pmu_enable(pmu);
+
+               perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
+       }
+       local_irq_restore(flags);
+}
+
 static void perf_event_switch(struct task_struct *task,
                              struct task_struct *next_prev, bool sched_in);
 

mm/mmap.c +1 -1

index 5f2b2b184c60..61978ad8c480 100644
@@ -2777,6 +2777,7 @@ int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
        /*
         * Remove the vma's, and unmap the actual pages
         */
+       perf_event_munmap();
        detach_vmas_to_be_unmapped(mm, vma, prev, end);
        unmap_region(mm, vma, prev, start, end);
 
--

2.17.1

Hi,

Quoted 11 lines · On Wed, Oct 24, 2018 at 8:12 AM <[email protected]> wrote:

From: Kan Liang <[email protected]>

To calculate the physical address, perf needs to walk the pages tables. The related mapping may has already been removed from pages table in some cases (e.g. large PEBS). The virtual address recorded in the first PEBS records may already be unmapped before draining PEBS buffers.

Add a munmap callback to notify the PMU of any unmapping, which only be invoked when the munmap is implemented.

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

Quoted 22 lines · Stephane Eranian

Hi,

Quoted 11 lines · On Wed, Oct 24, 2018 at 8:12 AM <[email protected]> wrote:

From: Kan Liang <[email protected]>

To calculate the physical address, perf needs to walk the pages tables. The related mapping may has already been removed from pages table in some cases (e.g. large PEBS). The virtual address recorded in the first PEBS records may already be unmapped before draining PEBS buffers.

Add a munmap callback to notify the PMU of any unmapping, which only be invoked when the munmap is implemented.

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

Quoted 2 lines

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

That is actually a different problem. And you're right, we never did fix that.

I agree with you that Kan's Changelog is somewhat cryptic; it took me at least 3 times reading and looking at what the patches actually do to understand :/

Quoted 3 lines · Peter Zijlstra

That is actually a different problem. And you're right, we never did fix that.

it is a different problem but the solution is the same: PERF_RECORD_UNMAP!

Quoted 1 line · Stephane Eranian

On Wed, Oct 24, 2018 at 5:23 PM Peter Zijlstra <[email protected]> wrote:

Quoted 4 lines

Quoted 3 lines

That is actually a different problem. And you're right, we never did fix that.

it is a different problem but the solution is the same: PERF_RECORD_UNMAP!

But he's not actually doing PERF_RECORD_UNMAP, nor is his perf_munmap() hook sufficient to actually generate those.

Now I agree that if he's going to do an munmap hook, he should do it 'right' and at the very least allow for PERF_RECORD_UNMAP to be done, but ideally simply pick up and finish that patch we had back then.

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

In that case, if I recall correctly, the vma for 1st mapping (now at 4k) and that of the 2nd mapping (4k)
get merged into a single 8k vma and this is what perf_events will record for PERF_RECORD_MMAP.
On the perf tool side, it is assumed that if two timestamped mappings overlap then, the latter overrides
the former. In this case, perf would loose the mapping of the first 4kb and assume all symbols comes from
2nd mapping. Hopefully I got the scenario right. If so, then you'd need PERF_RECORD_UNMAP to
disambiguate assuming the perf tool is modified accordingly.

Quoted 11 lines · Stephane Eranian

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

Quoted 12 lines

In that case, if I recall correctly, the vma for 1st mapping (now at 4k) and that of the 2nd mapping (4k)
get merged into a single 8k vma and this is what perf_events will record for PERF_RECORD_MMAP.
On the perf tool side, it is assumed that if two timestamped mappings overlap then, the latter overrides
the former. In this case, perf would loose the mapping of the first 4kb and assume all symbols comes from
2nd mapping. Hopefully I got the scenario right. If so, then you'd need PERF_RECORD_UNMAP to
disambiguate assuming the perf tool is modified accordingly.

Hi Stephane and Peter,

I went through the link(https://lkml.org/lkml/2017/1/27/452). I'm trying to understand the problematic case.

It looks like the issue can only be triggered by perf inject --jit. Because it can inject extra MMAP events.
As my understanding, Linux kernel only try to merge VMAs if they are both from anon or they are both from the same file. --jit breaks the rule, and makes the merged VMA partly from anon, partly from file. Now, there is a new MMAP event which range covers the modified VMA. Without the help of MUNMAP event, perf tool have no idea if the new one is a newly merged VMA (modified VMA + a new VMA) or a brand new VMA. Current code just simply overwrite the modified VMAs. The VMA information which --jit injected may be lost. The symbolization may be lost as well.

Except --jit, the VMAs information should be consistent between kernel and perf tools. We shouldn't observe the problem. MUNMAP event is not needed.

Is my understanding correct?

Do you have a test case for the problem?

Thanks,
Kan

Hi Kan,

I built a small test case for you to demonstrate the issue for code and data. Compile the test program and then do:
For text:

$ perf record  ./mmap
$ perf report -D | fgrep MMAP2

The test program mmaps 2 pages, unmaps the second, and remap 1 page over the freed space.
If you look at the MMAP2 record, you will not be able to reconstruct what happened and perf will
get confused should it try to symbolize from the address range

With Text:
PERF_RECORD_MMAP2 5937/5937: [0x400000(0x1000) @ 0 08:01 400938 824817672]: r-xp /home/eranian/mmap
PERF_RECORD_MMAP2 5937/5937: [0x7f7c01019000(0x2000) @ 0x7f7c01019000 00:00 0 0]: rwxp //anon
PERF_RECORD_MMAP2 5937/5937: [0x7f7c01019000(0x2000) @ 0x7f7c01019000 00:00 0 0]: rwxp //anon

^^^^^^^^^^^^^^^^^^^^^^^^ captures the whole VMA but not the mapping change in user space

For data:

$ perf record -d  ./mmap
$ perf report -D | fgrep MMAP2
With data:
PERF_RECORD_MMAP2 6430/6430: [0x400000(0x1000) @ 0 08:01 400938
3278843184]: r-xp /home/eranian/mmap
PERF_RECORD_MMAP2 6430/6430: [0x7f4aa704b000(0x2000) @ 0x7f4aa704b000
00:00 0 0]: rw-p //anon
PERF_RECORD_MMAP2 6430/6430: [0x7f4aa704b000(0x2000) @ 0x7f4aa704b000
00:00 0 0]: rw-p //anon

Same test case with data.
Perf will think the entire 2 pages have been replaced when in fact only the second has.
I believe the problem is likely to impact data and jitted code cache

#include <sys/types.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/mman.h>
#include <err.h>
#include <getopt.h>

int main(int argc, char **argv)
{
void *addr1, *addr2;
size_t pgsz = sysconf(_SC_PAGESIZE);
int n = 2;
int ret;
int c, mode = 0;

while ((c = getopt(argc, argv, "hd")) != -1) {
switch (c) {
case 'h':
printf("[-h]\tget this help\n");
printf("[-d]\tuse data mmaps (no PROT_EXEC)\n");
return 0;
case 'd':
mode = PROT_EXEC;
break;
default:
errx(1, "unknown option");
}
}
/* default to data */
if (mode == 0)
mode = PROT_WRITE;

/*
* mmap 2 contiugous pages
*/
addr1 = mmap(NULL, n * pgsz, PROT_READ| mode, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); if (addr1 == (void *)MAP_FAILED)
err(1, "mmap 1 failed");

printf("addr1=[%p : %p]\n", addr1, addr1 + n * pgsz);

/*
* unmap only the second page
*/
ret = munmap(addr1 + pgsz, pgsz);
if (ret == -1)
err(1, "munmp failed");

/*
* mmap 1 page at the location of the unmap page (should reuse virtual space)
* This creates a continuous region built from two mmaps and potentially two different sources
* especially with jitted runtimes
*/

addr2 = mmap(addr1 + pgsz, 1 * pgsz, PROT_READ|PROT_WRITE | mode, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);

printf("addr2=%p\n", addr2);

if (addr2 == (void *)MAP_FAILED)
err(1, "mmap 2 failed");
if (addr2 != (addr1 + pgsz))
errx(1, "wrong mmap2 address");

sleep(1);

return 0;
}

Quoted 55 lines · Kan

Quoted 11 lines · Stephane Eranian

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

Quoted 12 lines

In that case, if I recall correctly, the vma for 1st mapping (now at 4k) and that of the 2nd mapping (4k)
get merged into a single 8k vma and this is what perf_events will record for PERF_RECORD_MMAP.
On the perf tool side, it is assumed that if two timestamped mappings overlap then, the latter overrides
the former. In this case, perf would loose the mapping of the first 4kb and assume all symbols comes from
2nd mapping. Hopefully I got the scenario right. If so, then you'd need PERF_RECORD_UNMAP to
disambiguate assuming the perf tool is modified accordingly.

Hi Stephane and Peter,

I went through the link(https://lkml.org/lkml/2017/1/27/452). I'm trying to understand the problematic case.

It looks like the issue can only be triggered by perf inject --jit. Because it can inject extra MMAP events.
As my understanding, Linux kernel only try to merge VMAs if they are both from anon or they are both from the same file. --jit breaks the rule, and makes the merged VMA partly from anon, partly from file. Now, there is a new MMAP event which range covers the modified VMA. Without the help of MUNMAP event, perf tool have no idea if the new one is a newly merged VMA (modified VMA + a new VMA) or a brand new VMA. Current code just simply overwrite the modified VMAs. The VMA information which --jit injected may be lost. The symbolization may be lost as well.

Except --jit, the VMAs information should be consistent between kernel and perf tools. We shouldn't observe the problem. MUNMAP event is not needed.

Is my understanding correct?

Do you have a test case for the problem?

Thanks,
Kan

Quoted 89 lines · Stephane Eranian

Hi Kan,

I built a small test case for you to demonstrate the issue for code and data. Compile the test program and then do:
For text:

$ perf record  ./mmap
$ perf report -D | fgrep MMAP2

The test program mmaps 2 pages, unmaps the second, and remap 1 page over the freed space.
If you look at the MMAP2 record, you will not be able to reconstruct what happened and perf will
get confused should it try to symbolize from the address range

With Text:
PERF_RECORD_MMAP2 5937/5937: [0x400000(0x1000) @ 0 08:01 400938 824817672]: r-xp /home/eranian/mmap
PERF_RECORD_MMAP2 5937/5937: [0x7f7c01019000(0x2000) @ 0x7f7c01019000 00:00 0 0]: rwxp //anon
PERF_RECORD_MMAP2 5937/5937: [0x7f7c01019000(0x2000) @ 0x7f7c01019000 00:00 0 0]: rwxp //anon

^^^^^^^^^^^^^^^^^^^^^^^^ captures the whole VMA but not the mapping change in user space

For data:

$ perf record -d  ./mmap
$ perf report -D | fgrep MMAP2
With data:
PERF_RECORD_MMAP2 6430/6430: [0x400000(0x1000) @ 0 08:01 400938
3278843184]: r-xp /home/eranian/mmap
PERF_RECORD_MMAP2 6430/6430: [0x7f4aa704b000(0x2000) @ 0x7f4aa704b000
00:00 0 0]: rw-p //anon
PERF_RECORD_MMAP2 6430/6430: [0x7f4aa704b000(0x2000) @ 0x7f4aa704b000
00:00 0 0]: rw-p //anon

Same test case with data.
Perf will think the entire 2 pages have been replaced when in fact only the second has.
I believe the problem is likely to impact data and jitted code cache

#include <sys/types.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/mman.h>
#include <err.h>
#include <getopt.h>

int main(int argc, char **argv)
{
void *addr1, *addr2;
size_t pgsz = sysconf(_SC_PAGESIZE);
int n = 2;
int ret;
int c, mode = 0;

while ((c = getopt(argc, argv, "hd")) != -1) {
switch (c) {
case 'h':
printf("[-h]\tget this help\n");
printf("[-d]\tuse data mmaps (no PROT_EXEC)\n");
return 0;
case 'd':
mode = PROT_EXEC;
break;
default:
errx(1, "unknown option");
}
}
/* default to data */
if (mode == 0)
mode = PROT_WRITE;

/*
* mmap 2 contiugous pages
*/
addr1 = mmap(NULL, n * pgsz, PROT_READ| mode, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); if (addr1 == (void *)MAP_FAILED)
err(1, "mmap 1 failed");

printf("addr1=[%p : %p]\n", addr1, addr1 + n * pgsz);

/*
* unmap only the second page
*/
ret = munmap(addr1 + pgsz, pgsz);
if (ret == -1)
err(1, "munmp failed");

Quoted 6 lines

/*
* mmap 1 page at the location of the unmap page (should reuse virtual space)
* This creates a continuous region built from two mmaps and potentially two different sources
* especially with jitted runtimes
*/

The two mmaps are both anon. As my understanding, we cannot symbolize from the anonymous address, can we?

If we cannot, why we have to distinguish with them? I think we do not need to know their sources for symbolization.

As my understanding, only --jit can inject MMAP event, which tag an anon. Perf can symbolize the address after that. Then the unmap is needed.

Thanks,
Kan

Quoted 71 lines

addr2 = mmap(addr1 + pgsz, 1 * pgsz, PROT_READ|PROT_WRITE | mode, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);

printf("addr2=%p\n", addr2);

if (addr2 == (void *)MAP_FAILED)
err(1, "mmap 2 failed");
if (addr2 != (addr1 + pgsz))
errx(1, "wrong mmap2 address");

sleep(1);

return 0;
}

Quoted 55 lines · Kan

Quoted 11 lines · Stephane Eranian

The need for this new record type extends beyond physical address conversions and PEBS. A long while ago, someone reported issues with symbolization related to perf lacking munmap tracking. It had to do with vma merging. I think the sequence of mmaps was as follows in the problematic case:

1.   addr1 = mmap(8192);
2.   munmap(addr1 + 4096, 4096)
3.   addr2 = mmap(addr1+4096, 4096)

If successful, that yields addr2 = addr1 + 4096 (could also get the same without forcing the address).

Quoted 12 lines

In that case, if I recall correctly, the vma for 1st mapping (now at 4k) and that of the 2nd mapping (4k)
get merged into a single 8k vma and this is what perf_events will record for PERF_RECORD_MMAP.
On the perf tool side, it is assumed that if two timestamped mappings overlap then, the latter overrides
the former. In this case, perf would loose the mapping of the first 4kb and assume all symbols comes from
2nd mapping. Hopefully I got the scenario right. If so, then you'd need PERF_RECORD_UNMAP to
disambiguate assuming the perf tool is modified accordingly.

Hi Stephane and Peter,

I went through the link(https://lkml.org/lkml/2017/1/27/452). I'm trying to understand the problematic case.

It looks like the issue can only be triggered by perf inject --jit. Because it can inject extra MMAP events.
As my understanding, Linux kernel only try to merge VMAs if they are both from anon or they are both from the same file. --jit breaks the rule, and makes the merged VMA partly from anon, partly from file. Now, there is a new MMAP event which range covers the modified VMA. Without the help of MUNMAP event, perf tool have no idea if the new one is a newly merged VMA (modified VMA + a new VMA) or a brand new VMA. Current code just simply overwrite the modified VMAs. The VMA information which --jit injected may be lost. The symbolization may be lost as well.

Except --jit, the VMAs information should be consistent between kernel and perf tools. We shouldn't observe the problem. MUNMAP event is not needed.

Is my understanding correct?

Do you have a test case for the problem?

Thanks,
Kan

Quoted 8 lines

Quoted 6 lines

/*
* mmap 1 page at the location of the unmap page (should reuse virtual space)
* This creates a continuous region built from two mmaps and potentially two different sources
* especially with jitted runtimes
*/

The two mmaps are both anon. As my understanding, we cannot symbolize from the anonymous address, can we?

Can't we build the same test case using an actual file mapping (both mmap from the same file)?

kernel/events/core.c +18 -0

+	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
+		pmu = cpuctx->ctx.pmu;
+
+		if (!pmu->munmap)
+			continue;
+
+		perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+		perf_pmu_disable(pmu);
+
+		pmu->munmap();
+
+		perf_pmu_enable(pmu);
+
+		perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
+	}
+	local_irq_restore(flags);
+}
+
 static void perf_event_switch(struct task_struct *task,
 			      struct task_struct *next_prev, bool sched_in);

mm/mmap.c +0 -0

index 5f2b2b184c60..61978ad8c480 100644

mm/mmap.c +0 -0

 	detach_vmas_to_be_unmapped(mm, vma, prev, end);
 	unmap_region(mm, vma, prev, start, end);
 

Signature · 1 line

2.17.1

kernel/events/core.c static void perf_pmu_sched_task(struct task_struct *prev, · line 3193

Quoted 7 lines · On Wed, Oct 24, 2018 at 08:11:15AM -0700, [email protected] wrote:

kernel/events/core.c +7 -0

+void perf_event_munmap(void)
+{
+	struct perf_cpu_context *cpuctx;
+	unsigned long flags;
+	struct pmu *pmu;
+
+	local_irq_save(flags);

It is impossible to get here with IRQs already disabled.

Quoted 22 lines

kernel/events/core.c +18 -0

+	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
+		pmu = cpuctx->ctx.pmu;
+
+		if (!pmu->munmap)
+			continue;
+
+		perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+		perf_pmu_disable(pmu);
+
+		pmu->munmap();
+
+		perf_pmu_enable(pmu);
+
+		perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
+	}
+	local_irq_restore(flags);
+}
+
  static void perf_event_switch(struct task_struct *task,
  			      struct task_struct *next_prev, bool sched_in);

mm/mmap.c int do_munmap(struct mm_struct *mm, unsigned long start, size_t len, · line 2780

Quoted 9 lines

mm/mmap.c +1 -0

index 5f2b2b184c60..61978ad8c480 100644
@@ -2777,6 +2777,7 @@ int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
  	/*
  	 * Remove the vma's, and unmap the actual pages
  	 */
+	perf_event_munmap();

I think that if you add the munmap hook, you should do it right and at least do it such that we can solve the other munmap problem.

Quoted 6 lines

mm/mmap.c +0 -0

  	detach_vmas_to_be_unmapped(mm, vma, prev, end);
  	unmap_region(mm, vma, prev, start, end);
 

Signature · 1 line

2.17.1

Is this patch you mentioned?
https://lkml.org/lkml/2017/1/27/452