Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303937 > unrolled thread

Re: [RFC] perf record: missing buildid for callstack modules

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-07 23:00 +0100
    Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-07 23:10 +0100
      Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@gmail.com> - 2016-01-08 00:30 +0100
        Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-08 00:50 +0100
          Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-08 19:10 +0100
            Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-08 19:20 +0100
              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-11 18:40 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-11 19:30 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-11 21:10 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-12 11:50 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 12:40 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-12 13:20 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 14:50 +0100
                        Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:40 +0100
                          Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 16:40 +0100
                            Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 16:50 +0100
                              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 17:20 +0100
                                Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 17:30 +0100
                                  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 18:20 +0100
                                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:30 +0100
                                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-13 13:50 +0100
                                        Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-14 12:30 +0100
                                          Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-14 12:40 +0100
                                            Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-15 03:00 +0100
                                              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-15 10:40 +0100
                              Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-12 22:10 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-12 14:10 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:40 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 16:40 +0100
                        Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:30 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:30 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:00 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-13 16:30 +0100
            Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@kernel.org> - 2016-01-09 11:40 +0100
              Re: [RFC] perf record: missing buildid for callstack modules Adrian Hunter <adrian.hunter@intel.com> - 2016-01-11 10:40 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@kernel.org> - 2016-01-11 12:10 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Adrian Hunter <adrian.hunter@intel.com> - 2016-01-11 13:00 +0100

Page 1 of 2  [1] 2  Next page →

#1303937 — Re: [RFC] perf record: missing buildid for callstack modules

Em Thu, Jan 07, 2016 at 01:56:14PM -0800, Stephane Eranian escreveu:
> Hi,
> 
> Whenever you do:
> 
>     $ perf record -g -a sleep 10
> 
> Perf will collect the callstack for each sample. At the end of the
> run, perf record
> adds the buildid for all dso with at least one sample. But when it does this, it
> only looks at the sampled IP and ignore the modules traversed by the callstack.
> That means that, it is not possible to uniquely identify the modules executed,
> unless they had at least one IP sample captured. But this is not
> always the case.
> 
> How about providing an option to perf record to force collecting
> buildid for all IPs
> captured in the callstack? I understand that would cost more at the end of the
> collection, but this would be beneficial to several monitoring scenarios.

I agree, would consider applying a patch that provides the option but
does not do this by default.

- Arnaldo

[toc] | [next] | [standalone]

#1303944

On Thu, Jan 7, 2016 at 1:59 PM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Thu, Jan 07, 2016 at 01:56:14PM -0800, Stephane Eranian escreveu:
>> Hi,
>>
>> Whenever you do:
>>
>>     $ perf record -g -a sleep 10
>>
>> Perf will collect the callstack for each sample. At the end of the
>> run, perf record
>> adds the buildid for all dso with at least one sample. But when it does this, it
>> only looks at the sampled IP and ignore the modules traversed by the callstack.
>> That means that, it is not possible to uniquely identify the modules executed,
>> unless they had at least one IP sample captured. But this is not
>> always the case.
>>
>> How about providing an option to perf record to force collecting
>> buildid for all IPs
>> captured in the callstack? I understand that would cost more at the end of the
>> collection, but this would be beneficial to several monitoring scenarios.
>
> I agree, would consider applying a patch that provides the option but
> does not do this by default.
>
I agree, not the default.

[toc] | [prev] | [next] | [standalone]

#1303998

On January 8, 2016 7:00:35 AM GMT+09:00, Stephane Eranian <eranian@google.com> wrote:
>On Thu, Jan 7, 2016 at 1:59 PM, Arnaldo Carvalho de Melo
><acme@kernel.org> wrote:
>> Em Thu, Jan 07, 2016 at 01:56:14PM -0800, Stephane Eranian escreveu:
>>> Hi,
>>>
>>> Whenever you do:
>>>
>>>     $ perf record -g -a sleep 10
>>>
>>> Perf will collect the callstack for each sample. At the end of the
>>> run, perf record
>>> adds the buildid for all dso with at least one sample. But when it
>does this, it
>>> only looks at the sampled IP and ignore the modules traversed by the
>callstack.
>>> That means that, it is not possible to uniquely identify the modules
>executed,
>>> unless they had at least one IP sample captured. But this is not
>>> always the case.
>>>
>>> How about providing an option to perf record to force collecting
>>> buildid for all IPs
>>> captured in the callstack? I understand that would cost more at the
>end of the
>>> collection, but this would be beneficial to several monitoring
>scenarios.
>>
>> I agree, would consider applying a patch that provides the option but
>> does not do this by default.
>>
>I agree, not the default.

Hi Stephane,

Please see

https://lkml.org/lkml/2015/3/22/249

Thanks,
Namhyung

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1304017

Em Fri, Jan 08, 2016 at 07:47:03AM +0900, Namhyung Kim escreveu:
> On January 8, 2016 7:00:35 AM GMT+09:00, Stephane Eranian <eranian@google.com> wrote:
> >On Thu, Jan 7, 2016 at 1:59 PM, Arnaldo Carvalho de Melo
> ><acme@kernel.org> wrote:
> >> Em Thu, Jan 07, 2016 at 01:56:14PM -0800, Stephane Eranian escreveu:
> >>> Hi,
> >>>
> >>> Whenever you do:
> >>>
> >>>     $ perf record -g -a sleep 10
> >>>
> >>> Perf will collect the callstack for each sample. At the end of the
> >>> run, perf record
> >>> adds the buildid for all dso with at least one sample. But when it
> >does this, it
> >>> only looks at the sampled IP and ignore the modules traversed by the
> >callstack.
> >>> That means that, it is not possible to uniquely identify the modules
> >executed,
> >>> unless they had at least one IP sample captured. But this is not
> >>> always the case.
> >>>
> >>> How about providing an option to perf record to force collecting
> >>> buildid for all IPs
> >>> captured in the callstack? I understand that would cost more at the
> >end of the
> >>> collection, but this would be beneficial to several monitoring
> >scenarios.
> >>
> >> I agree, would consider applying a patch that provides the option but
> >> does not do this by default.
> >>
> >I agree, not the default.
> 
> Hi Stephane,
> 
> Please see
> 
> https://lkml.org/lkml/2015/3/22/249


Oops, Stephane, please try this, so that we can finally merge it :-\

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1304796

On Thu, Jan 7, 2016 at 3:47 PM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Fri, Jan 08, 2016 at 07:47:03AM +0900, Namhyung Kim escreveu:
>> On January 8, 2016 7:00:35 AM GMT+09:00, Stephane Eranian <eranian@google.com> wrote:
>> >On Thu, Jan 7, 2016 at 1:59 PM, Arnaldo Carvalho de Melo
>> ><acme@kernel.org> wrote:
>> >> Em Thu, Jan 07, 2016 at 01:56:14PM -0800, Stephane Eranian escreveu:
>> >>> Hi,
>> >>>
>> >>> Whenever you do:
>> >>>
>> >>>     $ perf record -g -a sleep 10
>> >>>
>> >>> Perf will collect the callstack for each sample. At the end of the
>> >>> run, perf record
>> >>> adds the buildid for all dso with at least one sample. But when it
>> >does this, it
>> >>> only looks at the sampled IP and ignore the modules traversed by the
>> >callstack.
>> >>> That means that, it is not possible to uniquely identify the modules
>> >executed,
>> >>> unless they had at least one IP sample captured. But this is not
>> >>> always the case.
>> >>>
>> >>> How about providing an option to perf record to force collecting
>> >>> buildid for all IPs
>> >>> captured in the callstack? I understand that would cost more at the
>> >end of the
>> >>> collection, but this would be beneficial to several monitoring
>> >scenarios.
>> >>
>> >> I agree, would consider applying a patch that provides the option but
>> >> does not do this by default.
>> >>
>> >I agree, not the default.
>>
>> Hi Stephane,
>>
>> Please see
>>
>> https://lkml.org/lkml/2015/3/22/249
>
>
> Oops, Stephane, please try this, so that we can finally merge it :-\
>
I will try it today. However, I am a bit worried about the performance
impact. Unless I am missing something in this approach we may end up
looking up N times the same module if it appears in N callstacks. In
Andi's suggested approach, there would be only one pass at the beginning
(or the end of the run). But you could miss some modules if they are gone
by the time you run the pass.

[toc] | [prev] | [next] | [standalone]

#1304817

Em Fri, Jan 08, 2016 at 10:01:24AM -0800, Stephane Eranian escreveu:
> On Thu, Jan 7, 2016 at 3:47 PM, Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> > Em Fri, Jan 08, 2016 at 07:47:03AM +0900, Namhyung Kim escreveu:
> >> On January 8, 2016 7:00:35 AM GMT+09:00, Stephane Eranian <eranian@google.com> wrote:
> >> >>> How about providing an option to perf record to force collecting
> >> >>> buildid for all IPs
> >> >>> captured in the callstack? I understand that would cost more at the
> >> >end of the
> >> >>> collection, but this would be beneficial to several monitoring
> >> >scenarios.

> >> >> I agree, would consider applying a patch that provides the option but
> >> >> does not do this by default.

> >> >I agree, not the default.

> >> Please see

> >> https://lkml.org/lkml/2015/3/22/249

> > Oops, Stephane, please try this, so that we can finally merge it :-\

> I will try it today. However, I am a bit worried about the performance
> impact. Unless I am missing something in this approach we may end up
> looking up N times the same module if it appears in N callstacks. In
> Andi's suggested approach, there would be only one pass at the beginning
> (or the end of the run). But you could miss some modules if they are gone
> by the time you run the pass.

For kernel modules, yeah, since we'll have to synthesize them at session
start, we could as well save the buildids at that point, but then this,
as well as the saving of buildids for normal DSOs is racy, since we
collect it just at the end of the session.

We already discussed how to solve it, and it involves extending once
more PERF_RECORD_MMAP, so that, when we load a DSO we stash its build-id
in a per-DSO data structure in the kernel, then, when generating
PERF_RECORD_MMAP3 we put the buildid there, this way if any of those
binaries gets replaced while we're recording samples, we would notice,
i.e. we wouldn't care that much about the pathname, looking everything
by the content based buildid instead.

And also for modules, what if a module is loaded during the session?
We'll miss, or gets replaced by a newer version? We'll miss as well.

We need to hook into module loading/unloading and generate
PERF_RECORD_MMAP3 records there as well.

