core/thread_flags: add thread flags group

[derMihai]

Contribution description

Adds thread flags group overlay.

From core/include/thread_flags_group.h:

A thread flags group allows setting thread flags for an arbitrary number of threads (called waiters) at the same time. The waiters can join and leave the group at any time. An additional advantage is that the signaler (the "flags setter") is de-coupled from the list of waiters, i.e. it does not need to know which specific thread must be signaled.

Example (waiter):

static tfg_t group = THREAD_FLAGS_GROUP_INIT;

// ...
 
tfg_entry_t entry;
thread_flags_group_join(&group, &entry);
 
while (!some_condition_is_met()) {
    thread_flags_wait_any(SOME_FLAG);
}

thread_flags_group_leave(&group, &entry);

Example (signaler):

fulfill_some_condition();
thread_flags_group_set(&group, SOME_FLAG);

Testing procedure

Run the test application on:

• native
• samd20-xpro

Issues/PRs references

This construct was suggested by @kaspar030 as alternative for wait queues in #21123

[maribu]

PR adds new feature to core --> 2 ACKs required.

Feel free to squash right away.

[kaspar030]

Why not a bit field for the pids that want to be group members?

[derMihai]

Why not a bit field for the pids that want to be group members?

We only expect a handful of waiters. In that case I guess it's probably faster than iterating the bitfield? On the other hand joining and especially leaving would probably be faster with a bitfield...

I'll prepare an alternative impl with bitfields.

[derMihai]

Why not a bit field for the pids that want to be group members?

I pushed a bitfield version. I'm explicitly not using the RIOT's bitfield implementation as it forces some bit ordering which I don't care about, and as result doesn't use the atomics in atomic_utils.h, which are a nice optimization.

[derMihai]

Any updates here?

[riot-ci]

Murdock results

:heavy_check_mark: PASSED

eb3c8c3f3f8c90e0851017bb3b7b6fd417e72527 core/thread_flags: add thread flags group

Success	Failures	Total	Runtime
10405	0	10405	12m:12s

Artifacts

• Documentation preview

[maribu]

UINT_WIDTH is not provided by the GCC 5.X series limits.h. GCC 7.X, to which the AVR tool has been updated in the recent Ubuntu package, does provide it.

IMO we should avoid cluttering core with workarounds for shitty historic toolchains that some distros keep shipping. Instead, we should IMO add the workaround to AVR.

I think something along the lines of:

diff --git a/cpu/avr8_common/Makefile.include b/cpu/avr8_common/Makefile.include
index 295bdea2aa..006c103e5b 100644
--- a/cpu/avr8_common/Makefile.include
+++ b/cpu/avr8_common/Makefile.include
@@ -12,3 +12,8 @@ LINKFLAGS += -Wl,--defsym=flash_printf=printf_P
 LINKFLAGS += -Wl,--defsym=flash_fprintf=fprintf_P
 LINKFLAGS += -Wl,--defsym=flash_snprintf=snprintf_P
 include $(RIOTMAKE)/arch/avr8.inc.mk
+
+# work around for shitty Ubuntu/Debian AVR toolchain. This can be dropped
+# when all commonly used versions of Ubuntu/Debian ship less historic
+# versions of GCC.
+CFLAGS += -DUINT_WIDTH=16

should work.

[derMihai]

UINT_WIDTH is not provided by the GCC 5.X series limits.h. GCC 7.X, to which the AVR tool has been updated in the recent Ubuntu package, does provide it.

IMO we should avoid cluttering core with workarounds for shitty historic toolchains that some distros keep shipping. Instead, we should IMO add the workaround to AVR.

I think something along the lines of:

diff --git a/cpu/avr8_common/Makefile.include b/cpu/avr8_common/Makefile.include
index 295bdea2aa..006c103e5b 100644
--- a/cpu/avr8_common/Makefile.include
+++ b/cpu/avr8_common/Makefile.include
@@ -12,3 +12,8 @@ LINKFLAGS += -Wl,--defsym=flash_printf=printf_P
 LINKFLAGS += -Wl,--defsym=flash_fprintf=fprintf_P
 LINKFLAGS += -Wl,--defsym=flash_snprintf=snprintf_P
 include $(RIOTMAKE)/arch/avr8.inc.mk
+
+# work around for shitty Ubuntu/Debian AVR toolchain. This can be dropped
+# when all commonly used versions of Ubuntu/Debian ship less historic
+# versions of GCC.
+CFLAGS += -DUINT_WIDTH=16

should work.

Looks like other compilers (including the ARM gcc) don't have this either...

[maribu]

Someone (tm) really should fix the CI image...

I wonder why it is different for legacy ARM and Cortex M. On Alpine, we are using the same GCC for all ARM systems.

[derMihai]

I added a fallback define, my ARM toolchain on Ubuntu 22.04 doesn't have UINT_WIDTH either.

[derMihai]

Now only a

$ make generate-Makefile.ci -C tests/core/thread_flags_group

is missing to get it past the CI :)

I also managed to shrink the RAM usage to widen the coverage.

[derMihai]

@kaspar030 I think I addressed your suggestions, can we merge this?

[derMihai]

@crasbe sorry I had a really busy week. I think I addressed your suggestions!