core/msg: yield after thread_flags_wake() in queue_msg()

[kaspar030]

Contribution description

Alternative version of #16751, doesn't require changing thread_yield_higher() semantics.

Without this fix, it is possible that a thread got set to "PENDING"
after being woken up by a message flag, but no scheduling gets
triggered. thread_flag_wake() (called by queue_msg()) does set sched_context_switch_request=1, which in all other cases
in msg.c will be used to conditionally call thread_yield_higher().
This PR adds the missing case in _msg_send().

Testing procedure

Issues/PRs references

#16751

[kaspar030]

@jia200x could you run your test on this one?

[kaspar030]

/opt/gcc-arm-none-eabi-9-2019-q4-major/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: /tmp/dwq.0.3668517265360921/a876dae8bca725c53c4e6673b74d694b/build/tests_gnrc_sixlowpan_frag_minfwd.elf section `.relocate' will not fit in region `rom'
/opt/gcc-arm-none-eabi-9-2019-q4-major/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: region `rom' overflowed by 4 bytes

Ouch. :)

[jia200x]

@jia200x could you run your test on this one?

Sure! Let's see

[jia200x]

tests/gnrc_netif passes, but I get many of these warnings:

RIOT/tests/gnrc_netif/bin/native/tests_gnrc_netif.elf: thread_yield_higher: interrupts are disabled - this should not be

[benpicco]

I think #16754 should fix this

[kaspar030]

tests/gnrc_netif passes, but I get many of these warnings:

@jia200x was it just make -Ctests/gnrc_netif all test? Then the warnings are gone. Maybe we tested this before #16754 was in, or with this PR not rebased? (I rebased it now)

[jia200x]

tests/gnrc_netif passes, but I get many of these warnings:

@jia200x was it just make -Ctests/gnrc_netif all test? Then the warnings are gone. Maybe we tested this before #16754 was in, or with this PR not rebased? (I rebased it now)

This is triggered when using #16748. I can rebase again and see if the warnings are gone

[kaspar030]

This is triggered when using #16748. I can rebase again and see if the warnings are gone

I cloned #16748, rebased to master, merged this PR and ran the tests/gnrc_netif tests on native, and don't see that warning.

[jia200x]

I cloned #16748, rebased to master, merged this PR and ran the tests/gnrc_netif tests on native, and don't see that warning.

Then I guess it's fixed

[kaspar030]

@OlegHahm should we backport this?

[OlegHahm]

To be honest, I'm somewhat reluctant to merge a kernel patch that late in the process. On the other hand a kernel bug is a kernel bug... Under which circumstances will the described bug occur?

[kaspar030]

Under which circumstances will the described bug occur?

It occurs if a thread sits waiting for multiple flags including THREAD_FLAG_MSG_WAITING (using thread_flags_wait_*()), then gets sent a message. The thread will end up marked PENDING and in the right runqueue, but if the message was sent from thread context, the task switch is not triggered. probably something else happens soon (any interrupt will do) and the scheduler gets called, but until then, the "highest priority runnable thread is always run" invariant is not honored.