Best thing we can do right now (by adding extra events to the 'perf
record' command line) is to hook into places to detect these issues and
at least warn the user that some of the DSOs with hits got updated
during the session.

Namhyungs approach is no better than what we do now, i.e. it doesn't
detects such flaps, but will be just fine if no updates take place,
guaranteeing that later, at analysis time, we pick the right
ELF/kallsyms files, keyed by their build ids.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1306540

On Fri, Jan 08, 2016 at 03:19:42PM -0300, Arnaldo Carvalho de Melo wrote:
> We already discussed how to solve it, and it involves extending once
> more PERF_RECORD_MMAP, so that, when we load a DSO we stash its build-id
> in a per-DSO data structure in the kernel, then, when generating
> PERF_RECORD_MMAP3 we put the buildid there, this way if any of those
> binaries gets replaced while we're recording samples, we would notice,
> i.e. we wouldn't care that much about the pathname, looking everything
> by the content based buildid instead.

Does the kernel even know about the buildid crap? AFAIK the binfmt stuff
doesn't know or care about things like that. Heck, we support binfmts
that do not even have a buildid.

[toc] | [prev] | [next] | [standalone]

#1306587

Em Mon, Jan 11, 2016 at 06:30:36PM +0100, Peter Zijlstra escreveu:
> On Fri, Jan 08, 2016 at 03:19:42PM -0300, Arnaldo Carvalho de Melo wrote:
> > We already discussed how to solve it, and it involves extending once
> > more PERF_RECORD_MMAP, so that, when we load a DSO we stash its build-id
> > in a per-DSO data structure in the kernel, then, when generating
> > PERF_RECORD_MMAP3 we put the buildid there, this way if any of those
> > binaries gets replaced while we're recording samples, we would notice,
> > i.e. we wouldn't care that much about the pathname, looking everything
> > by the content based buildid instead.
 
> Does the kernel even know about the buildid crap? AFAIK the binfmt stuff
> doesn't know or care about things like that. Heck, we support binfmts
> that do not even have a buildid.

Well, we need some cookie like that, build-id or something that allows
us to find the right binary for doing symbol resolution, annotation,
etc.

And we need to do it at mmap time, i.e. we don't know upfront what DSOs
and if we do at the end we will incur in things we also don't like:
workload wide scanning of used DSOs, and they could've been replaced...

If we do it using these build id ELF sections distros have for quite a
while or with something else, we'd have to try and see.

I.e. the MMAP3 would have whatever is in a content-based pre-calculated
unique cookie slot, per DSO. How to populate that slot? Well, at load
time we could get that build-id section and put there, for ELF DSOs.

Or we could generate that at load time and later recalculate when
looking for the DSO to match what is in a perf.data file.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1306643

Hi,

On Mon, Jan 11, 2016 at 10:22 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Mon, Jan 11, 2016 at 06:30:36PM +0100, Peter Zijlstra escreveu:
>> On Fri, Jan 08, 2016 at 03:19:42PM -0300, Arnaldo Carvalho de Melo wrote:
>> > We already discussed how to solve it, and it involves extending once
>> > more PERF_RECORD_MMAP, so that, when we load a DSO we stash its build-id
>> > in a per-DSO data structure in the kernel, then, when generating
>> > PERF_RECORD_MMAP3 we put the buildid there, this way if any of those
>> > binaries gets replaced while we're recording samples, we would notice,
>> > i.e. we wouldn't care that much about the pathname, looking everything
>> > by the content based buildid instead.
>
>> Does the kernel even know about the buildid crap? AFAIK the binfmt stuff
>> doesn't know or care about things like that. Heck, we support binfmts
>> that do not even have a buildid.
>
It does not have to right now.

> Well, we need some cookie like that, build-id or something that allows
> us to find the right binary for doing symbol resolution, annotation,
> etc.
>
Well, you need a cookie that the kernel can have access to quickly and cheaply.
That cookie would have to be accessible to the user as well and has to be unique
per DSO. So it would have to be in the DSO headers and calculated from  known
fileds in the DSO header (which fields depends on the DSO binary
format, does not
have to be ELF). And you'd need a unique cookie -> filepath mapping.
So yeah, it would have to be in a MMAP-type record.

> And we need to do it at mmap time, i.e. we don't know upfront what DSOs
> and if we do at the end we will incur in things we also don't like:
> workload wide scanning of used DSOs, and they could've been replaced...
>
> If we do it using these build id ELF sections distros have for quite a
> while or with something else, we'd have to try and see.
>
> I.e. the MMAP3 would have whatever is in a content-based pre-calculated
> unique cookie slot, per DSO. How to populate that slot? Well, at load
> time we could get that build-id section and put there, for ELF DSOs.
>
> Or we could generate that at load time and later recalculate when
> looking for the DSO to match what is in a perf.data file.
>
I can see clearly how to make this work for ELF + buildid. It it much harder
for formats without buildids. You'd have to have a rule between
the kernel and user tool that controls what fields in the DSO and used
to compute
the hash. Buildid are at most 20 bytes IIRC. So any hash would have to
fit in these.

[toc] | [prev] | [next] | [standalone]

#1307248

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Fri, Jan 08, 2016 at 03:19:42PM -0300, Arnaldo Carvalho de Melo wrote:
>
> > We already discussed how to solve it, and it involves extending once more 
> > PERF_RECORD_MMAP, so that, when we load a DSO we stash its build-id in a 
> > per-DSO data structure in the kernel, then, when generating PERF_RECORD_MMAP3 
> > we put the buildid there, this way if any of those binaries gets replaced 
> > while we're recording samples, we would notice, i.e. we wouldn't care that 
> > much about the pathname, looking everything by the content based buildid 
> > instead.
> 
> Does the kernel even know about the buildid crap? AFAIK the binfmt stuff doesn't 
> know or care about things like that. Heck, we support binfmts that do not even 
> have a buildid.

The kernel's exec() code does not care about the past, it will execute whatever is 
fit to execute right now.

But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
binary mismatch can be very convincing and can send developers down the wrong path 
for hours. I'd expect my tooling to not do that.

Path names alone (the thing that exec() cares about) are not unique enough to 
identify the binary that was profiled. So we need a content hash - hence the 
build-ID.

Can you suggest a better solution than a build-time calculated content hash?

As for binary formats that suck and don't allow for a content hash: we do our 
best, but of course the risk of data mismatch is there. We could perhaps cache the 
binary inode's mtime field to at least produce a 'profile data is older than 
binary/DSO modification date!' warning. (Which check won't catch all cases, like 
cross-system profiling data matches.)

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1307298

On Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar wrote:
> > Does the kernel even know about the buildid crap? AFAIK the binfmt stuff doesn't 
> > know or care about things like that. Heck, we support binfmts that do not even 
> > have a buildid.
> 
> The kernel's exec() code does not care about the past, it will execute whatever is 
> fit to execute right now.
> 
> But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> binary mismatch can be very convincing and can send developers down the wrong path 
> for hours. I'd expect my tooling to not do that.

Well, it really is rather a rare case, replacing binaries you're
profiling. Sure, if it happens (by accident or otherwise) it can be a
pain, but the cost of fixing this 'problem' is huge.

> Path names alone (the thing that exec() cares about) are not unique enough to 
> identify the binary that was profiled. So we need a content hash - hence the 
> build-ID.
> 
> Can you suggest a better solution than a build-time calculated content hash?

Not really, but the current 'solution' is a massive pain. The result is
that perf-record needs to do a full scan of the recorded data after
completion and look for buildids across the system.

On my system that pass takes longer than the actual workload (of
building a kernel). Furthermore, the resulting data is useless for me.

> As for binary formats that suck and don't allow for a content hash: we do our 
> best, but of course the risk of data mismatch is there. We could perhaps cache the 
> binary inode's mtime field to at least produce a 'profile data is older than 
> binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> cross-system profiling data matches.)

So my problem with the kernel side thing is that I fear it will, again,
be a partial solution, and we'll still end up scanning the perf-record
output, ie. nothing better than we are now.

Sure, maybe we can have binfmt_elf read the buildid and cache it
someplace, maybe we can even have the other binfmt thingies do something
similar (at small cost, we obviously cannot compute hashes over files at
exec() time, that would upset people).

But what do we do for DSOs, does dlopen() ever end up in the binfmt
code? I would think not, I would fully expect the dynamic linker to just
mmap() the relevant bits and be done with it.

And we cannot, at mmap() time, 'assume' the file is ELF and try prodding
into it to find a buildid or whatnot.

And all for some weird corner case.

~ Peter, who thinks buildid stuff stinks.

[toc] | [prev] | [next] | [standalone]

#1307349

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar wrote:
> > > Does the kernel even know about the buildid crap? AFAIK the binfmt stuff doesn't 
> > > know or care about things like that. Heck, we support binfmts that do not even 
> > > have a buildid.
> > 
> > The kernel's exec() code does not care about the past, it will execute whatever is 
> > fit to execute right now.
> > 
> > But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> > display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> > binary mismatch can be very convincing and can send developers down the wrong path 
> > for hours. I'd expect my tooling to not do that.
> 
> Well, it really is rather a rare case, replacing binaries you're
> profiling. Sure, if it happens (by accident or otherwise) it can be a
> pain, but the cost of fixing this 'problem' is huge.

But isn't this the common case for developers, who rebuild their binaries all the 
time, while profiling them? Looking at the wrong profile without having an 
indication that it's wrong is a problem.

> > Path names alone (the thing that exec() cares about) are not unique enough to 
> > identify the binary that was profiled. So we need a content hash - hence the 
> > build-ID.
> > 
> > Can you suggest a better solution than a build-time calculated content hash?
> 
> Not really, but the current 'solution' is a massive pain. The result is that 
> perf-record needs to do a full scan of the recorded data after completion and 
> look for buildids across the system.
> 
> On my system that pass takes longer than the actual workload (of building a 
> kernel). Furthermore, the resulting data is useless for me.

Hm, that's a powerful performance argument. Why is it so slow? I'd assume that by 
default we only need to save the build-ID itself per object - which is like 20 
bytes?

> > As for binary formats that suck and don't allow for a content hash: we do our 
> > best, but of course the risk of data mismatch is there. We could perhaps cache 
> > the binary inode's mtime field to at least produce a 'profile data is older 
> > than binary/DSO modification date!' warning. (Which check won't catch all 
> > cases, like cross-system profiling data matches.)
> 
> So my problem with the kernel side thing is that I fear it will, again, be a 
> partial solution, and we'll still end up scanning the perf-record output, ie. 
> nothing better than we are now.
> 
> Sure, maybe we can have binfmt_elf read the buildid and cache it someplace, 
> maybe we can even have the other binfmt thingies do something similar (at small 
> cost, we obviously cannot compute hashes over files at exec() time, that would 
> upset people).
> 
> But what do we do for DSOs, does dlopen() ever end up in the binfmt code? I 
> would think not, I would fully expect the dynamic linker to just mmap() the 
> relevant bits and be done with it.
> 
> And we cannot, at mmap() time, 'assume' the file is ELF and try prodding into it 
> to find a buildid or whatnot.
> 
> And all for some weird corner case.

So could we perhaps just switch the whole thing over to be mtime based: mtime is 
pretty indicative of whether a binary is the right one or not.

And mtime could be checked at perf report time, not at perf record time: we'd only 
have to check whether the mtime of the object we read at perf report time is newer 
than the mtime of the perf.data (the creation of the profile).

