On 2017/06/24 08:06PM, Masami Hiramatsu wrote:
> On Sat, 24 Jun 2017 02:30:21 +0900
> Masami Hiramatsu <mhira...@kernel.org> wrote:
>
> > On Thu, 22 Jun 2017 22:33:25 +0530
> > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote:
> >
> > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote:
> > > > On Thu, 22 Jun 2017 00:20:28 +0530
> > > > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote:
> > > >
> > > > > KPROBES_ON_FTRACE is only available on powerpc64le. Update comment to
> > > > > clarify this.
> > > > >
> > > > > Also, we should use an offset of 8 to ensure that the probe does not
> > > > > fall on ftrace location. The current offset of 4 will fall before the
> > > > > function local entry point and won't fire, while an offset of 12 or 16
> > > > > will fall on ftrace location. Offset 8 is currently guaranteed to not
> > > > > be
> > > > > the ftrace location.
> > > >
> > > > OK, these part seems good to me.
> > > >
> > > > >
> > > > > Finally, do not filter out symbols with a dot. Powerpc Elfv1 uses dot
> > > > > prefix for all functions and this prevents us from testing some of
> > > > > those
> > > > > symbols. Furthermore, with the patch to derive event names properly in
> > > > > the presence of ':' and '.', such names are accepted by kprobe_events
> > > > > and constitutes a good test for those symbols.
> > > >
> > > > Hmm, the reason why I added such filter was to avoid symbols including
> > > > gcc-generated suffixes like as .constprop or .isra etc.
> > >
> > > I see.
> > >
> > > I do wonder -- is there a problem if we try probing those symbols? On my
> > > local x86 vm, I don't see an issue probing it especially with the
> > > previous patch to enable probing with symbols having a '.' or ':'.
> > >
> > > Furthermore, since this is for testing kprobe_events, I feel it is good
> > > to try probing those symbols too to catch any weird errors we may hit.
> >
> > Yes, and that is not what this testcase is aiming to. That testcase should
> > be a separated one, with correct error handling.
Ok, I will re-send the patch by changing the pattern.
>
> Hi Naveen,
>
> Here is the testcase which I meant above. This may help if there is any
> regression related to this specific issue.
Nice!
I tested this and there are a few small issues on powerpc...
>
> Thank you,
>
> -----
>
> selftests: ftrace: Add a testcase for kprobe event naming
>
> From: Masami Hiramatsu <mhira...@kernel.org>
>
> Add a testcase for kprobe event naming. This testcase
> checks whether the kprobe events can automatically ganerate
> its event name on normal function and dot-suffixed function.
> Also it checks whether the kprobe events can correctly
> define new event with given event name and group name.
>
> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> ---
> .../ftrace/test.d/kprobe/kprobe_eventname.tc | 28
> ++++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644
> tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
> b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
> new file mode 100644
> index 0000000..d259031
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
> @@ -0,0 +1,28 @@
> +#!/bin/sh
> +# description: Kprobe event auto/manual naming
> +
> +disable_events
> +echo > kprobe_events
> +
> +:;: "Add an event on function without name" ;:
> +
> +FUNC=`grep -m 10 " [tT] [^.]*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d " "`
On powerpc, this always ends up using a blacklisted function. So, I
think we need a way to find a function that is not black listed.
However, one of the issues is that debugfs does not show all the address
ranges that are blacklisted. I am coming up with a way to address that
and will post patches once I have it working.
With those patches, we should be able to select symbols that are not
blacklisted.
> +echo p $FUNC > kprobe_events
> +test -d events/kprobes/p_${FUNC}_0 || exit_failure
> +
> +:;: "Add an event on function with new name" ;:
> +
> +echo p:event1 $FUNC > kprobe_events
> +test -d events/kprobes/event1 || exit_failure
> +
> +:;: "Add an event on function with new name and group" ;:
> +
> +echo p:kprobes2/event2 $FUNC > kprobe_events
> +test -d events/kprobes2/event2 || exit_failure
> +
> +:;: "Add an event on dot function without name" ;:
> +
> +FUNC=`grep -m 10 " [tT] .*\..*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d "
> "`
> +echo p $FUNC > kprobe_events
> +EVENT=`grep $FUNC kprobe_events | cut -f 1 -d " " | cut -f 2 -d:` ||
> exit_failure
> +test -d events/$EVENT || exit_failure
Probably good to add 'echo > kprobe_events' at the end just to clean up?
Thanks,
Naveen
