From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: Irina Tirdea <irina.tirdea@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>,
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>,
Namhyung Kim <namhyung@kernel.org>,
Pekka Enberg <penberg@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
Namhyung Kim <namhyung.kim@lge.com>,
Irina Tirdea <irina.tirdea@intel.com>
Subject: Re: [PATCH v4 4/6] perf tools: Try to find cross-built objdump path
Date: Tue, 16 Oct 2012 08:26:19 -0700 [thread overview]
Message-ID: <20121016152619.GE6807@ghostprotocols.net> (raw)
In-Reply-To: <1350344020-8071-5-git-send-email-irina.tirdea@gmail.com>
Em Tue, Oct 16, 2012 at 02:33:38AM +0300, Irina Tirdea escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
Well, by now it is not anymore from Namhyung, but based on a previous
patch by him, right?
I'm ok with the patch, thanks for addressing my suggestions, but please
resubmit with you as the patch author, giving credit for the original
patch by Namhyung.
Namhyung, are you ok with it? If so could I have a reviewed-by from you
or at least an acked-by? The SOB is to be dropped, as this is not coming
from you, but directly from Irina and as explained above, its a modified
patch.
- Arnaldo
> As we have architecture information of saved perf.data file, we can
> try to find cross-built objdump path.
>
> The triplets include support for Android (arm, x86 and mips architectures).
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
> tools/perf/Makefile | 2 +
> tools/perf/arch/common.c | 178 +++++++++++++++++++++++++++++++++++++++++
> tools/perf/arch/common.h | 10 +++
> tools/perf/builtin-annotate.c | 7 ++
> tools/perf/builtin-report.c | 7 ++
> tools/perf/util/annotate.h | 1 -
> 6 files changed, 204 insertions(+), 1 deletion(-)
> create mode 100644 tools/perf/arch/common.c
> create mode 100644 tools/perf/arch/common.h
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 99bf383..5906adb 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -428,6 +428,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
> LIB_OBJS += $(OUTPUT)ui/hist.o
> LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
>
> +LIB_OBJS += $(OUTPUT)arch/common.o
> +
> BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
> BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
> # Benchmark modules
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> new file mode 100644
> index 0000000..2367b25
> --- /dev/null
> +++ b/tools/perf/arch/common.c
> @@ -0,0 +1,178 @@
> +#include <stdio.h>
> +#include <sys/utsname.h>
> +#include "common.h"
> +#include "../util/debug.h"
> +
> +const char *const arm_triplets[] = {
> + "arm-eabi-",
> + "arm-linux-androideabi-",
> + "arm-unknown-linux-",
> + "arm-unknown-linux-gnu-",
> + "arm-unknown-linux-gnueabi-",
> + NULL
> +};
> +
> +const char *const powerpc_triplets[] = {
> + "powerpc-unknown-linux-gnu-",
> + "powerpc64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const s390_triplets[] = {
> + "s390-ibm-linux-",
> + NULL
> +};
> +
> +const char *const sh_triplets[] = {
> + "sh-unknown-linux-gnu-",
> + "sh64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const sparc_triplets[] = {
> + "sparc-unknown-linux-gnu-",
> + "sparc64-unknown-linux-gnu-",
> + NULL
> +};
> +
> +const char *const x86_triplets[] = {
> + "x86_64-pc-linux-gnu-",
> + "x86_64-unknown-linux-gnu-",
> + "i686-pc-linux-gnu-",
> + "i586-pc-linux-gnu-",
> + "i486-pc-linux-gnu-",
> + "i386-pc-linux-gnu-",
> + "i686-linux-android-",
> + "i686-android-linux-",
> + NULL
> +};
> +
> +const char *const mips_triplets[] = {
> + "mips-unknown-linux-gnu-",
> + "mipsel-linux-android-",
> + NULL
> +};
> +
> +static bool lookup_path(char *name)
> +{
> + bool found = false;
> + char *path, *tmp;
> + char buf[PATH_MAX];
> + char *env = getenv("PATH");
> +
> + if (!env)
> + return false;
> +
> + env = strdup(env);
> + if (!env)
> + return false;
> +
> + path = strtok_r(env, ":", &tmp);
> + while (path) {
> + scnprintf(buf, sizeof(buf), "%s/%s", path, name);
> + if (access(buf, F_OK) == 0) {
> + found = true;
> + break;
> + }
> + path = strtok_r(NULL, ":", &tmp);
> + }
> + free(env);
> + return found;
> +}
> +
> +static int lookup_triplets(const char *const *triplets, const char *name)
> +{
> + int i;
> + char buf[PATH_MAX];
> +
> + for (i = 0; triplets[i] != NULL; i++) {
> + scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
> + if (lookup_path(buf))
> + return i;
> + }
> + return -1;
> +}
> +
> +static int perf_session_env__lookup_binutils_path(struct perf_session_env *env,
> + const char *name,
> + const char **path)
> +{
> + int idx;
> + char *arch, *cross_env;
> + struct utsname uts;
> + const char *const *path_list;
> + char *buf = NULL;
> +
> + if (uname(&uts) < 0)
> + goto out;
> +
> + /*
> + * We don't need to try to find objdump path for native system.
> + * Just use default binutils path (e.g.: "objdump").
> + */
> + if (!strcmp(uts.machine, env->arch))
> + goto out;
> +
> + cross_env = getenv("CROSS_COMPILE");
> + if (cross_env) {
> + if (asprintf(&buf, "%s%s", cross_env, name) < 0)
> + goto out_error;
> + if (buf[0] == '/') {
> + if (access(buf, F_OK) == 0)
> + goto out;
> + goto out_error;
> + }
> + if (lookup_path(buf))
> + goto out;
> + free(buf);
> + }
> +
> + arch = env->arch;
> +
> + if (!strcmp(arch, "arm"))
> + path_list = arm_triplets;
> + else if (!strcmp(arch, "powerpc"))
> + path_list = powerpc_triplets;
> + else if (!strcmp(arch, "sh"))
> + path_list = sh_triplets;
> + else if (!strcmp(arch, "s390"))
> + path_list = s390_triplets;
> + else if (!strcmp(arch, "sparc"))
> + path_list = sparc_triplets;
> + else if (!strcmp(arch, "x86") || !strcmp(arch, "i386") ||
> + !strcmp(arch, "i486") || !strcmp(arch, "i586") ||
> + !strcmp(arch, "i686"))
> + path_list = x86_triplets;
> + else if (!strcmp(arch, "mips"))
> + path_list = mips_triplets;
> + else {
> + ui__error("binutils for %s not supported.\n", arch);
> + goto out_error;
> + }
> +
> + idx = lookup_triplets(path_list, name);
> + if (idx < 0) {
> + ui__error("Please install %s for %s.\n"
> + "You can add it to PATH, set CROSS_COMPILE or "
> + "override the default using --%s.\n",
> + name, arch, name);
> + goto out_error;
> + }
> +
> + if (asprintf(&buf, "%s%s", path_list[idx], name) < 0)
> + goto out_error;
> +
> +out:
> + *path = buf;
> + return 0;
> +out_error:
> + free(buf);
> + *path = NULL;
> + return -1;
> +}
> +
> +int perf_session_env__lookup_objdump(struct perf_session_env *env)
> +{
> + return perf_session_env__lookup_binutils_path(env, "objdump",
> + &objdump_path);
> +}
> diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
> new file mode 100644
> index 0000000..ede246e
> --- /dev/null
> +++ b/tools/perf/arch/common.h
> @@ -0,0 +1,10 @@
> +#ifndef ARCH_PERF_COMMON_H
> +#define ARCH_PERF_COMMON_H
> +
> +#include "../util/session.h"
> +
> +extern const char *objdump_path;
> +
> +int perf_session_env__lookup_objdump(struct perf_session_env *env);
> +
> +#endif /* ARCH_PERF_COMMON_H */
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 10d6ca4..e4df5da 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -28,6 +28,7 @@
> #include "util/hist.h"
> #include "util/session.h"
> #include "util/tool.h"
> +#include "arch/common.h"
>
> #include <linux/bitmap.h>
>
> @@ -186,6 +187,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
> goto out_delete;
> }
>
> + if (!objdump_path) {
> + ret = perf_session_env__lookup_objdump(&session->header.env);
> + if (ret)
> + goto out_delete;
> + }
> +
> ret = perf_session__process_events(session, &ann->tool);
> if (ret)
> goto out_delete;
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 3d30a9a..9c0fe68 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -33,6 +33,7 @@
> #include "util/thread.h"
> #include "util/sort.h"
> #include "util/hist.h"
> +#include "arch/common.h"
>
> #include <linux/bitmap.h>
>
> @@ -675,6 +676,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
> has_br_stack = perf_header__has_feat(&session->header,
> HEADER_BRANCH_STACK);
>
> + if (!objdump_path) {
> + ret = perf_session_env__lookup_objdump(&session->header.env);
> + if (ret)
> + goto error;
> + }
> +
> if (sort__branch_mode == -1 && has_br_stack)
> sort__branch_mode = 1;
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 39242dc..a4dd25a 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -154,6 +154,5 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
> #endif
>
> extern const char *disassembler_style;
> -extern const char *objdump_path;
>
> #endif /* __PERF_ANNOTATE_H */
> --
> 1.7.9.5
next prev parent reply other threads:[~2012-10-16 15:26 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-15 23:33 [PATCH v4 0/6] perf tools: fixes for Android Irina Tirdea
2012-10-15 23:33 ` [PATCH v4 1/6] perf tools: configure tmp path at build time Irina Tirdea
2012-10-16 15:18 ` Arnaldo Carvalho de Melo
2012-10-16 16:48 ` Christoph Hellwig
2012-10-21 16:24 ` Ingo Molnar
2012-10-22 7:38 ` Irina Tirdea
2012-10-15 23:33 ` [PATCH v4 2/6] perf tools: configure shell path at compile time Irina Tirdea
2012-10-16 15:19 ` Arnaldo Carvalho de Melo
2012-10-15 23:33 ` [PATCH v4 3/6] perf tools: add --addr2line command line option Irina Tirdea
2012-10-16 15:21 ` Arnaldo Carvalho de Melo
2012-10-22 9:06 ` Irina Tirdea
2012-10-15 23:33 ` [PATCH v4 4/6] perf tools: Try to find cross-built objdump path Irina Tirdea
2012-10-16 15:26 ` Arnaldo Carvalho de Melo [this message]
2012-10-17 4:22 ` Namhyung Kim
2012-10-25 7:59 ` [tip:perf/core] " tip-bot for Irina Tirdea
2012-10-15 23:33 ` [PATCH v4 5/6] perf tools: Try to find cross-built addr2line path Irina Tirdea
2012-10-16 15:26 ` Arnaldo Carvalho de Melo
2012-10-15 23:33 ` [PATCH v4 6/6] perf stat: implement --big-num grouping Irina Tirdea
2012-10-16 16:49 ` Christoph Hellwig
2012-10-21 16:28 ` Ingo Molnar
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=20121016152619.GE6807@ghostprotocols.net \
--to=acme@ghostprotocols.net \
--cc=a.p.zijlstra@chello.nl \
--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=mingo@kernel.org \
--cc=namhyung.kim@lge.com \
--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 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).