This does not solve rare corner cases like cross-system profiling, but I think the 
common case should not be burdened with the overhead of a rare case.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1307418

On Tue, Jan 12, 2016 at 01:18:05PM +0100, Ingo Molnar wrote:
> > Well, it really is rather a rare case, replacing binaries you're
> > profiling. Sure, if it happens (by accident or otherwise) it can be a
> > pain, but the cost of fixing this 'problem' is huge.
> 
> But isn't this the common case for developers, who rebuild their binaries all the 
> time, while profiling them? Looking at the wrong profile without having an 
> indication that it's wrong is a problem.

I tend to:

1:
	edit code
	compile code
	(perf) run code
	inspect profile
	goto 1

which does not have this problem at all. Only if you want to inspect
'old' profiles does this problem occur.

> > On my system that pass takes longer than the actual workload (of building a 
> > kernel). Furthermore, the resulting data is useless for me.
> 
> Hm, that's a powerful performance argument. Why is it so slow? I'd assume that by 
> default we only need to save the build-ID itself per object - which is like 20 
> bytes?

There is no buildid in the recorded data, I think it looks at every MMAP
record, finds the associated file, extracts the buildid and copies crap
into .debug directory.

Also, just parsing the gigabytes of data that comes out of perf-record
takes significant time, let alone poking around the filesystem and
copying files around.

Furthermore, I have 40 CPUs generating data, while only a single one is
doing all this post-processing.

# rm -rf ~/.debug/
# make O=defconfig-build/ clean; perf record make O=defconfig-build/ -j80 -s
# ls -lah perf.data
-rw------- 1 root root 2.7G Jan 12 14:18 perf.data
# du -sh ~/.debug/
240M    /root/.debug/

That's a lot of pointless work.


> > And all for some weird corner case.
> 
> So could we perhaps just switch the whole thing over to be mtime based: mtime is 
> pretty indicative of whether a binary is the right one or not.
> 
> And mtime could be checked at perf report time, not at perf record time: we'd only 
> have to check whether the mtime of the object we read at perf report time is newer 
> than the mtime of the perf.data (the creation of the profile).
> 
> This does not solve rare corner cases like cross-system profiling, but I think the 
> common case should not be burdened with the overhead of a rare case.

That might work, we have easy access to the mtime data for any file.

[toc] | [prev] | [next] | [standalone]

#1307486

Em Tue, Jan 12, 2016 at 02:40:12PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 12, 2016 at 01:18:05PM +0100, Ingo Molnar wrote:
> > > Well, it really is rather a rare case, replacing binaries you're
> > > profiling. Sure, if it happens (by accident or otherwise) it can be a
> > > pain, but the cost of fixing this 'problem' is huge.
> > 
> > But isn't this the common case for developers, who rebuild their binaries all the 
> > time, while profiling them? Looking at the wrong profile without having an 
> > indication that it's wrong is a problem.
> 
> I tend to:
> 
> 1:
> 	edit code
> 	compile code
> 	(perf) run code
> 	inspect profile
> 	goto 1
> 
> which does not have this problem at all. Only if you want to inspect
> 'old' profiles does this problem occur.
> 
> > > On my system that pass takes longer than the actual workload (of building a 
> > > kernel). Furthermore, the resulting data is useless for me.
> > 
> > Hm, that's a powerful performance argument. Why is it so slow? I'd assume that by 
> > default we only need to save the build-ID itself per object - which is like 20 
> > bytes?
> 
> There is no buildid in the recorded data, I think it looks at every MMAP
> record, finds the associated file, extracts the buildid and copies crap
> into .debug directory.

$ perf record -h build

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -B, --no-buildid      do not collect buildids in perf.data
    -N, --no-buildid-cache
                          do not update the buildid cache

[acme@zoo linux]$

> Also, just parsing the gigabytes of data that comes out of perf-record
> takes significant time, let alone poking around the filesystem and

Right, that is what we would elliminate with stashing the content-based
cookie into a PERF_RECORD_MMAP3 record.

> copying files around.
> 
> Furthermore, I have 40 CPUs generating data, while only a single one is
> doing all this post-processing.
> 
> # rm -rf ~/.debug/
> # make O=defconfig-build/ clean; perf record make O=defconfig-build/ -j80 -s
> # ls -lah perf.data
> -rw------- 1 root root 2.7G Jan 12 14:18 perf.data
> # du -sh ~/.debug/
> 240M    /root/.debug/
> 
> That's a lot of pointless work.

Right, for you -B is the only sane way (or doing that in ~/.perfconfig
and disabling this for good).

BTW, mtime would incur in postprocessing it all.

- Arnaldo
 
> > > And all for some weird corner case.
> > 
> > So could we perhaps just switch the whole thing over to be mtime based: mtime is 
> > pretty indicative of whether a binary is the right one or not.
> > 
> > And mtime could be checked at perf report time, not at perf record time: we'd only 
> > have to check whether the mtime of the object we read at perf report time is newer 
> > than the mtime of the perf.data (the creation of the profile).
> > 
> > This does not solve rare corner cases like cross-system profiling, but I think the 
> > common case should not be burdened with the overhead of a rare case.
> 
> That might work, we have easy access to the mtime data for any file.

[toc] | [prev] | [next] | [standalone]

#1307558

On Tue, Jan 12, 2016 at 11:38:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > Also, just parsing the gigabytes of data that comes out of perf-record
> > takes significant time, let alone poking around the filesystem and
> 
> Right, that is what we would elliminate with stashing the content-based
> cookie into a PERF_RECORD_MMAP3 record.

