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

From: Arnaldo Carvalho de Melo
Date: Fri Jan 27 2017 - 14:20:04 EST


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.

- Arnaldo