All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.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: Tue, 12 Jan 2016 12:35:21 +0100	[thread overview]
Message-ID: <20160112113521.GC6357@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20160112103943.GA6310@gmail.com>

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.

  reply	other threads:[~2016-01-12 11:35 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 [this message]
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
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=20160112113521.GC6357@twins.programming.kicks-ass.net \
    --to=peterz@infradead.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=mingo@kernel.org \
    --cc=namhyung@gmail.com \
    --cc=namhyung@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.