On Mon, Nov 11, 2013 at 10: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: >> Thanks Azat, I already saw you new patches. I think those patches don't have any big problem, also I'm not object with additional patch flows like what we do now, so I will take your patches. Btw, 3) I mentioned above is more like a bug, not feature, because there will miss sdt probe in current implementation. 4) definitely is a feature. Thank you. Jovi