linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] kselftest framework and test changes to use it
@ 2014-09-23 21:32 Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting Shuah Khan
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

This patch v2 series: (revised to address v1 comments)

Add kselftest framework for tests to use. This is a light
weight framework provides a set of interfaces to report test
results and test statistics on number of tests passed and failed.

Several tests are changed to use the framework to report results.

Shuah Khan (7):
  selftests: add kselftest framework for uniform test reporting
  selftests/breakpoints: change test to use ksft framework
  selftests/ipc: change test to use ksft framework
  selftests/kcmp: change test to use ksft framework
  selftests/mount: change test to use ksft framework
  selftests/ptrace: change test to use ksft framework
  selftests/timers: change test to use ksft framework

 .../selftests/breakpoints/breakpoint_test.c        | 10 +++---
 tools/testing/selftests/ipc/msgque.c               | 26 +++++++-------
 tools/testing/selftests/kcmp/kcmp_test.c           | 27 ++++++++++----
 tools/testing/selftests/kselftest.h                | 41 ++++++++++++++++++++++
 .../selftests/mount/unprivileged-remount-test.c    |  8 +++--
 tools/testing/selftests/ptrace/peeksiginfo.c       | 14 ++++----
 tools/testing/selftests/timers/posix_timers.c      | 14 ++++----
 7 files changed, 101 insertions(+), 39 deletions(-)
 create mode 100644 tools/testing/selftests/kselftest.h

-- 
1.9.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-24  8:09   ` Davidlohr Bueso
  2014-09-23 21:32 ` [PATCH v2 2/7] selftests/breakpoints: change test to use ksft framework Shuah Khan
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Add kselftest framework for tests to use. This is a light
weight framework provides a set of interfaces to report test
results. Tests can use these interfaces to report pass, and
fail cases as well as when failure is due to configuration
problems such as missing modules, or when a test that is should
fail, fails as expected, and a test that should fail, passes.
The framework uses POSIX standard return codes for reporting
results to address the needs of users that want to run the kernel
selftests from their user-space test suites and want to know why a
test failed. In addition, the framework includes interfaces to use
to report test statistics on number of tests passed and failed.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/kselftest.h | 41 +++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 tools/testing/selftests/kselftest.h

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
new file mode 100644
index 0000000..c73fc2c
--- /dev/null
+++ b/tools/testing/selftests/kselftest.h
@@ -0,0 +1,41 @@
+/*
+ * kselftest.h: kselftest framework return codes to include from
+ *		 selftests.
+ *
+ * Copyright (c) 2014 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef __KSELFTEST_H
+#define __KSELFTEST_H
+
+#include <stdlib.h>
+#include <unistd.h>
+
+/* counters */
+static int ksft_pass;
+static int ksft_fail;
+static int ksft_xfail;
+static int ksft_xpass;
+static int ksft_xskip;
+
+static inline void ksft_inc_pass_cnt(void) { ksft_pass++; }
+static inline void ksft_inc_fail_cnt(void) { ksft_fail++; }
+static inline void ksft_inc_xfail_cnt(void) { ksft_xfail++; }
+static inline void ksft_inc_xpass_cnt(void) { ksft_xpass++; }
+static inline void ksft_inc_xskip_cnt(void) { ksft_xskip++; }
+
+static inline void ksft_print_cnts(void)
+{
+	printf("Pass: %d Fail: %d Xfail: %d Xpass: %d, Xskip: %d\n",
+		ksft_pass, ksft_fail, ksft_xfail, ksft_xpass, ksft_xskip);
+}
+
+static inline int ksft_exit_pass(void) { exit(0); }
+static inline int ksft_exit_fail(void) { exit(1); }
+static inline int ksft_exit_xfail(void) { exit(2); }
+static inline int ksft_exit_xpass(void) { exit(3); }
+static inline int ksft_exit_skip(void) { exit(4); }
+
+#endif /* __KSELFTEST_H */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/7] selftests/breakpoints: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 3/7] selftests/ipc: " Shuah Khan
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change breakpoints test to use kselftest framework to report
test results.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/breakpoints/breakpoint_test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/breakpoint_test.c b/tools/testing/selftests/breakpoints/breakpoint_test.c
index a0743f3..120895a 100644
--- a/tools/testing/selftests/breakpoints/breakpoint_test.c
+++ b/tools/testing/selftests/breakpoints/breakpoint_test.c
@@ -17,6 +17,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 
+#include "../kselftest.h"
+
 
 /* Breakpoint access modes */
 enum {
@@ -42,7 +44,7 @@ static void set_breakpoint_addr(void *addr, int n)
 		     offsetof(struct user, u_debugreg[n]), addr);
 	if (ret) {
 		perror("Can't set breakpoint addr\n");
-		exit(-1);
+		ksft_exit_fail();
 	}
 }
 
@@ -105,7 +107,7 @@ static void toggle_breakpoint(int n, int type, int len,
 		     offsetof(struct user, u_debugreg[7]), dr7);
 	if (ret) {
 		perror("Can't set dr7");
-		exit(-1);
+		ksft_exit_fail();
 	}
 }
 
@@ -275,7 +277,7 @@ static void check_success(const char *msg)
 			msg2 = "Ok";
 		if (ptrace(PTRACE_POKEDATA, child_pid, &trapped, 1)) {
 			perror("Can't poke\n");
-			exit(-1);
+			ksft_exit_fail();
 		}
 	}
 
@@ -390,5 +392,5 @@ int main(int argc, char **argv)
 
 	wait(NULL);
 
-	return 0;
+	return ksft_exit_pass();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/7] selftests/ipc: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 2/7] selftests/breakpoints: change test to use ksft framework Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 4/7] selftests/kcmp: " Shuah Khan
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change ipc test to use kselftest framework to report
test results. With this change this test exits with
EXIT_FAIL instead of -errno. Changed print errno in
test fail messages to not loose that information.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/ipc/msgque.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/ipc/msgque.c b/tools/testing/selftests/ipc/msgque.c
index 552f081..1b2ce33 100644
--- a/tools/testing/selftests/ipc/msgque.c
+++ b/tools/testing/selftests/ipc/msgque.c
@@ -5,6 +5,8 @@
 #include <linux/msg.h>
 #include <fcntl.h>
 
+#include "../kselftest.h"
+
 #define MAX_MSG_SIZE		32
 
 struct msg1 {
@@ -195,58 +197,58 @@ int main(int argc, char **argv)
 
 	if (getuid() != 0) {
 		printf("Please run the test as root - Exiting.\n");
-		exit(1);
+		return ksft_exit_fail();
 	}
 
 	msgque.key = ftok(argv[0], 822155650);
 	if (msgque.key == -1) {
-		printf("Can't make key\n");
-		return -errno;
+		printf("Can't make key: %d\n", -errno);
+		return ksft_exit_fail();
 	}
 
 	msgque.msq_id = msgget(msgque.key, IPC_CREAT | IPC_EXCL | 0666);
 	if (msgque.msq_id == -1) {
 		err = -errno;
-		printf("Can't create queue\n");
+		printf("Can't create queue: %d\n", err);
 		goto err_out;
 	}
 
 	err = fill_msgque(&msgque);
 	if (err) {
-		printf("Failed to fill queue\n");
+		printf("Failed to fill queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = dump_queue(&msgque);
 	if (err) {
-		printf("Failed to dump queue\n");
+		printf("Failed to dump queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = check_and_destroy_queue(&msgque);
 	if (err) {
-		printf("Failed to check and destroy queue\n");
+		printf("Failed to check and destroy queue: %d\n", err);
 		goto err_out;
 	}
 
 	err = restore_queue(&msgque);
 	if (err) {
-		printf("Failed to restore queue\n");
+		printf("Failed to restore queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = check_and_destroy_queue(&msgque);
 	if (err) {
-		printf("Failed to test queue\n");
+		printf("Failed to test queue: %d\n", err);
 		goto err_out;
 	}
-	return 0;
+	return ksft_exit_pass();
 
 err_destroy:
 	if (msgctl(msgque.msq_id, IPC_RMID, 0)) {
 		printf("Failed to destroy queue: %d\n", -errno);
-		return -errno;
+		return ksft_exit_fail();
 	}
 err_out:
-	return err;
+	return ksft_exit_fail();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/7] selftests/kcmp: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
                   ` (2 preceding siblings ...)
  2014-09-23 21:32 ` [PATCH v2 3/7] selftests/ipc: " Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-23 21:32 ` [PATCH v2 5/7] selftests/mount: " Shuah Khan
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change kcmp test to use kselftest framework to report
test results and test statistics.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/kcmp/kcmp_test.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kcmp/kcmp_test.c b/tools/testing/selftests/kcmp/kcmp_test.c
index dbba408..a5a4da8 100644
--- a/tools/testing/selftests/kcmp/kcmp_test.c
+++ b/tools/testing/selftests/kcmp/kcmp_test.c
@@ -17,6 +17,8 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 
+#include "../kselftest.h"
+
 static long sys_kcmp(int pid1, int pid2, int type, int fd1, int fd2)
 {
 	return syscall(__NR_kcmp, pid1, pid2, type, fd1, fd2);
@@ -34,13 +36,13 @@ int main(int argc, char **argv)
 
 	if (fd1 < 0) {
 		perror("Can't create file");
-		exit(1);
+		ksft_exit_fail();
 	}
 
 	pid2 = fork();
 	if (pid2 < 0) {
 		perror("fork failed");
-		exit(1);
+		ksft_exit_fail();
 	}
 
 	if (!pid2) {
@@ -50,7 +52,7 @@ int main(int argc, char **argv)
 		fd2 = open(kpath, O_RDWR, 0644);
 		if (fd2 < 0) {
 			perror("Can't open file");
-			exit(1);
+			ksft_exit_fail();
 		}
 
 		/* An example of output and arguments */
@@ -74,23 +76,34 @@ int main(int argc, char **argv)
 		if (ret) {
 			printf("FAIL: 0 expected but %d returned (%s)\n",
 				ret, strerror(errno));
+			ksft_inc_fail_cnt();
 			ret = -1;
-		} else
+		} else {
 			printf("PASS: 0 returned as expected\n");
+			ksft_inc_pass_cnt();
+		}
 
 		/* Compare with self */
 		ret = sys_kcmp(pid1, pid1, KCMP_VM, 0, 0);
 		if (ret) {
 			printf("FAIL: 0 expected but %d returned (%s)\n",
 				ret, strerror(errno));
+			ksft_inc_fail_cnt();
 			ret = -1;
-		} else
+		} else {
 			printf("PASS: 0 returned as expected\n");
+			ksft_inc_pass_cnt();
+		}
+
+		ksft_print_cnts();
 
-		exit(ret);
+		if (ret)
+			ksft_exit_fail();
+		else
+			ksft_exit_pass();
 	}
 
 	waitpid(pid2, &status, P_ALL);
 
-	return 0;
+	return ksft_exit_pass();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/7] selftests/mount: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
                   ` (3 preceding siblings ...)
  2014-09-23 21:32 ` [PATCH v2 4/7] selftests/kcmp: " Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-23 22:41   ` Eric W. Biederman
  2014-09-23 21:32 ` [PATCH v2 6/7] selftests/ptrace: " Shuah Khan
  2014-09-23 21:33 ` [PATCH v2 7/7] selftests/timers: " Shuah Khan
  6 siblings, 1 reply; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change mount test to use kselftest framework to report
test results.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/mount/unprivileged-remount-test.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c
index 1b3ff2f..c64d442 100644
--- a/tools/testing/selftests/mount/unprivileged-remount-test.c
+++ b/tools/testing/selftests/mount/unprivileged-remount-test.c
@@ -13,6 +13,8 @@
 #include <stdbool.h>
 #include <stdarg.h>
 
+#include "../kselftest.h"
+
 #ifndef CLONE_NEWNS
 # define CLONE_NEWNS 0x00020000
 #endif
@@ -45,7 +47,7 @@ static void die(char *fmt, ...)
 	va_start(ap, fmt);
 	vfprintf(stderr, fmt, ap);
 	va_end(ap);
-	exit(EXIT_FAILURE);
+	ksft_exit_fail();
 }
 
 static void write_file(char *filename, char *fmt, ...)
@@ -176,7 +178,7 @@ bool test_unpriv_remount(int mount_flags, int remount_flags, int invalid_flags)
 		die("remount of /tmp with invalid flags "
 		    "succeeded unexpectedly\n");
 	}
-	exit(EXIT_SUCCESS);
+	return ksft_exit_pass();
 }
 
 static bool test_unpriv_remount_simple(int mount_flags)
@@ -238,5 +240,5 @@ int main(int argc, char **argv)
 	{
 		die("Default atime malfunctions\n");
 	}
-	return EXIT_SUCCESS;
+	return ksft_exit_pass();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/7] selftests/ptrace: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
                   ` (4 preceding siblings ...)
  2014-09-23 21:32 ` [PATCH v2 5/7] selftests/mount: " Shuah Khan
@ 2014-09-23 21:32 ` Shuah Khan
  2014-09-23 21:33 ` [PATCH v2 7/7] selftests/timers: " Shuah Khan
  6 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:32 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change ptrace test to use kselftest framework to report
test results.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/ptrace/peeksiginfo.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/ptrace/peeksiginfo.c b/tools/testing/selftests/ptrace/peeksiginfo.c
index c34cd8a..d940bfc 100644
--- a/tools/testing/selftests/ptrace/peeksiginfo.c
+++ b/tools/testing/selftests/ptrace/peeksiginfo.c
@@ -10,6 +10,7 @@
 #include <sys/mman.h>
 
 #include "linux/ptrace.h"
+#include "../kselftest.h"
 
 static int sys_rt_sigqueueinfo(pid_t tgid, int sig, siginfo_t *uinfo)
 {
@@ -151,7 +152,7 @@ out:
 int main(int argc, char *argv[])
 {
 	siginfo_t siginfo[SIGNR];
-	int i, exit_code = 1;
+	int i;
 	sigset_t blockmask;
 	pid_t child;
 
@@ -162,7 +163,7 @@ int main(int argc, char *argv[])
 	child = fork();
 	if (child == -1) {
 		err("fork() failed: %m");
-		return 1;
+		return ksft_exit_fail();
 	} else if (child == 0) {
 		pid_t ppid = getppid();
 		while (1) {
@@ -170,7 +171,7 @@ int main(int argc, char *argv[])
 				break;
 			sleep(1);
 		}
-		return 1;
+		return ksft_exit_fail();
 	}
 
 	/* Send signals in process-wide and per-thread queues */
@@ -185,7 +186,7 @@ int main(int argc, char *argv[])
 	}
 
 	if (sys_ptrace(PTRACE_ATTACH, child, NULL, NULL) == -1)
-		return 1;
+		return ksft_exit_fail();
 
 	waitpid(child, NULL, 0);
 
@@ -207,12 +208,11 @@ int main(int argc, char *argv[])
 		goto out;
 
 	printf("PASS\n");
-	exit_code = 0;
 out:
 	if (sys_ptrace(PTRACE_KILL, child, NULL, NULL) == -1)
-		return 1;
+		return ksft_exit_fail();
 
 	waitpid(child, NULL, 0);
 
-	return exit_code;
+	return ksft_exit_pass();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/7] selftests/timers: change test to use ksft framework
  2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
                   ` (5 preceding siblings ...)
  2014-09-23 21:32 ` [PATCH v2 6/7] selftests/ptrace: " Shuah Khan
@ 2014-09-23 21:33 ` Shuah Khan
  6 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 21:33 UTC (permalink / raw)
  To: akpm, gregkh, colin.king, dbueso, ebiederm, serge.hallyn, thierry, luto
  Cc: Shuah Khan, linux-api, linux-kernel

Change timers test to use kselftest framework to report
test results.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/timers/posix_timers.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index 41bd855..f87d970 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -15,6 +15,8 @@
 #include <time.h>
 #include <pthread.h>
 
+#include "../kselftest.h"
+
 #define DELAY 2
 #define USECS_PER_SEC 1000000
 
@@ -194,16 +196,16 @@ int main(int argc, char **argv)
 	printf("based timers if other threads run on the CPU...\n");
 
 	if (check_itimer(ITIMER_VIRTUAL) < 0)
-		return -1;
+		return ksft_exit_fail();
 
 	if (check_itimer(ITIMER_PROF) < 0)
-		return -1;
+		return ksft_exit_fail();
 
 	if (check_itimer(ITIMER_REAL) < 0)
-		return -1;
+		return ksft_exit_fail();
 
 	if (check_timer_create(CLOCK_THREAD_CPUTIME_ID) < 0)
-		return -1;
+		return ksft_exit_fail();
 
 	/*
 	 * It's unfortunately hard to reliably test a timer expiration
@@ -215,7 +217,7 @@ int main(int argc, char **argv)
 	 * find a better solution.
 	 */
 	if (check_timer_create(CLOCK_PROCESS_CPUTIME_ID) < 0)
-		return -1;
+		return ksft_exit_fail();
 
-	return 0;
+	return ksft_exit_pass();
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/7] selftests/mount: change test to use ksft framework
  2014-09-23 21:32 ` [PATCH v2 5/7] selftests/mount: " Shuah Khan
@ 2014-09-23 22:41   ` Eric W. Biederman
  2014-09-23 22:56     ` Shuah Khan
  0 siblings, 1 reply; 14+ messages in thread
From: Eric W. Biederman @ 2014-09-23 22:41 UTC (permalink / raw)
  To: Shuah Khan
  Cc: akpm, gregkh, colin.king, dbueso, serge.hallyn, thierry, luto,
	linux-api, linux-kernel

Shuah Khan <shuahkh@osg.samsung.com> writes:

> Change mount test to use kselftest framework to report
> test results.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

I am curious did you even run these tests?   I can't possibly see how
the tests would have passed with this change.  At the very least you
have taken this test from linear to exponential time complexity.

> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/mount/unprivileged-remount-test.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c
> index 1b3ff2f..c64d442 100644
> --- a/tools/testing/selftests/mount/unprivileged-remount-test.c
> +++ b/tools/testing/selftests/mount/unprivileged-remount-test.c
> @@ -13,6 +13,8 @@
>  #include <stdbool.h>
>  #include <stdarg.h>
>  
> +#include "../kselftest.h"
> +
>  #ifndef CLONE_NEWNS
>  # define CLONE_NEWNS 0x00020000
>  #endif
> @@ -45,7 +47,7 @@ static void die(char *fmt, ...)
>  	va_start(ap, fmt);
>  	vfprintf(stderr, fmt, ap);
>  	va_end(ap);
> -	exit(EXIT_FAILURE);
> +	ksft_exit_fail();
>  }
>  
>  static void write_file(char *filename, char *fmt, ...)
> @@ -176,7 +178,7 @@ bool test_unpriv_remount(int mount_flags, int remount_flags, int invalid_flags)
>  		die("remount of /tmp with invalid flags "
>  		    "succeeded unexpectedly\n");
>  	}
> -	exit(EXIT_SUCCESS);
> +	return ksft_exit_pass();

This change is a deep bug. 

This exit is a forked child process (not from the test itself). That a
few lines earlier in the test I test the value of in waitpid.

Also in your change commit comment if no where else you should have
called out that you were making a semantic change to the test.

>  }
>  
>  static bool test_unpriv_remount_simple(int mount_flags)
> @@ -238,5 +240,5 @@ int main(int argc, char **argv)
>  	{
>  		die("Default atime malfunctions\n");
>  	}
> -	return EXIT_SUCCESS;
> +	return ksft_exit_pass();
>  }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/7] selftests/mount: change test to use ksft framework
  2014-09-23 22:41   ` Eric W. Biederman
@ 2014-09-23 22:56     ` Shuah Khan
  2014-09-23 23:07       ` Eric W. Biederman
  0 siblings, 1 reply; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 22:56 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: akpm, gregkh, colin.king, dbueso, serge.hallyn, thierry, luto,
	linux-api, linux-kernel, Shuah Khan

On 09/23/2014 04:41 PM, Eric W. Biederman wrote:
> Shuah Khan <shuahkh@osg.samsung.com> writes:
> 
>> Change mount test to use kselftest framework to report
>> test results.
> 
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> I am curious did you even run these tests?   I can't possibly see how
> the tests would have passed with this change.  At the very least you
> have taken this test from linear to exponential time complexity.
> 

Oops. I meant to drop this commit from the series. Sorry for the noise.

-- Shuah

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/7] selftests/mount: change test to use ksft framework
  2014-09-23 22:56     ` Shuah Khan
@ 2014-09-23 23:07       ` Eric W. Biederman
  2014-09-23 23:18         ` Shuah Khan
  0 siblings, 1 reply; 14+ messages in thread
From: Eric W. Biederman @ 2014-09-23 23:07 UTC (permalink / raw)
  To: Shuah Khan
  Cc: akpm, gregkh, colin.king, dbueso, serge.hallyn, thierry, luto,
	linux-api, linux-kernel

Shuah Khan <shuahkh@osg.samsung.com> writes:

