xenomai.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] switchtest: test different migration types
@ 2022-12-05 15:21 T. Schaffner
  2022-12-05 15:21 ` [PATCH v2 1/3] switchtest: split the set_mode method T. Schaffner
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: T. Schaffner @ 2022-12-05 15:21 UTC (permalink / raw)
  To: xenomai; +Cc: florian.bezdeka, jan.kiszka, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Switchtest only tested intended switches between primary and secondary
mode in userland.

Let the rtuo thread also test fault based migrations and migrations that
occur while in kernel mode.

This patch series will also test the bug that was fixed in
829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5

Changes since v1:
- Squash patches 1 and 2 aswell as 3 and 4
- Call the correct rtswitch_to_xxx function after mode switch in the
  switchtest driver
- Only switch to secondary mode by using a linux syscall or by a call to
  cobalt_thread_relax()

Tobias Schaffner (3):
  switchtest: split the set_mode method
  switchtest: test multiple secondary switch cases
  switchtest: test mode switche while in kernel mode

 include/rtdm/uapi/testing.h         |  10 ++
 kernel/drivers/testing/switchtest.c |   8 ++
 testsuite/switchtest/switchtest.c   | 142 +++++++++++++++++++++-------
 3 files changed, 126 insertions(+), 34 deletions(-)

-- 
2.34.1


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

* [PATCH v2 1/3] switchtest: split the set_mode method
  2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
@ 2022-12-05 15:21 ` T. Schaffner
  2022-12-08 13:49   ` Jan Kiszka
  2022-12-05 15:21 ` [PATCH v2 2/3] switchtest: test multiple secondary switch cases T. Schaffner
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: T. Schaffner @ 2022-12-05 15:21 UTC (permalink / raw)
  To: xenomai; +Cc: florian.bezdeka, jan.kiszka, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Split the set_mode method in separate methods for switching to primary and
secondary mode for improved readability.

This also fixes a wrong evaluation of the mode variable that lead to never
checking the FPU registers in secondary mode.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 48 ++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index f2a5c1601..f8a37da79 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -440,15 +440,21 @@ static void *fpu_stress(void *cookie)
 	return NULL;
 }
 
-static void set_mode(const char *prefix, int fd, unsigned mode)
+static void switch_to_primary_mode(void)
 {
-	switch (mode) {
-	case 1:
-		cobalt_thread_harden();
-		return;
+	cobalt_thread_harden();
+	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
+		perror("Switch to primary mode failed.");
+		clean_exit(EXIT_FAILURE);
+	}
+}
 
-	case 2:
-		cobalt_thread_relax();
+static void switch_to_secondary_mode(void)
+{
+	cobalt_thread_relax();
+	if (!(cobalt_thread_mode() & (XNRELAX))) {
+		perror("Switch to secondary mode failed.");
+		clean_exit(EXIT_FAILURE);
 	}
 }
 
@@ -475,7 +481,7 @@ static void *rtup(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	set_mode("rtup", fd, 1);
+	switch_to_primary_mode();
 
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -556,7 +562,7 @@ static void *rtus(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	set_mode("rtus", fd, 2);
+	switch_to_secondary_mode();
 
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -637,8 +643,9 @@ static void *rtuo(void *cookie)
 	   allowed when suspended in ioctl. */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	mode = 1;
-	set_mode("rtuo", fd, mode);
+	switch_to_primary_mode();
+	mode = COBALT_PRIMARY;
+
 	do {
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
 	} while (err == -1 && errno == EINTR);
@@ -668,8 +675,10 @@ static void *rtuo(void *cookie)
 		}
 
 		expected = rtsw.from + i * 1000;
-		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS))
+		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
+		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_regs_set(fp_features, expected);
+		}
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
 		while (err == -1 && errno == EINTR)
 			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -682,16 +691,23 @@ static void *rtuo(void *cookie)
 		case -1:
 			clean_exit(EXIT_FAILURE);
 		}
-		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS)) {
+
+		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
+		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_val = check_fp_result(expected);
 			if (fp_val != expected)
 				handle_bad_fpreg(param->cpu, fp_val);
 		}
 
-		/* Switch mode. */
+		/* Switch between primary and secondary mode */
 		if (i % 3 == 2) {
-			mode = 3 - mode;
-			set_mode("rtuo", fd, mode);
+			if (mode == COBALT_PRIMARY) {
+				switch_to_secondary_mode();
+				mode = COBALT_SECONDARY;
+			} else {
+				switch_to_primary_mode();
+				mode = COBALT_PRIMARY;
+			}
 		}
 
 		if(++i == 4000000)
-- 
2.34.1


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

