All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Irina Tirdea <irina.tirdea@gmail.com>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	LKML <linux-kernel@vger.kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	David Ahern <dsahern@gmail.com>,
	Pekka Enberg <penberg@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	Irina Tirdea <irina.tirdea@intel.com>
Subject: Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time
Date: Mon, 1 Oct 2012 09:21:54 +0200	[thread overview]
Message-ID: <20121001072154.GA17143@gmail.com> (raw)
In-Reply-To: <87haqjvc6f.fsf@sejong.aot.lge.com>


* Namhyung Kim <namhyung@kernel.org> wrote:

> On Thu, 27 Sep 2012 14:25:10 +0300, Irina Tirdea wrote:
> >>> The perf built to run on the host needs to use arm-eabi-objdump from
> >>> the toolchain so that it can analyse data recorded on Android. This
> >>> patch is targeting this scenario, not the previous one. In this case,
> >>> the CROSS_COMPILE option would be different than arm-eabi- so using
> >>> $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden
> >>> when running make since there is no connection between the toolchain
> >>> used here and the path for objdump. I am always overriding objdump
> >>> when calling make, so I did not catch this.
> >>>
> >>> I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the
> >>> Makefile to handle the first scenario. I'll also explain this in the
> >>> commit message so that it is more clear and make the same change for
> >>> the addr2line patch.
> >>>
> >>> What do you think?
> >>
> >> I think the right thing to do is finding a correct objdump at runtime in
> >> some way.  Why do you want to make it compile-time configurable?
> >>
> >
> > The correct objdump path can be detected at runtime by setting the
> > toolchain path. But since the name is arm-eabi-objdump and not
> > objdump, it does not know to use it instead.
> >
> > The only way (I can think of) to change objdump at runtime would be to
> > use the --objdump option for perf annotate (and provide a similar
> > option for addr2line). The problem with this approach is that the user
> > has to be aware that perf annotate uses the objdump tool and that he
> > has to use the cross-compiler version instead. Since the user will
> > have perf compiled for host as part of his Android tree, he will
> > expect it to work without these further changes from his part. The
> > path for objdump can be set in the Android Makefile at compile time so
> > that the user doesn't need to be aware of it.
> 
> What I'm thinking is that perf can try to find cross-built 
> binutils when it detects perf.data file is came from other 
> machine/architecture. Fortunately perf_session_env was added 
> recently and it has the arch information from the file so we 
> can use it to find the path.
> 
> Following patch is a proof-of-concept patch and only build 
> tested. What do you think?  Could you play with it for some 
> time? :)

This is a pretty clever idea - much better than hard-coding 
architecture details at build time.

Thanks,

	Ingo

  parent reply	other threads:[~2012-10-01  7:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-23 19:27 [PATCH v2 0/4] perf: android: configure hardcoded paths Irina Tirdea
2012-09-23 19:27 ` [PATCH v2 1/4] perf tools: configure tmp path at build time Irina Tirdea
2012-09-23 19:27 ` [PATCH v2 2/4] perf tools: configure shell path at compile time Irina Tirdea
2012-09-23 19:27 ` [PATCH v2 3/4] perf annotate: configure objdump " Irina Tirdea
2012-09-25 13:08   ` Namhyung Kim
2012-09-27  0:51     ` Irina Tirdea
2012-09-27  1:52       ` Namhyung Kim
2012-09-27 11:25         ` Irina Tirdea
2012-09-27 13:16           ` Namhyung Kim
2012-09-27 13:29             ` [RFC v2] perf tools: Try to find cross-built objdump path Namhyung Kim
2012-10-01  0:41               ` [RFC v3] " Irina Tirdea
2012-09-30 23:37             ` [PATCH v2 3/4] perf annotate: configure objdump path at compile time Irina Tirdea
2012-10-01  7:21             ` Ingo Molnar [this message]
2012-10-01 14:27               ` Arnaldo Carvalho de Melo
2012-09-23 19:27 ` [PATCH v2 4/4] perf tools: configure addr2line " Irina Tirdea
2012-09-23 19:49   ` [PATCH v3 " Irina Tirdea

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=20121001072154.GA17143@gmail.com \
    --to=mingo@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=dsahern@gmail.com \
    --cc=irina.tirdea@gmail.com \
    --cc=irina.tirdea@intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    --cc=penberg@kernel.org \
    --cc=rostedt@goodmis.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.