cpu/msp430_common: Cleanup

[maribu]

Contribution description

void cpu_switch_context_exit(void) assigns sched_active_thread just before calling sched_run(). This is unneeded, as sched_run() will updated that anyway. Also generally speaking, changing internal scheduler data from outside the scheduler is a risky thing to do.

Testing procedure

Flash and run tests/thread_basic on a MSP430 based board.

Issues/PRs references

Spun out of #11226

[maribu]

I saw that @gschorcht recently authored a PR targeting the MSP430 platform that was reviewed by @jcarrano. Maybe one of them has the hardware required to test this PR and could review this one?

[gschorcht]

@maribu We both used IoTLab WSN430 nodes for testing.

[maribu]

@gschorcht: Thanks for pointing that out. The console output I got was

RIOT MSP430 hardware initialization complete.
main(): This is RIOT! (Version: 2019.04-devel-587-g9a2f6-msp430_cleanup)
first thread

second thread

So this seems to be working as expected :-)

[kaspar030]

Looking at the code, this should be tested with enabled module "schedstatistics", with "-DSCHED_TEST_STACK" and with "#define ENABLE_DEBUG 1" in sched.c...

[kaspar030]

But I agree that if the assignment is necessary, sth is weird.

[maribu]

I ran the test using this modifications:

diff --git a/core/sched.c b/core/sched.c
index a5a8d38fb5..bcb0baaad2 100644
--- a/core/sched.c
+++ b/core/sched.c
@@ -37,7 +37,7 @@
 #include "xtimer.h"
 #endif
 
-#define ENABLE_DEBUG (0)
+#define ENABLE_DEBUG (1)
 #include "debug.h"
 
 #if ENABLE_DEBUG
diff --git a/tests/thread_basic/Makefile b/tests/thread_basic/Makefile
index 61bc51c2c4..b393045c97 100644
--- a/tests/thread_basic/Makefile
+++ b/tests/thread_basic/Makefile
@@ -3,6 +3,7 @@ include ../Makefile.tests_common
 BOARD_INSUFFICIENT_MEMORY := nucleo-f031k6
 
 DISABLE_MODULE += auto_init
+USEMODULE += schedstatistics
 
 TEST_ON_CI_WHITELIST += all
 
diff --git a/tests/thread_basic/main.c b/tests/thread_basic/main.c
index 457668095a..c3f7335b89 100644
--- a/tests/thread_basic/main.c
+++ b/tests/thread_basic/main.c
@@ -21,7 +21,7 @@
 #include <stdio.h>
 #include "thread.h"
 
-char t2_stack[THREAD_STACKSIZE_MAIN];
+char t2_stack[THREAD_STACKSIZE_MAIN + THREAD_EXTRA_STACKSIZE_PRINTF];
 
 void *second_thread(void *arg)
 {

This is the output. (Apparently the stack of thread 3 is too small of debugging, but I have no idea where that thread is created.)

RIOT MSP430 hardware initialization complete.
sched_set_status: adding thread 1 to runqueue 15.
sched_set_status: adding thread 2 to runqueue 7.
sched_run: active thread: 0, next thread: 2
sched_run: done, changed sched_active_thread.
main(): This is RIOT! (Version: 2019.04-devel-587-g9a2f6-msp430_cleanup)
sched_set_status: adding thread 3 to runqueue 6.
first thread

sched_task_exit: ending thread 2...
sched_set_status: removing thread 2 to runqueue 7.
sched_run: active thread: 0, next thread: 3
sched_run: done, changed sched_active_thread.
second thread

sched_task_exit: ending thread 3...
sched_set_status: removing thread 3 to runqueue 6.
sched_run: active thread: 0, next thread: 1
Cannot debug, stack too small

What strikes me is the active thread: 0 in the output.

[maribu]

"-DSCHED_TEST_STACK"

missed that. I'll run it again tomorrow.

[maribu]

OK, did just run it again with CFLAGS += -DSCHED_TEST_STACK:

RIOT MSP430 hardware initialization complete.
sched_set_status: adding thread 1 to runqueue 15.
sched_set_status: adding thread 2 to runqueue 7.
sched_run: active thread: 0, next thread: 2
sched_run: done, changed sched_active_thread.
main(): This is RIOT! (Version: 2019.04-devel-587-g9a2f6-msp430_cleanup)
sched_set_status: adding thread 3 to runqueue 6.
first thread

sched_task_exit: ending thread 2...
sched_set_status: removing thread 2 to runqueue 7.
sched_run: active thread: 0, next thread: 3
sched_run: done, changed sched_active_thread.
second thread

sched_task_exit: ending thread 3...
sched_set_status: removing thread 3 to runqueue 6.
sched_run: active thread: 0, next thread: 1
Cannot debug, stack too small

[benpicco]

I checked core/sched.c and verified that sched_active_thread always gets set there.
@maribu provided test results.

[maribu]

Thanks for reviewing this!