Re: [PATCH v2 0/3] Perf support to SDT markers

[Hemant <hkshaw@linux.vnet.ibm.com>]

On 10/08/2013 11:33 AM, Masami Hiramatsu wrote:
> Hi,

Hi Masami and thanks a lot for testing these patches.

>
> (2013/10/07 15:46), Hemant Kumar wrote:
>> This patchset helps in probing dtrace style markers(SDT) present in user space
>> applications through perf. Notes/markes are placed at important places by the
>> developers. They have a negligible overhead when not enabled. We can enable
>> them and probe at these places and find some important information like the
>> arguments' values, etc.
> Thank you for updating!
> By the way, could you rebase this series to the latest -tip kernel next time?
> It is more convenient for us to test & merge it.

Yes, I will rebase this series next time.

>
> [...]
>> - Now use perf to list the markers in the app:
>> # perf probe --markers -x ./user_app
>>
>> %user_app:foo_start
>> %user_app:fun_start
> Here, what I got.
>
> $ ./perf probe --markers -x /lib64/librt.so
> %librt:lll_futex_wake
>
> OK, it lists up markers.
>
> $ ./perf probe --markers -x ./perf
> SDT markers not present!
>    Error : Failed to find SDT markers!(-1)
>
> OK, this shows an error, INHO, it'd better show which binary doesn't have markers.

OK, will include the binary name too.

>
> $ ./perf probe --markers
> Failed to open (null)
>    Error : Failed to find SDT markers!(-1)
>
> Hmm, it's a bit odd error. It should check if there is an elf binary is specified.

Yeah, correct.

>
> $ ./perf probe --markers /lib64/librt.so
> %librt:lll_futex_wake
>
> It seems to work without -x. However, that is only when the path is started with /.
> And also the usage is not described by --help.
>
> $ ./perf probe --markers ./perf
> Semantic error :File always requires line number or lazy pattern.
>    Error: Parse Error.  (-22)
>
> So, to avoid confusion, --markers should abort without -x(--exec) option.

Yes, we can restrict action of --markers for -x option only. I thought 
of keeping the same behavior as in case of function listing but I guess 
restricting it would be better since, --markers should only be available 
in user space.

>
>> - And  then use perf probe to add a probe point :
>>
>> # perf probe -x ./user_app -a '%user_app:foo_start'
> And also, without -x, -a also exits with an odd error message.
>
> $ ./perf probe -a %libm:slowexp_p6
> Failed to open (null)
>    Error: Failed to add events. (-1)

Ok, will rectify this behavior.

>
> I'll review each patch soon.

Thanks, looking forward to that.

- Hemant