| Message ID | f49dbddfbc7dee2f68a610bba468c39a4d017feb.1498070485.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, 22 Jun 2017 00:20:28 +0530 "Naveen N. Rao" <naveen.n.rao@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. 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 <naveen.n.rao@linux.vnet.ibm.com> > --- > 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 >
On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > On Thu, 22 Jun 2017 00:20:28 +0530 > "Naveen N. Rao" <naveen.n.rao@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. 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 <naveen.n.rao@linux.vnet.ibm.com> > > --- > > 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 <mhiramat@kernel.org> >
On Thu, 22 Jun 2017 22:33:25 +0530 "Naveen N. Rao" <naveen.n.rao@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.rao@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. 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 <naveen.n.rao@linux.vnet.ibm.com> > > > --- > > > 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 <mhiramat@kernel.org> > > >
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
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. 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. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)