Re: [PATCH 2/2] perf probe: add sdt probes arguments into the uprobe cmd string

[Hemant Kumar]

Hi Alexis,

On 11/17/2016 05:26 AM, Alexis Berlemont wrote:
> An sdt probe can be associated with arguments but they were not passed
> to the user probe tracing interface (uprobe_events); this patch adapts
> the sdt argument descriptors according to the uprobe input format.
>
> As the uprobe parser does not support scaled address mode, perf will
> skip arguments which cannot be adapted to the uprobe format.
>
> Here are the results:
>
> $ perf buildid-cache -v --add test_sdt
> $ perf probe -x test_sdt sdt_libfoo:table_frob
> $ perf probe -x test_sdt sdt_libfoo:table_diddle
> $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> $ perf script
> test_sdt  ...   666.255678:   sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> test_sdt  ...   666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> test_sdt  ...   666.255686:   sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> test_sdt  ...   666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> test_sdt  ...   666.255692:   sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> test_sdt  ...   666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8

Cool! Thanks for working on this.

I have a comment below.

> Signed-off-by: Alexis Berlemont <alexis.berlemont@xxxxxxxxx>
> ---
>   tools/perf/util/probe-file.c | 176 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 172 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 436b647..a97a170 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -28,6 +28,46 @@
>   #include "probe-file.h"
>   #include "session.h"
>   
> +#ifdef HAVE_GELF_GETNOTE_SUPPORT
> +
> +/*
> + * Local declarations needed for adjusting gcc/gas-generated registers
> + * before filling the uprobe tracer interface.
> + */
> +
> +struct sdt_reg_renaming {
> +	const char *sdt_name;
> +	const char *uprobe_name;
> +};
> +
> +#define REG_RENAMING(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> +#define REG_RENAMING_END {.sdt_name = NULL, .uprobe_name = NULL}
> +
> +static const struct sdt_reg_renaming sdt_reg_renaming_table[] = {
> +	REG_RENAMING(eax, ax),
> +	REG_RENAMING(rax, ax),
> +	REG_RENAMING(ebx, bx),
> +	REG_RENAMING(rbx, bx),
> +	REG_RENAMING(ecx, cx),
> +	REG_RENAMING(rcx, cx),
> +	REG_RENAMING(edx, dx),
> +	REG_RENAMING(rdx, dx),
> +	REG_RENAMING(esi, si),
> +	REG_RENAMING(rsi, si),
> +	REG_RENAMING(edi, di),
> +	REG_RENAMING(rdi, di),
> +	REG_RENAMING(ebp, bp),
> +	REG_RENAMING(rbp, bp),
> +	REG_RENAMING_END,
> +};

Please put the above in arch helper headers for x86, as these register 
names and their conversions are specific to x86.

[SNIP]

--
Thanks,
Hemant Kumar