linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>,
	Namhyung Kim <namhyung@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	"ak@linux.intel.com" <ak@linux.intel.com>
Subject: Re: [RFC] perf record: missing buildid for callstack modules
Date: Wed, 13 Jan 2016 10:57:38 +0100	[thread overview]
Message-ID: <20160113095738.GA9092@gmail.com> (raw)
In-Reply-To: <20160112142357.GS18367@kernel.org>


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

> Em Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar escreveu:
> > 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.)
> 
> So, we could think of this as: binary formats that want to aid
> observability tools to:
> 
> 1) Detect mismatches in contents for DSOs present at recording time to
>    those to be used at analysis time.
> 
> 2) Find symtabs, DSO binary contents, CFI tables, present in the DSO
>    where samples were taken.
> 
> Using mtime, as suggested in other messages will help with #1, but not
> with #2.

But but ... why is #2 a problem with mtime? If we have an out of date record in 
the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
out of date, most likely because the binary the developer is working on got 
rebuilt, or the system got upgraded - in both cases the developer does not care 
about the old records anymore...

What matters is #1, to detect mismatches, to be a reliable tool. Once we've 
detected that, we can inform the user and our job is mostly done.

But reliable != perfect time machine. Really, #2 is a second, third order concern 
that should never cause slowdowns on the magnitude that Peter is complaining 
about!

I realize that there might be special workflows (such as system-wide monitoring) 
where collecting at recording time might be useful, but those are not the common 
case - and they should not slow down the common case.

> Checking for inefficiencies in the current approach of
> right-after-recording post-processing looking for PERF_RECORD_MMAPs,
> Adrian suggested something here, also disabling the saving into
> ~/.debug/ will help, collecting numbers would be great.

I think Peter mentioned a number: the kernel build time almost _doubles_ with 
this. That's clearly unacceptable.

> But the mtime thing also requires traversing the whole perf.data
> contents looking for those paths in PERF_RECORD_MMAP records.

But why? Why cannot we do it at perf report time, when we will parse them anyway?

Thanks,

	Ingo

  reply	other threads:[~2016-01-13  9:58 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07 21:56 [RFC] perf record: missing buildid for callstack modules Stephane Eranian
2016-01-07 21:57 ` Andi Kleen
2016-01-07 22:00   ` Stephane Eranian
2016-01-07 21:59 ` Arnaldo Carvalho de Melo
2016-01-07 22:00   ` Stephane Eranian
2016-01-07 22:47     ` Namhyung Kim
2016-01-07 23:47       ` Arnaldo Carvalho de Melo
2016-01-08 18:01         ` Stephane Eranian
2016-01-08 18:19           ` Arnaldo Carvalho de Melo
2016-01-11 17:30             ` Peter Zijlstra
2016-01-11 18:22               ` Arnaldo Carvalho de Melo
2016-01-11 20:06                 ` Stephane Eranian
2016-01-12 10:39               ` Ingo Molnar
2016-01-12 11:35                 ` Peter Zijlstra
2016-01-12 12:18                   ` Ingo Molnar
2016-01-12 13:40                     ` Peter Zijlstra
2016-01-12 14:38                       ` Arnaldo Carvalho de Melo
2016-01-12 15:34                         ` Peter Zijlstra
2016-01-12 15:48                           ` Arnaldo Carvalho de Melo
2016-01-12 16:10                             ` Peter Zijlstra
2016-01-12 16:27                               ` Peter Zijlstra
2016-01-12 17:15                                 ` Arnaldo Carvalho de Melo
2016-01-13 10:21                                   ` Ingo Molnar
2016-01-13 12:40                                     ` Peter Zijlstra
2016-01-14 11:27                                       ` Ingo Molnar
2016-01-14 11:36                                         ` Peter Zijlstra
2016-01-15  1:59                                           ` Stephane Eranian
2016-01-15  9:34                                             ` Peter Zijlstra
2016-01-15 18:58                                               ` Stephane Eranian
2016-01-15 19:49                                                 ` Arnaldo Carvalho de Melo
2016-01-15 21:49                                                   ` Stephane Eranian
2016-01-15 21:36                                                 ` Peter Zijlstra
2016-01-12 21:02                             ` Stephane Eranian
2016-01-12 13:08                   ` One Thousand Gnomes
2016-01-12 14:34                   ` Arnaldo Carvalho de Melo
2016-01-12 15:37                     ` Peter Zijlstra
2016-01-13 10:25                       ` Ingo Molnar
2016-01-12 14:23                 ` Arnaldo Carvalho de Melo
2016-01-13  9:57                   ` Ingo Molnar [this message]
2016-01-13 15:27                     ` Arnaldo Carvalho de Melo
2016-01-19 14:56                     ` Namhyung Kim
2016-01-19 15:27                       ` Peter Zijlstra
2016-01-19 15:48                         ` Arnaldo Carvalho de Melo
2016-01-09 10:31           ` Namhyung Kim
2016-01-11  9:27             ` Adrian Hunter
2016-01-11 11:02               ` Namhyung Kim
2016-01-11 11:54                 ` Adrian Hunter

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=20160113095738.GA9092@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@gmail.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).