Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/29 09:57AM, Masami Hiramatsu wrote: > On Thu, 29 Jun 2017 00:13:24 +0530 > "Naveen N. Rao"wrote: > > > On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > > > 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 000..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. > > > > > > Hmm, if we increase the -m argument, like -m 100, is that still > > > in a blacklisted function? > > > > Yes, most of the initial symbols are exception vectors which are > > blacklisted. > > Hmm, then how about this? :) > > grep _stext -A 1000 | grep -m 10 " [tT] [^.]*$" /proc/kallsyms That just happened to block the test ;) I think your suggestion to use schedule/vfs_read is probably the best option. I will update your patch and post it. > > > > > > > > > > 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. > > > > > > Would you find that is only on powerpc or generic issue? > > > > I meant the address _ranges_ that are blacklisted such as the ones with > > __kprobes annotation and __entry_text and so on. > > I see, but we can also check the address by comparing the address > of symbols, which also can be retrieved from kallsyms. Yes, but this is very platform specific and will result in a lot of checks, arm64 especially. > Since the test case is also applied to stable kernel, I don't want > to make it depending on some special kernel tweaks. Sure, makes sense. For now, I think it's best to keep the test simple. But, we can enhance these later to consider the blacklist. Thanks, Naveen > > > > > > > > > > > > With those patches, we should be able to select symbols that are not > > > > blacklisted. > > > > > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > > > etc. > > > > Yes, I think this will be good for the generic test, but may not help > > selecting a dot symbol on powerpc. > > Right, and it depends on what gcc version and option is specified. > So, maybe we can skip the test if there is no such symbols. > > I intended to test the symbols with some dot-suffix, but it seems > ppc64 has dot-started symbols, right? If there is no dot-suffixed > symbols, I think this test should skip the test case. > > Thank you, > > > > -- > Masami Hiramatsu >
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/29 09:57AM, Masami Hiramatsu wrote: > On Thu, 29 Jun 2017 00:13:24 +0530 > "Naveen N. Rao" wrote: > > > On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > > > 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 000..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. > > > > > > Hmm, if we increase the -m argument, like -m 100, is that still > > > in a blacklisted function? > > > > Yes, most of the initial symbols are exception vectors which are > > blacklisted. > > Hmm, then how about this? :) > > grep _stext -A 1000 | grep -m 10 " [tT] [^.]*$" /proc/kallsyms That just happened to block the test ;) I think your suggestion to use schedule/vfs_read is probably the best option. I will update your patch and post it. > > > > > > > > > > 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. > > > > > > Would you find that is only on powerpc or generic issue? > > > > I meant the address _ranges_ that are blacklisted such as the ones with > > __kprobes annotation and __entry_text and so on. > > I see, but we can also check the address by comparing the address > of symbols, which also can be retrieved from kallsyms. Yes, but this is very platform specific and will result in a lot of checks, arm64 especially. > Since the test case is also applied to stable kernel, I don't want > to make it depending on some special kernel tweaks. Sure, makes sense. For now, I think it's best to keep the test simple. But, we can enhance these later to consider the blacklist. Thanks, Naveen > > > > > > > > > > > > With those patches, we should be able to select symbols that are not > > > > blacklisted. > > > > > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > > > etc. > > > > Yes, I think this will be good for the generic test, but may not help > > selecting a dot symbol on powerpc. > > Right, and it depends on what gcc version and option is specified. > So, maybe we can skip the test if there is no such symbols. > > I intended to test the symbols with some dot-suffix, but it seems > ppc64 has dot-started symbols, right? If there is no dot-suffixed > symbols, I think this test should skip the test case. > > Thank you, > > > > -- > Masami Hiramatsu >
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 29 Jun 2017 00:13:24 +0530 "Naveen N. Rao"wrote: > On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > > 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 000..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. > > > > Hmm, if we increase the -m argument, like -m 100, is that still > > in a blacklisted function? > > Yes, most of the initial symbols are exception vectors which are > blacklisted. Hmm, then how about this? :) grep _stext -A 1000 | grep -m 10 " [tT] [^.]*$" /proc/kallsyms > > > > > > 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. > > > > Would you find that is only on powerpc or generic issue? > > I meant the address _ranges_ that are blacklisted such as the ones with > __kprobes annotation and __entry_text and so on. I see, but we can also check the address by comparing the address of symbols, which also can be retrieved from kallsyms. Since the test case is also applied to stable kernel, I don't want to make it depending on some special kernel tweaks. > > > > > > > > With those patches, we should be able to select symbols that are not > > > blacklisted. > > > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > > etc. > > Yes, I think this will be good for the generic test, but may not help > selecting a dot symbol on powerpc. Right, and it depends on what gcc version and option is specified. So, maybe we can skip the test if there is no such symbols. I intended to test the symbols with some dot-suffix, but it seems ppc64 has dot-started symbols, right? If there is no dot-suffixed symbols, I think this test should skip the test case. Thank you, -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 29 Jun 2017 00:13:24 +0530 "Naveen N. Rao" wrote: > On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > > 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 000..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. > > > > Hmm, if we increase the -m argument, like -m 100, is that still > > in a blacklisted function? > > Yes, most of the initial symbols are exception vectors which are > blacklisted. Hmm, then how about this? :) grep _stext -A 1000 | grep -m 10 " [tT] [^.]*$" /proc/kallsyms > > > > > > 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. > > > > Would you find that is only on powerpc or generic issue? > > I meant the address _ranges_ that are blacklisted such as the ones with > __kprobes annotation and __entry_text and so on. I see, but we can also check the address by comparing the address of symbols, which also can be retrieved from kallsyms. Since the test case is also applied to stable kernel, I don't want to make it depending on some special kernel tweaks. > > > > > > > > With those patches, we should be able to select symbols that are not > > > blacklisted. > > > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > > etc. > > Yes, I think this will be good for the generic test, but may not help > selecting a dot symbol on powerpc. Right, and it depends on what gcc version and option is specified. So, maybe we can skip the test if there is no such symbols. I intended to test the symbols with some dot-suffix, but it seems ppc64 has dot-started symbols, right? If there is no dot-suffixed symbols, I think this test should skip the test case. Thank you, -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > 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 000..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. > > Hmm, if we increase the -m argument, like -m 100, is that still > in a blacklisted function? Yes, most of the initial symbols are exception vectors which are blacklisted. > > > 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. > > Would you find that is only on powerpc or generic issue? I meant the address _ranges_ that are blacklisted such as the ones with __kprobes annotation and __entry_text and so on. > > > > > With those patches, we should be able to select symbols that are not > > blacklisted. > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > etc. Yes, I think this will be good for the generic test, but may not help selecting a dot symbol on powerpc. Thanks, Naveen
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/28 11:16PM, Masami Hiramatsu wrote: > > > 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 000..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. > > Hmm, if we increase the -m argument, like -m 100, is that still > in a blacklisted function? Yes, most of the initial symbols are exception vectors which are blacklisted. > > > 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. > > Would you find that is only on powerpc or generic issue? I meant the address _ranges_ that are blacklisted such as the ones with __kprobes annotation and __entry_text and so on. > > > > > With those patches, we should be able to select symbols that are not > > blacklisted. > > Or, maybe we can use generic function, like "schedule" or "vfs_read" > etc. Yes, I think this will be good for the generic test, but may not help selecting a dot symbol on powerpc. Thanks, Naveen
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Wed, 28 Jun 2017 14:58:46 +0530 "Naveen N. Rao"wrote: > On 2017/06/24 08:06PM, Masami Hiramatsu wrote: > > On Sat, 24 Jun 2017 02:30:21 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 22:33:25 +0530 > > > "Naveen N. Rao" wrote: > > > > > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > > > "Naveen N. Rao" 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 > > > > 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 > > --- > > .../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 000..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. Hmm, if we increase the -m argument, like -m 100, is that still in a blacklisted function? > 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. Would you find that is only on powerpc or generic issue? > > With those patches, we should be able to select symbols that are not > blacklisted. Or, maybe we can use generic function, like "schedule" or "vfs_read" etc. > > > +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 >
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Wed, 28 Jun 2017 14:58:46 +0530 "Naveen N. Rao" wrote: > On 2017/06/24 08:06PM, Masami Hiramatsu wrote: > > On Sat, 24 Jun 2017 02:30:21 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 22:33:25 +0530 > > > "Naveen N. Rao" wrote: > > > > > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > > > "Naveen N. Rao" 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 > > > > 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 > > --- > > .../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 000..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. Hmm, if we increase the -m argument, like -m 100, is that still in a blacklisted function? > 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. Would you find that is only on powerpc or generic issue? > > With those patches, we should be able to select symbols that are not > blacklisted. Or, maybe we can use generic function, like "schedule" or "vfs_read" etc. > > > +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]
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/24 08:06PM, Masami Hiramatsu wrote: > On Sat, 24 Jun 2017 02:30:21 +0900 > Masami Hiramatsuwrote: > > > On Thu, 22 Jun 2017 22:33:25 +0530 > > "Naveen N. Rao" wrote: > > > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > > "Naveen N. Rao" 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 > > 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 > --- > .../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 000..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
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/24 08:06PM, Masami Hiramatsu wrote: > On Sat, 24 Jun 2017 02:30:21 +0900 > Masami Hiramatsu wrote: > > > On Thu, 22 Jun 2017 22:33:25 +0530 > > "Naveen N. Rao" wrote: > > > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > > "Naveen N. Rao" 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 > > 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 > --- > .../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 000..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
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Sat, 24 Jun 2017 02:30:21 +0900 Masami Hiramatsuwrote: > On Thu, 22 Jun 2017 22:33:25 +0530 > "Naveen N. Rao" wrote: > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > "Naveen N. Rao" 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. Hi Naveen, Here is the testcase which I meant above. This may help if there is any regression related to this specific issue. Thank you, - selftests: ftrace: Add a testcase for kprobe event naming From: Masami Hiramatsu 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 --- .../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 000..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 " "` +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 -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Sat, 24 Jun 2017 02:30:21 +0900 Masami Hiramatsu wrote: > On Thu, 22 Jun 2017 22:33:25 +0530 > "Naveen N. Rao" wrote: > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > "Naveen N. Rao" 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. Hi Naveen, Here is the testcase which I meant above. This may help if there is any regression related to this specific issue. Thank you, - selftests: ftrace: Add a testcase for kprobe event naming From: Masami Hiramatsu 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 --- .../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 000..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 " "` +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 -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 22 Jun 2017 22:33:25 +0530 "Naveen N. Rao"wrote: > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > On Thu, 22 Jun 2017 00:20:28 +0530 > > "Naveen N. Rao" 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. Thank you, > > Thanks for the review! > - Naveen > > > > So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, > > could you update the filter as "^.*\\..*" ? > > > > Thank you, > > > > > > > > Signed-off-by: Naveen N. Rao > > > --- > > > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git > > > a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > index f4d1ff785d67..d209c071b2c0 100644 > > > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > @@ -2,16 +2,16 @@ > > > # description: Register/unregister many kprobe events > > > > > > # ftrace fentry skip size depends on the machine architecture. > > > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > > > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > > > case `uname -m` in > > >x86_64|i[3456]86) OFFS=5;; > > > - ppc*) OFFS=4;; > > > + ppc64le) OFFS=8;; > > >*) OFFS=0;; > > > esac > > > > > > echo "Setup up to 256 kprobes" > > > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > > > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > > > > kprobe_events ||: > > > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > > > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > > > > > echo 1 > events/kprobes/enable > > > echo 0 > events/kprobes/enable > > > -- > > > 2.13.1 > > > > > > > > > -- > > Masami Hiramatsu > > > -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 22 Jun 2017 22:33:25 +0530 "Naveen N. Rao" wrote: > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > On Thu, 22 Jun 2017 00:20:28 +0530 > > "Naveen N. Rao" 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. Thank you, > > Thanks for the review! > - Naveen > > > > So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, > > could you update the filter as "^.*\\..*" ? > > > > Thank you, > > > > > > > > Signed-off-by: Naveen N. Rao > > > --- > > > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git > > > a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > index f4d1ff785d67..d209c071b2c0 100644 > > > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > > @@ -2,16 +2,16 @@ > > > # description: Register/unregister many kprobe events > > > > > > # ftrace fentry skip size depends on the machine architecture. > > > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > > > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > > > case `uname -m` in > > >x86_64|i[3456]86) OFFS=5;; > > > - ppc*) OFFS=4;; > > > + ppc64le) OFFS=8;; > > >*) OFFS=0;; > > > esac > > > > > > echo "Setup up to 256 kprobes" > > > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > > > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > > > > kprobe_events ||: > > > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > > > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > > > > > echo 1 > events/kprobes/enable > > > echo 0 > events/kprobes/enable > > > -- > > > 2.13.1 > > > > > > > > > -- > > Masami Hiramatsu > > > -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > On Thu, 22 Jun 2017 00:20:28 +0530 > "Naveen N. Rao"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. Thanks for the review! - Naveen > So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, > could you update the filter as "^.*\\..*" ? > > Thank you, > > > > > Signed-off-by: Naveen N. Rao > > --- > > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git > > a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > index f4d1ff785d67..d209c071b2c0 100644 > > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > @@ -2,16 +2,16 @@ > > # description: Register/unregister many kprobe events > > > > # ftrace fentry skip size depends on the machine architecture. > > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > > case `uname -m` in > >x86_64|i[3456]86) OFFS=5;; > > - ppc*) OFFS=4;; > > + ppc64le) OFFS=8;; > >*) OFFS=0;; > > esac > > > > echo "Setup up to 256 kprobes" > > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events > > ||: > > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > > > echo 1 > events/kprobes/enable > > echo 0 > events/kprobes/enable > > -- > > 2.13.1 > > > > > -- > Masami Hiramatsu >
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > On Thu, 22 Jun 2017 00:20:28 +0530 > "Naveen N. Rao" 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. Thanks for the review! - Naveen > So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, > could you update the filter as "^.*\\..*" ? > > Thank you, > > > > > Signed-off-by: Naveen N. Rao > > --- > > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git > > a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > index f4d1ff785d67..d209c071b2c0 100644 > > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > > @@ -2,16 +2,16 @@ > > # description: Register/unregister many kprobe events > > > > # ftrace fentry skip size depends on the machine architecture. > > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > > case `uname -m` in > >x86_64|i[3456]86) OFFS=5;; > > - ppc*) OFFS=4;; > > + ppc64le) OFFS=8;; > >*) OFFS=0;; > > esac > > > > echo "Setup up to 256 kprobes" > > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events > > ||: > > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > > > echo 1 > events/kprobes/enable > > echo 0 > events/kprobes/enable > > -- > > 2.13.1 > > > > > -- > Masami Hiramatsu >
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 22 Jun 2017 00:20:28 +0530 "Naveen N. Rao"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. So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, could you update the filter as "^.*\\..*" ? Thank you, > > Signed-off-by: Naveen N. Rao > --- > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > index f4d1ff785d67..d209c071b2c0 100644 > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > @@ -2,16 +2,16 @@ > # description: Register/unregister many kprobe events > > # ftrace fentry skip size depends on the machine architecture. > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > case `uname -m` in >x86_64|i[3456]86) OFFS=5;; > - ppc*) OFFS=4;; > + ppc64le) OFFS=8;; >*) OFFS=0;; > esac > > echo "Setup up to 256 kprobes" > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > echo 1 > events/kprobes/enable > echo 0 > events/kprobes/enable > -- > 2.13.1 > -- Masami Hiramatsu
Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc
On Thu, 22 Jun 2017 00:20:28 +0530 "Naveen N. Rao" 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. So if the Powerpc Elfv1 use dot prefix, that is OK, in that case, could you update the filter as "^.*\\..*" ? Thank you, > > Signed-off-by: Naveen N. Rao > --- > tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > index f4d1ff785d67..d209c071b2c0 100644 > --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc > @@ -2,16 +2,16 @@ > # description: Register/unregister many kprobe events > > # ftrace fentry skip size depends on the machine architecture. > -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc > +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le > case `uname -m` in >x86_64|i[3456]86) OFFS=5;; > - ppc*) OFFS=4;; > + ppc64le) OFFS=8;; >*) OFFS=0;; > esac > > echo "Setup up to 256 kprobes" > -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \ > +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||: > > echo 1 > events/kprobes/enable > echo 0 > events/kprobes/enable > -- > 2.13.1 > -- Masami Hiramatsu