* [PATCH v2 2/3] switchtest: test multiple secondary switch cases
  2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
  2022-12-05 15:21 ` [PATCH v2 1/3] switchtest: split the set_mode method T. Schaffner
@ 2022-12-05 15:21 ` T. Schaffner
  2022-12-05 17:44   ` Florian Bezdeka
  2022-12-08 13:53   ` Jan Kiszka
  2022-12-05 15:21 ` [PATCH v2 3/3] switchtest: test mode switche while in kernel mode T. Schaffner
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: T. Schaffner @ 2022-12-05 15:21 UTC (permalink / raw)
  To: xenomai; +Cc: florian.bezdeka, jan.kiszka, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Switches to secondary mode do not only occur when a relax is explicitly
called via the cobalt api but also e.g. when a linux syscall is called in
primary mode.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 testsuite/switchtest/switchtest.c | 117 ++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 38 deletions(-)

diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index f8a37da79..12516186b 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -34,7 +34,9 @@
 #include <semaphore.h>
 #include <setjmp.h>
 #include <getopt.h>
+#include <asm/unistd.h>
 #include <asm/xenomai/features.h>
+#include <asm/xenomai/syscall.h>
 #include <asm/xenomai/uapi/fptest.h>
 #include <cobalt/trace.h>
 #include <rtdm/testing.h>
@@ -75,6 +77,14 @@ typedef enum {
 	UFPS = 4	 /* use the FPU while in secondary mode. */
 } fpflags;
 
+#define TASK_SWITCH_MODES 3
+
+enum task_switch_mode {
+	TASK_SWITCH_PREVIOUS = 0,
+	TASK_SWITCH_NEXT = 1,
+	TASK_SWITCH_MODE = 2
+};
+
 struct cpu_tasks;
 
 struct task_params {
@@ -297,6 +307,57 @@ static int printout(const char *fmt, ...)
 #define check_fp_result(__expected)	\
 	fp_regs_check(fp_features, __expected, printout)
 
+static void _assert_primary_mode(char const *calling_func)
+{
+	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
+		fprintf(stderr,
+			"Switch to primary mode failed in %s.",
+			calling_func);
+		clean_exit(EXIT_FAILURE);
+	}
+}
+
+#define assert_primary_mode() _assert_primary_mode(__func__)
+
+static void _assert_secondary_mode(char const *calling_func)
+{
+	if (!(cobalt_thread_mode() & (XNRELAX))) {
+		fprintf(stderr,
+			"Switch to secondary mode failed in %s.",
+			calling_func);
+		clean_exit(EXIT_FAILURE);
+	}
+}
+
+#define assert_secondary_mode() _assert_secondary_mode(__func__)
+
+static void switch_to_primary_mode(void)
+{
+	cobalt_thread_harden();
+	assert_primary_mode();
+}
+
+static void switch_to_secondary_mode(void)
+{
+	cobalt_thread_relax();
+	assert_secondary_mode();
+}
+
+static void switch_to_secondary_mode_by_using_linux_syscall(void)
+{
+	syscall(__NR_gettid);
+	assert_secondary_mode();
+}
+
+#define SWITCH_FUNC_COUNT 4
+
+static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
+	switch_to_secondary_mode,
+	switch_to_primary_mode,
+	switch_to_secondary_mode_by_using_linux_syscall,
+	switch_to_primary_mode,
+};
+
 static void *sleeper_switcher(void *cookie)
 {
 	struct task_params *param = (struct task_params *) cookie;
@@ -354,13 +415,13 @@ static void *sleeper_switcher(void *cookie)
 		if (tasks_count == 1)
 			continue;
 
-		switch (i % 3) {
-		case 0:
+		switch (i % TASK_SWITCH_MODES) {
+		case TASK_SWITCH_PREVIOUS:
 			/* to == from means "return to last task" */
 			rtsw.to = rtsw.from;
 			break;
 
-		case 1:
+		case TASK_SWITCH_NEXT:
 			if (++to == rtsw.from)
 				++to;
 			if (to > tasks_count - 1)
@@ -440,24 +501,6 @@ static void *fpu_stress(void *cookie)
 	return NULL;
 }
 
-static void switch_to_primary_mode(void)
-{
-	cobalt_thread_harden();
-	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
-		perror("Switch to primary mode failed.");
-		clean_exit(EXIT_FAILURE);
-	}
-}
-
-static void switch_to_secondary_mode(void)
-{
-	cobalt_thread_relax();
-	if (!(cobalt_thread_mode() & (XNRELAX))) {
-		perror("Switch to secondary mode failed.");
-		clean_exit(EXIT_FAILURE);
-	}
-}
-
 static void *rtup(void *cookie)
 {
 	struct task_params *param = (struct task_params *) cookie;
@@ -493,13 +536,13 @@ static void *rtup(void *cookie)
 	for (;;) {
 		unsigned expected, fp_val;
 
-		switch (i % 3) {
-		case 0:
+		switch (i % TASK_SWITCH_MODES) {
+		case TASK_SWITCH_PREVIOUS:
 			/* to == from means "return to last task" */
 			rtsw.to = rtsw.from;
 			break;
 
-		case 1:
+		case TASK_SWITCH_NEXT:
 			if (++to == rtsw.from)
 				++to;
 			if (to > tasks_count - 1)
@@ -574,13 +617,13 @@ static void *rtus(void *cookie)
 	for (;;) {
 		unsigned expected, fp_val;
 
-		switch (i % 3) {
-		case 0:
+		switch (i % TASK_SWITCH_MODES) {
+		case TASK_SWITCH_PREVIOUS:
 			/* to == from means "return to last task" */
 			rtsw.to = rtsw.from;
 			break;
 
-		case 1:
+		case TASK_SWITCH_NEXT:
 			if (++to == rtsw.from)
 				++to;
 			if (to > tasks_count - 1)
@@ -656,13 +699,13 @@ static void *rtuo(void *cookie)
 	for (;;) {
 		unsigned expected, fp_val;
 
-		switch (i % 3) {
-		case 0:
+		switch (i % TASK_SWITCH_MODES) {
+		case TASK_SWITCH_PREVIOUS:
 			/* to == from means "return to last task" */
 			rtsw.to = rtsw.from;
 			break;
 
-		case 1:
+		case TASK_SWITCH_NEXT:
 			if (++to == rtsw.from)
 				++to;
 			if (to > tasks_count - 1)
@@ -679,6 +722,7 @@ static void *rtuo(void *cookie)
 		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
 			fp_regs_set(fp_features, expected);
 		}
+
 		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
 		while (err == -1 && errno == EINTR)
 			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
@@ -700,14 +744,11 @@ static void *rtuo(void *cookie)
 		}
 
 		/* Switch between primary and secondary mode */
-		if (i % 3 == 2) {
-			if (mode == COBALT_PRIMARY) {
-				switch_to_secondary_mode();
-				mode = COBALT_SECONDARY;
-			} else {
-				switch_to_primary_mode();
-				mode = COBALT_PRIMARY;
-			}
+		if (i % TASK_SWITCH_MODES == TASK_SWITCH_MODE) {
+			uint switch_iteration = (i / TASK_SWITCH_MODES %
+				SWITCH_FUNC_COUNT);
+			switch_funcs[switch_iteration]();
+			mode = !mode;
 		}
 
 		if(++i == 4000000)
-- 
2.34.1


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

* [PATCH v2 3/3] switchtest: test mode switche while in kernel mode
  2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
  2022-12-05 15:21 ` [PATCH v2 1/3] switchtest: split the set_mode method T. Schaffner
  2022-12-05 15:21 ` [PATCH v2 2/3] switchtest: test multiple secondary switch cases T. Schaffner
@ 2022-12-05 15:21 ` T. Schaffner
  2022-12-08 14:08   ` Jan Kiszka
  2022-12-05 17:16 ` [PATCH v2 0/3] switchtest: test different migration types Jan Kiszka
  2022-12-05 17:32 ` Florian Bezdeka
  4 siblings, 1 reply; 13+ messages in thread
From: T. Schaffner @ 2022-12-05 15:21 UTC (permalink / raw)
  To: xenomai; +Cc: florian.bezdeka, jan.kiszka, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

Make the rtuo thread switch between primary and secondary mode while in the
RTTST_RTIOC_SWTEST_SWITCH_TO syscall.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 include/rtdm/uapi/testing.h         | 10 ++++++++++
 kernel/drivers/testing/switchtest.c |  8 ++++++++
 testsuite/switchtest/switchtest.c   | 21 +++++++++++++++++++--
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/include/rtdm/uapi/testing.h b/include/rtdm/uapi/testing.h
index f8207b8c7..c389cdc25 100644
--- a/include/rtdm/uapi/testing.h
+++ b/include/rtdm/uapi/testing.h
@@ -24,6 +24,7 @@
 #define _RTDM_UAPI_TESTING_H
 
 #include <linux/types.h>
+#include <stdbool.h>
 
 #define RTTST_PROFILE_VER		2
 
@@ -71,9 +72,18 @@ struct rttst_swtest_task {
 #define RTTST_SWTEST_USE_FPU		0x2 /* Only for kernel-space tasks. */
 #define RTTST_SWTEST_FREEZE		0x4 /* Only for kernel-space tasks. */
 
+/**
+ * @brief parameter for the RTTST_RTIOC_SWTEST_SWITCH_TO syscall
+ * @anchor rttst_swtest_dir
+ *
+ * This structure is used to tell the RTTST_RTIOC_SWTEST_SWITCH_TO syscall
+ * which threads should be exchanged and if the mode (primary/secondary) of the
+ * from thread should be switched.
+ */
 struct rttst_swtest_dir {
 	unsigned int from;
 	unsigned int to;
+	bool switch_mode;
 };
 
 struct rttst_swtest_error {
diff --git a/kernel/drivers/testing/switchtest.c b/kernel/drivers/testing/switchtest.c
index 312b4d870..e2c0f4bcd 100644
--- a/kernel/drivers/testing/switchtest.c
+++ b/kernel/drivers/testing/switchtest.c
@@ -647,6 +647,10 @@ static int rtswitch_ioctl_nrt(struct rtdm_fd *fd,
 				    arg,
 				    sizeof(fromto));
 
+		if (fromto.switch_mode) {
+			xnthread_harden();
+			return rtswitch_to_rt(ctx, fromto.from, fromto.to);
+		}
 		return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
 
 	case RTTST_RTIOC_SWTEST_GET_SWITCHES_COUNT:
@@ -701,6 +705,10 @@ static int rtswitch_ioctl_rt(struct rtdm_fd *fd,
 				    arg,
 				    sizeof(fromto));
 
+		if (fromto.switch_mode) {
+			xnthread_relax(0, 0);
+			return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
+		}
 		return rtswitch_to_rt(ctx, fromto.from, fromto.to);
 
 	case RTTST_RTIOC_SWTEST_GET_LAST_ERROR:
diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
index 12516186b..490e39f1f 100644
--- a/testsuite/switchtest/switchtest.c
+++ b/testsuite/switchtest/switchtest.c
@@ -358,6 +358,8 @@ static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
 	switch_to_primary_mode,
 };
 
+#define MODE_SWITCHES_KERNEL 2
+
 static void *sleeper_switcher(void *cookie)
 {
 	struct task_params *param = (struct task_params *) cookie;
@@ -377,6 +379,7 @@ static void *sleeper_switcher(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -517,6 +520,7 @@ static void *rtup(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -598,6 +602,7 @@ static void *rtus(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -679,6 +684,7 @@ static void *rtuo(void *cookie)
 		clean_exit(EXIT_FAILURE);
 	}
 
+	rtsw.switch_mode = false;
 	rtsw.from = param->swt.index;
 	to = param->swt.index;
 
@@ -727,6 +733,9 @@ static void *rtuo(void *cookie)
 		while (err == -1 && errno == EINTR)
 			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
 
+		// Return to default: do not switch in syscall
+		rtsw.switch_mode = false;
+
 		switch (err) {
 		case 0:
 			break;
@@ -746,8 +755,16 @@ static void *rtuo(void *cookie)
 		/* Switch between primary and secondary mode */
 		if (i % TASK_SWITCH_MODES == TASK_SWITCH_MODE) {
 			uint switch_iteration = (i / TASK_SWITCH_MODES %
-				SWITCH_FUNC_COUNT);
-			switch_funcs[switch_iteration]();
+				(SWITCH_FUNC_COUNT + MODE_SWITCHES_KERNEL));
+
+			if (switch_iteration < SWITCH_FUNC_COUNT) {
+				switch_funcs[switch_iteration]();
+			} else {
+				// Switch mode on next
+				// RTTST_RTIOC_SWTEST_SWITCH_TO syscall
+				rtsw.switch_mode = true;
+			}
+
 			mode = !mode;
 		}
 
-- 
2.34.1


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

* Re: [PATCH v2 0/3] switchtest: test different migration types
  2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
                   ` (2 preceding siblings ...)
  2022-12-05 15:21 ` [PATCH v2 3/3] switchtest: test mode switche while in kernel mode T. Schaffner
@ 2022-12-05 17:16 ` Jan Kiszka
  2022-12-05 18:29   ` Schaffner, Tobias
  2022-12-05 17:32 ` Florian Bezdeka
  4 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2022-12-05 17:16 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: florian.bezdeka

