[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:23:21 +0400
On 11 Nov 2013 19:11, "Jovi Zhangwei" <jovi.zhangwei@xxxxxxxxx> wrote:
>
> 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.
Hi Jovi, you absolutely right.
However 3 depends from 4, and vise versa.
Thanks.
>
> Thank you.
>
> Jovi
Other related posts:
