Subject: Re: [PATCH 2/2] perf probe: add sdt probes arguments into the uprobe
 cmd string
To: Alexis Berlemont <alexis.berlemont@gmail.com>,
        linux-kernel@vger.kernel.org
References: <20161116235601.13433-1-alexis.berlemont@gmail.com>
 <20161116235601.13433-3-alexis.berlemont@gmail.com>
Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
        alexander.shishkin@linux.intel.com
From: Hemant Kumar <hemant@linux.vnet.ibm.com>
Date: Thu, 17 Nov 2016 14:34:05 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <20161116235601.13433-3-alexis.berlemont@gmail.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Message-Id: <52251cbe-6825-1d03-7b5b-e7517549caa0@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2721
Lines: 82

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@gmail.com>
> ---
>   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