[PATCH 0/5] Syscall restart fix, smokey fixes

[PATCH 0/5] Syscall restart fix, smokey fixes

[Jan Kiszka]

This also replaces the previously sent version of
"testsuite/smokey: gdb: Catch dup and write errors".

Jan

Jan Kiszka (5):
  testsuite/smokey/gdb: Catch dup and write errors
  testsuite/smokey/gdb: Cleanup check helpers
  cobalt/thread: Clear XNSYSRST when migrating to Linux
  cobalt/posix/clock: Remove unneeded local variable
  testsuite/smokey/sigdebug: Fix check_no_error and some callers

 kernel/cobalt/posix/clock.c          |  3 +-
 kernel/cobalt/thread.c               |  5 ++++
 testsuite/smokey/gdb/gdb.c           | 56 +++++++++++++++++++-----------------
 testsuite/smokey/sigdebug/sigdebug.c | 15 ++++++----
 4 files changed, 44 insertions(+), 35 deletions(-)

-- 
2.16.4

[PATCH 1/5] testsuite/smokey/gdb: Catch dup and write errors

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Reported by compiler settings in travis.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 testsuite/smokey/gdb/gdb.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/testsuite/smokey/gdb/gdb.c b/testsuite/smokey/gdb/gdb.c
index a0308d9b2e..7a20314e0b 100644
--- a/testsuite/smokey/gdb/gdb.c
+++ b/testsuite/smokey/gdb/gdb.c
@@ -1,7 +1,7 @@
 /*
  * gdb test.
  *
- * Copyright (C) Siemens AG, 2015
+ * Copyright (C) Siemens AG, 2015-2019
  *
  * Authors.
  *  Jan Kiszka <jan.kiszka@siemens.com>
@@ -104,7 +104,10 @@ static void wait_for_pattern(const char *string)
 
 static void send_command(const char *string, int wait_for_prompt)
 {
-	write(pipe_in[1], string, strlen(string));
+	int ret;
+
+	ret = write(pipe_in[1], string, strlen(string));
+	check("write(pipe_in)", ret, strlen(string));
 	if (wait_for_prompt)
 		wait_for_pattern("(gdb) ");
 }
@@ -142,8 +145,12 @@ static void eval_expression_inner(const char *fn, int line,
 				  const char *expression,
 				  const char *expected_value)
 {
-	write(pipe_in[1], "p ", 2);
-	write(pipe_in[1], expression, strlen(expression));
+	int ret;
+
+	ret = write(pipe_in[1], "p ", 2);
+	check("write(pipe_in)", ret, 2);
+	ret = write(pipe_in[1], expression, strlen(expression));
+	check("write(pipe_in)", ret, strlen(expression));
 	send_command("\n", 0);
 
 	check_output(fn, line, expression, "$");
@@ -261,13 +268,16 @@ static int run_gdb(struct smokey_test *t, int argc, char *const argv[])
 		case 0:
 			/* redirect input */
 			close(0);
-			dup(pipe_in[0]);
+			err = dup(pipe_in[0]);
+			check_no_error("dup(pipe_in)", err < 0 ? err : 0);
 
 			/* redirect output and stderr */
 			close(1);
-			dup(pipe_out[1]);
+			err = dup(pipe_out[1]);
+			check_no_error("dup(pipe_out)", err < 0 ? err : 0);
 			close(2);
-			dup(1);
+			err = dup(1);
+			check_no_error("dup(1)", err < 0 ? err : 0);
 
 			snprintf(run_param, sizeof(run_param), "--run=%d",
 				 t->__reserved.id);
-- 
2.16.4

[PATCH 2/5] testsuite/smokey/gdb: Cleanup check helpers

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

check_inner() is only called with exp_flag = 1.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 testsuite/smokey/gdb/gdb.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/testsuite/smokey/gdb/gdb.c b/testsuite/smokey/gdb/gdb.c
index 7a20314e0b..2fad3da19a 100644
--- a/testsuite/smokey/gdb/gdb.c
+++ b/testsuite/smokey/gdb/gdb.c
@@ -25,37 +25,29 @@
 static void send_command(const char *string, int wait_for_prompt);
 
 static void check_inner(const char *fn, int line, const char *msg,
-			int status, int expected, int exp_flag)
+			int status, int expected)
 {
-	if (exp_flag) {
-		if (status == expected)
-			return;
-	}
-	else if (status != -1)
-			return;
+	if (status == expected)
+		return;
 
 	rt_print_flush_buffers();
-	if (exp_flag)
-		fprintf(stderr, "FAILURE %s:%d: %s returned %d instead of %d - %s\n",
-				fn, line, msg, status, expected, strerror(status));
-	else
-		fprintf(stderr, "FAILURE %s:%d: %s returned %d [errno=%d] - %s\n",
-				fn, line, msg, status, expected, strerror(expected));
+	fprintf(stderr, "FAILURE %s:%d: %s returned %d instead of %d - %s\n",
+		fn, line, msg, status, expected, strerror(status));
 	send_command("q\n", 0);
 	exit(EXIT_FAILURE);
 }
 
 #define check(msg, status, expected) ({					\
-int __status = status;						\
-check_inner(__FUNCTION__, __LINE__, msg, __status, expected, 1);	\
-__status;							\
+	int __status = status;						\
+	check_inner(__FUNCTION__, __LINE__, msg, __status, expected);	\
+	__status;							\
 })
 
 #define check_no_error(msg, status) ({					\
-int __status = status;						\
-check_inner(__FUNCTION__, __LINE__, msg,			\
-__status < 0 ? errno : __status, 0, 1);			\
-__status;							\
+	int __status = status;						\
+	check_inner(__func__, __LINE__, msg,				\
+		    __status < 0 ? errno : __status, 0);		\
+	__status;							\
 })
 
 smokey_test_plugin(gdb,
-- 
2.16.4

[PATCH 3/5] cobalt/thread: Clear XNSYSRST when migrating to Linux

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This fixes the issue of spurious syscall-restart detection in
clock_nanosleep and select when a signal was delivered. In that case,
Linux replaces the ESYSRESTART with EINTR and does not perform any
restart attempt. As a signal delivery implies migration, clearing
XNSYSRST in the migration path resolves the issue.

Fixes: 36132cdb21cd ("cobalt/kernel: Allow to restart clock_nanosleep and select after signal processing")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kernel/cobalt/thread.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index eacdd9a089..666b4d4ca0 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -2171,6 +2171,11 @@ void xnthread_relax(int notify, int reason)
 		set_cpus_allowed_ptr(p, cpumask_of(cpu));
 	}
 #endif
+	/*
+	 * After migration there will be no syscall restart (rather a signal
+	 * delivery).
+	 */
+	xnthread_clear_localinfo(thread, XNSYSRST);
 
 	ipipe_clear_thread_flag(TIP_MAYDAY);
 
-- 
2.16.4

[PATCH 4/5] cobalt/posix/clock: Remove unneeded local variable

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kernel/cobalt/posix/clock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
index 6ba389efd0..561358e749 100644
--- a/kernel/cobalt/posix/clock.c
+++ b/kernel/cobalt/posix/clock.c
@@ -276,7 +276,6 @@ int __cobalt_clock_nanosleep(clockid_t clock_id, int flags,
 	struct restart_block *restart;
 	struct xnthread *cur;
 	xnsticks_t timeout, rem;
-	int ret = 0;
 	spl_t s;
 
 	trace_cobalt_clock_nanosleep(clock_id, flags, rqt);
@@ -347,7 +346,7 @@ int __cobalt_clock_nanosleep(clockid_t clock_id, int flags,
 
 	xnlock_put_irqrestore(&nklock, s);
 
-	return ret;
+	return 0;
 }
 
 COBALT_SYSCALL(clock_nanosleep, primary,
-- 
2.16.4

[PATCH 5/5] testsuite/smokey/sigdebug: Fix check_no_error and some callers

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Aligning check_no_error with the gdb test case revealed a number of
smaller issues in the sigdebug test - and the bigger one with syscall
restarted fixed in "cobalt/thread: Clear XNSYSRST when migrating to
Linux".

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 testsuite/smokey/sigdebug/sigdebug.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/testsuite/smokey/sigdebug/sigdebug.c b/testsuite/smokey/sigdebug/sigdebug.c
index 87f0be6310..15f9e4dae3 100644
--- a/testsuite/smokey/sigdebug/sigdebug.c
+++ b/testsuite/smokey/sigdebug/sigdebug.c
@@ -1,10 +1,10 @@
 /*
  * Functional testing of unwanted domain switch debugging mechanism.
  *
- * Copyright (C) Siemens AG, 2012-2014
+ * Copyright (C) Siemens AG, 2012-2019
  *
  * Authors:
- *  Jan Kiszka  <jan.kiszka@siemens.com>
+ *  Jan Kiszka <jan.kiszka@siemens.com>
  *
  * Released under the terms of GPLv2.
  */
@@ -53,7 +53,7 @@ static void check_inner(const char *fn, int line, const char *msg,
 	pthread_setmode_np(PTHREAD_WARNSW, 0, NULL);
 	rt_print_flush_buffers();
 	fprintf(stderr, "FAILURE %s:%d: %s returned %d instead of %d - %s\n",
-		fn, line, msg, status, expected, strerror(-status));
+		fn, line, msg, status, expected, strerror(status));
 	exit(EXIT_FAILURE);
 }
 
@@ -77,7 +77,7 @@ static void check_sigdebug_inner(const char *fn, int line, const char *reason)
 #define check_no_error(msg, status) ({					\
 	int __status = status;						\
 	check_inner(__func__, __LINE__, msg,				\
-		    __status < 0 ? __status : 0, 0);			\
+		    __status < 0 ? errno : __status, 0);		\
 	__status;							\
 })
 
@@ -107,7 +107,7 @@ static void *rt_thread_body(void *cookie)
 	err = sem_post(&send_signal);
 	check_no_error("sem_post", err);
 	err = clock_nanosleep(CLOCK_MONOTONIC, 0, &delay, NULL);
-	check_no_error("clock_nanosleep", err);
+	check("clock_nanosleep", err, EINTR);
 	check_sigdebug_received("SIGDEBUG_MIGRATE_SIGNAL");
 
 	smokey_trace("relaxed mutex owner");
@@ -116,6 +116,8 @@ static void *rt_thread_body(void *cookie)
 		err = pthread_mutex_lock(&prio_invert);
 		check_no_error("pthread_mutex_lock", err);
 		check_sigdebug_received("SIGDEBUG_MIGRATE_PRIOINV");
+		err = pthread_mutex_unlock(&prio_invert);
+		check_no_error("pthread_mutex_unlock", err);
 	} else {
 		smokey_note("sigdebug \"SIGDEBUG_MIGRATE_PRIOINV\" skipped "
 			    "(no kernel support)");
@@ -263,7 +265,8 @@ static int run_sigdebug(struct smokey_test *t, int argc, char *const argv[])
 	munlockall();
 	setup_checkdebug(SIGDEBUG_NOMLOCK);
 	err = pthread_create(&rt_thread, &attr, rt_thread_body, NULL);
-	check("pthread_setschedparam", err, EINTR);
+	/* Note: EINTR is against the spec, but that's OK in this scenario. */
+	check("pthread_create", err, EINTR);
 	check_sigdebug_received("SIGDEBUG_NOMLOCK");
 	mlockall(MCL_CURRENT | MCL_FUTURE);
 
-- 
2.16.4