[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 18:58:24 +0400
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.
>
>>
>> Thanks for your effort.
>>
>> Jovi
>
>
>
> --
> Respectfully
> Azat Khuzhin
--
Respectfully
Azat Khuzhin
Other related posts:
