core: provide dummy implementation of thread and mutex for riotboot

[benpicco]

Contribution description

Riotboot does not enter thread mode.
It can still be desirable to use periph code in this environment, e.g. SPI drivers that make use of mutex.
Since there are no threads, the implementation can be very simple: Busy wait on lock with the expectation to be un-locked from e.g. the TX complete ISR.

mtd_spi_nor nakes use of thread_yield() so provide a dummy implementation of that which is simple a no-op.

Testing procedure

Add

DISABLE_MODULE += core_thread

to some single-threaded tests. You might have to bump ISR_STACKSIZE.

Issues/PRs references

split off #17379

[chrysn]

I think this is split out from #17379 and not 17341.

[fjmolinas]

There are some uncrustify comments, maybe @maribu could take a look at this one?

[maribu]

I would like to see some safeguards, e.g. a static_assert() on max kernel threads being 1 in the fallback implementations of mutex_lock() and thread_yield() so that dropping core_mutex and core_thread really fails to compile unless one is doing something like RIOT boot.

I think @kaspar030 has a strong opinion on approaches like this. I personally do like this, though ;)

[kaspar030]

I think @kaspar030 has a strong opinion on approaches like this. I personally do like this, though ;)

If this is like, +26 lines as in this PR, I think it is ok.
But, riotboot can't just use the regular code but "MAXTHRADS=1"?

[jue89]

Ah sweet, I like this. We removed mutexes for RIOTBOOT, as well. ROM was tight and our hack was required to fit our RIOTBOOT in 8kB.

[kaspar030]

Ah sweet, I like this. We removed mutexes for RIOTBOOT, as well. ROM was tight and our hack was required to fit our RIOTBOOT in 8kB.

oooooooooook. ;)

[jue89]

Ah sweet, I like this. We removed mutexes for RIOTBOOT, as well. ROM was tight and our hack was required to fit our RIOTBOOT in 8kB.

oooooooooook. ;)

Yeah, we are using an external SPI NOR flash to copy over new app images. This riotboot_mtd (if we apply the common name schema that arose during the last month) is still on my list of stuff to be upstreamed.

[fjmolinas]

I would like to see some safeguards, e.g. a static_assert() on max kernel threads being 1 in the fallback implementations of mutex_lock() and thread_yield() so that dropping core_mutex and core_thread really fails to compile unless one is doing something like RIOT boot.

I think @kaspar030 has a strong opinion on approaches like this. I personally do like this, though ;)

I think @maribu suggestion on providing static asserts is still missing.

[benpicco]

Added the static_assert

[maribu]

Looks good to me, please squash.

One last bike-shedding item: Does it really make sense to make core_mutex a module? I'd argue that just makeing core_thread a module and simply providing optimised mutex implementations for the case with and without core_thread would reflect the behaviour better. After all, the mutex implementation would still work fully as expected in either case.

[benpicco]

Well the dummy mutex still works with threads enabled if there is only a single thread (that's how I tested it with a modified tests/buttons).

Since SUBMODULES is set to 1 there is an implicit core_mutex module anyway that we have to disable in order to avoid compiling mutex.c, so it felt most natural to do it that way.

[maribu]

Since SUBMODULES is set to 1 there is an implicit core_mutex module anyway that we have to disable in order to avoid compiling mutex.c, so it felt most natural to do it that way.

I understand that, but this seems to be an implementation detail to me. From the users point of view the mutex is always provided and always fulfills the API agreements.

Maybe it would be better to haven an #if MAXTHREADS < 2 ... #else in the mutex code? (With the C file effectively being empty now in case of MAXTHREADS < 2 - but I still hope to have all builds except maybe native with LTO=1, so that we can get rid of all static inline stuff.)

[benpicco]

Yes this works well.
The upside is that now to test this, just add

DISABLE_MODULE += core_thread

to some simple examples, e.g. for the riotboot use case tests/vfs_default with CFLAGS += -DISR_STACKSIZE=2048

[maribu]

ACK, code looks good and I am trusting your test results.

I really like this one!

[fjmolinas]

ACK

[fjmolinas]

ACK