> On 09/23/2014 04:41 PM, Eric W. Biederman wrote:
>> Shuah Khan <shuahkh@osg.samsung.com> writes:
>> 
>>> Change mount test to use kselftest framework to report
>>> test results.
>> 
>> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> 
>> I am curious did you even run these tests?   I can't possibly see how
>> the tests would have passed with this change.  At the very least you
>> have taken this test from linear to exponential time complexity.
>> 
>
> Oops. I meant to drop this commit from the series. Sorry for the
> noise.

No problem.  Except for the unnecessary semantic change the rest of the
change looked fine.  I just wanted to make certain you didn't break my
test by accident.

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/7] selftests/mount: change test to use ksft framework
  2014-09-23 23:07       ` Eric W. Biederman
@ 2014-09-23 23:18         ` Shuah Khan
  0 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-23 23:18 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: akpm, gregkh, colin.king, dbueso, serge.hallyn, thierry, luto,
	linux-api, linux-kernel, Shuah Khan

On 09/23/2014 05:07 PM, Eric W. Biederman wrote:
> Shuah Khan <shuahkh@osg.samsung.com> writes:
> 
>> On 09/23/2014 04:41 PM, Eric W. Biederman wrote:
>>> Shuah Khan <shuahkh@osg.samsung.com> writes:
>>>
>>>> Change mount test to use kselftest framework to report
>>>> test results.
>>>
>>> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>>>
>>> I am curious did you even run these tests?   I can't possibly see how
>>> the tests would have passed with this change.  At the very least you
>>> have taken this test from linear to exponential time complexity.
>>>
>>
>> Oops. I meant to drop this commit from the series. Sorry for the
>> noise.
> 
> No problem.  Except for the unnecessary semantic change the rest of the
> change looked fine.  I just wanted to make certain you didn't break my
> test by accident.
> 

Thanks for catching it. The results didn't look right and I meant
to delay this change for later after another round of testing.
I will fix it without the unnecessary change and send it out
separately.

thanks,
-- Shuah

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting
  2014-09-23 21:32 ` [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting Shuah Khan
@ 2014-09-24  8:09   ` Davidlohr Bueso
  2014-09-24 22:06     ` Shuah Khan
  0 siblings, 1 reply; 14+ messages in thread
From: Davidlohr Bueso @ 2014-09-24  8:09 UTC (permalink / raw)
  To: Shuah Khan
  Cc: akpm, gregkh, colin.king, ebiederm, serge.hallyn, thierry, luto,
	linux-api, linux-kernel

On Tue, 2014-09-23 at 15:32 -0600, Shuah Khan wrote:
> Add kselftest framework for tests to use. This is a light
> weight framework provides a set of interfaces to report test
> results. Tests can use these interfaces to report pass, and
> fail cases as well as when failure is due to configuration
> problems such as missing modules, or when a test that is should
> fail, fails as expected, and a test that should fail, passes.
> The framework uses POSIX standard return codes for reporting
> results to address the needs of users that want to run the kernel
> selftests from their user-space test suites and want to know why a
> test failed. In addition, the framework includes interfaces to use
> to report test statistics on number of tests passed and failed.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/kselftest.h | 41 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 tools/testing/selftests/kselftest.h
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> new file mode 100644
> index 0000000..c73fc2c
> --- /dev/null
> +++ b/tools/testing/selftests/kselftest.h
> @@ -0,0 +1,41 @@
> +/*
> + * kselftest.h: kselftest framework return codes to include from
> + *		 selftests.
> + *
> + * Copyright (c) 2014 Shuah Khan <shuahkh@osg.samsung.com>
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + *
> + * This file is released under the GPLv2.
> + */
> +#ifndef __KSELFTEST_H
> +#define __KSELFTEST_H
> +
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +/* counters */
> +static int ksft_pass;
> +static int ksft_fail;
> +static int ksft_xfail;
> +static int ksft_xpass;
> +static int ksft_xskip;

unsigned int?

> +static inline void ksft_inc_pass_cnt(void) { ksft_pass++; }
> +static inline void ksft_inc_fail_cnt(void) { ksft_fail++; }
> +static inline void ksft_inc_xfail_cnt(void) { ksft_xfail++; }
> +static inline void ksft_inc_xpass_cnt(void) { ksft_xpass++; }
> +static inline void ksft_inc_xskip_cnt(void) { ksft_xskip++; }

It would probably make sense to have the counters in a structures,
something like: struct ksft_counter { ... } ksft_cnt;

Then just pass it around the proposed functions as arguments. That also
minimizes a bit the global variables and would allow you to easily
change it in the future.

> +static inline void ksft_print_cnts(void)
> +{
> +	printf("Pass: %d Fail: %d Xfail: %d Xpass: %d, Xskip: %d\n",
> +		ksft_pass, ksft_fail, ksft_xfail, ksft_xpass, ksft_xskip);
> +}

Same here, just do:

static inline void ksft_print_cnts(struct ksft_counter counters)
{ 
	... 
}

Otherwise looks good.

> +static inline int ksft_exit_pass(void) { exit(0); }
> +static inline int ksft_exit_fail(void) { exit(1); }
> +static inline int ksft_exit_xfail(void) { exit(2); }
> +static inline int ksft_exit_xpass(void) { exit(3); }
> +static inline int ksft_exit_skip(void) { exit(4); }
> +
> +#endif /* __KSELFTEST_H */



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting
  2014-09-24  8:09   ` Davidlohr Bueso
