[ktap] Re: [PATCH 0/4] stapsdt initial support

  • From: Azat Khuzhin <a3at.mail@xxxxxxxxx>
  • To: Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx>
  • Date: Mon, 11 Nov 2013 19:04:24 +0400

On Mon, Nov 11, 2013 at 6:58 PM, Azat Khuzhin <a3at.mail@xxxxxxxxx> wrote:
> On Mon, Nov 11, 2013 at 11:08 AM, Azat Khuzhin <a3at.mail@xxxxxxxxx> wrote:
>> On Mon, Nov 11, 2013 at 6:24 AM, Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx> 
>> wrote:
>>> On Sun, Nov 10, 2013 at 3:50 AM, Azat Khuzhin <a3at.mail@xxxxxxxxx> wrote:
>>>> Jovi, If you prefer github, here is the link:
>>>> https://github.com/azat/ktap/compare/sdt-probes-v2
>>>>
>>> I didn't expect you implement it so fast, that's amazing :)
>>>
>>> Email would be more ease to review and post comments, the comments
>>> in github can be lost easily.
>>>
>>> Some comments from my side:
>>>
>>> 1) use "sdt" instead of "stapsdt"
>>>     "sdt" would be much shorter, and it's so weird to use "stap"
>>> naming in ktap script. :)
>>
>> It was already done before me.
>> And yeah I also though about it.
>> I will rename it shortly.
>
> Fixed in 0005-sdt-replace-stapsdt-to-just-sdt.patch
>
>>
>>>
>>> 2) Segmentation fault for "trace sdt:/lib64/libc.so.6 {...}"
>>>     No symbol given in that case.
>>
>> Yeah, thanks!
>>
>> This is a common case for parse_events_resolve_symbol(),
>> I will send another patch, because it also affects uprobes.
>
> Fixed in 0006-stap-fix-possible-segmentation-fault-without-symbol.patch
>
>>
>>>
>>> 3) mulitple probe location with same sdt name
>>>     For example(in my x86-64 machine):
>>>     #readelf -n /lib64/libc.so.6 | grep lll_futex_wake
>>>       Name: lll_futex_wake
>>>       Name: lll_futex_wake
>>>
>>>     So in this case, script like "trace
>>> sdt:/lib64/libc.so.6:lll_futex_wake" should match
>>>     two probe location, so we have to write two uprobe events into debugfs.
>>>
>>> 4) support glob sdt name
>>>     For example,"trace sdt:/lib64/libc.so.6:*" will match all sdt
>>> probes in /lib64/libc.so.6
>>>
>>> It seems that 3) and 4) need some big code change on eventdef.c,maybe we can
>>> abstract "add_uprobe" common function, it can be called when parse
>>> each valid sdt
>>> probe successfully. Please feel free to change that code to make it better.
>>
>> I've already thought about this, but I hoped that this can be separate 
>> feature:
>>
>>>> This implementation still needs in functions to obtain list of all 
>>>> available
>>>> markers, and also all available symbols (like perf) I think, to use 
>>>> patterns
>>>> for example, however this can wait a little.
>>
>> And it's simpler to track smaller patch series.

I also noticed that I have a typo std/sdt, so I rebased existing patches.

>>
>>>
>>> Thanks for your effort.
>>>
>>> Jovi
>>
>>
>>
>> --
>> Respectfully
>> Azat Khuzhin
>
>
>
> --
> Respectfully
> Azat Khuzhin



-- 
Respectfully
Azat Khuzhin

Other related posts: