I'm trying to port some code I've used on many other systems/OSes. It uses critical sections to safeguard manipulating critical data. When I look at the Teensy 3 code it looks like the code simply uses __disable_irq and __enable_irq to surround critical sections. The problem is that those functions unconditionally disable and reenable interrupts. The problem comes up if one piece of code has a critical section in it so calls __disable_irq before calling some other function from inside its critical section that also has a critical section. When that nested function calls __enable_irq at the end of its critical section it would allow interrupts again even though the code was still executing inside the critical section of the calling function. That would be wrong and could cause problems that were very hard (actually almost impossible) to hunt down since the outer critical section code would no longer be protected from interrupts. Without looking at every use of __disable/enable_irq in the Teensy software and all the libraries I don't know if there is an actual problem due to this issue but I worry there could be. And it's an issue for the code I'm writing where I want to implement event logging to trace all the operations of a program running on the Teensy so I can figure out why I'm getting hard faults when using low power modes.
When I've written Cortex M3 code I've used critical section code that was:
Looking at the Freescale MQX RTOS for the processor used on the T3 I see they use a similar but even more complex implementation to account for nesting in their mqx_prv.h file. They seem to be keeping track of different status register values to maybe maintain different task priority levels. But that's something we don't need with this Teensy 3 code. Their code is:
Since I know bugs caused by incorrect handling of nested critical sections can be hard to handle I'm just wondering if anyone (Paul?) has checked the use of the critical sections already in the code to make sure there is no nesting. I think for my code I'll just go back to using the same code I used on the M3 processors. Just wanted to provide others a head's up if they start writing critical section code that has any nested calls in it. (I do realize critical sections should be kept very short but I have often seen cases where they make function calls that can cause these problems and I have been burnt by these types of problems before requiring much wasted debug time; typically fixing someone else's code).
When I've written Cortex M3 code I've used critical section code that was:
Code:
static int lock_nesting_count; /// Tracks nested critical sections
/**
* Enter a critical section in way that supports nested calls.
*
* @details
* This function can be called to allow code to enter a critical section.
* It differs from the CMSIS function __disable_irq() (which it uses) in that
* it supports nested calls. See file header for more info.
*
* Critical sections entered by a call to this function should be exited
* by a call to the core_critical_exit() function defined below.
*/
void
core_critical_enter(void)
{
__disable_irq();
++lock_nesting_count;
}
/**
* Exit critical section and enable interrupts if needed.
*
* @details
* This function can be called to exit a critical section that was entered
* by a call to the core_critical_enter() function. This function should be
* used in place of the __enable_irq() function because it correctly handles
* nested calls.
*/
void
core_critical_exit(void)
{
/* Unlock interrupts only when we are exiting the outermost nested call. */
--lock_nesting_count;
if (lock_nesting_count == 0) {
__enable_irq();
}
}Looking at the Freescale MQX RTOS for the processor used on the T3 I see they use a similar but even more complex implementation to account for nesting in their mqx_prv.h file. They seem to be keeping track of different status register values to maybe maintain different task priority levels. But that's something we don't need with this Teensy 3 code. Their code is:
Code:
#if MQX_USE_INTERRUPTS
#define _INT_DISABLE_CODE() \
if (kernel_data->ACTIVE_PTR->DISABLED_LEVEL == 0) { \
_PSP_SET_DISABLE_SR(kernel_data->DISABLE_SR); \
} /* Endif */ \
++kernel_data->ACTIVE_PTR->DISABLED_LEVEL;
#define _INT_ENABLE_CODE() \
if (kernel_data->ACTIVE_PTR->DISABLED_LEVEL) { \
if (--kernel_data->ACTIVE_PTR->DISABLED_LEVEL == 0) { \
if (kernel_data->IN_ISR) { \
_PSP_SET_ENABLE_SR(kernel_data->INTERRUPT_CONTEXT_PTR->ENABLE_SR); \
} else { \
_PSP_SET_ENABLE_SR(kernel_data->ACTIVE_SR); \
} /* Endif */ \
} /* Endif */ \
} /* Endif */
#else
#define _INT_DISABLE_CODE()
#define _INT_ENABLE_CODE()
#endifSince I know bugs caused by incorrect handling of nested critical sections can be hard to handle I'm just wondering if anyone (Paul?) has checked the use of the critical sections already in the code to make sure there is no nesting. I think for my code I'll just go back to using the same code I used on the M3 processors. Just wanted to provide others a head's up if they start writing critical section code that has any nested calls in it. (I do realize critical sections should be kept very short but I have often seen cases where they make function calls that can cause these problems and I have been burnt by these types of problems before requiring much wasted debug time; typically fixing someone else's code).
Last edited:
