Concerns about the atomicity of vTaskSuspendAll()

~~~~~~ vTaskSuspendAll() ~~~~~~ is used to lock taskswitches without disabling interrupts. Pretty good. But this function works by just incrementing a global variable ~~~~~~ ++uxSchedulerSuspended; ~~~~~~ On many platforms including AVR and ARM Cortex M this operation compiles to something like ~~~~~~ load uxSchedulerSuspendeded into register increment register store register into uxSchedulerSuspendeded ~~~~~~ which is obviously NOT ATOMIC. Therefore the variable uxSchedulerSuspendeded may get a wrong value which can lead to unintended unlocking of the kernel with catastrophic consequences. I suggest to modify the locking function like this ~~~~~~ void vTaskSuspendAll( void ) { portATOMIC_INCREMENT( uxSchedulerSuspended); } ~~~~~~ and to implement a macro portATOMIC_INCREMENT() for all platforms that lack an atomic increment machine instruction.

Concerns about the atomicity of vTaskSuspendAll()

Well he sure might be right. I suppose there is only such a thing as an atomic read. Not an atomic read-increment-write to a global. Native size or not. Does this need to be fixed?

Concerns about the atomicity of vTaskSuspendAll()

When I modify globals I use something like this ~~~~ void FlagClear(int i) { // safely clear a flag vPortEnterCritical(); // disable FreeRTOS interrupts Flags &= ~i; // clear the flag vPortExitCritical(); // put interrupts back like they were } ~~~~ Of course that has the problem of Enter and Exit not working right…

Concerns about the atomicity of vTaskSuspendAll()

Sorry not to answer this sooner – I’ve been thrown out somewhat by the forum software being updated and then having to re-register to get notifications of people posting. This topic has been discussed a few times, you should be able to find it in the archive on the FreeRTOS.org website. Basically the key to this is that each task maintains its own context, and a context switch cannot occur if the variable is non zero. So, as long as the writing from the register back into the memory is atomic, it is not a problem. In your example, consider the following scenario, which starts with uxSchedulerSuspended at zero.
/* This load stores the variable's value in a register. */
load uxSchedulerSuspendeded into register

 ... Now a context switch causes another task to run, and
the other task uses the same variable.  The other task 
will see the variable as zero because the variable has 
not yet been updated by the original task... Eventually 
the original task runs again.  **That can only happen 
when uxSchedulerSuspended is once again zero**, and when 
the original task runs again the contents of the CPU 
registers are restored to exactly how they were when it 
was switched out - therefore the value it read into the 
register still matches the value of the 
xuSchedulerSuspended variable ... 

/* The value will be incremented to be equal to 1. */
increment register

/* The value restored to uxSchedulerSuspended will be the 
correct value of 1, even though the variable was used 
by other tasks in the mean time. */
store register into uxSchedulerSuspendeded
Regards.

Concerns about the atomicity of vTaskSuspendAll()

Thanks Richard. Okay. That explains it. This particular variable ‘self protects’ itself.

Concerns about the atomicity of vTaskSuspendAll()

YUP – got it … good news! uxSchedulerSuspended is binary, not counting because of it’s special use. Especially it is not used whithin one of the ..fromISR functions. So everything is fine. Thank you Richard!