@ 2014-09-24 22:06     ` Shuah Khan
  0 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2014-09-24 22:06 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: akpm, gregkh, colin.king, ebiederm, serge.hallyn, thierry, luto,
	linux-api, linux-kernel, Shuah Khan

On 09/24/2014 02:09 AM, Davidlohr Bueso wrote:

>> +/* counters */
>> +static int ksft_pass;
>> +static int ksft_fail;
>> +static int ksft_xfail;
>> +static int ksft_xpass;
>> +static int ksft_xskip;
> 
> unsigned int?

Yes unsigned int is a better choice.

> 
>> +static inline void ksft_inc_pass_cnt(void) { ksft_pass++; }
>> +static inline void ksft_inc_fail_cnt(void) { ksft_fail++; }
>> +static inline void ksft_inc_xfail_cnt(void) { ksft_xfail++; }
>> +static inline void ksft_inc_xpass_cnt(void) { ksft_xpass++; }
>> +static inline void ksft_inc_xskip_cnt(void) { ksft_xskip++; }
> 
> It would probably make sense to have the counters in a structures,
> something like: struct ksft_counter { ... } ksft_cnt;
> 
> Then just pass it around the proposed functions as arguments. That also
> minimizes a bit the global variables and would allow you to easily
> change it in the future.

How does the following look?

struct ksft_count
{
        unsigned int ksft_pass;
        unsigned int ksft_fail;
        unsigned int ksft_xfail;
        unsigned int ksft_xpass;
        unsigned int ksft_xskip;
};

static ksft_count ksft_cnt;

static inline void ksft_inc_pass_cnt(void) { ksft_cnt.ksft_pass++; }
static inline void ksft_inc_fail_cnt(void) { ksft_cnt.ksft_fail++; }
static inline void ksft_inc_xfail_cnt(void) { ksft_cnt.ksft_xfail++; }
static inline void ksft_inc_xpass_cnt(void) { ksft_cnt.ksft_xpass++; }
static inline void ksft_inc_xskip_cnt(void) { ksft_cnt.ksft_xskip++; }

With this approach, tests don't have to define their own counter
variable and pass it in. I am looking to abstract the framework
as much as possible.

thanks,
-- Shuah


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-09-24 22:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-23 21:32 [PATCH v2 0/7] kselftest framework and test changes to use it Shuah Khan
2014-09-23 21:32 ` [PATCH v2 1/7] selftests: add kselftest framework for uniform test reporting Shuah Khan
2014-09-24  8:09   ` Davidlohr Bueso
2014-09-24 22:06     ` Shuah Khan
2014-09-23 21:32 ` [PATCH v2 2/7] selftests/breakpoints: change test to use ksft framework Shuah Khan
2014-09-23 21:32 ` [PATCH v2 3/7] selftests/ipc: " Shuah Khan
2014-09-23 21:32 ` [PATCH v2 4/7] selftests/kcmp: " Shuah Khan
2014-09-23 21:32 ` [PATCH v2 5/7] selftests/mount: " Shuah Khan
2014-09-23 22:41   ` Eric W. Biederman
2014-09-23 22:56     ` Shuah Khan
2014-09-23 23:07       ` Eric W. Biederman
2014-09-23 23:18         ` Shuah Khan
2014-09-23 21:32 ` [PATCH v2 6/7] selftests/ptrace: " Shuah Khan
2014-09-23 21:33 ` [PATCH v2 7/7] selftests/timers: " Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).