On 05.12.22 16:21, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Switchtest only tested intended switches between primary and secondary
> mode in userland.
> 
> Let the rtuo thread also test fault based migrations and migrations that
> occur while in kernel mode.
> 
> This patch series will also test the bug that was fixed in
> 829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5
> 

Does it trigger over an unpatched version?

Thanks for looking into this!

Jan

> Changes since v1:
> - Squash patches 1 and 2 aswell as 3 and 4
> - Call the correct rtswitch_to_xxx function after mode switch in the
>   switchtest driver
> - Only switch to secondary mode by using a linux syscall or by a call to
>   cobalt_thread_relax()
> 
> Tobias Schaffner (3):
>   switchtest: split the set_mode method
>   switchtest: test multiple secondary switch cases
>   switchtest: test mode switche while in kernel mode
> 
>  include/rtdm/uapi/testing.h         |  10 ++
>  kernel/drivers/testing/switchtest.c |   8 ++
>  testsuite/switchtest/switchtest.c   | 142 +++++++++++++++++++++-------
>  3 files changed, 126 insertions(+), 34 deletions(-)
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux


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

* Re: [PATCH v2 0/3] switchtest: test different migration types
  2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
                   ` (3 preceding siblings ...)
  2022-12-05 17:16 ` [PATCH v2 0/3] switchtest: test different migration types Jan Kiszka
@ 2022-12-05 17:32 ` Florian Bezdeka
  2022-12-05 21:04   ` Schaffner, Tobias
  4 siblings, 1 reply; 13+ messages in thread
From: Florian Bezdeka @ 2022-12-05 17:32 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: jan.kiszka

On Mon, 2022-12-05 at 16:21 +0100, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Switchtest only tested intended switches between primary and secondary
> mode in userland.
> 
> Let the rtuo thread also test fault based migrations and migrations that
> occur while in kernel mode.
> 
> This patch series will also test the bug that was fixed in
> 829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5
> 
> Changes since v1:
> - Squash patches 1 and 2 aswell as 3 and 4
> - Call the correct rtswitch_to_xxx function after mode switch in the
>   switchtest driver
> - Only switch to secondary mode by using a linux syscall or by a call to
>   cobalt_thread_relax()

Why has the part with a faulting syscall been removed?

Just in case my previous comment was misread: My idea was not to remove
it but to move it into the switchtest uapi, similar to the switch_mode
flag that you have introduced in patch 3/3.

Something like "trigger_fault" in addition to switch_mode should do it.
That would decouple the switchtest utility from y2038 stuff.

> 
> Tobias Schaffner (3):
>   switchtest: split the set_mode method
>   switchtest: test multiple secondary switch cases
>   switchtest: test mode switche while in kernel mode
> 
>  include/rtdm/uapi/testing.h         |  10 ++
>  kernel/drivers/testing/switchtest.c |   8 ++
>  testsuite/switchtest/switchtest.c   | 142 +++++++++++++++++++++-------
>  3 files changed, 126 insertions(+), 34 deletions(-)
> 


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

* Re: [PATCH v2 2/3] switchtest: test multiple secondary switch cases
  2022-12-05 15:21 ` [PATCH v2 2/3] switchtest: test multiple secondary switch cases T. Schaffner
@ 2022-12-05 17:44   ` Florian Bezdeka
  2022-12-08 13:53   ` Jan Kiszka
  1 sibling, 0 replies; 13+ messages in thread
From: Florian Bezdeka @ 2022-12-05 17:44 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: jan.kiszka

On Mon, 2022-12-05 at 16:21 +0100, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Switches to secondary mode do not only occur when a relax is explicitly
> called via the cobalt api but also e.g. when a linux syscall is called in
> primary mode.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 117 ++++++++++++++++++++----------
>  1 file changed, 79 insertions(+), 38 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index f8a37da79..12516186b 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -34,7 +34,9 @@
>  #include <semaphore.h>
>  #include <setjmp.h>
>  #include <getopt.h>
> +#include <asm/unistd.h>
>  #include <asm/xenomai/features.h>
> +#include <asm/xenomai/syscall.h>
>  #include <asm/xenomai/uapi/fptest.h>
>  #include <cobalt/trace.h>
>  #include <rtdm/testing.h>
> @@ -75,6 +77,14 @@ typedef enum {
>  	UFPS = 4	 /* use the FPU while in secondary mode. */
>  } fpflags;
>  
> +#define TASK_SWITCH_MODES 3
> +
> +enum task_switch_mode {
> +	TASK_SWITCH_PREVIOUS = 0,
> +	TASK_SWITCH_NEXT = 1,
> +	TASK_SWITCH_MODE = 2
> +};
> +
>  struct cpu_tasks;
>  
>  struct task_params {
> @@ -297,6 +307,57 @@ static int printout(const char *fmt, ...)
>  #define check_fp_result(__expected)	\
>  	fp_regs_check(fp_features, __expected, printout)
>  
> +static void _assert_primary_mode(char const *calling_func)
> +{
> +	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> +		fprintf(stderr,
> +			"Switch to primary mode failed in %s.",
> +			calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}
> +
> +#define assert_primary_mode() _assert_primary_mode(__func__)
> +
> +static void _assert_secondary_mode(char const *calling_func)
> +{
> +	if (!(cobalt_thread_mode() & (XNRELAX))) {
> +		fprintf(stderr,
> +			"Switch to secondary mode failed in %s.",
> +			calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}

Just a very minor thing: I (personally) would prefer to bail out if
everything is OK. Applies here and above. Simply inverse the condition.
Nice side effect: fprintf fits into one line. I guess Jan will decide
;-)

static void _assert_secondary_mode(char const *calling_func)
{
	if ((cobalt_thread_mode() & (XNRELAX)))
		return;

	fprintf(stderr, "Switch to secondary mode failed in %s.", calling_func);
	clean_exit(EXIT_FAILURE);
}


> +
> +#define assert_secondary_mode() _assert_secondary_mode(__func__)
> +
> +static void switch_to_primary_mode(void)
> +{
> +	cobalt_thread_harden();
> +	assert_primary_mode();
> +}
> +
> +static void switch_to_secondary_mode(void)
> +{
> +	cobalt_thread_relax();
> +	assert_secondary_mode();
> +}
> +
> +static void switch_to_secondary_mode_by_using_linux_syscall(void)
> +{
> +	syscall(__NR_gettid);
> +	assert_secondary_mode();
> +}
> +
> +#define SWITCH_FUNC_COUNT 4
> +
> +static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
> +	switch_to_secondary_mode,
> +	switch_to_primary_mode,
> +	switch_to_secondary_mode_by_using_linux_syscall,
> +	switch_to_primary_mode,
> +};
> +
>  static void *sleeper_switcher(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -354,13 +415,13 @@ static void *sleeper_switcher(void *cookie)
>  		if (tasks_count == 1)
>  			continue;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -440,24 +501,6 @@ static void *fpu_stress(void *cookie)
>  	return NULL;
>  }
>  
> -static void switch_to_primary_mode(void)
> -{
> -	cobalt_thread_harden();
> -	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> -		perror("Switch to primary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -
> -static void switch_to_secondary_mode(void)
> -{
> -	cobalt_thread_relax();
> -	if (!(cobalt_thread_mode() & (XNRELAX))) {
> -		perror("Switch to secondary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -
>  static void *rtup(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -493,13 +536,13 @@ static void *rtup(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -574,13 +617,13 @@ static void *rtus(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -656,13 +699,13 @@ static void *rtuo(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -679,6 +722,7 @@ static void *rtuo(void *cookie)
>  		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_regs_set(fp_features, expected);
>  		}
> +
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -700,14 +744,11 @@ static void *rtuo(void *cookie)
>  		}
>  
>  		/* Switch between primary and secondary mode */
> -		if (i % 3 == 2) {
> -			if (mode == COBALT_PRIMARY) {
> -				switch_to_secondary_mode();
> -				mode = COBALT_SECONDARY;
> -			} else {
> -				switch_to_primary_mode();
> -				mode = COBALT_PRIMARY;
> -			}
> +		if (i % TASK_SWITCH_MODES == TASK_SWITCH_MODE) {
> +			uint switch_iteration = (i / TASK_SWITCH_MODES %
> +				SWITCH_FUNC_COUNT);
> +			switch_funcs[switch_iteration]();
> +			mode = !mode;
>  		}
>  
>  		if(++i == 4000000)


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

* Re: [PATCH v2 0/3] switchtest: test different migration types
  2022-12-05 17:16 ` [PATCH v2 0/3] switchtest: test different migration types Jan Kiszka
@ 2022-12-05 18:29   ` Schaffner, Tobias
  0 siblings, 0 replies; 13+ messages in thread
From: Schaffner, Tobias @ 2022-12-05 18:29 UTC (permalink / raw)
  To: Kiszka, Jan, xenomai; +Cc: Bezdeka, Florian

On 05.12.22 18:16, Jan Kiszka wrote:
> On 05.12.22 16:21, T. Schaffner wrote:
>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>
>> Switchtest only tested intended switches between primary and secondary
>> mode in userland.
>>
>> Let the rtuo thread also test fault based migrations and migrations that
>> occur while in kernel mode.
>>
>> This patch series will also test the bug that was fixed in
>> 829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5
>>
> 
> Does it trigger over an unpatched version?
> 
> Thanks for looking into this!
> 
> Jan

Hi Jan,

yes, the patches trigger the described FPU errors when your fix is removed.

Best

>> Changes since v1:
>> - Squash patches 1 and 2 aswell as 3 and 4
>> - Call the correct rtswitch_to_xxx function after mode switch in the
>>    switchtest driver
>> - Only switch to secondary mode by using a linux syscall or by a call to
>>    cobalt_thread_relax()
>>
>> Tobias Schaffner (3):
>>    switchtest: split the set_mode method
>>    switchtest: test multiple secondary switch cases
>>    switchtest: test mode switche while in kernel mode
>>
>>   include/rtdm/uapi/testing.h         |  10 ++
>>   kernel/drivers/testing/switchtest.c |   8 ++
>>   testsuite/switchtest/switchtest.c   | 142 +++++++++++++++++++++-------
>>   3 files changed, 126 insertions(+), 34 deletions(-)
>>
> 

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

* Re: [PATCH v2 0/3] switchtest: test different migration types
  2022-12-05 17:32 ` Florian Bezdeka
@ 2022-12-05 21:04   ` Schaffner, Tobias
  0 siblings, 0 replies; 13+ messages in thread
From: Schaffner, Tobias @ 2022-12-05 21:04 UTC (permalink / raw)
  To: Bezdeka, Florian, xenomai; +Cc: Kiszka, Jan

On 05.12.22 18:32, Bezdeka, Florian (T CED SES-DE) wrote:
> On Mon, 2022-12-05 at 16:21 +0100, T. Schaffner wrote:
>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>
>> Switchtest only tested intended switches between primary and secondary
>> mode in userland.
>>
>> Let the rtuo thread also test fault based migrations and migrations that
>> occur while in kernel mode.
>>
>> This patch series will also test the bug that was fixed in
>> 829d4d33cb7aa3ee485ad56da6ac0c3b0ce5dbe5
>>
>> Changes since v1:
>> - Squash patches 1 and 2 aswell as 3 and 4
>> - Call the correct rtswitch_to_xxx function after mode switch in the
>>    switchtest driver
>> - Only switch to secondary mode by using a linux syscall or by a call to
>>    cobalt_thread_relax()
> 
> Why has the part with a faulting syscall been removed?
> 
> Just in case my previous comment was misread: My idea was not to remove
> it but to move it into the switchtest uapi, similar to the switch_mode
> flag that you have introduced in patch 3/3.
> 
> Something like "trigger_fault" in addition to switch_mode should do it.
> That would decouple the switchtest utility from y2038 stuff.
I added an additional switchtest syscall which triggers a fault in the 
way you suggested. Again this does not always trigger a migration and 
the assertion gets raised.

I would like to examine that further till I fully understand what is 
going on here. I would suggest to create a new patch series for that to 
allow this patch series with the FPU test for Jans fix to get merged 
soon. What do you think?

>>
>> Tobias Schaffner (3):
>>    switchtest: split the set_mode method
>>    switchtest: test multiple secondary switch cases
>>    switchtest: test mode switche while in kernel mode
>>
>>   include/rtdm/uapi/testing.h         |  10 ++
>>   kernel/drivers/testing/switchtest.c |   8 ++
>>   testsuite/switchtest/switchtest.c   | 142 +++++++++++++++++++++-------
>>   3 files changed, 126 insertions(+), 34 deletions(-)
>>
> 

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

* Re: [PATCH v2 1/3] switchtest: split the set_mode method
  2022-12-05 15:21 ` [PATCH v2 1/3] switchtest: split the set_mode method T. Schaffner
@ 2022-12-08 13:49   ` Jan Kiszka
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2022-12-08 13:49 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: florian.bezdeka

On 05.12.22 16:21, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Split the set_mode method in separate methods for switching to primary and
> secondary mode for improved readability.
> 
> This also fixes a wrong evaluation of the mode variable that lead to never
> checking the FPU registers in secondary mode.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 48 ++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index f2a5c1601..f8a37da79 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -440,15 +440,21 @@ static void *fpu_stress(void *cookie)
>  	return NULL;
>  }
>  
> -static void set_mode(const char *prefix, int fd, unsigned mode)
> +static void switch_to_primary_mode(void)
>  {
> -	switch (mode) {
> -	case 1:
> -		cobalt_thread_harden();
> -		return;
> +	cobalt_thread_harden();
> +	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> +		perror("Switch to primary mode failed.");
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}
>  
> -	case 2:
> -		cobalt_thread_relax();
> +static void switch_to_secondary_mode(void)
> +{
> +	cobalt_thread_relax();
> +	if (!(cobalt_thread_mode() & (XNRELAX))) {

Redundant braces around XNRELAX.

> +		perror("Switch to secondary mode failed.");
> +		clean_exit(EXIT_FAILURE);
>  	}
>  }
>  
> @@ -475,7 +481,7 @@ static void *rtup(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	set_mode("rtup", fd, 1);
> +	switch_to_primary_mode();
>  
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -556,7 +562,7 @@ static void *rtus(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	set_mode("rtus", fd, 2);
> +	switch_to_secondary_mode();
>  
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -637,8 +643,9 @@ static void *rtuo(void *cookie)
>  	   allowed when suspended in ioctl. */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	mode = 1;
> -	set_mode("rtuo", fd, mode);
> +	switch_to_primary_mode();
> +	mode = COBALT_PRIMARY;
> +
>  	do {
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
>  	} while (err == -1 && errno == EINTR);
> @@ -668,8 +675,10 @@ static void *rtuo(void *cookie)
>  		}
>  
>  		expected = rtsw.from + i * 1000;
> -		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS))
> +		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
> +		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_regs_set(fp_features, expected);
> +		}
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -682,16 +691,23 @@ static void *rtuo(void *cookie)
>  		case -1:
>  			clean_exit(EXIT_FAILURE);
>  		}
> -		if ((mode && param->fp & UFPP) || (!mode && param->fp & UFPS)) {
> +
> +		if ((mode == COBALT_PRIMARY && param->fp & UFPP) ||
> +		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_val = check_fp_result(expected);
>  			if (fp_val != expected)
>  				handle_bad_fpreg(param->cpu, fp_val);
>  		}
>  
> -		/* Switch mode. */
> +		/* Switch between primary and secondary mode */
>  		if (i % 3 == 2) {
> -			mode = 3 - mode;
> -			set_mode("rtuo", fd, mode);
> +			if (mode == COBALT_PRIMARY) {
> +				switch_to_secondary_mode();
> +				mode = COBALT_SECONDARY;
> +			} else {
> +				switch_to_primary_mode();
> +				mode = COBALT_PRIMARY;
> +			}
>  		}
>  
>  		if(++i == 4000000)

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


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

* Re: [PATCH v2 2/3] switchtest: test multiple secondary switch cases
  2022-12-05 15:21 ` [PATCH v2 2/3] switchtest: test multiple secondary switch cases T. Schaffner
  2022-12-05 17:44   ` Florian Bezdeka
@ 2022-12-08 13:53   ` Jan Kiszka
  2022-12-08 14:03     ` Jan Kiszka
  1 sibling, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2022-12-08 13:53 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: florian.bezdeka

On 05.12.22 16:21, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Switches to secondary mode do not only occur when a relax is explicitly
> called via the cobalt api but also e.g. when a linux syscall is called in
> primary mode.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  testsuite/switchtest/switchtest.c | 117 ++++++++++++++++++++----------
>  1 file changed, 79 insertions(+), 38 deletions(-)
> 
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index f8a37da79..12516186b 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -34,7 +34,9 @@
>  #include <semaphore.h>
>  #include <setjmp.h>
>  #include <getopt.h>
> +#include <asm/unistd.h>
>  #include <asm/xenomai/features.h>
> +#include <asm/xenomai/syscall.h>
>  #include <asm/xenomai/uapi/fptest.h>
>  #include <cobalt/trace.h>
>  #include <rtdm/testing.h>
> @@ -75,6 +77,14 @@ typedef enum {
>  	UFPS = 4	 /* use the FPU while in secondary mode. */
>  } fpflags;
>  
> +#define TASK_SWITCH_MODES 3
> +
> +enum task_switch_mode {
> +	TASK_SWITCH_PREVIOUS = 0,
> +	TASK_SWITCH_NEXT = 1,
> +	TASK_SWITCH_MODE = 2
> +};
> +
>  struct cpu_tasks;
>  
>  struct task_params {
> @@ -297,6 +307,57 @@ static int printout(const char *fmt, ...)
>  #define check_fp_result(__expected)	\
>  	fp_regs_check(fp_features, __expected, printout)
>  
> +static void _assert_primary_mode(char const *calling_func)
> +{
> +	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> +		fprintf(stderr,
> +			"Switch to primary mode failed in %s.",
> +			calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}
> +
> +#define assert_primary_mode() _assert_primary_mode(__func__)
> +
> +static void _assert_secondary_mode(char const *calling_func)
> +{
> +	if (!(cobalt_thread_mode() & (XNRELAX))) {
> +		fprintf(stderr,
> +			"Switch to secondary mode failed in %s.",
> +			calling_func);
> +		clean_exit(EXIT_FAILURE);
> +	}
> +}
> +
> +#define assert_secondary_mode() _assert_secondary_mode(__func__)
> +
> +static void switch_to_primary_mode(void)
> +{
> +	cobalt_thread_harden();
> +	assert_primary_mode();
> +}
> +
> +static void switch_to_secondary_mode(void)
> +{
> +	cobalt_thread_relax();
> +	assert_secondary_mode();
> +}
> +
> +static void switch_to_secondary_mode_by_using_linux_syscall(void)
> +{
> +	syscall(__NR_gettid);
> +	assert_secondary_mode();
> +}
> +
> +#define SWITCH_FUNC_COUNT 4
> +
> +static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
> +	switch_to_secondary_mode,
> +	switch_to_primary_mode,
> +	switch_to_secondary_mode_by_using_linux_syscall,
> +	switch_to_primary_mode,
> +};
> +
>  static void *sleeper_switcher(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -354,13 +415,13 @@ static void *sleeper_switcher(void *cookie)
>  		if (tasks_count == 1)
>  			continue;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -440,24 +501,6 @@ static void *fpu_stress(void *cookie)
>  	return NULL;
>  }
>  
> -static void switch_to_primary_mode(void)
> -{
> -	cobalt_thread_harden();
> -	if (cobalt_thread_mode() & (XNRELAX|XNWEAK)) {
> -		perror("Switch to primary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -
> -static void switch_to_secondary_mode(void)
> -{
> -	cobalt_thread_relax();
> -	if (!(cobalt_thread_mode() & (XNRELAX))) {
> -		perror("Switch to secondary mode failed.");
> -		clean_exit(EXIT_FAILURE);
> -	}
> -}
> -

Please locate those two already at the right spot in patch 1. Then you
don't need to move them around here which makes the patch smaller.

>  static void *rtup(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -493,13 +536,13 @@ static void *rtup(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -574,13 +617,13 @@ static void *rtus(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -656,13 +699,13 @@ static void *rtuo(void *cookie)
>  	for (;;) {
>  		unsigned expected, fp_val;
>  
> -		switch (i % 3) {
> -		case 0:
> +		switch (i % TASK_SWITCH_MODES) {
> +		case TASK_SWITCH_PREVIOUS:
>  			/* to == from means "return to last task" */
>  			rtsw.to = rtsw.from;
>  			break;
>  
> -		case 1:
> +		case TASK_SWITCH_NEXT:
>  			if (++to == rtsw.from)
>  				++to;
>  			if (to > tasks_count - 1)
> @@ -679,6 +722,7 @@ static void *rtuo(void *cookie)
>  		    (mode == COBALT_SECONDARY && param->fp & UFPS)) {
>  			fp_regs_set(fp_features, expected);
>  		}
> +
>  		err = ioctl(fd, RTTST_RTIOC_SWTEST_SWITCH_TO, &rtsw);
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
> @@ -700,14 +744,11 @@ static void *rtuo(void *cookie)
>  		}
>  
>  		/* Switch between primary and secondary mode */
> -		if (i % 3 == 2) {
> -			if (mode == COBALT_PRIMARY) {
> -				switch_to_secondary_mode();
> -				mode = COBALT_SECONDARY;
> -			} else {
> -				switch_to_primary_mode();
> -				mode = COBALT_PRIMARY;
> -			}
> +		if (i % TASK_SWITCH_MODES == TASK_SWITCH_MODE) {
> +			uint switch_iteration = (i / TASK_SWITCH_MODES %
> +				SWITCH_FUNC_COUNT);
> +			switch_funcs[switch_iteration]();
> +			mode = !mode;
>  		}
>  
>  		if(++i == 4000000)

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


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

* Re: [PATCH v2 2/3] switchtest: test multiple secondary switch cases
  2022-12-08 13:53   ` Jan Kiszka
@ 2022-12-08 14:03     ` Jan Kiszka
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2022-12-08 14:03 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: florian.bezdeka

On 08.12.22 14:53, Jan Kiszka wrote:
> On 05.12.22 16:21, T. Schaffner wrote:
>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>
>> Switches to secondary mode do not only occur when a relax is explicitly
>> called via the cobalt api but also e.g. when a linux syscall is called in
>> primary mode.
>>

Oh, and what does this change contribute /wrt enhancing test coverage?
The reasoning here is not yet clear.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


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

* Re: [PATCH v2 3/3] switchtest: test mode switche while in kernel mode
  2022-12-05 15:21 ` [PATCH v2 3/3] switchtest: test mode switche while in kernel mode T. Schaffner
@ 2022-12-08 14:08   ` Jan Kiszka
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2022-12-08 14:08 UTC (permalink / raw)
  To: T. Schaffner, xenomai; +Cc: florian.bezdeka

Subject typo: "...mode switches ..."

On 05.12.22 16:21, T. Schaffner wrote:
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Make the rtuo thread switch between primary and secondary mode while in the
> RTTST_RTIOC_SWTEST_SWITCH_TO syscall.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  include/rtdm/uapi/testing.h         | 10 ++++++++++
>  kernel/drivers/testing/switchtest.c |  8 ++++++++
>  testsuite/switchtest/switchtest.c   | 21 +++++++++++++++++++--
>  3 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/include/rtdm/uapi/testing.h b/include/rtdm/uapi/testing.h
> index f8207b8c7..c389cdc25 100644
> --- a/include/rtdm/uapi/testing.h
> +++ b/include/rtdm/uapi/testing.h
> @@ -24,6 +24,7 @@
>  #define _RTDM_UAPI_TESTING_H
>  
>  #include <linux/types.h>
> +#include <stdbool.h>
>  
>  #define RTTST_PROFILE_VER		2
>  
> @@ -71,9 +72,18 @@ struct rttst_swtest_task {
>  #define RTTST_SWTEST_USE_FPU		0x2 /* Only for kernel-space tasks. */
>  #define RTTST_SWTEST_FREEZE		0x4 /* Only for kernel-space tasks. */
>  
> +/**
> + * @brief parameter for the RTTST_RTIOC_SWTEST_SWITCH_TO syscall
> + * @anchor rttst_swtest_dir
> + *
> + * This structure is used to tell the RTTST_RTIOC_SWTEST_SWITCH_TO syscall
> + * which threads should be exchanged and if the mode (primary/secondary) of the
> + * from thread should be switched.
> + */
>  struct rttst_swtest_dir {
>  	unsigned int from;
>  	unsigned int to;
> +	bool switch_mode;
>  };
>  
>  struct rttst_swtest_error {
> diff --git a/kernel/drivers/testing/switchtest.c b/kernel/drivers/testing/switchtest.c
> index 312b4d870..e2c0f4bcd 100644
> --- a/kernel/drivers/testing/switchtest.c
> +++ b/kernel/drivers/testing/switchtest.c
> @@ -647,6 +647,10 @@ static int rtswitch_ioctl_nrt(struct rtdm_fd *fd,
>  				    arg,
>  				    sizeof(fromto));
>  
> +		if (fromto.switch_mode) {
> +			xnthread_harden();
> +			return rtswitch_to_rt(ctx, fromto.from, fromto.to);
> +		}
>  		return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
>  
>  	case RTTST_RTIOC_SWTEST_GET_SWITCHES_COUNT:
> @@ -701,6 +705,10 @@ static int rtswitch_ioctl_rt(struct rtdm_fd *fd,
>  				    arg,
>  				    sizeof(fromto));
>  
> +		if (fromto.switch_mode) {
> +			xnthread_relax(0, 0);
> +			return rtswitch_to_nrt(ctx, fromto.from, fromto.to);
> +		}
>  		return rtswitch_to_rt(ctx, fromto.from, fromto.to);
>  
>  	case RTTST_RTIOC_SWTEST_GET_LAST_ERROR:
> diff --git a/testsuite/switchtest/switchtest.c b/testsuite/switchtest/switchtest.c
> index 12516186b..490e39f1f 100644
> --- a/testsuite/switchtest/switchtest.c
> +++ b/testsuite/switchtest/switchtest.c
> @@ -358,6 +358,8 @@ static void (*switch_funcs[SWITCH_FUNC_COUNT]) (void) = {
>  	switch_to_primary_mode,
>  };
>  
> +#define MODE_SWITCHES_KERNEL 2
> +
>  static void *sleeper_switcher(void *cookie)
>  {
>  	struct task_params *param = (struct task_params *) cookie;
> @@ -377,6 +379,7 @@ static void *sleeper_switcher(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -517,6 +520,7 @@ static void *rtup(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -598,6 +602,7 @@ static void *rtus(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -679,6 +684,7 @@ static void *rtuo(void *cookie)
>  		clean_exit(EXIT_FAILURE);
>  	}
>  
> +	rtsw.switch_mode = false;
>  	rtsw.from = param->swt.index;
>  	to = param->swt.index;
>  
> @@ -727,6 +733,9 @@ static void *rtuo(void *cookie)
>  		while (err == -1 && errno == EINTR)
>  			err = ioctl(fd, RTTST_RTIOC_SWTEST_PEND, &param->swt);
>  
> +		// Return to default: do not switch in syscall

We generally avoid "//"-style comments (kernel style).

> +		rtsw.switch_mode = false;
> +
>  		switch (err) {
>  		case 0:
>  			break;
> @@ -746,8 +755,16 @@ static void *rtuo(void *cookie)
>  		/* Switch between primary and secondary mode */
>  		if (i % TASK_SWITCH_MODES == TASK_SWITCH_MODE) {
>  			uint switch_iteration = (i / TASK_SWITCH_MODES %
> -				SWITCH_FUNC_COUNT);
> -			switch_funcs[switch_iteration]();
> +				(SWITCH_FUNC_COUNT + MODE_SWITCHES_KERNEL));
> +
> +			if (switch_iteration < SWITCH_FUNC_COUNT) {
> +				switch_funcs[switch_iteration]();
> +			} else {
> +				// Switch mode on next
> +				// RTTST_RTIOC_SWTEST_SWITCH_TO syscall
> +				rtsw.switch_mode = true;

+2 iterations because we flip rt->nrt and then back to rt this way?

> +			}
> +
>  			mode = !mode;
>  		}
>  

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


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

end of thread, other threads:[~2022-12-08 14:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 15:21 [PATCH v2 0/3] switchtest: test different migration types T. Schaffner
2022-12-05 15:21 ` [PATCH v2 1/3] switchtest: split the set_mode method T. Schaffner
2022-12-08 13:49   ` Jan Kiszka
2022-12-05 15:21 ` [PATCH v2 2/3] switchtest: test multiple secondary switch cases T. Schaffner
2022-12-05 17:44   ` Florian Bezdeka
2022-12-08 13:53   ` Jan Kiszka
2022-12-08 14:03     ` Jan Kiszka
2022-12-05 15:21 ` [PATCH v2 3/3] switchtest: test mode switche while in kernel mode T. Schaffner
2022-12-08 14:08   ` Jan Kiszka
2022-12-05 17:16 ` [PATCH v2 0/3] switchtest: test different migration types Jan Kiszka
2022-12-05 18:29   ` Schaffner, Tobias
2022-12-05 17:32 ` Florian Bezdeka
2022-12-05 21:04   ` Schaffner, Tobias

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).