Re: [PATCH v2 2/3] Support for perf to probe into SDT markers:

[Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>]

(2013/10/07 15:48), Hemant Kumar wrote:
> This allows perf to probe into the sdt markers/notes present in
> the libraries and executables. We try to find the associated location
> and handle prelinking (since, stapsdt notes section is not allocated
> during runtime). Prelinking is handled with the help of base
> section which is allocated during runtime. This address can be compared
> with the address retrieved from the notes' description. If its different,
> we can take this difference and then add to the note's location.
> 
> We can use existing '-a/--add' option to add events for sdt markers.
> Also, we can add multiple events at once using the same '-a' option.
> 
> Usage:
> perf probe -x /lib64/libc.so.6 -a 'my_event=%libc:setjmp'
> 
> or
> 
> perf probe -x /lib64/libc.so.6 %libc:setjmp
> 
> Output (corresponding to the first usage):
> Added new event:
>   libc:my_event        (on 0x35981)
> 
> You can now use it in all perf tools, such as:
> 
>     perf record -e libc:my_event -aR sleep 1
> 
> 
> Signed-off-by: Hemant Kumar Shaw <hkshaw@linux.vnet.ibm.com>
> ---
>  tools/perf/builtin-probe.c    |   11 +++++
>  tools/perf/util/probe-event.c |   89 +++++++++++++++++++++++++++++++++++++----
>  tools/perf/util/probe-event.h |    2 +
>  tools/perf/util/symbol-elf.c  |   80 +++++++++++++++++++++++++++++++++++++
>  tools/perf/util/symbol.h      |    3 +
>  5 files changed, 177 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index cbd2383..6f09723 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -370,6 +370,17 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  			pr_err("Error: Don't use --markers with --funcs.\n");
>  			usage_with_options(probe_usage, options);
>  		}
> +		if (params.mod_events) {
> +			ret = add_perf_probe_events(params.events,
> +						    params.nevents,
> +						    params.max_probe_points,
> +						    params.target,
> +						    params.force_add);
> +			if (ret < 0) {
> +				pr_err(" Error: Failed to add events. "
> +				       " (%d)\n", ret);
> +			}
> +		}

What is this code for? params.sdt is true only if "--markers" is set, and that
should not be used with --add and --del, because it's an action "query markers".
We should give an error and abort here.

Other points are covered by Namhyung's review(thanks!).

Thank you!

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com