Re: [PATCH] handle munmap records in tools/perf was: Re: perf/jit doesn't cope well with mprotect() to jit containing pages

[Stephane Eranian]

On Fri, Jan 27, 2017 at 11:18 AM, Arnaldo Carvalho de Melo
<arnaldo.melo@xxxxxxxxx> wrote:
> Em Fri, Jan 27, 2017 at 10:10:09AM -0800, Stephane Eranian escreveu:
>> On Fri, Jan 27, 2017 at 10:05 AM, Arnaldo Carvalho de Melo
>> <arnaldo.melo@xxxxxxxxx> wrote:
>> > Em Fri, Jan 27, 2017 at 09:46:55AM -0800, Stephane Eranian escreveu:
>> >> On Fri, Jan 27, 2017 at 9:38 AM, Arnaldo Carvalho de Melo
>> >> <arnaldo.melo@xxxxxxxxx> wrote:
>> >> > Em Fri, Jan 27, 2017 at 12:43:05PM -0300, Arnaldo Carvalho de Melo escreveu:
>> >> >> Em Fri, Jan 27, 2017 at 02:07:02PM +0100, Peter Zijlstra escreveu:
>> >> >> > Something like the (compile tested only) below might be sufficient to
>> >> >> > disambiguate things. It would need a corresponding tools/perf patch of
>> >> >> > course, but I'm not too familiar with that code anymore.
>> >> >>
>> >> >> I'm working on patch to do feature test, fallback and handling of the
>> >> >> event, etc, will post later.
>> >> >
>> >> > Just compile tested, need to build a kernel with PeterZ's patch to test,
>> >> > feel free to go from there if in a hurry.
>> >> >
>> >> >
>> >> > The place where the map is yanked out of the thread's maps rbtree is at
>> >> >
>> >> > machine__process_munmap_event()
>> >> >
>> >> > The rest is making sure the tool works with older kernels, deals with
>> >> > endianness in the record in a perf.data file for cross platform
>> >> > analysis, hooking it to the various tools where handling this event
>> >> > makes sense.
>> >> >
>> >> At first glance this patch handles the munmap() well. But it will not solve
>> >> the case of Andres. Unless you're telling me that the kernel with Peterz's patch
>> >
>> >
>> > Nah, I just tried to implement support for the facility PeterZ was
>> > proposing for the kernel, not trying to solve Andres, silly me ;-)
>> >
>> > But then it doesn't even does that well, as it needs to take munmap.len
>> > into account, to possibly split the map if they aren't a perfect match
>> > (start, len).
>> >
>> Ah, yes, that's correct. You need to consider len and possibly split
>> the maps you have.
>
> In fact we need to replicate the kernel's do_munmap() routine, as the
> munmap range may straddle multiple existing maps, possibly splitting the
> first and/or the last, ditching whatever is in between.
>
Yes. So this is not a small change in perf unfortunately.