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