Again, how would you go about getting that cookie for a DSO? The whole
kernel isn't involved with dlopen(), all it sees is a mmap(PROT_EXEC).

> BTW, mtime would incur in postprocessing it all.

mtime can still warn you if things are non-matching at report time
without this post-processing, and thereby solves the problem of staring
at broken/wrong data.

It doesn't get you right data, but knowing your data is broken allows
you to manually do things 'right'.

[toc] | [prev] | [next] | [standalone]

#1307572

Em Tue, Jan 12, 2016 at 04:34:40PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 12, 2016 at 11:38:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Also, just parsing the gigabytes of data that comes out of perf-record
> > > takes significant time, let alone poking around the filesystem and
> > 
> > Right, that is what we would elliminate with stashing the content-based
> > cookie into a PERF_RECORD_MMAP3 record.
> 
> Again, how would you go about getting that cookie for a DSO? The whole
> kernel isn't involved with dlopen(), all it sees is a mmap(PROT_EXEC).
> 
> > BTW, mtime would incur in postprocessing it all.
> 
> mtime can still warn you if things are non-matching at report time
> without this post-processing, and thereby solves the problem of staring
> at broken/wrong data.

How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
if we don't look at those records? What mtime are you talking about?
 
> It doesn't get you right data, but knowing your data is broken allows
> you to manually do things 'right'.

[toc] | [prev] | [next] | [standalone]

#1307592

On Tue, Jan 12, 2016 at 12:48:12PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 12, 2016 at 04:34:40PM +0100, Peter Zijlstra escreveu:
> > On Tue, Jan 12, 2016 at 11:38:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Also, just parsing the gigabytes of data that comes out of perf-record
> > > > takes significant time, let alone poking around the filesystem and
> > > 
> > > Right, that is what we would elliminate with stashing the content-based
> > > cookie into a PERF_RECORD_MMAP3 record.
> > 
> > Again, how would you go about getting that cookie for a DSO? The whole
> > kernel isn't involved with dlopen(), all it sees is a mmap(PROT_EXEC).
> > 
> > > BTW, mtime would incur in postprocessing it all.
> > 
> > mtime can still warn you if things are non-matching at report time
> > without this post-processing, and thereby solves the problem of staring
> > at broken/wrong data.
> 
> How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
> if we don't look at those records?

Kernel side, the vma has a vm_file member. So
vma->vm_file->f_inode->i_mtime will get us that for all file based
mmap()s.

(or maybe ctime, not sure how popular it is these days to switch off
mtime accounting).

> What mtime are you talking about?

perf-report looks at those records, it can compare the recorded mtime vs
the currently observed mtime and complain if non-matching.

[toc] | [prev] | [next] | [standalone]

#1307608

On Tue, Jan 12, 2016 at 05:10:27PM +0100, Peter Zijlstra wrote:
> > How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
> > if we don't look at those records?
> 
> Kernel side, the vma has a vm_file member. So
> vma->vm_file->f_inode->i_mtime will get us that for all file based
> mmap()s.
> 
> (or maybe ctime, not sure how popular it is these days to switch off
> mtime accounting).

And this would (obviously) be a good time to see what else we'd like to
stuff in there.

---
 include/uapi/linux/perf_event.h | 26 +++++++++++++++++++++++++-
 kernel/events/core.c            | 19 +++++++++++++++----
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1afe9623c1a7..a0f43140a0e2 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -340,7 +340,8 @@ struct perf_event_attr {
 				comm_exec      :  1, /* flag comm events that are due to an exec */
 				use_clockid    :  1, /* use @clockid for time fields */
 				context_switch :  1, /* context switch data */
-				__reserved_1   : 37;
+				mmap3          :  1, /* include mmap with mtime */
+				__reserved_1   : 36;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
@@ -856,6 +857,29 @@ enum perf_event_type {
 	 */
 	PERF_RECORD_SWITCH_CPU_WIDE		= 15,
 
+	/*
+	 * The MMAP3 records are an augmented version of MMAP2, they add
+	 * mtime.
+	 *
+	 * struct {
+	 *	struct perf_event_header	header;
+	 *
+	 *	u32				pid, tid;
+	 *	u64				addr;
+	 *	u64				len;
+	 *	u64				pgoff;
+	 *	u32				maj;
+	 *	u32				min;
+	 *	u64				ino;
+	 *	u64				ino_generation;
+	 *	u32				prot, flags;
+	 *	u64				mtime;
+	 *	char				filename[];
+	 *	struct sample_id		sample_id;
+	 * }
+	 */
+	PERF_RECORD_MMAP3			= 16,
+
 	PERF_RECORD_MAX,			/* non-ABI */
 };
 
diff --git a/kernel/events/core.c b/kernel/events/core.c
index bf8244190d0f..d400da14b923 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5661,7 +5661,7 @@ perf_event_aux(perf_event_aux_output_cb output, void *data,
 /*
  * task tracking -- fork/exit
  *
- * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap_data | attr.task
+ * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap3 | attr.mmap_data | attr.task
  */
 
 struct perf_task_event {
@@ -5682,8 +5682,8 @@ struct perf_task_event {
 static int perf_event_task_match(struct perf_event *event)
 {
 	return event->attr.comm  || event->attr.mmap ||
-	       event->attr.mmap2 || event->attr.mmap_data ||
-	       event->attr.task;
+	       event->attr.mmap2 || event->attr.mmap3 ||
+	       event->attr.mmap_data || event->attr.task;
 }
 
 static void perf_event_task_output(struct perf_event *event,
@@ -5872,6 +5872,7 @@ struct perf_mmap_event {
 	u64			ino;
 	u64			ino_generation;
 	u32			prot, flags;
+	u64			mtime;
 
 	struct {
 		struct perf_event_header	header;
@@ -5892,7 +5893,7 @@ static int perf_event_mmap_match(struct perf_event *event,
 	int executable = vma->vm_flags & VM_EXEC;
 
 	return (!executable && event->attr.mmap_data) ||
-	       (executable && (event->attr.mmap || event->attr.mmap2));
+	       (executable && (event->attr.mmap || event->attr.mmap2 || event-attr.mmap3));
 }
 
 static void perf_event_mmap_output(struct perf_event *event,
@@ -5916,6 +5917,9 @@ static void perf_event_mmap_output(struct perf_event *event,
 		mmap_event->event_id.header.size += sizeof(mmap_event->prot);
 		mmap_event->event_id.header.size += sizeof(mmap_event->flags);
 	}
+	if (event->attr.mmap3) {
+		mmap_event->event_id.header.size += sizeof(mmap_event->mtime);
+	}
 
 	perf_event_header__init_id(&mmap_event->event_id.header, &sample, event);
 	ret = perf_output_begin(&handle, event,
@@ -5936,6 +5940,9 @@ static void perf_event_mmap_output(struct perf_event *event,
 		perf_output_put(&handle, mmap_event->prot);
 		perf_output_put(&handle, mmap_event->flags);
 	}
+	if (event->attr.mmap3) {
+		perf_output_put(&handle, mmap_event->mtime);
+	}
 
 	__output_copy(&handle, mmap_event->file_name,
 				   mmap_event->file_size);
@@ -5954,6 +5961,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 	int maj = 0, min = 0;
 	u64 ino = 0, gen = 0;
 	u32 prot = 0, flags = 0;
+	u64 mtime = 0;
 	unsigned int size;
 	char tmp[16];
 	char *buf = NULL;
@@ -5984,6 +5992,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 		gen = inode->i_generation;
 		maj = MAJOR(dev);
 		min = MINOR(dev);
+		mtime = timespec_to_ns(inode->i_mtime);
 
 		if (vma->vm_flags & VM_READ)
 			prot |= PROT_READ;
@@ -6054,6 +6063,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 	mmap_event->ino_generation = gen;
 	mmap_event->prot = prot;
 	mmap_event->flags = flags;
+	mmap_event->mtime = mtime;
 
 	if (!(vma->vm_flags & VM_EXEC))
 		mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA;
@@ -6096,6 +6106,7 @@ void perf_event_mmap(struct vm_area_struct *vma)
 		/* .ino_generation (attr_mmap2 only) */
 		/* .prot (attr_mmap2 only) */
 		/* .flags (attr_mmap2 only) */
+		/* .mtime (attr_mmap3 only) */
 	};
 
 	perf_event_mmap_event(&mmap_event);

[toc] | [prev] | [next] | [standalone]

#1307648

Em Tue, Jan 12, 2016 at 05:27:19PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 12, 2016 at 05:10:27PM +0100, Peter Zijlstra wrote:
> > > How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
> > > if we don't look at those records?

> > Kernel side, the vma has a vm_file member. So

Gotcha, via PERF_RECORD_MMAP3, using something we have (mtime) that
while not good as a content-based cookie (that we don't easily have at
PERF_RECORD_MMAP3 generation time) will allow us to do minimal detection
of mismatched DSOs at analysis time.

> > vma->vm_file->f_inode->i_mtime will get us that for all file based
> > mmap()s.

> > (or maybe ctime, not sure how popular it is these days to switch off
> > mtime accounting).
 
> And this would (obviously) be a good time to see what else we'd like to
> stuff in there.

Perhaps a version number? Else we'll be using those reserved bits again
when we decide to introduce MMAP4 ;-\

But I still think we should have a content-based cookie to be delivered
via that record, so at a minimum leave a cookie there starting with a
size, that, for now, would be zero. If present, that would get into the
current build-id infrastructure existing in the tooling side, something
like:

+	/*
+	 * The MMAP3 records are an augmented version of MMAP2, they add
+	 * mtime and an optional content-based cookie, if available.
+	 *	struct perf_event_header	header;
+	 *
+	 *	u32				pid, tid;
+	 *	u64				addr;
+	 *	u64				len;
+	 *	u64				pgoff;
+	 *	u32				maj;
+	 *	u32				min;
+	 *	u64				ino;
+	 *	u64				ino_generation;
+	 *	u32				prot, flags;
+	 *	u64				mtime;
+	 *      u32				cookie_len
+	 *	char				cookie[2+];
+	 *	char				filename[];
+	 *	struct sample_id		sample_id;

Then someone (I want to do this, after processing more stuff from a
neverending backlog) should try to experiment with the varios ELF
loaders, etc to, in the process of loading, set that content-based
cookie somehow (ioctl? prctl? whatever).

- Arnaldo

> ---
>  include/uapi/linux/perf_event.h | 26 +++++++++++++++++++++++++-
>  kernel/events/core.c            | 19 +++++++++++++++----
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 1afe9623c1a7..a0f43140a0e2 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -340,7 +340,8 @@ struct perf_event_attr {
>  				comm_exec      :  1, /* flag comm events that are due to an exec */
>  				use_clockid    :  1, /* use @clockid for time fields */
>  				context_switch :  1, /* context switch data */
> -				__reserved_1   : 37;
> +				mmap3          :  1, /* include mmap with mtime */
> +				__reserved_1   : 36;
>  
>  	union {
>  		__u32		wakeup_events;	  /* wakeup every n events */
> @@ -856,6 +857,29 @@ enum perf_event_type {
>  	 */
>  	PERF_RECORD_SWITCH_CPU_WIDE		= 15,
>  
> +	/*
> +	 * The MMAP3 records are an augmented version of MMAP2, they add
> +	 * mtime.
> +	 *
> +	 * struct {
> +	 *	struct perf_event_header	header;
> +	 *
> +	 *	u32				pid, tid;
> +	 *	u64				addr;
> +	 *	u64				len;
> +	 *	u64				pgoff;
> +	 *	u32				maj;
> +	 *	u32				min;
> +	 *	u64				ino;
> +	 *	u64				ino_generation;
> +	 *	u32				prot, flags;
> +	 *	u64				mtime;
> +	 *	char				filename[];
> +	 *	struct sample_id		sample_id;
> +	 * }
> +	 */
> +	PERF_RECORD_MMAP3			= 16,
> +
>  	PERF_RECORD_MAX,			/* non-ABI */
>  };
>  
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index bf8244190d0f..d400da14b923 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5661,7 +5661,7 @@ perf_event_aux(perf_event_aux_output_cb output, void *data,
>  /*
>   * task tracking -- fork/exit
>   *
> - * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap_data | attr.task
> + * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap3 | attr.mmap_data | attr.task
>   */
>  
>  struct perf_task_event {
> @@ -5682,8 +5682,8 @@ struct perf_task_event {
>  static int perf_event_task_match(struct perf_event *event)
>  {
>  	return event->attr.comm  || event->attr.mmap ||
> -	       event->attr.mmap2 || event->attr.mmap_data ||
> -	       event->attr.task;
> +	       event->attr.mmap2 || event->attr.mmap3 ||
> +	       event->attr.mmap_data || event->attr.task;
>  }
>  
>  static void perf_event_task_output(struct perf_event *event,
> @@ -5872,6 +5872,7 @@ struct perf_mmap_event {
>  	u64			ino;
>  	u64			ino_generation;
>  	u32			prot, flags;
> +	u64			mtime;
>  
>  	struct {
>  		struct perf_event_header	header;
> @@ -5892,7 +5893,7 @@ static int perf_event_mmap_match(struct perf_event *event,
>  	int executable = vma->vm_flags & VM_EXEC;
>  
>  	return (!executable && event->attr.mmap_data) ||
> -	       (executable && (event->attr.mmap || event->attr.mmap2));
> +	       (executable && (event->attr.mmap || event->attr.mmap2 || event-attr.mmap3));
>  }
>  
>  static void perf_event_mmap_output(struct perf_event *event,
> @@ -5916,6 +5917,9 @@ static void perf_event_mmap_output(struct perf_event *event,
>  		mmap_event->event_id.header.size += sizeof(mmap_event->prot);
>  		mmap_event->event_id.header.size += sizeof(mmap_event->flags);
>  	}
> +	if (event->attr.mmap3) {
> +		mmap_event->event_id.header.size += sizeof(mmap_event->mtime);
> +	}
>  
>  	perf_event_header__init_id(&mmap_event->event_id.header, &sample, event);
>  	ret = perf_output_begin(&handle, event,
> @@ -5936,6 +5940,9 @@ static void perf_event_mmap_output(struct perf_event *event,
>  		perf_output_put(&handle, mmap_event->prot);
>  		perf_output_put(&handle, mmap_event->flags);
>  	}
> +	if (event->attr.mmap3) {
> +		perf_output_put(&handle, mmap_event->mtime);
> +	}
>  
>  	__output_copy(&handle, mmap_event->file_name,
>  				   mmap_event->file_size);
> @@ -5954,6 +5961,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
>  	int maj = 0, min = 0;
>  	u64 ino = 0, gen = 0;
>  	u32 prot = 0, flags = 0;
> +	u64 mtime = 0;
>  	unsigned int size;
>  	char tmp[16];
>  	char *buf = NULL;
> @@ -5984,6 +5992,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
>  		gen = inode->i_generation;
>  		maj = MAJOR(dev);
>  		min = MINOR(dev);
> +		mtime = timespec_to_ns(inode->i_mtime);
>  
>  		if (vma->vm_flags & VM_READ)
>  			prot |= PROT_READ;
> @@ -6054,6 +6063,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
>  	mmap_event->ino_generation = gen;
>  	mmap_event->prot = prot;
>  	mmap_event->flags = flags;
> +	mmap_event->mtime = mtime;
>  
>  	if (!(vma->vm_flags & VM_EXEC))
>  		mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA;
> @@ -6096,6 +6106,7 @@ void perf_event_mmap(struct vm_area_struct *vma)
>  		/* .ino_generation (attr_mmap2 only) */
>  		/* .prot (attr_mmap2 only) */
>  		/* .flags (attr_mmap2 only) */
> +		/* .mtime (attr_mmap3 only) */
>  	};
>  
>  	perf_event_mmap_event(&mmap_event);

[toc] | [prev] | [next] | [standalone]

#1308261

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> > And this would (obviously) be a good time to see what else we'd like to
> > stuff in there.
> 
> Perhaps a version number? Else we'll be using those reserved bits again
> when we decide to introduce MMAP4 ;-\

No version numbers please. Cannot we have a size field and be done with it? The 
size is the 'version', if we only ever expand the record. (which is the typical 
case)

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web