All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Martin Liška" <mliska@suse.cz>
Cc: David Ahern <dsahern@gmail.com>,
	a.p.zijlstra@chello.nl, paulus@samba.org, mingo@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf: introduce new two options for objdump.
Date: Wed, 20 May 2015 10:05:15 -0300	[thread overview]
Message-ID: <20150520130515.GB2955@kernel.org> (raw)
In-Reply-To: <555C85C9.4050104@suse.cz>

Em Wed, May 20, 2015 at 03:02:01PM +0200, Martin Liška escreveu:
> On 05/19/2015 04:33 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, May 19, 2015 at 08:29:58AM -0600, David Ahern escreveu:
> >>On 5/19/15 8:08 AM, Arnaldo Carvalho de Melo wrote:
> >>>Em Thu, Jan 29, 2015 at 02:03:46PM +0100, Martin Liška escreveu:
> >>>>Newly introduced options usedfull for off-box usage:
> >
> >>>    New useful options for off-box usage:
> >
> >>>>   --objdump-prefix: specify prefix to add to the absolute paths
> >>>>   where objdump looks for source files.
> >>>>   --objdump-prefix-strip: indicate how many initial directory names
> >>>>   to strip off the hardwired absolute paths, passed to objdump.
> >>>>   It has no effect without --objdump-prefix.
> >
> >>>Does this, for instance, allows one looking at source code + assembly
> >>>with debuginfo packages? Is that a usecase you tested this against?
> >
> >>>Anyway, sorry for the delay in processing this, fell thru the cracks ;-\
> >
> >>>Can you please respin this on top of my current perf/core, at:
> >
> >>>git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> >
> >>Perhaps it should be generalized a bit? Right now we have symfs for the
> >>location of the tree for binaries. What about a srcfs for a source tree?
> >
> >That is a good idea indeed.
> >
> >- Arnaldo
> >
> 
> Hello.
> 
> I was motivated to be able to annotate source code, realized out-of-box. More precisely, on a machine A
> I run perf record, I moved the report file to machine B and I use these options to annotate source code.
> 
> Let's consider source code location
> 
> Machine A:
> P1=/a/b/c/d/source-files
> 
> Machine B:
> P2=/x/y/z/source-files
> 
> Using just a single option (--objdump-prefix) works, but one how to place source code on machine B to:
> /a/b/c/d/x/y/z/source-files
> 
> That's quite cumbersome, thus adding second option, one can strip '/a/b/c/d', with --objdump-prefix-strip=4.
> Still, no ideal, but it's quite flexible. A new 'srcfs' can used as --objdump-prefix, am I right?

I guess so, and yeah, the feature implemented in your patch has value,
thanks for collaborating! It is just that it may fit better as a
complement to the --symfs stuff, that is also for off-box analysis:

--symfs=<directory>::
        Look for files with symbols relative to this directory.

> Respun version of patch is attached.

Thanks, will wait for further discussion before applying,

- Arnaldo
 
> Thanks,
> Martin
> 

> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index e9cd39a..4d2a5be 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -90,6 +90,12 @@ OPTIONS
>  --objdump=<path>::
>          Path to objdump binary.
>  
> +--objdump-prefix=<path>::
> +	Specify prefix to add to the absolute paths where objdump looks for source files.
> +
> +--objdump-prefix-strip=<level>::
> +	Indicate how many initial directory names to strip off the hardwired absolute paths, passed to objdump. It has no effect without --objdump-prefix.
> +
>  --skip-missing::
>  	Skip symbols that cannot be annotated.
>  
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index c33b69f..2b3cb21 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -286,6 +286,12 @@ OPTIONS
>  --objdump=<path>::
>          Path to objdump binary.
>  
> +--objdump-prefix=<path>::
> +	Specify prefix to add to the absolute paths where objdump looks for source files.
> +
> +--objdump-prefix-strip=<level>::
> +	Indicate how many initial directory names to strip off the hardwired absolute paths, passed to objdump. It has no effect without --objdump-prefix.
> +
>  --group::
>  	Show event group information together.
>  
> diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> index 9e5b07eb..2f269e9 100644
> --- a/tools/perf/Documentation/perf-top.txt
> +++ b/tools/perf/Documentation/perf-top.txt
> @@ -157,6 +157,16 @@ Default is to monitor all CPUS.
>  --asm-raw::
>  	Show raw instruction encoding of assembly instructions.
>  
> +--objdump=<path>::
> +        Path to objdump binary.
> +
> +--objdump-prefix=<path>::
> +	Specify prefix to add to the absolute paths where objdump looks for source files.
> +
> +--objdump-prefix-strip=<level>::
> +	Indicate how many initial directory names to strip off the hardwired absolute paths, passed to objdump. It has no effect without --objdump-prefix.
> +
> +
>  -g::
>  	Enables call-graph (stack chain/backtrace) recording.
>  
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index b57a027..9f0690d 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -324,6 +324,11 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>  		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
>  	OPT_STRING(0, "objdump", &objdump_path, "path",
>  		   "objdump binary to use for disassembly and annotations"),
> +	OPT_STRING(0, "objdump-prefix", &objdump_prefix_path, "path",
> +		   "prefix to the absolute paths where objdump looks for source files"),
> +	OPT_STRING(0, "objdump-prefix-strip", &objdump_prefix_strip, "level",
> +		   "Indicate how many initial directory names to strip off the hardwired "
> +		   "absolute paths, passed to objdump."),
>  	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
>  		    "Show event group information together"),
>  	OPT_END()
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 56025d9..3328fb8 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -715,6 +715,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
>  		    "add last branch records to call history"),
>  	OPT_STRING(0, "objdump", &objdump_path, "path",
>  		   "objdump binary to use for disassembly and annotations"),
> +	OPT_STRING(0, "objdump-prefix", &objdump_prefix_path, "path",
> +		   "prefix to the absolute paths where objdump looks for source files"),
> +	OPT_STRING(0, "objdump-prefix-strip", &objdump_prefix_strip, "level",
> +		   "Indicate how many initial directory names to strip off the hardwired "
> +		   "absolute paths, passed to objdump."),
>  	OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle,
>  		    "Disable symbol demangling"),
>  	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 6b98742..24bdb08 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1150,6 +1150,11 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
>  		    "Enable kernel symbol demangling"),
>  	OPT_STRING(0, "objdump", &objdump_path, "path",
>  		    "objdump binary to use for disassembly and annotations"),
> +	OPT_STRING(0, "objdump-prefix", &objdump_prefix_path, "path",
> +		   "prefix to the absolute paths where objdump looks for source files"),
> +	OPT_STRING(0, "objdump-prefix-strip", &objdump_prefix_strip, "level",
> +		   "Indicate how many initial directory names to strip off the hardwired "
> +		   "absolute paths, passed to objdump."),
>  	OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style",
>  		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
>  	OPT_STRING('u', "uid", &target->uid_str, "user", "user to profile"),
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 7f5bdfc..c5373f4 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -23,6 +23,8 @@
>  
>  const char 	*disassembler_style;
>  const char	*objdump_path;
> +const char	*objdump_prefix_path;
> +const char	*objdump_prefix_strip;
>  static regex_t	 file_lineno;
>  
>  static struct ins *ins__find(const char *name);
> @@ -1039,12 +1041,16 @@ fallback:
>  	}
>  
>  	snprintf(command, sizeof(command),
> -		 "%s %s%s --start-address=0x%016" PRIx64
> +		 "%s %s%s %s%s %s%s --start-address=0x%016" PRIx64
>  		 " --stop-address=0x%016" PRIx64
>  		 " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
>  		 objdump_path ? objdump_path : "objdump",
>  		 disassembler_style ? "-M " : "",
>  		 disassembler_style ? disassembler_style : "",
> +		 objdump_prefix_path ? "--prefix=" : "",
> +		 objdump_prefix_path ? objdump_prefix_path : "",
> +		 objdump_prefix_strip ? "--prefix-strip=" : "",
> +		 objdump_prefix_strip ? objdump_prefix_strip : "",
>  		 map__rip_2objdump(map, sym->start),
>  		 map__rip_2objdump(map, sym->end),
>  		 symbol_conf.annotate_asm_raw ? "" : "--no-show-raw",
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index cadbdc9..fd9782e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -168,5 +168,7 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
>  #endif
>  
>  extern const char	*disassembler_style;
> +extern const char	*objdump_prefix_path;
> +extern const char	*objdump_prefix_strip;
>  
>  #endif	/* __PERF_ANNOTATE_H */


  reply	other threads:[~2015-05-20 13:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-29 13:03 [PATCH] perf: introduce new two options for objdump Martin Liška
2015-05-19 14:08 ` Arnaldo Carvalho de Melo
2015-05-19 14:29   ` David Ahern
2015-05-19 14:33     ` Arnaldo Carvalho de Melo
2015-05-20 13:02       ` Martin Liška
2015-05-20 13:05         ` Arnaldo Carvalho de Melo [this message]
2015-06-01 11:10           ` Martin Liška
2015-06-01 12:54             ` Arnaldo Carvalho de Melo
2015-06-01 13:58               ` Martin Liška
2015-06-01 14:17                 ` Arnaldo Carvalho de Melo

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=20150520130515.GB2955@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mliska@suse.cz \
    --cc=paulus@samba.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.