tracing: make ftrace tests to be extended

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh +11 -11

 LOOP=400
 
 # In kernel which is older than 2.6.32, we set global clock

@@ -24,23 +27,23 @@ else
         old_kernel=0
 fi
 
-for ((; ;))
-{
-	if [ $old_kernel -eq 1 ];
-	then
-		for ((i = 0; i < $LOOP; i++))
-		{
+while true; do
+	i=0;
+	if [ $old_kernel -eq 1 ]; then
+		while [ $i -lt $LOOP ]; do
 			echo 1 > "$TRACING_PATH"/options/global-clock
 			echo 0 > "$TRACING_PATH"/options/global-clock
-		}
+			i=$((i + 1))
+		done
 	else
-		for ((i = 0; i < $LOOP; i++))
-		{
+		while [ $i -lt $LOOP ]; do
 			echo local > "$TRACING_PATH"/trace_clock
 			echo global > "$TRACING_PATH"/trace_clock
-		}
+			i=$((i + 1))
+		done
+
 	fi
 
 	sleep 1
-}
+done
 

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +0 -0

index 47d42bc..a24008a 100755

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +3 -4

@@ -33,10 +34,9 @@ trap kill_this_pid SIGUSR1
 
 LOOP=20
 
-for ((; ;))
-{
-	for ((i = 0; i < $LOOP; i++))
-	{
+while true; do
+	i=0;
+	while [ $i -lt $LOOP ]; do
 		cat "$TRACING_PATH"/trace_pipe > /dev/null &
 
 		this_pid=$!

@@ -45,8 +45,7 @@ for ((; ;))
 		wait $this_pid

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh +9 -9

+nr_cpus=`tst_ncpus`
 
 if [ ! -e "$TRACING_PATH"/function_profile_enabled ]; then
         should_skip=1

@@ -28,18 +30,17 @@ if [ $? -eq 0 ]; then
 	should_skip=1
 fi
 
-for ((; ;))
-{
+while true; do
 	if [ $should_skip -eq 1 ]; then
 		sleep 2
 		continue
 	fi
-
-	for ((i = 0; i < $LOOP; i++))
-	{
-		cat "$TRACING_PATH"/trace_stat/function0 > /dev/null 2>&1
-	}
+	cpu=$($rand_gen 0 $((nr_cpus - 1)))
+	i=0;
+	while [ $i -lt $LOOP ]; do
+		cat "$TRACING_PATH"/trace_stat/function${cpu} > /dev/null 2>&1
+		i=$((i + 1))
+	done
 
 	sleep 1
-}
-
+done

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh +2 -4

index fbaceb8..6090a91 100755
@@ -15,12 +15,13 @@
 
 MAX_LATENCY=100000
 
-for ((; ;))
-{
-	for ((i = 0; i < $MAX_LATENCY; i += 400))
-	{
+while true; do
+	i=0;

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Wednesday, May 4, 2016 10:56:30 PM
Subject: Re: [LTP] [PATCH V2 9/9] ftrace_stress: cleanup and use ltp API

Hi!

Quoted 8 lines

---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_buffer_size_kb.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_buffer_size_kb.sh

(quoted diff) +0 -0

@@ -26,20 +26,20 @@ if [ $step -eq 0 ]; then
 	LOOP=50
 fi
 

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_buffer_size_kb.sh if [ $step -eq 0 ]; then · line 31

Quoted 7 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_buffer_size_kb.sh +2 -4

-for ((; ;))
-{
+while true; do
 	new_size=1
-	for ((i = 0; i < $LOOP; i++))
-	{
+	i=0;

The semicolon after i=0 is useless (and in all i=0; lines in this patch).

...

Quoted 13 lines

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh index de6bbea..70135c1 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_set_event.sh +0 -0

@@ -13,6 +13,9 @@
 #
 #
 ###############################################################################
 

Review line 17

Quoted 1 line

(quoted diff) +1 -0

+. test.sh

No need to source test.sh for the tst_kvercmp which is a separate binary.

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh line 18

Quoted 1 line

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh +1 -0

+TRACING_PATH=/sys/kernel/debug/tracing/

Shouldn't this be propagated from the main script? What's the point of redefining it here?

Quoted 51 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_clock.sh +11 -11

 LOOP=400
 
 # In kernel which is older than 2.6.32, we set global clock

@@ -24,23 +27,23 @@ else
         old_kernel=0
 fi
 
-for ((; ;))
-{
-	if [ $old_kernel -eq 1 ];
-	then
-		for ((i = 0; i < $LOOP; i++))
-		{
+while true; do
+	i=0;
+	if [ $old_kernel -eq 1 ]; then
+		while [ $i -lt $LOOP ]; do
 			echo 1 > "$TRACING_PATH"/options/global-clock
 			echo 0 > "$TRACING_PATH"/options/global-clock
-		}
+			i=$((i + 1))
+		done
 	else
-		for ((i = 0; i < $LOOP; i++))
-		{
+		while [ $i -lt $LOOP ]; do
 			echo local > "$TRACING_PATH"/trace_clock
 			echo global > "$TRACING_PATH"/trace_clock
-		}
+			i=$((i + 1))
+		done
+
 	fi
 
 	sleep 1
-}
+done
 

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh index 47d42bc..a24008a 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_set_event.sh +0 -0

@@ -13,10 +13,11 @@
 #
 #
 ###############################################################################
 

Review line 17

Quoted 1 line

(quoted diff) +1 -0

+. test.sh

You don't have to add test.sh for tst_sleep.

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +0 -2

 ftrace_sleep()
 {
-	# usleep is not a standard command?
-	usleep 200000 2> /dev/null

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh line 23

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +1 -0

+	tst_sleep 200000us
 	if [ $? -ne 0 ]; then
 		sleep 1
 	fi

You should remove the fallback to sleep 1 if the usleep wasn't found. Ideally the whole function ftrace_sleep should be removed and you should just call tst_sleep from the main loop.

Quoted 13 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +3 -4

@@ -33,10 +34,9 @@ trap kill_this_pid SIGUSR1
 
 LOOP=20
 
-for ((; ;))
-{
-	for ((i = 0; i < $LOOP; i++))
-	{
+while true; do
+	i=0;
+	while [ $i -lt $LOOP ]; do
 		cat "$TRACING_PATH"/trace_pipe > /dev/null &
 

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh for ((; ;)) · line 51

Quoted 13 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +3 -4

 		this_pid=$!

@@ -45,8 +45,7 @@ for ((; ;))
 		wait $this_pid
 		this_pid=0
 		ftrace_sleep
-	}
-
+		i=$((i + 1))
+	done
 	sleep 2
-}
-
+done

And you should also fix the kill called by the full path in this script. It should really be just kill not /bin/kill.

Quoted 11 lines

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh index d7e6fd3..2691991 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh

(quoted diff) +0 -0

@@ -16,6 +16,8 @@
 LOOP=200
 

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh line 19

Quoted 2 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh +1 -0

 should_skip=0
+rand_gen=tst_random

What's the point of assigning tst_random into a variable instead of using it directly?

Quoted 31 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_stat.sh +9 -9

+nr_cpus=`tst_ncpus`
 
 if [ ! -e "$TRACING_PATH"/function_profile_enabled ]; then
         should_skip=1

@@ -28,18 +30,17 @@ if [ $? -eq 0 ]; then
 	should_skip=1
 fi
 
-for ((; ;))
-{
+while true; do
 	if [ $should_skip -eq 1 ]; then
 		sleep 2
 		continue
 	fi
-
-	for ((i = 0; i < $LOOP; i++))
-	{
-		cat "$TRACING_PATH"/trace_stat/function0 > /dev/null 2>&1
-	}
+	cpu=$($rand_gen 0 $((nr_cpus - 1)))
+	i=0;
+	while [ $i -lt $LOOP ]; do
+		cat "$TRACING_PATH"/trace_stat/function${cpu} > /dev/null 2>&1
+		i=$((i + 1))
+	done
 
 	sleep 1
-}
-
+done

...

Quoted 24 lines

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh index fbaceb8..6090a91 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh +5 -5

@@ -15,12 +15,13 @@
 
 MAX_LATENCY=100000
 
-for ((; ;))
-{
-	for ((i = 0; i < $MAX_LATENCY; i += 400))
-	{
+while true; do
+	i=0;
+	while [ $i -lt $MAX_LATENCY ]; do
 		echo $i > "$TRACING_PATH"/tracing_max_latency
-	}
+		i=$((i + 400))
+	done
 

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh +2 -1

 	sleep 1
-}
+
+Vdone

^
Typo?

--
Cyril Hrubis
[email protected]

Quoted 24 lines

Quoted 24 lines

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh index fbaceb8..6090a91 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh +5 -5

@@ -15,12 +15,13 @@
 
 MAX_LATENCY=100000
 
-for ((; ;))
-{
-	for ((i = 0; i < $MAX_LATENCY; i += 400))
-	{
+while true; do
+	i=0;
+	while [ $i -lt $MAX_LATENCY ]; do
 		echo $i > "$TRACING_PATH"/tracing_max_latency
-	}
+		i=$((i + 400))
+	done
 

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_tracing_max_latency.sh +2 -1

 	sleep 1
-}
+
+Vdone

^
Typo?

Here it is weird, I checked my repo and my email in ltp list, there is no char 'V', I will have a deeper check.

From: "Chunyu Hu" <[email protected]>
To: "Cyril Hrubis" <[email protected]>
Cc: [email protected], [email protected]
Sent: Wednesday, May 4, 2016 11:42:48 PM
Subject: Re: [LTP] [PATCH V2 9/9] ftrace_stress: cleanup and use ltp API

Hi Cyril,

Quoted 13 lines

Quoted 13 lines

diff --git a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh index 47d42bc..a24008a 100755
---
a/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh
+++
b/testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_set_event.sh +0 -0

@@ -13,10 +13,11 @@
 #
 #
 ###############################################################################
 

Quoted 3 lines

Review line 17

Quoted 1 line

(quoted diff) +1 -0

+. test.sh

You don't have to add test.sh for tst_sleep.

OK. will remove this.

Quoted 4 lines

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +0 -2

 ftrace_sleep()
 {
-	# usleep is not a standard command?
-	usleep 200000 2> /dev/null

Quoted 8 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh line 23

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +1 -0

+	tst_sleep 200000us
 	if [ $? -ne 0 ]; then
 		sleep 1
 	fi

You should remove the fallback to sleep 1 if the usleep wasn't found. Ideally the whole function ftrace_sleep should be removed and you should just call tst_sleep from the main loop.

I think using tst_sleep directly is good way. thanks.

Quoted 29 lines

Quoted 16 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +3 -4

@@ -33,10 +34,9 @@ trap kill_this_pid SIGUSR1
 
 LOOP=20
 
-for ((; ;))
-{
-	for ((i = 0; i < $LOOP; i++))
-	{
+while true; do
+	i=0;
+	while [ $i -lt $LOOP ]; do
 		cat "$TRACING_PATH"/trace_pipe > /dev/null &
 
 		this_pid=$!

@@ -45,8 +45,7 @@ for ((; ;))
 		wait $this_pid

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh for ((; ;)) · line 51

Quoted 10 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress/ftrace_trace_pipe.sh +3 -4

 		this_pid=0
 		ftrace_sleep
-	}
-
+		i=$((i + 1))
+	done
 	sleep 2
-}
-
+done

And you should also fix the kill called by the full path in this script. It should really be just kill not /bin/kill.

After i changing it to kill directly and retest it, i hit err message:

ftrace_trace_pipe.sh: line 18: kill: SIGKILL: invalid signal specification

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Tuesday, May 10, 2016 10:13:00 PM
Subject: Re: [LTP] [PATCH V2 9/9] ftrace_stress: cleanup and use ltp API

Hi!

Quoted 3 lines

After i changing it to kill directly and retest it, i hit err message:

ftrace_trace_pipe.sh: line 18: kill: SIGKILL: invalid signal specification

That is because POSIX only accepts symbolic names without the SIG prefix while kill from procps works with both.

The correct way to send SIGKILL is 'kill -KILL $pid'.

--
Cyril Hrubis
[email protected]

include $(top_srcdir)/include/mk/generic_leaf_target.mk

testcases/lib/tst_random.c new file +64 -0

new file mode 100644
index 0000000..91a5247
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Author: Chunyu Hu <[email protected]>
+ *
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/time.h>
+
+static void print_help(void)
+{
+	printf("Usage: tst_random <value1> [value2]\n");
+	printf("       Generated random will be between value1 and value2.\n");
+	printf("       If only value1 is specified, value2 will treated as 0.\n");
+}
+
+static int get_seed(void)
+{
+	struct timeval tv;
+	gettimeofday(&tv, NULL);
+	return tv.tv_usec;
+}
+
+static long rand_range(long min, long max)
+{
+	return rand() % (max - min + 1) + min;
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+	long min = 0, max = 0, tmp = 0;
+	long rval = 0;
+	char *end;
+
+	while ((opt = getopt(argc, argv, ":h")) != -1) {
+		switch (opt) {
+		case 'h':
+			print_help();
+			return 0;
+		default:
+			print_help();
+			return 1;
+		}
+	}
+

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Wednesday, May 4, 2016 9:34:53 PM
Subject: Re: [LTP] [PATCH V2 6/9] testcases/lib: Add tst_random decmical integer generator

Hi!

Quoted 77 lines

include $(top_srcdir)/include/mk/generic_leaf_target.mk

testcases/lib/tst_random.c new file +32 -0

new file mode 100644
index 0000000..91a5247
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Author: Chunyu Hu <[email protected]>
+ *
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/time.h>
+
+static void print_help(void)
+{
+	printf("Usage: tst_random <value1> [value2]\n");
+	printf("       Generated random will be between value1 and value2.\n");
+	printf("       If only value1 is specified, value2 will treated as

0.\n");

testcases/lib/tst_random.c new file +37 -0

+}
+
+static int get_seed(void)
+{
+	struct timeval tv;
+	gettimeofday(&tv, NULL);
+	return tv.tv_usec;
+}
+
+static long rand_range(long min, long max)
+{
+	return rand() % (max - min + 1) + min;
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+	long min = 0, max = 0, tmp = 0;
+	long rval = 0;
+	char *end;
+
+	while ((opt = getopt(argc, argv, ":h")) != -1) {
+		switch (opt) {
+		case 'h':
+			print_help();
+			return 0;
+		default:
+			print_help();
+			return 1;
+		}
+	}
+
+	if (argc < 2 || argc > 3) {
+		print_help();
+		return 1;
+	}
+

testcases/lib/tst_random.c line 74

Quoted 5 lines

testcases/lib/tst_random.c new file +5 -0

+	if (optind >= argc) {
+		fprintf(stderr, "ERROR: Expected one or two range_val arguments\n\n");
+		print_help();
+		return 1;
+	}

We exit if getopt() finds any options in argv[] so optind == 1 here and since we check for argc < 2 before this this if is never true.

testcases/lib/tst_random.c line 76

Quoted 2 lines

testcases/lib/tst_random.c new file +1 -0

+	max = strtol(argv[optind], &end, 10);

Here as well optind == 1 by definition.

testcases/lib/tst_random.c line 82

Quoted 7 lines

testcases/lib/tst_random.c new file +6 -0

+	if (argv[optind] == end) {
+		fprintf(stderr, "ERROR: Invalid range value1 '%s'\n\n",
+			argv[optind]);
+		print_help();
+		return 1;
+	}

Well if we bother with strtol() we may check that argv[1] != '\0' and end == '\0' which will ensure that whole parameter was a valid string.

testcases/lib/tst_random.c line 85

Quoted 2 lines

testcases/lib/tst_random.c new file +2 -0

+	if (argc == 3) {
+		min = strtol(argv[optind+1], &end, 10);

Here as well use just 2 instead of optind + 1.

testcases/lib/tst_random.c line 96

Quoted 12 lines

testcases/lib/tst_random.c new file +11 -0

+		if (argv[optind+1] == end) {
+			fprintf(stderr, "ERROR: Invalid range value2 '%s'\n\n",
+				argv[optind+1]);
+			print_help();
+			return 1;
+		}
+		if (min > max) {
+			tmp = min;
+			min = max;
+			max = tmp;
+		}

Well we may also do:

	rval = (min > max) ? rand_range(max, min) : rand_range(min, max);

In the code below instead, but that is merely cosmetic.

testcases/lib/tst_random.c line 103

Quoted 8 lines

testcases/lib/tst_random.c new file +7 -0

+	}
+
+	srand(get_seed());
+	rval = rand_range(min, max);
+	printf("%ld\n", rval);
+
+	return rval;

We should just return 0 here since the return value is limited to one byte. Exporting interface that works only in certain cases is a recipe for a disaster.

--
Cyril Hrubis
[email protected]

Looks like we just need to compare with ( *end != '\0' ), as argv[1] or argv[2] can't be '\0' (NULL) as it has been filtered by (argc <2 || argc > 3).

And i just verified this with this debug line:

   printf("end = %p, *end = %c\n", end, *end);

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Wednesday, May 4, 2016 11:55:31 PM
Subject: Re: [LTP] [PATCH V2 6/9] testcases/lib: Add tst_random decmical integer generator

Hi!

Quoted 7 lines

Looks like we just need to compare with ( *end != '\0' ), as argv[1] or argv[2] can't be '\0' (NULL) as it has been filtered by (argc <2 || argc > 3).

And i just verified this with this debug line:

   printf("end = %p, *end = %c\n", end, *end);

What will happen when you call:

tst_random "" ""

Or more realistic case when FOO is not defined and you do:

tst_random "$FOO"

Note that empty string in C is an array with first element set to 0.

--
Cyril Hrubis
[email protected]

From: "Chunyu Hu" <[email protected]>
To: "Cyril Hrubis" <[email protected]>
Cc: [email protected], [email protected]
Sent: Thursday, May 5, 2016 12:28:30 AM
Subject: Re: [LTP] [PATCH V2 6/9] testcases/lib: Add tst_random decmical integer generator

----- Original Message -----

Quoted 12 lines

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Wednesday, May 4, 2016 11:55:31 PM
Subject: Re: [LTP] [PATCH V2 6/9] testcases/lib: Add tst_random decmical integer generator

Hi!

Quoted 4 lines

Looks like we just need to compare with ( *end != '\0' ), as argv[1] or argv[2] can't be '\0' (NULL) as it has been filtered by (argc <2 || argc

Quoted 1 line

3).

Quoted 6 lines

Quoted 3 lines

And i just verified this with this debug line:

   printf("end = %p, *end = %c\n", end, *end);

What will happen when you call:

Quoted 7 lines

tst_random "" ""

Or more realistic case when FOO is not defined and you do:

tst_random "$FOO"

Note that empty string in C is an array with first element set to 0.

I was so hurried to send out the patch :/. You are right. I missed your mentioned here. I added some debug line, proving you are right.

        printf("argv[1] = [%d]\n", argv[1][0]);
        printf("argv[3] = [%d]\n", argv[3]);
        printf("end = %p, *end = [%d]\n", end, *end);

./a.out "" ""

argv[1] = [0]
argv[3] = [0]
end = 0x7ffc3d49861f, *end = [0]

I will reorganize this check later, my brain is so slow now. Thank you.

Quoted 5 lines

--
Cyril Hrubis
[email protected]

--
Regards,
Chunyu Hu

--
Mailing list info: https://lists.linux.it/listinfo/ltp

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +23 -56

index d9f7f8b..80c914c 100755
@@ -21,87 +21,92 @@
 ##                                                                       ##
 ###########################################################################
 
-
 export TCID="ftrace-stress-test"
 export TST_TOTAL=1
 export TST_COUNT=1
 
 . ftrace_lib.sh
 
-test_success=true
+test_targets=" \
+trace_pipe current_tracer ftrace_enabled function_profile_enabled \
+set_event set_ftrace_pid stack_max_size stack_trace trace trace_clock \
+trace_options trace_stat tracing_enabled tracing_max_latency \
+tracing_on function_profile_enabled buffer_size_kb"
 
-export_pids()
+get_skip_targets()
 {
-	export pid1 pid2 pid3 pid4 pid5 pid6 pid7 pid8 pid9 pid10 pid11 pid12 \
-		pid13 pid14 pid15 pid16
-
-	export NR_PIDS=16
+	NR_PIDS=0
+	for target in ${test_targets}; do
+		if [ ! -e $TRACING_PATH/$target ] &&
+			[ ! -e /proc/sys/kernel/$target ]; then
+			eval skip_$target=1
+			tst_resm TINFO "$target is not supported. Skip it."
+		else
+			eval skip_$target=0
+			NR_PIDS=$((NR_PIDS + 1))
+		fi
+	done
+	# Export it before sub case is lanuched.
+	export NR_PIDS
 }
 
-test_stress()
+should_skip_target()
 {
-	export_pids
-
-	$SPATH/ftrace_trace_clock.sh &
-	pid1=$!
-	$SPATH/ftrace_current_tracer.sh &
-	pid2=$!
-	$SPATH/ftrace_trace_options.sh &
-	pid3=$!
-	$SPATH/ftrace_tracing_max_latency.sh &
-	pid4=$!
-	$SPATH/ftrace_stack_trace.sh &
-	pid5=$!
-	$SPATH/ftrace_stack_max_size.sh &
-	pid6=$!
-	$SPATH/ftrace_tracing_on.sh &
-	pid7=$!
-	$SPATH/ftrace_tracing_enabled.sh &
-	pid8=$!
-	$SPATH/ftrace_set_event.sh &
-	pid9=$!
-	$SPATH/ftrace_buffer_size.sh &
-	pid10=$!
-	$SPATH/ftrace_trace.sh &
-	pid11=$!
-	$SPATH/ftrace_trace_pipe.sh &
-	pid12=$!
-	$SPATH/ftrace_ftrace_enabled.sh &
-	pid13=$!
-	$SPATH/ftrace_set_ftrace_pid.sh &
-	pid14=$!
-	$SPATH/ftrace_profile_enabled.sh &
-	pid15=$!
-	$SPATH/ftrace_trace_stat.sh &
-	pid16=$!
+	local var=skip_$1
+	local val=${!var}
+	[ "$val" = 1 ]
 }
 
 test_kill()
 {
-	kill -KILL $pid1 || test_success=false
-	kill -KILL $pid2 || test_success=false
-	kill -KILL $pid3 || test_success=false
-	kill -KILL $pid4 || test_success=false
-	kill -KILL $pid5 || test_success=false
-	kill -KILL $pid6 || test_success=false
-	kill -KILL $pid7 || test_success=false
-	kill -KILL $pid8 || test_success=false
-	kill -KILL $pid9 || test_success=false
-	kill -KILL $pid10 || test_success=false
-	kill -KILL $pid11 || test_success=false
-	kill -USR1 $pid12 || test_success=false
-	kill -KILL $pid13 || test_success=false
-	kill -KILL $pid14 || test_success=false

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +19 -0

 	clean_up
 }
 
+test_stress()
+{
+	local index=0;
+
+	tst_resm TINFO "Test targets: ${test_targets}"
+
+	get_skip_targets
+	for target in ${test_targets}; do
+		if should_skip_target $target; then
+			continue
+		fi
+		sh ftrace_${target}.sh &
+		eval pid${index}=$!
+		tst_resm TINFO "Start pid${index}=$! $SPATH/ftrace_${target}.sh"
+		index=$((index + 1))
+	done
+	export_pids
+}
+

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +1 -6

-if $test_success; then
-	tst_resm TPASS "finished running the test. Run dmesg to double-check for bugs"
-else
-	tst_resm TFAIL "please check log message."
-	exit 1
-fi
+tst_exit

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Thursday, May 5, 2016 12:50:50 AM
Subject: Re: [LTP] [PATCH V2 4/9] ftrace_stress: skip unsupported tests

Hi!

Quoted 77 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +20 -36

index d9f7f8b..80c914c 100755
@@ -21,87 +21,92 @@
 ##
 ##
 ###########################################################################
 
-
 export TCID="ftrace-stress-test"
 export TST_TOTAL=1
 export TST_COUNT=1
 
 . ftrace_lib.sh
 
-test_success=true
+test_targets=" \
+trace_pipe current_tracer ftrace_enabled function_profile_enabled \
+set_event set_ftrace_pid stack_max_size stack_trace trace trace_clock \
+trace_options trace_stat tracing_enabled tracing_max_latency \
+tracing_on function_profile_enabled buffer_size_kb"
 
-export_pids()
+get_skip_targets()
 {
-	export pid1 pid2 pid3 pid4 pid5 pid6 pid7 pid8 pid9 pid10 pid11 pid12 \
-		pid13 pid14 pid15 pid16
-
-	export NR_PIDS=16
+	NR_PIDS=0
+	for target in ${test_targets}; do
+		if [ ! -e $TRACING_PATH/$target ] &&
+			[ ! -e /proc/sys/kernel/$target ]; then
+			eval skip_$target=1
+			tst_resm TINFO "$target is not supported. Skip it."
+		else
+			eval skip_$target=0
+			NR_PIDS=$((NR_PIDS + 1))
+		fi
+	done
+	# Export it before sub case is lanuched.
+	export NR_PIDS
 }
 
-test_stress()
+should_skip_target()
 {
-	export_pids
-
-	$SPATH/ftrace_trace_clock.sh &
-	pid1=$!
-	$SPATH/ftrace_current_tracer.sh &
-	pid2=$!
-	$SPATH/ftrace_trace_options.sh &
-	pid3=$!
-	$SPATH/ftrace_tracing_max_latency.sh &
-	pid4=$!
-	$SPATH/ftrace_stack_trace.sh &
-	pid5=$!
-	$SPATH/ftrace_stack_max_size.sh &
-	pid6=$!
-	$SPATH/ftrace_tracing_on.sh &
-	pid7=$!
-	$SPATH/ftrace_tracing_enabled.sh &
-	pid8=$!
-	$SPATH/ftrace_set_event.sh &
-	pid9=$!
-	$SPATH/ftrace_buffer_size.sh &
-	pid10=$!
-	$SPATH/ftrace_trace.sh &
-	pid11=$!
-	$SPATH/ftrace_trace_pipe.sh &
-	pid12=$!
-	$SPATH/ftrace_ftrace_enabled.sh &
-	pid13=$!

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 56

Quoted 8 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +2 -6

-	$SPATH/ftrace_set_ftrace_pid.sh &
-	pid14=$!
-	$SPATH/ftrace_profile_enabled.sh &
-	pid15=$!
-	$SPATH/ftrace_trace_stat.sh &
-	pid16=$!
+	local var=skip_$1
+	local val=${!var}

Quoted 23 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +3 -16

+	[ "$val" = 1 ]
 }
 
 test_kill()
 {
-	kill -KILL $pid1 || test_success=false
-	kill -KILL $pid2 || test_success=false
-	kill -KILL $pid3 || test_success=false
-	kill -KILL $pid4 || test_success=false
-	kill -KILL $pid5 || test_success=false
-	kill -KILL $pid6 || test_success=false
-	kill -KILL $pid7 || test_success=false
-	kill -KILL $pid8 || test_success=false
-	kill -KILL $pid9 || test_success=false
-	kill -KILL $pid10 || test_success=false
-	kill -KILL $pid11 || test_success=false
-	kill -USR1 $pid12 || test_success=false
-	kill -KILL $pid13 || test_success=false
-	kill -KILL $pid14 || test_success=false
-	kill -KILL $pid15 || test_success=false
-	kill -KILL $pid16 || test_success=false
+	tst_resm TINFO "killing ${pid0}"
+	kill -USR1 ${pid0}

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 69

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +6 -0

+	wait ${pid0}
+
+	local p=1;
+	while [ $p -lt $NR_PIDS ]; do
+                local kill_pid=pid${p}
+                kill -KILL ${!kill_pid}

                                ^
				Bashism.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 73

Quoted 5 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +4 -0

+		tst_resm TINFO "killing ${!kill_pid}"
+                wait ${!kill_pid}
+                p=$((p + 1))
+        done

This part seems to have misformatted whitespaces, please fix that.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 75

Quoted 1 line

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +0 -0

 	sleep 2

The wait is done here, OK. But you should have dropped the sleep 2.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 76

Quoted 1 line

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +0 -0

 	clean_up

Quoted 30 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +27 -0

 }
 
+test_stress()
+{
+	local index=0;
+
+	tst_resm TINFO "Test targets: ${test_targets}"
+
+	get_skip_targets
+	for target in ${test_targets}; do
+		if should_skip_target $target; then
+			continue
+		fi
+		sh ftrace_${target}.sh &
+		eval pid${index}=$!
+		tst_resm TINFO "Start pid${index}=$! $SPATH/ftrace_${target}.sh"
+		index=$((index + 1))
+	done
+	export_pids
+}
+
+export_pids()
+{
+	local p=0
+	while [ $p -lt $NR_PIDS ]; do
+		export pid${p}
+		p=$((p + 1))
+	done
+}
 

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 107

Quoted 1 line

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +1 -0

+cd ftrace_stress/

Why the cd? It does not seem to be needed.

Quoted 9 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +1 -1

 # ----------------------------
-echo "Ftrace Stress Test Begin"
+tst_resm TINFO "Ftrace Stress Test Begin"
 
 save_old_setting
 

@@ -113,11 +118,6 @@ test_wait
 

Quoted 5 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +1 -1

 test_kill
 
-echo "Ftrace Stress Test End"
+tst_resm TINFO "Finished running the test. Run dmesg to double-check for

bugs"

Well most of the things I've pointed out seems to be fixed but I still fail to see the restore_old_setting here. Or do I miss someting?

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh test_wait · line 123

Quoted 8 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +0 -2

-if $test_success; then
-	tst_resm TPASS "finished running the test. Run dmesg to double-check for

bugs"

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +1 -4

-else
-	tst_resm TFAIL "please check log message."
-	exit 1
-fi
+tst_exit

--
Cyril Hrubis
[email protected]

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 69

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +4 -0

+	local p=1;
+	while [ $p -lt $NR_PIDS ]; do
+                local kill_pid=pid${p}
+                kill -KILL ${!kill_pid}

                                ^
				Bashism.

I am trying to change to use this way

    local pid_var=pid${p}
                eval local kill_pid=\$${pid_var}
                tst_resm TINFO "killing ${kill_pid}"
                eval kill -KILL $kill_pid
                wait ${kill_pid}

maybe i'm still using bashism? thanks. if it's ok then i will use this way.

Ideally i should find a dash to try. but i have not found the package in rhel.

Quoted 9 lines

Quoted 9 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +8 -0

+export_pids()
+{
+	local p=0
+	while [ $p -lt $NR_PIDS ]; do
+		export pid${p}
+		p=$((p + 1))
+	done
+}

Quoted 3 lines

Quoted 1 line

+cd ftrace_stress/

Why the cd? It does not seem to be needed.

as currently we are in ftrace_test/, but sub stresses cases are in ftrace_test/ftrace_stress/

in test_stress(), i start the sub script using

  sh ftrace_${target}.sh &

This make the path shorter. If you don't have objection, I will move the cd ftrace_stress/ into test_stress(). What do you think of this?

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Tuesday, May 10, 2016 10:25:04 PM
Subject: Re: [LTP] [PATCH V2 4/9] ftrace_stress: skip unsupported tests

Hi!

Quoted 18 lines

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 69

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +4 -0

+	local p=1;
+	while [ $p -lt $NR_PIDS ]; do
+                local kill_pid=pid${p}
+                kill -KILL ${!kill_pid}

                                ^
				Bashism.

I am trying to change to use this way

    local pid_var=pid${p}
                eval local kill_pid=\$${pid_var}
                tst_resm TINFO "killing ${kill_pid}"
                eval kill -KILL $kill_pid
                wait ${kill_pid}

maybe i'm still using bashism? thanks. if it's ok then i will use this way.

Quoted 2 lines

Ideally i should find a dash to try. but i have not found the package in rhel.

You can always compile it from source:

http://gondor.apana.org.au/~herbert/dash/files/

And there also seems to be debian devel perl script that can check for bashism, maybe we can start using it to check shell scripts:

https://anonscm.debian.org/cgit/collab-maint/devscripts.git/tree/scripts/checkbashisms.pl

Quoted 20 lines

Quoted 9 lines

Quoted 9 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh +8 -0

+export_pids()
+{
+	local p=0
+	while [ $p -lt $NR_PIDS ]; do
+		export pid${p}
+		p=$((p + 1))
+	done
+}

Quoted 3 lines

Quoted 1 line

+cd ftrace_stress/

Why the cd? It does not seem to be needed.

as currently we are in ftrace_test/, but sub stresses cases are in ftrace_test/ftrace_stress/

in test_stress(), i start the sub script using

  sh ftrace_${target}.sh &

Quoted 3 lines

This make the path shorter. If you don't have objection, I will move the cd ftrace_stress/ into test_stress(). What do you think of this?

The stress tests are installed into $DESTDIR/testcases/bin/ aren't they?

If that is the case these scripts are in $PATH and could be executed without fiddling with full path.

--
Cyril Hrubis
[email protected]

testcases/kernel/tracing/ftrace_test/ftrace_regression01.sh new file +27 -0

+	fi
+
+	for i in $(seq $LOOP); do
+		echo 1 >  $TSTACK_TRACE_PATH
+		echo userstacktrace > $TRACING_PATH/trace_options
+		grep -q "^userstacktrace"  $TRACING_PATH/trace_options
+		if [ $? -ne 0 ]; then
+			tst_brkm TBROK "Failed to set userstacktrace"
+		fi
+
+		if [ -f "$EXC_PAGE_FAULT_ENABLE" ]; then
+			exc_page_fault_enable=`cat $EXC_PAGE_FAULT_ENABLE`
+			echo 1 > $EXC_PAGE_FAULT_ENABLE
+		else
+			mm_page_fault_enable=`cat MM_PAGE_FAULT_ENABLE`
+			echo 1 > $MM_PAGE_FAULT_ENABLE
+		fi
+	done
+
+	if [ -f "$EXC_PAGE_FAULT_ENABLE" ]; then
+		echo "$exc_page_fault_enable" > $EXC_PAGE_FAULT_ENABLE
+	else
+		echo "$mm_page_fault_enable" > $MM_PAGE_FAULT_ENABLE
+	fi
+
+	tst_resm TPASS "Finished running the test"
+}

Hi!

testcases/kernel/tracing/ftrace_test/ftrace_regression01.sh line 52

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_regression01.sh new file +4 -0

+ftrace_userstacktrace_test()
+{
+	if [ ! -e "$TSTACK_TRACE_PATH" ]; then
+		tst_brkm TCONF ignored "Stack Tracer is not cofigured in This kernel"

The 'ignored' should really be removed here, it's a leftover when the tst_brkm called the broken-by-design binary.

Quoted 28 lines

testcases/kernel/tracing/ftrace_test/ftrace_regression01.sh new file +27 -0

+	fi
+
+	for i in $(seq $LOOP); do
+		echo 1 >  $TSTACK_TRACE_PATH
+		echo userstacktrace > $TRACING_PATH/trace_options
+		grep -q "^userstacktrace"  $TRACING_PATH/trace_options
+		if [ $? -ne 0 ]; then
+			tst_brkm TBROK "Failed to set userstacktrace"
+		fi
+
+		if [ -f "$EXC_PAGE_FAULT_ENABLE" ]; then
+			exc_page_fault_enable=`cat $EXC_PAGE_FAULT_ENABLE`
+			echo 1 > $EXC_PAGE_FAULT_ENABLE
+		else
+			mm_page_fault_enable=`cat MM_PAGE_FAULT_ENABLE`
+			echo 1 > $MM_PAGE_FAULT_ENABLE
+		fi
+	done
+
+	if [ -f "$EXC_PAGE_FAULT_ENABLE" ]; then
+		echo "$exc_page_fault_enable" > $EXC_PAGE_FAULT_ENABLE
+	else
+		echo "$mm_page_fault_enable" > $MM_PAGE_FAULT_ENABLE
+	fi
+
+	tst_resm TPASS "Finished running the test"
+}

Signature · 2 lines

Cyril Hrubis
[email protected]

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +16 -0

+###########################################################################
+##                                                                       ##
+## Copyright (c) 2010 FUJITSU LIMITED                                    ##
+##                                                                       ##
+## This program is free software: you can redistribute it and/or modify  ##
+## it under the terms of the GNU General Public License as published by  ##
+## the Free Software Foundation, either version 3 of the License, or     ##
+## (at your option) any later version.                                   ##
+##                                                                       ##
+## This program is distributed in the hope that it will be useful,       ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of        ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the          ##
+## GNU General Public License for more details.                          ##
+##                                                                       ##
+## You should have received a copy of the GNU General Public License     ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.  ##

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +96 -0

+export FPATH="$TPATH/ftrace_function"
+export RPATH="$TPATH/ftrace_regression"
+export SPATH="$TPATH/ftrace_stress"
+
+. test.sh
+
+test_interval=$1
+
+save_old_setting()
+{
+	cd $TRACING_PATH
+
+	old_trace_options=( `cat trace_options` )
+	old_tracing_on=`cat tracing_on`
+	old_tracing_enabled=`cat tracing_enabled`
+	old_buffer_size=`cat buffer_size_kb`
+
+	if [ -e stack_max_size ]; then
+		old_stack_tracer_enabled=`cat /proc/sys/kernel/stack_tracer_enabled`
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		old_ftrace_enabled=`cat /proc/sys/kernel/ftrace_enabled`
+	fi
+
+	if [ -e "function_profile_enabled" ]; then
+		old_profile_enabled=`cat function_profile_enabled`
+	fi
+
+	cd - > /dev/null
+}
+
+
+restore_old_setting()
+{
+	cd $TRACING_PATH
+
+	echo nop > current_tracer
+	echo 0 > events/enable
+	echo 0 > tracing_max_latency 2> /dev/null
+
+	if [ -e trace_clock ]; then
+		echo local > trace_clock
+	fi
+
+	if [ -e "function_pofile_enabled" ]; then
+		echo $old_profile_enabled > function_profile_enabled
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		echo $old_ftrace_enabled > /proc/sys/kernel/ftrace_enabled
+	fi
+
+	if [ -e stack_max_size ]; then
+		echo $old_stack_tracer_enabled > /proc/sys/kernel/stack_tracer_enabled
+		echo 0 > stack_max_size
+	fi
+
+	echo $old_buffer_size > buffer_size_kb
+	echo $old_tracing_on > tracing_on
+	echo $old_tracing_enabled > tracing_enabled
+
+	for option in $old_trace_options
+	do
+		echo $option > trace_options 2> /dev/null
+	done
+
+	echo > trace
+
+	cd - > /dev/null
+}
+
+clean_up()
+{
+	restore_old_setting
+
+	umount $DEBUGFS_PATH
+	rmdir $DEBUGFS_PATH
+}
+
+clean_up_exit()
+{
+	clean_up
+	exit 1
+}
+
+test_begin()
+{
+	start_time=`date +%s`
+}
+
+test_wait()
+{
+	for ((; ;))
+	{
+		sleep 2

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +88 -0

new file mode 100755
index 0000000..d9f7f8b
@@ -0,0 +1,123 @@
+#! /bin/sh
+
+###########################################################################
+##                                                                       ##
+## Copyright (c) 2010 FUJITSU LIMITED                                    ##
+##                                                                       ##
+## This program is free software: you can redistribute it and/or modify  ##
+## it under the terms of the GNU General Public License as published by  ##
+## the Free Software Foundation, either version 3 of the License, or     ##
+## (at your option) any later version.                                   ##
+##                                                                       ##
+## This program is distributed in the hope that it will be useful,       ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of        ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the          ##
+## GNU General Public License for more details.                          ##
+##                                                                       ##
+## You should have received a copy of the GNU General Public License     ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.  ##
+##                                                                       ##
+## Author: Li Zefan <[email protected]>                                ##
+##                                                                       ##
+###########################################################################
+
+
+export TCID="ftrace-stress-test"
+export TST_TOTAL=1
+export TST_COUNT=1
+
+. ftrace_lib.sh
+
+test_success=true
+
+export_pids()
+{
+	export pid1 pid2 pid3 pid4 pid5 pid6 pid7 pid8 pid9 pid10 pid11 pid12 \
+		pid13 pid14 pid15 pid16
+
+	export NR_PIDS=16
+}
+
+test_stress()
+{
+	export_pids
+
+	$SPATH/ftrace_trace_clock.sh &
+	pid1=$!
+	$SPATH/ftrace_current_tracer.sh &
+	pid2=$!
+	$SPATH/ftrace_trace_options.sh &
+	pid3=$!
+	$SPATH/ftrace_tracing_max_latency.sh &
+	pid4=$!
+	$SPATH/ftrace_stack_trace.sh &
+	pid5=$!
+	$SPATH/ftrace_stack_max_size.sh &
+	pid6=$!
+	$SPATH/ftrace_tracing_on.sh &
+	pid7=$!
+	$SPATH/ftrace_tracing_enabled.sh &
+	pid8=$!
+	$SPATH/ftrace_set_event.sh &
+	pid9=$!
+	$SPATH/ftrace_buffer_size.sh &
+	pid10=$!
+	$SPATH/ftrace_trace.sh &
+	pid11=$!
+	$SPATH/ftrace_trace_pipe.sh &
+	pid12=$!
+	$SPATH/ftrace_ftrace_enabled.sh &
+	pid13=$!
+	$SPATH/ftrace_set_ftrace_pid.sh &
+	pid14=$!
+	$SPATH/ftrace_profile_enabled.sh &
+	pid15=$!
+	$SPATH/ftrace_trace_stat.sh &
+	pid16=$!
+}
+
+test_kill()
+{
+	kill -KILL $pid1 || test_success=false
+	kill -KILL $pid2 || test_success=false
+	kill -KILL $pid3 || test_success=false
+	kill -KILL $pid4 || test_success=false
+	kill -KILL $pid5 || test_success=false
+	kill -KILL $pid6 || test_success=false
+	kill -KILL $pid7 || test_success=false
+	kill -KILL $pid8 || test_success=false

Hi!

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 1

Quoted 7 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +1 -0

new file mode 100755
index 0000000..ea082dc
@@ -0,0 +1,156 @@
+#! /bin/sh

^
The space should be removed

Quoted 17 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +16 -0

+###########################################################################
+##                                                                       ##
+## Copyright (c) 2010 FUJITSU LIMITED                                    ##
+##                                                                       ##
+## This program is free software: you can redistribute it and/or modify  ##
+## it under the terms of the GNU General Public License as published by  ##
+## the Free Software Foundation, either version 3 of the License, or     ##
+## (at your option) any later version.                                   ##
+##                                                                       ##
+## This program is distributed in the hope that it will be useful,       ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of        ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the          ##
+## GNU General Public License for more details.                          ##
+##                                                                       ##
+## You should have received a copy of the GNU General Public License     ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.  ##

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 28

Quoted 10 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +10 -0

+##                                                                       ##
+## Author: Li Zefan <[email protected]>                                ##
+##                                                                       ##
+###########################################################################
+
+cd $LTPROOT/testcases/bin
+
+export TPATH="$PWD"
+export DEBUGFS_PATH="$PWD/debugfs"
+export TRACING_PATH="$PWD/debugfs/tracing"

The test must not create files and mount things outside of the test temporary directory. Expecially not in $LTPROOT. You should call tst_tmpdir to create temporary directory instead, then create directory there.

Also the ftrace filesystem seems to be mounted on /sys/kernel/debug/tracing/ on recent distributions, why don't we just set TRACING_PATH accodingly if it's already mounted and fallback to mounting it only when debugfs couldn't be found in /proc/mounts?

Quoted 96 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +96 -0

+export FPATH="$TPATH/ftrace_function"
+export RPATH="$TPATH/ftrace_regression"
+export SPATH="$TPATH/ftrace_stress"
+
+. test.sh
+
+test_interval=$1
+
+save_old_setting()
+{
+	cd $TRACING_PATH
+
+	old_trace_options=( `cat trace_options` )
+	old_tracing_on=`cat tracing_on`
+	old_tracing_enabled=`cat tracing_enabled`
+	old_buffer_size=`cat buffer_size_kb`
+
+	if [ -e stack_max_size ]; then
+		old_stack_tracer_enabled=`cat /proc/sys/kernel/stack_tracer_enabled`
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		old_ftrace_enabled=`cat /proc/sys/kernel/ftrace_enabled`
+	fi
+
+	if [ -e "function_profile_enabled" ]; then
+		old_profile_enabled=`cat function_profile_enabled`
+	fi
+
+	cd - > /dev/null
+}
+
+
+restore_old_setting()
+{
+	cd $TRACING_PATH
+
+	echo nop > current_tracer
+	echo 0 > events/enable
+	echo 0 > tracing_max_latency 2> /dev/null
+
+	if [ -e trace_clock ]; then
+		echo local > trace_clock
+	fi
+
+	if [ -e "function_pofile_enabled" ]; then
+		echo $old_profile_enabled > function_profile_enabled
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		echo $old_ftrace_enabled > /proc/sys/kernel/ftrace_enabled
+	fi
+
+	if [ -e stack_max_size ]; then
+		echo $old_stack_tracer_enabled > /proc/sys/kernel/stack_tracer_enabled
+		echo 0 > stack_max_size
+	fi
+
+	echo $old_buffer_size > buffer_size_kb
+	echo $old_tracing_on > tracing_on
+	echo $old_tracing_enabled > tracing_enabled
+
+	for option in $old_trace_options
+	do
+		echo $option > trace_options 2> /dev/null
+	done
+
+	echo > trace
+
+	cd - > /dev/null
+}
+
+clean_up()
+{
+	restore_old_setting
+
+	umount $DEBUGFS_PATH
+	rmdir $DEBUGFS_PATH
+}
+
+clean_up_exit()
+{
+	clean_up
+	exit 1
+}
+
+test_begin()
+{
+	start_time=`date +%s`
+}
+
+test_wait()
+{
+	for ((; ;))
+	{
+		sleep 2

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 133

Quoted 10 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +10 -0

+
+		cur_time=`date +%s`
+		elapsed=$(( $cur_time - $start_time ))
+
+		# run the test for $test_interval secs
+		if [ $elapsed -ge $test_interval ]; then
+			break
+		fi
+	}
+}

Why don't we just do sleep $test_interval instead?

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 145

Quoted 11 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +11 -0

+trap clean_up_exit INT
+
+tst_require_root
+
+# Don't run the test on kernels older than 2.6.34, otherwise
+# it can crash the system if the kernel is not latest-stable
+tst_kvercmp 2 6 34
+if [ $? -eq 0 ]; then
+	tst_brkm TCONF ignored "The test should be run in kernels >= 2.6.34. Skip the test..."
+	exit 0
+fi

Don't do this. Really the whole point of LTP is to validate kernel, if we skip known bugs by default we would give users false impression that the kernel was thoroughly tested.

Also since you include the test.sh here the tst_brkm format is different and it exits the test, so the exit 0 is never reached.

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 156

Quoted 10 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +10 -0

+mkdir $DEBUGFS_PATH
+mount -t debugfs xxx $DEBUGFS_PATH
+
+# Check to see tracing feature is supported or not
+if [ ! -d $TRACING_PATH ]; then
+	tst_brkm TCONF ignored "Tracing is not supported. Skip the test..."
+	umount $DEBUGFS_PATH
+	rmdir $DEBUGFS_PATH
+	exit 0
+fi

Here as well, the tst_brkm from test.sh exit the test. Ideally you should setup TST_CLEANUP to point to a cleanup function once you do the test initalization which would be then executed before the test exits automatically.

...

Quoted 94 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +88 -0

new file mode 100755
index 0000000..d9f7f8b
@@ -0,0 +1,123 @@
+#! /bin/sh
+
+###########################################################################
+##                                                                       ##
+## Copyright (c) 2010 FUJITSU LIMITED                                    ##
+##                                                                       ##
+## This program is free software: you can redistribute it and/or modify  ##
+## it under the terms of the GNU General Public License as published by  ##
+## the Free Software Foundation, either version 3 of the License, or     ##
+## (at your option) any later version.                                   ##
+##                                                                       ##
+## This program is distributed in the hope that it will be useful,       ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of        ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the          ##
+## GNU General Public License for more details.                          ##
+##                                                                       ##
+## You should have received a copy of the GNU General Public License     ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.  ##
+##                                                                       ##
+## Author: Li Zefan <[email protected]>                                ##
+##                                                                       ##
+###########################################################################
+
+
+export TCID="ftrace-stress-test"
+export TST_TOTAL=1
+export TST_COUNT=1
+
+. ftrace_lib.sh
+
+test_success=true
+
+export_pids()
+{
+	export pid1 pid2 pid3 pid4 pid5 pid6 pid7 pid8 pid9 pid10 pid11 pid12 \
+		pid13 pid14 pid15 pid16
+
+	export NR_PIDS=16
+}
+
+test_stress()
+{
+	export_pids
+
+	$SPATH/ftrace_trace_clock.sh &
+	pid1=$!
+	$SPATH/ftrace_current_tracer.sh &
+	pid2=$!
+	$SPATH/ftrace_trace_options.sh &
+	pid3=$!
+	$SPATH/ftrace_tracing_max_latency.sh &
+	pid4=$!
+	$SPATH/ftrace_stack_trace.sh &
+	pid5=$!
+	$SPATH/ftrace_stack_max_size.sh &
+	pid6=$!
+	$SPATH/ftrace_tracing_on.sh &
+	pid7=$!
+	$SPATH/ftrace_tracing_enabled.sh &
+	pid8=$!
+	$SPATH/ftrace_set_event.sh &
+	pid9=$!
+	$SPATH/ftrace_buffer_size.sh &
+	pid10=$!
+	$SPATH/ftrace_trace.sh &
+	pid11=$!
+	$SPATH/ftrace_trace_pipe.sh &
+	pid12=$!
+	$SPATH/ftrace_ftrace_enabled.sh &
+	pid13=$!
+	$SPATH/ftrace_set_ftrace_pid.sh &
+	pid14=$!
+	$SPATH/ftrace_profile_enabled.sh &
+	pid15=$!
+	$SPATH/ftrace_trace_stat.sh &
+	pid16=$!
+}
+
+test_kill()
+{
+	kill -KILL $pid1 || test_success=false
+	kill -KILL $pid2 || test_success=false
+	kill -KILL $pid3 || test_success=false
+	kill -KILL $pid4 || test_success=false
+	kill -KILL $pid5 || test_success=false
+	kill -KILL $pid6 || test_success=false
+	kill -KILL $pid7 || test_success=false
+	kill -KILL $pid8 || test_success=false

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 98

Quoted 10 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +10 -0

+	kill -KILL $pid9 || test_success=false
+	kill -KILL $pid10 || test_success=false
+	kill -KILL $pid11 || test_success=false
+	kill -USR1 $pid12 || test_success=false
+	kill -KILL $pid13 || test_success=false
+	kill -KILL $pid14 || test_success=false
+	kill -KILL $pid15 || test_success=false
+	kill -KILL $pid16 || test_success=false
+
+	sleep 2

I remember telling you to call wait on each pid instead of the sleep 2 here.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 104

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +6 -0

+	clean_up
+}
+
+
+# ----------------------------
+echo "Ftrace Stress Test Begin"

That should be tst_resm TINFO "..." but that is very minor.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 114

Quoted 9 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +9 -0

+save_old_setting
+
+test_begin
+
+test_stress
+
+test_wait
+
+test_kill

Hmm, shouldn't you restore old settings here?

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 116

Quoted 1 line

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +1 -0

+echo "Ftrace Stress Test End"

Here as well.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 123

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +6 -0

+if $test_success; then
+	tst_resm TPASS "finished running the test. Run dmesg to double-check for bugs"
+else
+	tst_resm TFAIL "please check log message."
+	exit 1
+fi

You should call tst_exit at the end of the test instead of doing exit 1 after the tst_resm. This is another leftover caused by the broken-by-desing tst_resm and tst_brkm binaries.

Signature · 2 lines

Cyril Hrubis
[email protected]

From: "Cyril Hrubis" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected], [email protected]
Sent: Thursday, May 5, 2016 12:26:56 AM
Subject: Re: [LTP] [PATCH V2 1/9] tracing: reorganize ftrace-stress tests to general tests

Hi!

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 1

Quoted 8 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +1 -0

new file mode 100755
index 0000000..ea082dc
@@ -0,0 +1,156 @@
+#! /bin/sh

^
The space should be removed

Quoted 41 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +2 -0

+###########################################################################
+##

##
+## Copyright (c) 2010 FUJITSU LIMITED
##
+##
##
+## This program is free software: you can redistribute it and/or modify ##
+## it under the terms of the GNU General Public License as published by ##
+## the Free Software Foundation, either version 3 of the License, or ##
+## (at your option) any later version.
##
+##
##
+## This program is distributed in the hope that it will be useful, ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ##
+## GNU General Public License for more details.
##
+##
##
+## You should have received a copy of the GNU General Public License ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.
##
+##
##
+## Author: Li Zefan <[email protected]>
##
+##
##

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +4 -0

+###########################################################################
+
+cd $LTPROOT/testcases/bin
+

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 28

Quoted 3 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +3 -0

+export TPATH="$PWD"
+export DEBUGFS_PATH="$PWD/debugfs"
+export TRACING_PATH="$PWD/debugfs/tracing"

The test must not create files and mount things outside of the test temporary directory. Expecially not in $LTPROOT. You should call tst_tmpdir to create temporary directory instead, then create directory there.

Also the ftrace filesystem seems to be mounted on /sys/kernel/debug/tracing/ on recent distributions, why don't we just set TRACING_PATH accodingly if it's already mounted and fallback to mounting it only when debugfs couldn't be found in /proc/mounts?

Quoted 100 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +100 -0

+export FPATH="$TPATH/ftrace_function"
+export RPATH="$TPATH/ftrace_regression"
+export SPATH="$TPATH/ftrace_stress"
+
+. test.sh
+
+test_interval=$1
+
+save_old_setting()
+{
+	cd $TRACING_PATH
+
+	old_trace_options=( `cat trace_options` )
+	old_tracing_on=`cat tracing_on`
+	old_tracing_enabled=`cat tracing_enabled`
+	old_buffer_size=`cat buffer_size_kb`
+
+	if [ -e stack_max_size ]; then
+		old_stack_tracer_enabled=`cat /proc/sys/kernel/stack_tracer_enabled`
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		old_ftrace_enabled=`cat /proc/sys/kernel/ftrace_enabled`
+	fi
+
+	if [ -e "function_profile_enabled" ]; then
+		old_profile_enabled=`cat function_profile_enabled`
+	fi
+
+	cd - > /dev/null
+}
+
+
+restore_old_setting()
+{
+	cd $TRACING_PATH
+
+	echo nop > current_tracer
+	echo 0 > events/enable
+	echo 0 > tracing_max_latency 2> /dev/null
+
+	if [ -e trace_clock ]; then
+		echo local > trace_clock
+	fi
+
+	if [ -e "function_pofile_enabled" ]; then
+		echo $old_profile_enabled > function_profile_enabled
+	fi
+
+	if [ -e "/proc/sys/kernel/ftrace_enabled" ]; then
+		echo $old_ftrace_enabled > /proc/sys/kernel/ftrace_enabled
+	fi
+
+	if [ -e stack_max_size ]; then
+		echo $old_stack_tracer_enabled > /proc/sys/kernel/stack_tracer_enabled
+		echo 0 > stack_max_size
+	fi
+
+	echo $old_buffer_size > buffer_size_kb
+	echo $old_tracing_on > tracing_on
+	echo $old_tracing_enabled > tracing_enabled
+
+	for option in $old_trace_options
+	do
+		echo $option > trace_options 2> /dev/null
+	done
+
+	echo > trace
+
+	cd - > /dev/null
+}
+
+clean_up()
+{
+	restore_old_setting
+
+	umount $DEBUGFS_PATH
+	rmdir $DEBUGFS_PATH
+}
+
+clean_up_exit()
+{
+	clean_up
+	exit 1
+}
+
+test_begin()
+{
+	start_time=`date +%s`
+}
+
+test_wait()
+{
+	for ((; ;))
+	{
+		sleep 2
+
+		cur_time=`date +%s`
+		elapsed=$(( $cur_time - $start_time ))
+

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 133

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +6 -0

+		# run the test for $test_interval secs
+		if [ $elapsed -ge $test_interval ]; then
+			break
+		fi
+	}
+}

Why don't we just do sleep $test_interval instead?

Quoted 8 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +8 -0

+trap clean_up_exit INT
+
+tst_require_root
+
+# Don't run the test on kernels older than 2.6.34, otherwise
+# it can crash the system if the kernel is not latest-stable
+tst_kvercmp 2 6 34
+if [ $? -eq 0 ]; then

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh

Quoted 4 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +1 -0

+	tst_brkm TCONF ignored "The test should be run in kernels >= 2.6.34. Skip

the test..."

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +2 -0

+	exit 0
+fi

Don't do this. Really the whole point of LTP is to validate kernel, if we skip known bugs by default we would give users false impression that the kernel was thoroughly tested.

Also since you include the test.sh here the tst_brkm format is different and it exits the test, so the exit 0 is never reached.

Quoted 7 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +7 -0

+mkdir $DEBUGFS_PATH
+mount -t debugfs xxx $DEBUGFS_PATH
+
+# Check to see tracing feature is supported or not
+if [ ! -d $TRACING_PATH ]; then
+	tst_brkm TCONF ignored "Tracing is not supported. Skip the test..."
+	umount $DEBUGFS_PATH

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh line 145

Quoted 3 lines

testcases/kernel/tracing/ftrace_test/ftrace_lib.sh new file +3 -0

+	rmdir $DEBUGFS_PATH
+	exit 0
+fi

Here as well, the tst_brkm from test.sh exit the test. Ideally you should setup TST_CLEANUP to point to a cleanup function once you do the test initalization which would be then executed before the test exits automatically.

...

Quoted 118 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +4 -0

new file mode 100755
index 0000000..d9f7f8b
@@ -0,0 +1,123 @@
+#! /bin/sh
+
+###########################################################################
+##

##
+## Copyright (c) 2010 FUJITSU LIMITED
##
+##
##
+## This program is free software: you can redistribute it and/or modify ##
+## it under the terms of the GNU General Public License as published by ##
+## the Free Software Foundation, either version 3 of the License, or ##
+## (at your option) any later version.
##
+##
##
+## This program is distributed in the hope that it will be useful, ##
+## but WITHOUT ANY WARRANTY; without even the implied warranty of ##
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ##
+## GNU General Public License for more details.
##
+##
##
+## You should have received a copy of the GNU General Public License ##
+## along with this program. If not, see <http://www.gnu.org/licenses/>.
##
+##
##
+## Author: Li Zefan <[email protected]>
##
+##
##

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +72 -0

+###########################################################################
+
+
+export TCID="ftrace-stress-test"
+export TST_TOTAL=1
+export TST_COUNT=1
+
+. ftrace_lib.sh
+
+test_success=true
+
+export_pids()
+{
+	export pid1 pid2 pid3 pid4 pid5 pid6 pid7 pid8 pid9 pid10 pid11 pid12 \
+		pid13 pid14 pid15 pid16
+
+	export NR_PIDS=16
+}
+
+test_stress()
+{
+	export_pids
+
+	$SPATH/ftrace_trace_clock.sh &
+	pid1=$!
+	$SPATH/ftrace_current_tracer.sh &
+	pid2=$!
+	$SPATH/ftrace_trace_options.sh &
+	pid3=$!
+	$SPATH/ftrace_tracing_max_latency.sh &
+	pid4=$!
+	$SPATH/ftrace_stack_trace.sh &
+	pid5=$!
+	$SPATH/ftrace_stack_max_size.sh &
+	pid6=$!
+	$SPATH/ftrace_tracing_on.sh &
+	pid7=$!
+	$SPATH/ftrace_tracing_enabled.sh &
+	pid8=$!
+	$SPATH/ftrace_set_event.sh &
+	pid9=$!
+	$SPATH/ftrace_buffer_size.sh &
+	pid10=$!
+	$SPATH/ftrace_trace.sh &
+	pid11=$!
+	$SPATH/ftrace_trace_pipe.sh &
+	pid12=$!
+	$SPATH/ftrace_ftrace_enabled.sh &
+	pid13=$!
+	$SPATH/ftrace_set_ftrace_pid.sh &
+	pid14=$!
+	$SPATH/ftrace_profile_enabled.sh &
+	pid15=$!
+	$SPATH/ftrace_trace_stat.sh &
+	pid16=$!
+}
+
+test_kill()
+{
+	kill -KILL $pid1 || test_success=false
+	kill -KILL $pid2 || test_success=false
+	kill -KILL $pid3 || test_success=false
+	kill -KILL $pid4 || test_success=false
+	kill -KILL $pid5 || test_success=false
+	kill -KILL $pid6 || test_success=false
+	kill -KILL $pid7 || test_success=false
+	kill -KILL $pid8 || test_success=false
+	kill -KILL $pid9 || test_success=false
+	kill -KILL $pid10 || test_success=false
+	kill -KILL $pid11 || test_success=false
+	kill -USR1 $pid12 || test_success=false
+	kill -KILL $pid13 || test_success=false

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 98

Quoted 5 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +5 -0

+	kill -KILL $pid14 || test_success=false
+	kill -KILL $pid15 || test_success=false
+	kill -KILL $pid16 || test_success=false
+
+	sleep 2

I remember telling you to call wait on each pid instead of the sleep 2 here.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 104

Quoted 6 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +6 -0

+	clean_up
+}
+
+
+# ----------------------------
+echo "Ftrace Stress Test Begin"

That should be tst_resm TINFO "..." but that is very minor.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 114

Quoted 9 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +9 -0

+save_old_setting
+
+test_begin
+
+test_stress
+
+test_wait
+
+test_kill

Hmm, shouldn't you restore old settings here?

Quoted 1 line

+echo "Ftrace Stress Test End"

Here as well.

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh line 123

Quoted 7 lines

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +2 -0

+if $test_success; then
+	tst_resm TPASS "finished running the test. Run dmesg to double-check for

bugs"

testcases/kernel/tracing/ftrace_test/ftrace_stress_test.sh new file +4 -0

+else
+	tst_resm TFAIL "please check log message."
+	exit 1
+fi

You should call tst_exit at the end of the test instead of doing exit 1 after the tst_resm. This is another leftover caused by the broken-by-desing tst_resm and tst_brkm binaries.

--
Cyril Hrubis
[email protected]

From: "Chunyu Hu" <[email protected]>
To: "Cyril Hrubis" <[email protected]>
Cc: [email protected]
Sent: Thursday, May 5, 2016 6:11:04 PM
Subject: Re: [LTP] [PATCH V2 1/9] tracing: reorganize ftrace-stress tests to general tests

Thanks. Some issue has been done in patch 2/9, will fix other issues you mentioned in next step.