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.

[richard-damon]

My understanding is that BaseType_t is intended to normally be a type that is a "natural" size for the processor, and thus normally atomically accessible. But, it is possible that it might not be atomic.

First thought is that something like portTICK_TYPE_ENTER_CRITICAL could be defined (that defaults to using a critical section) could be defined such that ports that know that accesses to BaseType_t can be done without a critical section could do so.

So, all the task{ENTER|EXIT}CRITICAL calls around a single BaseType_t access could be changed to a portBASETYPE{ENTER|EXIT}_CRITICAL call, that defaults to being defined to just be the critical section, but then the major ports that don't need it can define those operations away.

[n9wxu]

@GuilhermeGiacomoSimoes did you raise a PR so we can review your changes?