RE: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
From: Masami Hiramatsu > Sent: 11 May 2020 10:28 ... > > We may not avoid fixing related failures after your change: > > 1) We have to reuse built-in echo (do alias echo=echo) if we want to > > test common_pid for histogram. > > 2) We have to reuse built-in echo if some new tests want to interpret > > backslash escapes in future. > > 1) yes, that's what I need to do for avoiding "pid" key histogram > (but I think we should have better way to test it) > 2) No, in that case you should use "/bin/echo -e" explicitly. >dash's built-in echo doesn't support it. > > > Is it simple to provide two implementations of echo?(built-in echo and > > echo command?) and then just apply echo command for kprobe_syntax_errors.tc? > > Hmm, OK, there might be another reason we reconsider this patch. > > - Alisasing echo (this patch) can avoid dash related issues but > this also makes "echo" running in another process implicitly. > > - Using /bin/echo for backslash explicitly will be missed unless > user runs it on dash, but it will keep "echo" in same process. Why not change to use printf - probably a builtin in both shells? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Mon, 11 May 2020 15:22:25 +0800 Xiao Yang wrote: > On 2020/5/7 17:15, Masami Hiramatsu wrote: > > On Thu, 7 May 2020 14:45:16 +0800 > > Xiao Yang wrote: > > > >> On 2020/5/1 21:38, Masami Hiramatsu wrote: > >>> Since the built-in echo has different behavior in POSIX shell > >>> (dash) and bash, we forcibly use /bin/echo -E (not interpret > >>> backslash escapes) by default. > >>> > >>> This also fixes some test cases which expects built-in > >>> echo command. > >>> > >>> Reported-by: Liu Yiding > >>> Signed-off-by: Masami Hiramatsu > >>> --- > >>>tools/testing/selftests/ftrace/test.d/functions|3 +++ > >>>.../test.d/trigger/trigger-trace-marker-hist.tc|2 +- > >>>.../trigger-trace-marker-synthetic-kernel.tc |4 > >>>.../trigger/trigger-trace-marker-synthetic.tc |4 ++-- > >>>4 files changed, 10 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/tools/testing/selftests/ftrace/test.d/functions > >>> b/tools/testing/selftests/ftrace/test.d/functions > >>> index 5d4550591ff9..ea59b6ea2c3e 100644 > >>> --- a/tools/testing/selftests/ftrace/test.d/functions > >>> +++ b/tools/testing/selftests/ftrace/test.d/functions > >>> @@ -1,3 +1,6 @@ > >>> +# Since the built-in echo has different behavior in POSIX shell (dash) > >>> and > >>> +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes). > >>> +alias echo="/bin/echo -E" > >> Hi Masami, Steven > >> > >> It seems that only kprobe_syntax_errors.tc is impacted by the issue > >> currently. Is it necessary for all tests to use /bin/echo and could we > >> just make kprobe_syntax_errors.tc use /bin/echo? > > > > Yes, I would like to unify the "echo"'s behavior among the testcases > > instead of patching each failure in the future. > > Or would you have any concern on it? > Hi Masami, > > Very sorry for the late reply. > > We may not avoid fixing related failures after your change: > 1) We have to reuse built-in echo (do alias echo=echo) if we want to > test common_pid for histogram. > 2) We have to reuse built-in echo if some new tests want to interpret > backslash escapes in future. 1) yes, that's what I need to do for avoiding "pid" key histogram (but I think we should have better way to test it) 2) No, in that case you should use "/bin/echo -e" explicitly. dash's built-in echo doesn't support it. > Is it simple to provide two implementations of echo?(built-in echo and > echo command?) and then just apply echo command for kprobe_syntax_errors.tc? Hmm, OK, there might be another reason we reconsider this patch. - Alisasing echo (this patch) can avoid dash related issues but this also makes "echo" running in another process implicitly. - Using /bin/echo for backslash explicitly will be missed unless user runs it on dash, but it will keep "echo" in same process. So both have pros/cons, but your idea will be locally effected. OK, I'll retry it. Thank you, -- Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On 2020/5/7 17:15, Masami Hiramatsu wrote:
On Thu, 7 May 2020 14:45:16 +0800
Xiao Yang wrote:
On 2020/5/1 21:38, Masami Hiramatsu wrote:
Since the built-in echo has different behavior in POSIX shell
(dash) and bash, we forcibly use /bin/echo -E (not interpret
backslash escapes) by default.
This also fixes some test cases which expects built-in
echo command.
Reported-by: Liu Yiding
Signed-off-by: Masami Hiramatsu
---
tools/testing/selftests/ftrace/test.d/functions|3 +++
.../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
.../trigger-trace-marker-synthetic-kernel.tc |4
.../trigger/trigger-trace-marker-synthetic.tc |4 ++--
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/functions
b/tools/testing/selftests/ftrace/test.d/functions
index 5d4550591ff9..ea59b6ea2c3e 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -1,3 +1,6 @@
+# Since the built-in echo has different behavior in POSIX shell (dash) and
+# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
+alias echo="/bin/echo -E"
Hi Masami, Steven
It seems that only kprobe_syntax_errors.tc is impacted by the issue
currently. Is it necessary for all tests to use /bin/echo and could we
just make kprobe_syntax_errors.tc use /bin/echo?
Yes, I would like to unify the "echo"'s behavior among the testcases
instead of patching each failure in the future.
Or would you have any concern on it?
Hi Masami,
Very sorry for the late reply.
We may not avoid fixing related failures after your change:
1) We have to reuse built-in echo (do alias echo=echo) if we want to
test common_pid for histogram.
2) We have to reuse built-in echo if some new tests want to interpret
backslash escapes in future.
Is it simple to provide two implementations of echo?(built-in echo and
echo command?) and then just apply echo command for kprobe_syntax_errors.tc?
BTW: My suggestion may not be correct.
Best Regards,
Xiao Yang
Thank you,
Best Regards,
Xiao Yang
clear_trace() { # reset trace output
echo> trace
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
index ab6bedb25736..b3f70f53ee69 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
@@ -30,7 +30,7 @@ fi
echo "Test histogram trace_marker tigger"
-echo 'hist:keys=common_pid'> events/ftrace/print/trigger
+echo 'hist:keys=ip'> events/ftrace/print/trigger
for i in `seq 1 10` ; do echo "hello"> trace_marker; done
grep 'hitcount: *10$' events/ftrace/print/hist> /dev/null || \
fail "hist trigger did not trigger correct times on trace_marker"
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
index 18b4d1c2807e..c1625d945f4d 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
@@ -44,6 +44,10 @@ echo 'latency u64 lat'> synthetic_events
echo 'hist:keys=pid:ts0=common_timestamp.usecs'>
events/sched/sched_waking/trigger
echo
'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'>
events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat'> events/synthetic/latency/trigger
+
+# We have to use the built-in echo here because waking up pid must be same
+# as echoing pid.
+alias echo=echo
sleep 1
echo "hello"> trace_marker
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
index dd262d6d0db6..23e52c8d71de 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
@@ -36,8 +36,8 @@ fi
echo "Test histogram trace_marker to trace_marker latency histogram trigger"
echo 'latency u64 lat'> synthetic_events
-echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"'>
events/ftrace/print/trigger
-echo 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
if buf == "end"'>> events/ftrace/print/trigger
+echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"'>
events/ftrace/print/trigger
+echo 'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat) if buf
== "end"'>> events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat'> events/synthetic/latency/trigger
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Thu, 7 May 2020 15:32:46 -0500 "Zanussi, Tom" wrote: > Hi, > > On 5/7/2020 12:25 PM, Steven Rostedt wrote: > > On Fri, 8 May 2020 00:50:28 +0900 > > Masami Hiramatsu wrote: > > > Yes, I need Tom's review for this change. As far as I can test, this > fixes the test failure. If this isn't acceptable, we can use "alias > echo=echo" > for this test case. > > >>> > >>> I still don't see how changing "keys=common_pid" to "keys=ip" has anything > >>> to do with the echo patch. If that is a problem, it should be a different > >>> patch with explanation to why "keys=common_pid" is broken. > >> > >> This test case uses a trace_marker event to make a histogram with > >> the common_pid key, and it expects the "echo" command is built-in command > >> so that the pid is same while writing several events to trace_marker. > >> I changed it to "ip" which is always same if trace_marker interface is > >> used. > > > > Can you explicitly state that in your change log? It wasn't obvious from > > what you meant with: > > > > "This also fixes some test cases which expects built-in echo command." OK, will add the description. > > > > With that change, > > Reviewed-by: Tom Zanussi Thanks Tom! > > Thanks, > > Tom > > > Thanks! > > > > -- Steve > > -- Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
Hi, On 5/7/2020 12:25 PM, Steven Rostedt wrote: On Fri, 8 May 2020 00:50:28 +0900 Masami Hiramatsu wrote: Yes, I need Tom's review for this change. As far as I can test, this fixes the test failure. If this isn't acceptable, we can use "alias echo=echo" for this test case. I still don't see how changing "keys=common_pid" to "keys=ip" has anything to do with the echo patch. If that is a problem, it should be a different patch with explanation to why "keys=common_pid" is broken. This test case uses a trace_marker event to make a histogram with the common_pid key, and it expects the "echo" command is built-in command so that the pid is same while writing several events to trace_marker. I changed it to "ip" which is always same if trace_marker interface is used. Can you explicitly state that in your change log? It wasn't obvious from what you meant with: "This also fixes some test cases which expects built-in echo command." With that change, Reviewed-by: Tom Zanussi Thanks, Tom Thanks! -- Steve
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Fri, 8 May 2020 00:50:28 +0900 Masami Hiramatsu wrote: > > > Yes, I need Tom's review for this change. As far as I can test, this > > > fixes the test failure. If this isn't acceptable, we can use "alias > > > echo=echo" > > > for this test case. > > > > > > > I still don't see how changing "keys=common_pid" to "keys=ip" has anything > > to do with the echo patch. If that is a problem, it should be a different > > patch with explanation to why "keys=common_pid" is broken. > > This test case uses a trace_marker event to make a histogram with > the common_pid key, and it expects the "echo" command is built-in command > so that the pid is same while writing several events to trace_marker. > I changed it to "ip" which is always same if trace_marker interface is > used. Can you explicitly state that in your change log? It wasn't obvious from what you meant with: "This also fixes some test cases which expects built-in echo command." Thanks! -- Steve
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Thu, 07 May 2020 11:22:28 +0200 Andreas Schwab wrote: > On Mai 01 2020, Masami Hiramatsu wrote: > > > Since the built-in echo has different behavior in POSIX shell > > (dash) and bash, we forcibly use /bin/echo -E (not interpret > > backslash escapes) by default. > > How about using printf instead (at least where it matters)? Hmm, I think replacing all echos with printf in the testcase might be much harder than this... Thank you, -- Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Thu, 7 May 2020 09:12:07 -0400 Steven Rostedt wrote: > On Sat, 2 May 2020 12:08:42 +0900 > Masami Hiramatsu wrote: > > > > > diff --git > > > > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > > > > > > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > > index ab6bedb25736..b3f70f53ee69 100644 > > > > --- > > > > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > > +++ > > > > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > > @@ -30,7 +30,7 @@ fi > > > > > > > > echo "Test histogram trace_marker tigger" > > > > > > > > -echo 'hist:keys=common_pid' > events/ftrace/print/trigger > > > > +echo 'hist:keys=ip' > events/ftrace/print/trigger > > > > > > This is doing more than just changing the echo being used. It's changing > > > the test being done. > > > > Yes, I need Tom's review for this change. As far as I can test, this > > fixes the test failure. If this isn't acceptable, we can use "alias > > echo=echo" > > for this test case. > > > > I still don't see how changing "keys=common_pid" to "keys=ip" has anything > to do with the echo patch. If that is a problem, it should be a different > patch with explanation to why "keys=common_pid" is broken. This test case uses a trace_marker event to make a histogram with the common_pid key, and it expects the "echo" command is built-in command so that the pid is same while writing several events to trace_marker. I changed it to "ip" which is always same if trace_marker interface is used. Thank you, > > -- Steve > -- Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Sat, 2 May 2020 12:08:42 +0900 Masami Hiramatsu wrote: > > > diff --git > > > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > > > > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > index ab6bedb25736..b3f70f53ee69 100644 > > > --- > > > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > +++ > > > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc > > > @@ -30,7 +30,7 @@ fi > > > > > > echo "Test histogram trace_marker tigger" > > > > > > -echo 'hist:keys=common_pid' > events/ftrace/print/trigger > > > +echo 'hist:keys=ip' > events/ftrace/print/trigger > > > > This is doing more than just changing the echo being used. It's changing > > the test being done. > > Yes, I need Tom's review for this change. As far as I can test, this > fixes the test failure. If this isn't acceptable, we can use "alias echo=echo" > for this test case. > I still don't see how changing "keys=common_pid" to "keys=ip" has anything to do with the echo patch. If that is a problem, it should be a different patch with explanation to why "keys=common_pid" is broken. -- Steve
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Mai 01 2020, Masami Hiramatsu wrote: > Since the built-in echo has different behavior in POSIX shell > (dash) and bash, we forcibly use /bin/echo -E (not interpret > backslash escapes) by default. How about using printf instead (at least where it matters)? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Thu, 7 May 2020 14:45:16 +0800
Xiao Yang wrote:
> On 2020/5/1 21:38, Masami Hiramatsu wrote:
> > Since the built-in echo has different behavior in POSIX shell
> > (dash) and bash, we forcibly use /bin/echo -E (not interpret
> > backslash escapes) by default.
> >
> > This also fixes some test cases which expects built-in
> > echo command.
> >
> > Reported-by: Liu Yiding
> > Signed-off-by: Masami Hiramatsu
> > ---
> > tools/testing/selftests/ftrace/test.d/functions|3 +++
> > .../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
> > .../trigger-trace-marker-synthetic-kernel.tc |4
> > .../trigger/trigger-trace-marker-synthetic.tc |4 ++--
> > 4 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/functions
> > b/tools/testing/selftests/ftrace/test.d/functions
> > index 5d4550591ff9..ea59b6ea2c3e 100644
> > --- a/tools/testing/selftests/ftrace/test.d/functions
> > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > @@ -1,3 +1,6 @@
> > +# Since the built-in echo has different behavior in POSIX shell (dash) and
> > +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
> > +alias echo="/bin/echo -E"
> Hi Masami, Steven
>
> It seems that only kprobe_syntax_errors.tc is impacted by the issue
> currently. Is it necessary for all tests to use /bin/echo and could we
> just make kprobe_syntax_errors.tc use /bin/echo?
Yes, I would like to unify the "echo"'s behavior among the testcases
instead of patching each failure in the future.
Or would you have any concern on it?
Thank you,
>
> Best Regards,
> Xiao Yang
>
> >
> > clear_trace() { # reset trace output
> > echo> trace
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > index ab6bedb25736..b3f70f53ee69 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > @@ -30,7 +30,7 @@ fi
> >
> > echo "Test histogram trace_marker tigger"
> >
> > -echo 'hist:keys=common_pid'> events/ftrace/print/trigger
> > +echo 'hist:keys=ip'> events/ftrace/print/trigger
> > for i in `seq 1 10` ; do echo "hello"> trace_marker; done
> > grep 'hitcount: *10$' events/ftrace/print/hist> /dev/null || \
> > fail "hist trigger did not trigger correct times on trace_marker"
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > index 18b4d1c2807e..c1625d945f4d 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > @@ -44,6 +44,10 @@ echo 'latency u64 lat'> synthetic_events
> > echo 'hist:keys=pid:ts0=common_timestamp.usecs'>
> > events/sched/sched_waking/trigger
> > echo
> > 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'>
> > events/ftrace/print/trigger
> > echo 'hist:keys=common_pid,lat:sort=lat'>
> > events/synthetic/latency/trigger
> > +
> > +# We have to use the built-in echo here because waking up pid must be same
> > +# as echoing pid.
> > +alias echo=echo
> > sleep 1
> > echo "hello"> trace_marker
> >
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > index dd262d6d0db6..23e52c8d71de 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > @@ -36,8 +36,8 @@ fi
> > echo "Test histogram trace_marker to trace_marker latency histogram
> > trigger"
> >
> > echo 'latency u64 lat'> synthetic_events
> > -echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"'>
> > events/ftrace/print/trigger
> > -echo
> > 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> > if buf == "end"'>> events/ftrace/print/trigger
> > +echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"'>
> > events/ftrace/print/trigger
> > +echo
> > 'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> > if buf == "end"'>> events/ftrace/print/trigger
> > echo 'hist:keys=common_pid,lat:sort=lat'>
> > events/synthetic/latency/trigger
> > echo -n "start"> trace_marker
> > echo -n "end"> trace_marker
> >
> >
> >
> > .
> >
>
>
>
--
Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On 2020/5/1 21:38, Masami Hiramatsu wrote:
Since the built-in echo has different behavior in POSIX shell
(dash) and bash, we forcibly use /bin/echo -E (not interpret
backslash escapes) by default.
This also fixes some test cases which expects built-in
echo command.
Reported-by: Liu Yiding
Signed-off-by: Masami Hiramatsu
---
tools/testing/selftests/ftrace/test.d/functions|3 +++
.../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
.../trigger-trace-marker-synthetic-kernel.tc |4
.../trigger/trigger-trace-marker-synthetic.tc |4 ++--
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/functions
b/tools/testing/selftests/ftrace/test.d/functions
index 5d4550591ff9..ea59b6ea2c3e 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -1,3 +1,6 @@
+# Since the built-in echo has different behavior in POSIX shell (dash) and
+# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
+alias echo="/bin/echo -E"
Hi Masami, Steven
It seems that only kprobe_syntax_errors.tc is impacted by the issue
currently. Is it necessary for all tests to use /bin/echo and could we
just make kprobe_syntax_errors.tc use /bin/echo?
Best Regards,
Xiao Yang
clear_trace() { # reset trace output
echo> trace
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
index ab6bedb25736..b3f70f53ee69 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
@@ -30,7 +30,7 @@ fi
echo "Test histogram trace_marker tigger"
-echo 'hist:keys=common_pid'> events/ftrace/print/trigger
+echo 'hist:keys=ip'> events/ftrace/print/trigger
for i in `seq 1 10` ; do echo "hello"> trace_marker; done
grep 'hitcount: *10$' events/ftrace/print/hist> /dev/null || \
fail "hist trigger did not trigger correct times on trace_marker"
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
index 18b4d1c2807e..c1625d945f4d 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
@@ -44,6 +44,10 @@ echo 'latency u64 lat'> synthetic_events
echo 'hist:keys=pid:ts0=common_timestamp.usecs'>
events/sched/sched_waking/trigger
echo
'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'>
events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat'> events/synthetic/latency/trigger
+
+# We have to use the built-in echo here because waking up pid must be same
+# as echoing pid.
+alias echo=echo
sleep 1
echo "hello"> trace_marker
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
index dd262d6d0db6..23e52c8d71de 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
@@ -36,8 +36,8 @@ fi
echo "Test histogram trace_marker to trace_marker latency histogram trigger"
echo 'latency u64 lat'> synthetic_events
-echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"'>
events/ftrace/print/trigger
-echo 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
if buf == "end"'>> events/ftrace/print/trigger
+echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"'>
events/ftrace/print/trigger
+echo 'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat) if buf
== "end"'>> events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat'> events/synthetic/latency/trigger
echo -n "start"> trace_marker
echo -n "end"> trace_marker
.
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Fri, 1 May 2020 10:19:42 -0400
Steven Rostedt wrote:
> On Fri, 1 May 2020 22:38:00 +0900
> Masami Hiramatsu wrote:
>
> > Since the built-in echo has different behavior in POSIX shell
> > (dash) and bash, we forcibly use /bin/echo -E (not interpret
> > backslash escapes) by default.
> >
> > This also fixes some test cases which expects built-in
> > echo command.
> >
> > Reported-by: Liu Yiding
> > Signed-off-by: Masami Hiramatsu
> > ---
> > tools/testing/selftests/ftrace/test.d/functions|3 +++
> > .../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
> > .../trigger-trace-marker-synthetic-kernel.tc |4
> > .../trigger/trigger-trace-marker-synthetic.tc |4 ++--
> > 4 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/functions
> > b/tools/testing/selftests/ftrace/test.d/functions
> > index 5d4550591ff9..ea59b6ea2c3e 100644
> > --- a/tools/testing/selftests/ftrace/test.d/functions
> > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > @@ -1,3 +1,6 @@
> > +# Since the built-in echo has different behavior in POSIX shell (dash) and
> > +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
> > +alias echo="/bin/echo -E"
> >
> > clear_trace() { # reset trace output
> > echo > trace
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > index ab6bedb25736..b3f70f53ee69 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> > @@ -30,7 +30,7 @@ fi
> >
> > echo "Test histogram trace_marker tigger"
> >
> > -echo 'hist:keys=common_pid' > events/ftrace/print/trigger
> > +echo 'hist:keys=ip' > events/ftrace/print/trigger
>
> This is doing more than just changing the echo being used. It's changing
> the test being done.
Yes, I need Tom's review for this change. As far as I can test, this
fixes the test failure. If this isn't acceptable, we can use "alias echo=echo"
for this test case.
Thank you,
>
> > for i in `seq 1 10` ; do echo "hello" > trace_marker; done
> > grep 'hitcount: *10$' events/ftrace/print/hist > /dev/null || \
> > fail "hist trigger did not trigger correct times on trace_marker"
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > index 18b4d1c2807e..c1625d945f4d 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> > @@ -44,6 +44,10 @@ echo 'latency u64 lat' > synthetic_events
> > echo 'hist:keys=pid:ts0=common_timestamp.usecs' >
> > events/sched/sched_waking/trigger
> > echo
> > 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'
> > > events/ftrace/print/trigger
> > echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
> > +
> > +# We have to use the built-in echo here because waking up pid must be same
> > +# as echoing pid.
> > +alias echo=echo
> > sleep 1
> > echo "hello" > trace_marker
> >
> > diff --git
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> >
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > index dd262d6d0db6..23e52c8d71de 100644
> > ---
> > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > +++
> > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> > @@ -36,8 +36,8 @@ fi
> > echo "Test histogram trace_marker to trace_marker latency histogram
> > trigger"
> >
> > echo 'latency u64 lat' > synthetic_events
> > -echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"' >
> > events/ftrace/print/trigger
> > -echo
> > 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> > if buf == "end"' >> events/ftrace/print/trigger
> > +echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"' >
> > events/ftrace/print/trigger
> > +echo
> > 'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> > if buf == "end"' >> events/ftrace/print/trigger
>
> This too. And it's not explained in the change log why. In fact, these
> changes look like they belong in a separate patch.
>
> -- Steve
>
> > echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
> > echo -n "start" > trace_marker
> > echo -n "end" > trace_marker
>
--
Masami Hiramatsu
Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
On Fri, 1 May 2020 22:38:00 +0900
Masami Hiramatsu wrote:
> Since the built-in echo has different behavior in POSIX shell
> (dash) and bash, we forcibly use /bin/echo -E (not interpret
> backslash escapes) by default.
>
> This also fixes some test cases which expects built-in
> echo command.
>
> Reported-by: Liu Yiding
> Signed-off-by: Masami Hiramatsu
> ---
> tools/testing/selftests/ftrace/test.d/functions|3 +++
> .../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
> .../trigger-trace-marker-synthetic-kernel.tc |4
> .../trigger/trigger-trace-marker-synthetic.tc |4 ++--
> 4 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/functions
> b/tools/testing/selftests/ftrace/test.d/functions
> index 5d4550591ff9..ea59b6ea2c3e 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -1,3 +1,6 @@
> +# Since the built-in echo has different behavior in POSIX shell (dash) and
> +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
> +alias echo="/bin/echo -E"
>
> clear_trace() { # reset trace output
> echo > trace
> diff --git
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> index ab6bedb25736..b3f70f53ee69 100644
> ---
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> +++
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
> @@ -30,7 +30,7 @@ fi
>
> echo "Test histogram trace_marker tigger"
>
> -echo 'hist:keys=common_pid' > events/ftrace/print/trigger
> +echo 'hist:keys=ip' > events/ftrace/print/trigger
This is doing more than just changing the echo being used. It's changing
the test being done.
> for i in `seq 1 10` ; do echo "hello" > trace_marker; done
> grep 'hitcount: *10$' events/ftrace/print/hist > /dev/null || \
> fail "hist trigger did not trigger correct times on trace_marker"
> diff --git
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
>
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> index 18b4d1c2807e..c1625d945f4d 100644
> ---
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> +++
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
> @@ -44,6 +44,10 @@ echo 'latency u64 lat' > synthetic_events
> echo 'hist:keys=pid:ts0=common_timestamp.usecs' >
> events/sched/sched_waking/trigger
> echo
> 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'
> > events/ftrace/print/trigger
> echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
> +
> +# We have to use the built-in echo here because waking up pid must be same
> +# as echoing pid.
> +alias echo=echo
> sleep 1
> echo "hello" > trace_marker
>
> diff --git
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
>
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> index dd262d6d0db6..23e52c8d71de 100644
> ---
> a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> +++
> b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
> @@ -36,8 +36,8 @@ fi
> echo "Test histogram trace_marker to trace_marker latency histogram trigger"
>
> echo 'latency u64 lat' > synthetic_events
> -echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"' >
> events/ftrace/print/trigger
> -echo
> 'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> if buf == "end"' >> events/ftrace/print/trigger
> +echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"' >
> events/ftrace/print/trigger
> +echo
> 'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
> if buf == "end"' >> events/ftrace/print/trigger
This too. And it's not explained in the change log why. In fact, these
changes look like they belong in a separate patch.
-- Steve
> echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
> echo -n "start" > trace_marker
> echo -n "end" > trace_marker
[PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo
Since the built-in echo has different behavior in POSIX shell
(dash) and bash, we forcibly use /bin/echo -E (not interpret
backslash escapes) by default.
This also fixes some test cases which expects built-in
echo command.
Reported-by: Liu Yiding
Signed-off-by: Masami Hiramatsu
---
tools/testing/selftests/ftrace/test.d/functions|3 +++
.../test.d/trigger/trigger-trace-marker-hist.tc|2 +-
.../trigger-trace-marker-synthetic-kernel.tc |4
.../trigger/trigger-trace-marker-synthetic.tc |4 ++--
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/functions
b/tools/testing/selftests/ftrace/test.d/functions
index 5d4550591ff9..ea59b6ea2c3e 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -1,3 +1,6 @@
+# Since the built-in echo has different behavior in POSIX shell (dash) and
+# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
+alias echo="/bin/echo -E"
clear_trace() { # reset trace output
echo > trace
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
index ab6bedb25736..b3f70f53ee69 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-hist.tc
@@ -30,7 +30,7 @@ fi
echo "Test histogram trace_marker tigger"
-echo 'hist:keys=common_pid' > events/ftrace/print/trigger
+echo 'hist:keys=ip' > events/ftrace/print/trigger
for i in `seq 1 10` ; do echo "hello" > trace_marker; done
grep 'hitcount: *10$' events/ftrace/print/hist > /dev/null || \
fail "hist trigger did not trigger correct times on trace_marker"
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
index 18b4d1c2807e..c1625d945f4d 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic-kernel.tc
@@ -44,6 +44,10 @@ echo 'latency u64 lat' > synthetic_events
echo 'hist:keys=pid:ts0=common_timestamp.usecs' >
events/sched/sched_waking/trigger
echo
'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).latency($lat)'
> events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
+
+# We have to use the built-in echo here because waking up pid must be same
+# as echoing pid.
+alias echo=echo
sleep 1
echo "hello" > trace_marker
diff --git
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
index dd262d6d0db6..23e52c8d71de 100644
---
a/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
+++
b/tools/testing/selftests/ftrace/test.d/trigger/trigger-trace-marker-synthetic.tc
@@ -36,8 +36,8 @@ fi
echo "Test histogram trace_marker to trace_marker latency histogram trigger"
echo 'latency u64 lat' > synthetic_events
-echo 'hist:keys=common_pid:ts0=common_timestamp.usecs if buf == "start"' >
events/ftrace/print/trigger
-echo
'hist:keys=common_pid:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
if buf == "end"' >> events/ftrace/print/trigger
+echo 'hist:keys=ip:ts0=common_timestamp.usecs if buf == "start"' >
events/ftrace/print/trigger
+echo
'hist:keys=ip:lat=common_timestamp.usecs-$ts0:onmatch(ftrace.print).latency($lat)
if buf == "end"' >> events/ftrace/print/trigger
echo 'hist:keys=common_pid,lat:sort=lat' > events/synthetic/latency/trigger
echo -n "start" > trace_marker
echo -n "end" > trace_marker
