[PATCH] ftracetest: Do not use usleep

[PATCH] ftracetest: Do not use usleep

[Namhyung Kim]

The usleep is only provided on distros from Redhat so running ftracetest
on other distro resulted in failures due to the missing usleep.

The reason of using [u]sleep in the test was to generate (scheduler)
events. But as we use 'cat trace | grep | wc -l' to read the events,
the command themselves already generate some events before reading the
trace file so no need to call [u]sleep explicitly.

Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Reported-by: Luis Henriques <luis.henriques@canonical.com>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/testing/selftests/ftrace/test.d/event/event-enable.tc     | 3 ---
 tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/event/event-enable.tc b/tools/testing/selftests/ftrace/test.d/event/event-enable.tc
index 668616d9bb03..7c7b4ce383a3 100644
--- a/tools/testing/selftests/ftrace/test.d/event/event-enable.tc
+++ b/tools/testing/selftests/ftrace/test.d/event/event-enable.tc
@@ -21,7 +21,6 @@ reset_tracer
 do_reset
 
 echo 'sched:sched_switch' > set_event
-usleep 1
 
 count=`cat trace | grep sched_switch | wc -l`
 if [ $count -eq 0 ]; then
@@ -31,7 +30,6 @@ fi
 do_reset
 
 echo 1 > events/sched/sched_switch/enable
-usleep 1
 
 count=`cat trace | grep sched_switch | wc -l`
 if [ $count -eq 0 ]; then
@@ -41,7 +39,6 @@ fi
 do_reset
 
 echo 0 > events/sched/sched_switch/enable
-usleep 1
 
 count=`cat trace | grep sched_switch | wc -l`
 if [ $count -ne 0 ]; then
diff --git a/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc b/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
index 655c415b6e7f..08a9459224b0 100644
--- a/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
+++ b/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
@@ -21,7 +21,6 @@ reset_tracer
 do_reset
 
 echo 'sched:*' > set_event
-usleep 1
 
 count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
 if [ $count -lt 3 ]; then
@@ -31,7 +30,6 @@ fi
 do_reset
 
 echo 1 > events/sched/enable
-usleep 1
 
 count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
 if [ $count -lt 3 ]; then
@@ -41,7 +39,6 @@ fi
 do_reset
 
 echo 0 > events/sched/enable
-usleep 1
 
 count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
 if [ $count -ne 0 ]; then
-- 
2.3.3

Re: [PATCH] ftracetest: Do not use usleep

[Shuah Khan]

On 03/25/2015 06:32 PM, Namhyung Kim wrote:
> The usleep is only provided on distros from Redhat so running ftracetest
> on other distro resulted in failures due to the missing usleep.
> 
> The reason of using [u]sleep in the test was to generate (scheduler)
> events. But as we use 'cat trace | grep | wc -l' to read the events,
> the command themselves already generate some events before reading the
> trace file so no need to call [u]sleep explicitly.
> 
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Dave Jones <davej@codemonkey.org.uk>
> Reported-by: Luis Henriques <luis.henriques@canonical.com>
> Tested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---

Hi Steve,

I will pull this in for 4.1. Assume that is ok. Just confirming.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH] ftracetest: Do not use usleep

[Steven Rostedt]

On Thu, 26 Mar 2015 09:32:23 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The usleep is only provided on distros from Redhat so running ftracetest
> on other distro resulted in failures due to the missing usleep.
> 
> The reason of using [u]sleep in the test was to generate (scheduler)
> events. But as we use 'cat trace | grep | wc -l' to read the events,
> the command themselves already generate some events before reading the
> trace file so no need to call [u]sleep explicitly.

Note, opening "trace" via cat stops tracing. There is a possible race
where the cat will not produce events. My worry is that if the shell
implements its own "cat" command, it may not fork, and open the trace
file. Which would not have any events in it, and opening it will
disable the rest of the command from having events.

What about using:

 ping localhost -c 1

?

-- Steve

Re: [PATCH] ftracetest: Do not use usleep

[Namhyung Kim]

Hi Steve,

On Mon, Mar 30, 2015 at 05:15:11PM -0400, Steven Rostedt wrote:
> On Thu, 26 Mar 2015 09:32:23 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > The usleep is only provided on distros from Redhat so running ftracetest
> > on other distro resulted in failures due to the missing usleep.
> > 
> > The reason of using [u]sleep in the test was to generate (scheduler)
> > events. But as we use 'cat trace | grep | wc -l' to read the events,
> > the command themselves already generate some events before reading the
> > trace file so no need to call [u]sleep explicitly.
> 
> Note, opening "trace" via cat stops tracing. There is a possible race
> where the cat will not produce events. My worry is that if the shell
> implements its own "cat" command, it may not fork, and open the trace
> file. Which would not have any events in it, and opening it will
> disable the rest of the command from having events.

I understand your point.  But this is not just cat, it needs grep and
wc also.  So I think there should be scheduler event(s).

> 
> What about using:
> 
>  ping localhost -c 1
> 
> ?

I'm okay with ping though but worried if some tiny system might lack
the ping command..

Thanks,
Namhyung

Re: [PATCH] ftracetest: Do not use usleep

[Steven Rostedt]

On Tue, 31 Mar 2015 09:48:55 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> I understand your point.  But this is not just cat, it needs grep and
> wc also.  So I think there should be scheduler event(s).
>

Yes, and cat opens 'trace', which will disable tracing, and if that
happens before grep and wc, then those events will not show up.

There was a time when these tests use to fail without adding something
to force a schedule, that's why I feel it's safe to add something.

-- Steve

Re: [PATCH] ftracetest: Do not use usleep

[Pádraig Brady]

On 31/03/15 01:48, Namhyung Kim wrote:
> Hi Steve,
> 
> On Mon, Mar 30, 2015 at 05:15:11PM -0400, Steven Rostedt wrote:
>> On Thu, 26 Mar 2015 09:32:23 +0900
>> Namhyung Kim <namhyung@kernel.org> wrote:
>>
>>> The usleep is only provided on distros from Redhat so running ftracetest
>>> on other distro resulted in failures due to the missing usleep.
>>>
>>> The reason of using [u]sleep in the test was to generate (scheduler)
>>> events. But as we use 'cat trace | grep | wc -l' to read the events,
>>> the command themselves already generate some events before reading the
>>> trace file so no need to call [u]sleep explicitly.
>>
>> Note, opening "trace" via cat stops tracing. There is a possible race
>> where the cat will not produce events. My worry is that if the shell
>> implements its own "cat" command, it may not fork, and open the trace
>> file. Which would not have any events in it, and opening it will
>> disable the rest of the command from having events.
> 
> I understand your point.  But this is not just cat, it needs grep and
> wc also.  So I think there should be scheduler event(s).
> 
>>
>> What about using:
>>
>>  ping localhost -c 1
>>
>> ?
> 
> I'm okay with ping though but worried if some tiny system might lack
> the ping command..

I'd use a fallback method like:

  yield() { sleep .001 || usleep 1 || sleep 1; }

Then just s/usleep 1/yield/

cheers,
Pádraig.

Re: [PATCH] ftracetest: Do not use usleep

[Steven Rostedt]

On Tue, 31 Mar 2015 02:08:06 +0100
Pádraig Brady <P@draigBrady.com> wrote:

> > I'm okay with ping though but worried if some tiny system might lack
> > the ping command..
> 
> I'd use a fallback method like:
> 
>   yield() { sleep .001 || usleep 1 || sleep 1; }
> 
> Then just s/usleep 1/yield/

yield() { ping localhost -c 1 || sleep .001 || usleep 1 || sleep 1; } ;-)


-- Steve