improvement suggestions for FreeRTOS v4.7.0
Hi Richard,
First of all thanks a lot for FreeRTOS. It is really a very efficient OS for small embedded systems.
I’d like to suggest you some improvements for FreeRTOS.
1. If I compile FreeRTOS v4.7.0 by RVDS v2.2 for an ARM9, it produces the following warnings:
compiling ..srcfreertosqueue.c…
"..srcFreeRTOSQUEUE.C", line 726: Warning: #191-D: type qualifier is meaningless on cast type
vTaskPriorityInherit( ( void * const ) pxQueue->pxMutexHolder );
^
"..srcFreeRTOSQUEUE.C", line 904: Warning: #191-D: type qualifier is meaningless on cast type
vTaskPriorityDisinherit( ( void * const ) pxQueue->pxMutexHolder );
Please, remove the unnecessary "const" identifier in the next version.
2. The configuration interface of the current implementation of FreeRTOS requires the following
definition: configMINIMAL_STACK_SIZE …
It would be great if xTaskCreate() ensures the minimal stack usage for any task. The current
implementation of FreeRTOS accepts any stack size.
3. The FreeRTOS doesn’t provide any stack check during the runtime, what is very simple to implement.
With each new version of FreeRTOS I have to patch tasks.c to implement the stack check procedure. I
appreciate it very much if you provide this functionality in the next version of FreeRTOS.
My implementation of stack checking is:
– it is important that the current task doesn’t corrupt its own TCB (we want to know which task causes
problems) => needs modification of prvAllocateTCBAndStack() and prvDeleteTCB()
– the stack and TCB must be allocated in one block:
HEAP: actual impl.: stack check impl.:
0x00 | … | | … |
+————–+ |+————+|
| TCB | || STACK ||
+————–+ |+————+|
| STACK | || TCB ||
+————–+ |+————+|
0xFF | … | | … |
– after CONTEXT_SAVE a stack check function is called => needs infos about the current task
(stack bottom, top of stack, task name for diagnostics)
– the stack is initialized by 0xA5, this value can be used to perform fast stack check
– the OS core should provide read-only access to TCB to get the stack properties
Maxim
========================================================================================================
Listing:
signed portBASE_TYPE xTaskCreate( pdTASK_CODE pvTaskCode, const signed portCHAR * const pcName, unsigned portSHORT usStackDepth, void *pvParameters, unsigned portBASE_TYPE uxPriority, xTaskHandle *pxCreatedTask )
{
signed portBASE_TYPE xReturn;
tskTCB * pxNewTCB;
#if ( configUSE_TRACE_FACILITY == 1 )
static unsigned portBASE_TYPE uxTaskNumber = 0;
#endif
/* ensure the user has specified at least the minimal stack size for the new task*/
if (usStackDepth < configMINIMAL_STACK_SIZE)
{
usStackDepth = configMINIMAL_STACK_SIZE;
}
…
}
static tskTCB *prvAllocateTCBAndStack( unsigned portSHORT usStackDepth )
{
tskTCB* pxNewTCB = NULL;
portSTACK_TYPE* pxMemBlock;
/* Allocate space for the stack and TCB used by the task being created. */
pxMemBlock = pvPortMalloc( ((size_t)usStackDepth)*sizeof(portSTACK_TYPE) + sizeof( tskTCB ) );
if( pxMemBlock != NULL )
{
//place the TCB behind the stack, needed for stack check!
//the actual task must not corrupt its own TCB in case of stack overflow
pxNewTCB = (tskTCB*)(pxMemBlock + usStackDepth);
//set the bottom of the stack:
pxNewTCB->pxStack = pxMemBlock;
/* Just to help debugging. */
(void)memset( pxMemBlock, tskSTACK_FILL_BYTE, usStackDepth * sizeof( portSTACK_TYPE ) );
}
return pxNewTCB;
}
static void prvDeleteTCB( tskTCB *pxTCB )
{
/* Free up the memory allocated by the scheduler for the task. It is up to
the task to free any memory allocated at the application level. */
vPortFree( pxTCB->pxStack );
}
/// implements stack check. this function is called after "context save" to
/// to check if the actual task produces stack overflow.
/// in case of stack overflow, or stack usage grater than 75%, the program
/// execution stops at a breakpoint, in release mode an exception is raised.
///
/// param ppvCurrentTCB current task control block reference
/// Note tskTCB_DEBUG is a copy of tskTCB, which is defined in task.c,
/// well-defines access function would be much better
void vTaskStackCheck(void** ppvCurrentTCB)
{
//get the pointer to the actual task control block:
tskTCB_DEBUG* pxTCB = (tskTCB_DEBUG*)(*ppvCurrentTCB);
//calculate the lowest alowed stack address
unsigned long* pulStackBottom = pxTCB->pxStack;
unsigned long StackDepth = (unsigned long)&pxTCB->pxTopOfStack
– (unsigned long)pxTCB->pxStack;
//set the pointer to 25% location (75% stack usage):
unsigned long* pulStackCritical = pulStackBottom + (StackDepth /4 / 4);
if ( (pxTCB->pxTopOfStack < pxTCB->pxStack)
|| (STACK_INIT_VALUE != *pulStackBottom))
{
//ALARM!!! Stack overflow! The memory is inconsistent!
#ifdef NDEBUG
raise(30);
#else
__breakpoint(0);
#endif
for(;;);
}
if (STACK_INIT_VALUE != *pulStackCritical)
{
//WARNING!!! Critical stack usage > 75% !
#ifndef NDEBUG
__breakpoint(0);
#endif
}
}
improvement suggestions for FreeRTOS v4.7.0
> First of all thanks a lot for FreeRTOS. It is really a very
> efficient OS for
> small embedded systems.
Thanks.
> I’d like to suggest you some improvements for FreeRTOS.
Sure – I’m always interested in users comments.
> 1. If I compile FreeRTOS v4.7.0 by RVDS v2.2 for an ARM9, it
> produces the following
> warnings:
I’m a bit stuck with RVDS as I only have an eval license. I would appreciate you sending me you project so I can hold it on file for others wishing to use RVDS. Currently I only have a SAM7 project with this compiler.
> compiling ..srcfreertosqueue.c…
> "..srcFreeRTOSQUEUE.C", line 726: Warning: #191-D: type
> qualifier
> is meaningless on cast type
> vTaskPriorityInherit( ( void * const ) pxQueue->pxMutexHolder );
> ^
> "..srcFreeRTOSQUEUE.C", line 904: Warning: #191-D: type
> qualifier
> is meaningless on cast type
> vTaskPriorityDisinherit( ( void * const )
> pxQueue->pxMutexHolder );
>
> Please, remove the unnecessary "const" identifier in the
> next version.
I’m afraid this annoyance is a consequence of using so many different compilers to compile the same code. If I remove the const then other compilers will generate a warning that the the types differ. Some compilers complain with the const/volatile qualifier, some complain without it. As this warning is benign I would suggest just switching it off.
>
> 2. The configuration interface of the current implementation
> of FreeRTOS requires
> the following
> definition: configMINIMAL_STACK_SIZE …
> It would be great if xTaskCreate() ensures the minimal
> stack usage for any
> task. The current
> implementation of FreeRTOS accepts any stack size.
The problem here is actually the name of the constant. It would be better named configIDLE_TASK_STACK_SIZE – as this is the only place the const is actually used in the code. If the idle task does nothing then the value I set in the demo can be used. If you add an idle task hook then you might need to increase it.
The demo tasks also use the definition for convenience as the demos get built for lots of different targets, but the demos are not actually part of the kernel itself.
> 3. The FreeRTOS doesn’t provide any stack check during the
> runtime, what is
> very simple to implement.
> With each new version of FreeRTOS I have to patch tasks.c
> to implement the
> stack check procedure. I
> appreciate it very much if you provide this functionality
> in the next version
> of FreeRTOS.
> My implementation of stack checking is:
<implementation snipped>
SafeRTOS performs a stack check prior to the context being saved, so the context saving itself will not corrupt anything. How this is done is port specific as this is into the portable layer. My intention for FreeRTOS.org is to add a stack overflow check in the common code – tasks.c (outside of the port layer) – this will catch the error of a stack being in an overflowed state but will not prevent the context save from actually being the cause of the overflow. This allows useful error reporting while not actually providing an error recovery mechanism (other than halting the system). I see your solution is a bit more sophisticated with a warning level.
Regards.