Minimize the use of the taskENTER_CRITICAL and taskEXIT_CRITICAL

[n9wxu]

The following forum discussion points out a few instances where a critical section is wrapping a write to a variable of BaseType_t. BaseType_t is intended to be a datatype that the architecture will access atomically which should eliminate the need for a critical section.

https://forums.freertos.org/t/seemingly-excessive-use-of-taskenter-critical-and-taskexit-critical-macros-in-the-kernel/20044

The feature request is to remove the excess critical sections identified in the forum post. Please use one PR per "fixed" file to simplify code review and/or roll-back as changes to the critical sections can be tricky to test and review.

[GuilhermeGiacomoSimoes]

@n9wxu I seing this discussion on forum, and i have three doubt:

1) How I can see, the BaseType_t is a type that is hardware independent, and so, the BaseType_t is not guarantee that is running in atomic context. If the BaseType_t is not guarantee that is running on atomic context, the taskENTER_CRITICAL() is required because this not know, of course, if is running in atomic context.

2) if exists two tasks executing in atomic context, in a cpu multi-core, I think that the taskNETER_CRITICAL() is required for access control in critical region.

3) Why the 8-bits is relationship with the problmen ? Why the 8bits architecture can create a problemn in execute atomic tasks?

I'm not an expert in FreeRTOS, but I can help with this simple task.

I modified this files queue.c tasks.c timers.c Removing the taskENTER_CRITIAL().

The tests make in AVR architectures (that is 8bits) looks like OK. I compile the kernel using the Demos and running it in qemu.

example the compile in AVRATMega_323_WinAVR

Size before:
rtosdemo.elf  :
section                      size      addr
.text                       13510         0
.data                          80   8388704
.bss                         1671   8388784
.comment                       18         0
.note.gnu.avr.deviceinfo       60         0
.debug_aranges                456         0
.debug_info                 25963         0
.debug_abbrev                6628         0
.debug_line                 33922         0
.debug_frame                 3860         0
.debug_str                   6480         0
.debug_line_str               851         0
.debug_loclists             14248         0
.debug_rnglists               431         0
Total                      108178

Size after:
rtosdemo.elf  :
section                      size      addr
.text                       13510         0
.data                          80   8388704
.bss                         1671   8388784
.comment                       18         0
.note.gnu.avr.deviceinfo       60         0
.debug_aranges                456         0
.debug_info                 25963         0
.debug_abbrev                6628         0
.debug_line                 33922         0
.debug_frame                 3860         0
.debug_str                   6480         0
.debug_line_str               851         0
.debug_loclists             14248         0
.debug_rnglists               431         0
Total                      108178

Errors: none

I really think that I need execute more specific test. I could not execute unit tests, if any body can help me, i thanks.

[GuilhermeGiacomoSimoes]

@n9wxu I was study about this BaseType_t and I can see that this can has any type, like: int, char, and this storage a states or result functions of the OS.

Then, I think that the BaseType_t when the OS is running in 8bit cpu , if BaseType_t is bigger than 8 bit (like type int, usually 16 bits), we don't access this is atomic context because this types is don't atomic in this CPUs.

But, that said, I'm unable to think why this change (remove task{ENTER|EXIT}_CRITICAL()) can be a good idea.