From ddc89fa9851a85b1d5dfe40bfa9b84b5c1e514df Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Tue, 28 Nov 2023 07:40:11 -0500 Subject: [PATCH 01/14] POSIX - Switch from posix timers to a timer thread to fix signal handling with non-FreeRTOS pthreads Improve upon the elegant approach of using signals to cause task/pthreads suspension and scheduler execution by using directed signals. This fixes: - Deadlocks in non-FreeRTOS pthreads - Multiple FreeRTOS tasks(pthreads) incorrectly running at the same time By directing the signals using pthread_kill() the signal handler in the presently running FreeRTOS task/pthread will be called, ensuring that the scheduler runs both in the context of a FreeRTOS task/pthread and from the presently executing FreeRTOS task/pthread. Details ============== The POSIX port uses signals to preempt FreeRTOS tasks (implemented as pthreads), a very neat and elegant approach to forcing tasks/pthreads to suspend and run the scheduler. Signal handlers are process global. Posix timers generate signals when the timer expires, and the signal is sent to the currently running pthread. In systems where there are pthreads that are NOT a result of creating FreeRTOS tasks, such as the entry point thread that calls main(), or user created pthreads, this poses a serious issue. While the POSIX port only allows a single FreeRTOS pthread to run at once, by causing all suspended threads to not be scheduled due to their waiting on a pthread condition variable, this isn't the case with non-FreeRTOS pthreads. Thus it is possible that a non-FreeRTOS pthread is running when the timer expires and the signal is generated. This results in the signal handler running in the non-FreeRTOS thread. The sequence of events results in these events from signal handler context: - vPortSystemTickHandler() being called - The scheduler running - Selecting another FreeRTOS task to run and switching the active task - The newly selected task released from suspension by pthread_cond_signal() - The presently active thread calling event_wait() - The pthread calling pthread_cond_wait(), suspending the thread and allowing the host OS scheduler to schedule another thread to run. If this occurs from a non-FreeRTOS thread this results in: - The active FreeRTOS pthread (Task A/Thread A) continuing to run (as the signal handler that calls event_wait() ran instead in a non-FreeRTOS pthread. - The pthread where the signal handler did run (Thread B) will call event_wait() and pthread_cond_wait(), but on the condition variable of the previously active FreeRTOS task, oops. This causes the non-FreeRTOS pthread to block unexpectedly relative to what the developer might have expected. - The newly selected FreeRTOS Task (Task C/Thread C) will resume and start running. At this point Task A/Thread A is running concurrently with Task C/Thread C. While this may not necessarily be an issue, it does not replicate the expected behavior of a single Task running at once. Note that Thread B will resume if/when Task A/ThreadA is switched to. However, this could be delayed by an arbitrary amount of time, or could never occur. Also note that if there are multiple non-FreeRTOS pthreads that Thread D, E, F...etc could suffer the same fate as Thread B, if the scheduler were to suspend Task C/Thread C and resume Task E/Thread E. Implementation ============== Timer details ------------- A standalone pthread for the signal generation thread was chosen, rather than using a posix timer_settime() handler function because the latter creates a temporary pthread for each handler callback. This makes debugging much more difficult due to gdb detecting the creation and destruction of these temporary threads. Signal delivery -------------- While signal handlers are per-thread, it is possible for pthreads to selectively block signals, rather than using thread directed signals. However, the approach of blocking signals in non-FreeRTOS pthreads adds complexity to each of these non-FreeRTOS pthreads including ensuring that these signals are blocked at thread creation, prior to the thread starting up. Directed signals removes the requirement for non-FreeRTOS pthreads to be aware of and take action to protect against these signals, reducing complexity. --- portable/ThirdParty/GCC/Posix/port.c | 69 +++++++++++----------------- 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 5dc3d73e2ff..d312aa48d4d 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -60,6 +60,7 @@ #include #include #include +#include #ifdef __APPLE__ #include @@ -103,6 +104,9 @@ static pthread_t hMainThread = ( pthread_t ) NULL; static volatile BaseType_t uxCriticalNesting; /*-----------------------------------------------------------*/ +static pthread_t hTimerTickThread; +static bool xTimerTickThreadShouldRun; + static BaseType_t xSchedulerEnd = pdFALSE; /*-----------------------------------------------------------*/ @@ -231,6 +235,10 @@ BaseType_t xPortStartScheduler( void ) sigwait( &xSignals, &iSignal ); } + /* asking timer thread to shut down */ + xTimerTickThreadShouldRun = false; + pthread_join( hTimerTickThread, NULL ); + /* Cancel the Idle task and free its resources */ #if ( INCLUDE_xTaskGetIdleTaskHandle == 1 ) vPortCancelThread( xTaskGetIdleTaskHandle() ); @@ -250,24 +258,8 @@ BaseType_t xPortStartScheduler( void ) void vPortEndScheduler( void ) { - struct itimerval itimer; - struct sigaction sigtick; Thread_t * xCurrentThread; - /* Stop the timer and ignore any pending SIGALRMs that would end - * up running on the main thread when it is resumed. */ - itimer.it_value.tv_sec = 0; - itimer.it_value.tv_usec = 0; - - itimer.it_interval.tv_sec = 0; - itimer.it_interval.tv_usec = 0; - ( void ) setitimer( ITIMER_REAL, &itimer, NULL ); - - sigtick.sa_flags = 0; - sigtick.sa_handler = SIG_IGN; - sigemptyset( &sigtick.sa_mask ); - sigaction( SIGALRM, &sigtick, NULL ); - /* Signal the scheduler to exit its loop. */ xSchedulerEnd = pdTRUE; ( void ) pthread_kill( hMainThread, SIG_RESUME ); @@ -366,38 +358,31 @@ static uint64_t prvStartTimeNs; * to adjust timing according to full demo requirements */ /* static uint64_t prvTickCount; */ +static void* prvTimerTickHandler(void *arg) +{ + while( xTimerTickThreadShouldRun ) + { + /* + * signal to the active task to cause tick handling or + * preemption (if enabled) + */ + Thread_t * thread = prvGetThreadFromTask( xTaskGetCurrentTaskHandle() ); + pthread_kill( thread->pthread, SIGALRM ); + + usleep( portTICK_RATE_MICROSECONDS ); + } + + return NULL; +} + /* * Setup the systick timer to generate the tick interrupts at the required * frequency. */ void prvSetupTimerInterrupt( void ) { - struct itimerval itimer; - int iRet; - - /* Initialise the structure with the current timer information. */ - iRet = getitimer( ITIMER_REAL, &itimer ); - - if( iRet == -1 ) - { - prvFatalError( "getitimer", errno ); - } - - /* Set the interval between timer events. */ - itimer.it_interval.tv_sec = 0; - itimer.it_interval.tv_usec = portTICK_RATE_MICROSECONDS; - - /* Set the current count-down. */ - itimer.it_value.tv_sec = 0; - itimer.it_value.tv_usec = portTICK_RATE_MICROSECONDS; - - /* Set-up the timer interrupt. */ - iRet = setitimer( ITIMER_REAL, &itimer, NULL ); - - if( iRet == -1 ) - { - prvFatalError( "setitimer", errno ); - } + xTimerTickThreadShouldRun = true; + pthread_create( &hTimerTickThread, NULL, prvTimerTickHandler, NULL ); prvStartTimeNs = prvGetTimeNs(); } From 88d3540b54d837f1e06e9dd3837316ca5f425c14 Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Tue, 28 Nov 2023 08:57:37 -0500 Subject: [PATCH 02/14] POSIX port - Cancel and join all FreeRTOS managed pthreads upon shutdown For a clean shutdown where memory is freed, it is necessary for all pthreads to be joined at shutdown. Previously there was explicit cancellation of the idle task and timer daemon task, however there may be a number of other tasks in the system, both system created and user created, and those tasks/threads were being left at shutdown. This change calls pthread_cancel()/pthread_join() on all FreeRTOS managed pthreads upon shutdown. --- portable/ThirdParty/GCC/Posix/port.c | 41 ++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index d312aa48d4d..c5864f82315 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -69,6 +69,7 @@ /* Scheduler includes. */ #include "FreeRTOS.h" #include "task.h" +#include "list.h" #include "timers.h" #include "utils/wait_for_event.h" /*-----------------------------------------------------------*/ @@ -82,6 +83,7 @@ typedef struct THREAD void * pvParams; BaseType_t xDying; struct event * ev; + ListItem_t xThreadListItem; } Thread_t; /* @@ -102,6 +104,7 @@ static sigset_t xAllSignals; static sigset_t xSchedulerOriginalSignalMask; static pthread_t hMainThread = ( pthread_t ) NULL; static volatile BaseType_t uxCriticalNesting; +static List_t xThreadList; /*-----------------------------------------------------------*/ static pthread_t hTimerTickThread; @@ -178,6 +181,11 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread->ev = event_create(); + /* Add the new thread in xThreadList. */ + vListInitialiseItem( &thread->xThreadListItem ); + listSET_LIST_ITEM_OWNER( &thread->xThreadListItem, thread ); + vListInsertEnd( &xThreadList, &thread->xThreadListItem ); + vPortEnterCritical(); iRet = pthread_create( &thread->pthread, &xThreadAttributes, @@ -210,6 +218,8 @@ BaseType_t xPortStartScheduler( void ) { int iSignal; sigset_t xSignals; + ListItem_t * pxIterator; + const ListItem_t * pxEndMarker; hMainThread = pthread_self(); @@ -239,15 +249,23 @@ BaseType_t xPortStartScheduler( void ) xTimerTickThreadShouldRun = false; pthread_join( hTimerTickThread, NULL ); - /* Cancel the Idle task and free its resources */ - #if ( INCLUDE_xTaskGetIdleTaskHandle == 1 ) - vPortCancelThread( xTaskGetIdleTaskHandle() ); - #endif + /* + * cancel and join any remaining pthreads + * to ensure their resources are freed + * + * https://stackoverflow.com/a/5612424 + */ + pxEndMarker = listGET_END_MARKER( &xThreadList ); + for( pxIterator = listGET_HEAD_ENTRY( &xThreadList ); pxIterator != pxEndMarker; pxIterator = listGET_NEXT( pxIterator ) ) + { + Thread_t *pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); + + pthread_cancel( pxThread->pthread ); + event_signal( pxThread->ev ); - #if ( configUSE_TIMERS == 1 ) - /* Cancel the Timer task and free its resources */ - vPortCancelThread( xTimerGetTimerDaemonTaskHandle() ); - #endif /* configUSE_TIMERS */ + pthread_join( pxThread->pthread, NULL ); + event_delete( pxThread->ev ); + } /* Restore original signal mask. */ ( void ) pthread_sigmask( SIG_SETMASK, &xSchedulerOriginalSignalMask, NULL ); @@ -444,10 +462,14 @@ void vPortCancelThread( void * pxTaskToDelete ) { Thread_t * pxThreadToCancel = prvGetThreadFromTask( pxTaskToDelete ); + /* Remove the thread from xThreadList. */ + uxListRemove( &pxThreadToCancel->xThreadListItem ); + /* * The thread has already been suspended so it can be safely cancelled. */ pthread_cancel( pxThreadToCancel->pthread ); + event_signal( pxThreadToCancel->ev ); pthread_join( pxThreadToCancel->pthread, NULL ); event_delete( pxThreadToCancel->ev ); } @@ -543,6 +565,9 @@ static void prvSetupSignalsAndSchedulerPolicy( void ) hMainThread = pthread_self(); + /* Setup thread list to record all the task which are not deleted. */ + vListInitialise( &xThreadList ); + /* Initialise common signal masks. */ sigfillset( &xAllSignals ); From 6b698ff6bc539a932b28aa5220fc978a3d585667 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:08:03 +0000 Subject: [PATCH 03/14] Fix potential race condition --- portable/ThirdParty/GCC/Posix/port.c | 41 +++++++++++++--------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index c5864f82315..34293f01a88 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -105,11 +105,7 @@ static sigset_t xSchedulerOriginalSignalMask; static pthread_t hMainThread = ( pthread_t ) NULL; static volatile BaseType_t uxCriticalNesting; static List_t xThreadList; -/*-----------------------------------------------------------*/ - static pthread_t hTimerTickThread; -static bool xTimerTickThreadShouldRun; - static BaseType_t xSchedulerEnd = pdFALSE; /*-----------------------------------------------------------*/ @@ -181,13 +177,14 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread->ev = event_create(); - /* Add the new thread in xThreadList. */ vListInitialiseItem( &thread->xThreadListItem ); listSET_LIST_ITEM_OWNER( &thread->xThreadListItem, thread ); - vListInsertEnd( &xThreadList, &thread->xThreadListItem ); vPortEnterCritical(); + /* Add the new thread in xThreadList. */ + vListInsertEnd( &xThreadList, &thread->xThreadListItem ); + iRet = pthread_create( &thread->pthread, &xThreadAttributes, prvWaitForStart, thread ); @@ -245,10 +242,6 @@ BaseType_t xPortStartScheduler( void ) sigwait( &xSignals, &iSignal ); } - /* asking timer thread to shut down */ - xTimerTickThreadShouldRun = false; - pthread_join( hTimerTickThread, NULL ); - /* * cancel and join any remaining pthreads * to ensure their resources are freed @@ -261,8 +254,6 @@ BaseType_t xPortStartScheduler( void ) Thread_t *pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); pthread_cancel( pxThread->pthread ); - event_signal( pxThread->ev ); - pthread_join( pxThread->pthread, NULL ); event_delete( pxThread->ev ); } @@ -276,14 +267,15 @@ BaseType_t xPortStartScheduler( void ) void vPortEndScheduler( void ) { - Thread_t * xCurrentThread; + /* Stop the timer tick thread. */ + pthread_cancel( hTimerTickThread ); + pthread_join( hTimerTickThread, NULL ); /* Signal the scheduler to exit its loop. */ xSchedulerEnd = pdTRUE; ( void ) pthread_kill( hMainThread, SIG_RESUME ); - xCurrentThread = prvGetThreadFromTask( xTaskGetCurrentTaskHandle() ); - prvSuspendSelf( xCurrentThread ); + pthread_exit( NULL ); } /*-----------------------------------------------------------*/ @@ -378,19 +370,24 @@ static uint64_t prvStartTimeNs; static void* prvTimerTickHandler(void *arg) { - while( xTimerTickThreadShouldRun ) + for(;;) { /* * signal to the active task to cause tick handling or * preemption (if enabled) */ - Thread_t * thread = prvGetThreadFromTask( xTaskGetCurrentTaskHandle() ); - pthread_kill( thread->pthread, SIGALRM ); + TaskHandle_t hCurrentTask; + Thread_t * thread; + hCurrentTask = xTaskGetCurrentTaskHandle(); + if( hCurrentTask != NULL ) + { + thread = prvGetThreadFromTask( hCurrentTask ); + pthread_kill( thread->pthread, SIGALRM ); + } usleep( portTICK_RATE_MICROSECONDS ); + pthread_testcancel(); } - - return NULL; } /* @@ -399,7 +396,6 @@ static void* prvTimerTickHandler(void *arg) */ void prvSetupTimerInterrupt( void ) { - xTimerTickThreadShouldRun = true; pthread_create( &hTimerTickThread, NULL, prvTimerTickHandler, NULL ); prvStartTimeNs = prvGetTimeNs(); @@ -463,13 +459,14 @@ void vPortCancelThread( void * pxTaskToDelete ) Thread_t * pxThreadToCancel = prvGetThreadFromTask( pxTaskToDelete ); /* Remove the thread from xThreadList. */ + vPortEnterCritical(); uxListRemove( &pxThreadToCancel->xThreadListItem ); + vPortExitCritical(); /* * The thread has already been suspended so it can be safely cancelled. */ pthread_cancel( pxThreadToCancel->pthread ); - event_signal( pxThreadToCancel->ev ); pthread_join( pxThreadToCancel->pthread, NULL ); event_delete( pxThreadToCancel->ev ); } From 7ba4124c785e400cb0e8c3c17a49f390e90ba77e Mon Sep 17 00:00:00 2001 From: Ching-Hsin Lee Date: Wed, 10 Jan 2024 13:18:55 +0800 Subject: [PATCH 04/14] Add back the pthread stack fit --- portable/ThirdParty/GCC/Posix/port.c | 37 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 34293f01a88..391abe4ff64 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -60,7 +60,6 @@ #include #include #include -#include #ifdef __APPLE__ #include @@ -104,9 +103,10 @@ static sigset_t xAllSignals; static sigset_t xSchedulerOriginalSignalMask; static pthread_t hMainThread = ( pthread_t ) NULL; static volatile BaseType_t uxCriticalNesting; -static List_t xThreadList; -static pthread_t hTimerTickThread; static BaseType_t xSchedulerEnd = pdFALSE; +static pthread_t hTimerTickThread; +static uint64_t prvStartTimeNs; +static List_t xThreadList; /* The list to track all the pthreads which are not deleted. */ /*-----------------------------------------------------------*/ static void prvSetupSignalsAndSchedulerPolicy( void ); @@ -130,6 +130,7 @@ void prvFatalError( const char * pcCall, fprintf( stderr, "%s: %s\n", pcCall, strerror( iErrno ) ); abort(); } +/*-----------------------------------------------------------*/ /* * See header file for description. @@ -166,13 +167,15 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread->pvParams = pvParameters; thread->xDying = pdFALSE; + /* Ensure ulStackSize is at least PTHREAD_STACK_MIN */ + ulStackSize = ( ulStackSize < PTHREAD_STACK_MIN ) ? PTHREAD_STACK_MIN : ulStackSize; + pthread_attr_init( &xThreadAttributes ); - iRet = pthread_attr_setstack( &xThreadAttributes, pxEndOfStack, ulStackSize ); + iRet = pthread_attr_setstacksize( &xThreadAttributes, ulStackSize ); if( iRet != 0 ) { - fprintf( stderr, "[WARN] pthread_attr_setstack failed with return value: %d. Default stack will be used.\n", iRet ); - fprintf( stderr, "[WARN] Increase the stack size to PTHREAD_STACK_MIN.\n" ); + fprintf( stderr, "[WARN] pthread_attr_setstacksize failed with return value: %d. Default stack size will be used.\n", iRet ); } thread->ev = event_create(); @@ -242,13 +245,9 @@ BaseType_t xPortStartScheduler( void ) sigwait( &xSignals, &iSignal ); } - /* - * cancel and join any remaining pthreads - * to ensure their resources are freed - * - * https://stackoverflow.com/a/5612424 - */ + /* Cancel all the running thread. */ pxEndMarker = listGET_END_MARKER( &xThreadList ); + for( pxIterator = listGET_HEAD_ENTRY( &xThreadList ); pxIterator != pxEndMarker; pxIterator = listGET_NEXT( pxIterator ) ) { Thread_t *pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); @@ -258,6 +257,16 @@ BaseType_t xPortStartScheduler( void ) event_delete( pxThread->ev ); } + /* + * clear out the variable that is used to end the scheduler, otherwise + * subsequent scheduler restarts will end immediately. + */ + xSchedulerEnd = pdFALSE; + + /* Reset the pthread_once_t structure. This is required if the port + * starts the scheduler again. */ + hSigSetupThread = PTHREAD_ONCE_INIT; + /* Restore original signal mask. */ ( void ) pthread_sigmask( SIG_SETMASK, &xSchedulerOriginalSignalMask, NULL ); @@ -362,7 +371,7 @@ static uint64_t prvGetTimeNs( void ) return ( uint64_t ) t.tv_sec * ( uint64_t ) 1000000000UL + ( uint64_t ) t.tv_nsec; } -static uint64_t prvStartTimeNs; +/*-----------------------------------------------------------*/ /* commented as part of the code below in vPortSystemTickHandler, * to adjust timing according to full demo requirements */ @@ -453,6 +462,7 @@ void vPortThreadDying( void * pxTaskToDelete, pxThread->xDying = pdTRUE; } +/*-----------------------------------------------------------*/ void vPortCancelThread( void * pxTaskToDelete ) { @@ -542,6 +552,7 @@ static void prvSuspendSelf( Thread_t * thread ) * - A thread with all signals blocked with pthread_sigmask(). */ event_wait( thread->ev ); + pthread_testcancel(); } /*-----------------------------------------------------------*/ From 3dade5b5a5a1378ccab80718a1f501f7485a0fa1 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:20:51 +0000 Subject: [PATCH 05/14] UPdate format --- portable/ThirdParty/GCC/Posix/port.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 391abe4ff64..d912557962a 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -106,7 +106,7 @@ static volatile BaseType_t uxCriticalNesting; static BaseType_t xSchedulerEnd = pdFALSE; static pthread_t hTimerTickThread; static uint64_t prvStartTimeNs; -static List_t xThreadList; /* The list to track all the pthreads which are not deleted. */ +static List_t xThreadList; /* The list to track all the pthreads which are not deleted. */ /*-----------------------------------------------------------*/ static void prvSetupSignalsAndSchedulerPolicy( void ); @@ -250,7 +250,7 @@ BaseType_t xPortStartScheduler( void ) for( pxIterator = listGET_HEAD_ENTRY( &xThreadList ); pxIterator != pxEndMarker; pxIterator = listGET_NEXT( pxIterator ) ) { - Thread_t *pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); + Thread_t * pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); pthread_cancel( pxThread->pthread ); pthread_join( pxThread->pthread, NULL ); @@ -377,9 +377,9 @@ static uint64_t prvGetTimeNs( void ) * to adjust timing according to full demo requirements */ /* static uint64_t prvTickCount; */ -static void* prvTimerTickHandler(void *arg) +static void * prvTimerTickHandler( void * arg ) { - for(;;) + for( ; ; ) { /* * signal to the active task to cause tick handling or @@ -389,13 +389,15 @@ static void* prvTimerTickHandler(void *arg) Thread_t * thread; hCurrentTask = xTaskGetCurrentTaskHandle(); + if( hCurrentTask != NULL ) { thread = prvGetThreadFromTask( hCurrentTask ); pthread_kill( thread->pthread, SIGALRM ); } + usleep( portTICK_RATE_MICROSECONDS ); - pthread_testcancel(); + pthread_testcancel(); } } From 0fac0859af251a7bf33ebfc6c3f00f3fe0d7d6c5 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:32:14 +0000 Subject: [PATCH 06/14] Add back heap setup code --- portable/ThirdParty/GCC/Posix/port.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index d912557962a..04f883119bd 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -105,8 +105,9 @@ static pthread_t hMainThread = ( pthread_t ) NULL; static volatile BaseType_t uxCriticalNesting; static BaseType_t xSchedulerEnd = pdFALSE; static pthread_t hTimerTickThread; +static bool xTimerTickThreadShouldRun; static uint64_t prvStartTimeNs; -static List_t xThreadList; /* The list to track all the pthreads which are not deleted. */ +static List_t xThreadList; /*-----------------------------------------------------------*/ static void prvSetupSignalsAndSchedulerPolicy( void ); @@ -277,7 +278,7 @@ BaseType_t xPortStartScheduler( void ) void vPortEndScheduler( void ) { /* Stop the timer tick thread. */ - pthread_cancel( hTimerTickThread ); + xTimerTickThreadShouldRun = false; pthread_join( hTimerTickThread, NULL ); /* Signal the scheduler to exit its loop. */ @@ -370,7 +371,6 @@ static uint64_t prvGetTimeNs( void ) return ( uint64_t ) t.tv_sec * ( uint64_t ) 1000000000UL + ( uint64_t ) t.tv_nsec; } - /*-----------------------------------------------------------*/ /* commented as part of the code below in vPortSystemTickHandler, @@ -379,7 +379,7 @@ static uint64_t prvGetTimeNs( void ) static void * prvTimerTickHandler( void * arg ) { - for( ; ; ) + while( xTimerTickThreadShouldRun ) { /* * signal to the active task to cause tick handling or @@ -399,7 +399,10 @@ static void * prvTimerTickHandler( void * arg ) usleep( portTICK_RATE_MICROSECONDS ); pthread_testcancel(); } + + return NULL; } +/*-----------------------------------------------------------*/ /* * Setup the systick timer to generate the tick interrupts at the required @@ -407,6 +410,7 @@ static void * prvTimerTickHandler( void * arg ) */ void prvSetupTimerInterrupt( void ) { + xTimerTickThreadShouldRun = true; pthread_create( &hTimerTickThread, NULL, prvTimerTickHandler, NULL ); prvStartTimeNs = prvGetTimeNs(); From a7d39c3e3a1390c3e0cde3eda87a35edfa3181f6 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:38:18 +0000 Subject: [PATCH 07/14] format and header file --- portable/ThirdParty/GCC/Posix/port.c | 1 + 1 file changed, 1 insertion(+) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 04f883119bd..43fff7f5d33 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -60,6 +60,7 @@ #include #include #include +#include #ifdef __APPLE__ #include From 14903c380ea276f2766a5a096353da285831b2b8 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:39:27 +0000 Subject: [PATCH 08/14] Remove redundent cancellation point --- portable/ThirdParty/GCC/Posix/port.c | 1 - 1 file changed, 1 deletion(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 43fff7f5d33..4c61ff544cc 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -398,7 +398,6 @@ static void * prvTimerTickHandler( void * arg ) } usleep( portTICK_RATE_MICROSECONDS ); - pthread_testcancel(); } return NULL; From 5ed9c7022b9c95c33ca72ebbab40397f9386f24f Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:45:02 +0000 Subject: [PATCH 09/14] Add back event signal --- portable/ThirdParty/GCC/Posix/port.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 4c61ff544cc..f1bf2365ed3 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -255,6 +255,7 @@ BaseType_t xPortStartScheduler( void ) Thread_t * pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); pthread_cancel( pxThread->pthread ); + event_signal( pxThread->pthread ); pthread_join( pxThread->pthread, NULL ); event_delete( pxThread->ev ); } @@ -483,6 +484,7 @@ void vPortCancelThread( void * pxTaskToDelete ) * The thread has already been suspended so it can be safely cancelled. */ pthread_cancel( pxThreadToCancel->pthread ); + event_signal( pxThreadToCancel->ev ); pthread_join( pxThreadToCancel->pthread, NULL ); event_delete( pxThreadToCancel->ev ); } From b6c0c51cbe74429fffb0ba48551907f4bb94eed6 Mon Sep 17 00:00:00 2001 From: "Ching-Hsin,Lee" Date: Wed, 10 Jan 2024 05:48:35 +0000 Subject: [PATCH 10/14] Revert timer tick function --- portable/ThirdParty/GCC/Posix/port.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index f1bf2365ed3..4653d6f1963 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -255,7 +255,7 @@ BaseType_t xPortStartScheduler( void ) Thread_t * pxThread = ( Thread_t * ) listGET_LIST_ITEM_OWNER( pxIterator ); pthread_cancel( pxThread->pthread ); - event_signal( pxThread->pthread ); + event_signal( pxThread->ev ); pthread_join( pxThread->pthread, NULL ); event_delete( pxThread->ev ); } @@ -387,17 +387,8 @@ static void * prvTimerTickHandler( void * arg ) * signal to the active task to cause tick handling or * preemption (if enabled) */ - TaskHandle_t hCurrentTask; - Thread_t * thread; - - hCurrentTask = xTaskGetCurrentTaskHandle(); - - if( hCurrentTask != NULL ) - { - thread = prvGetThreadFromTask( hCurrentTask ); - pthread_kill( thread->pthread, SIGALRM ); - } - + Thread_t * thread = prvGetThreadFromTask( xTaskGetCurrentTaskHandle() ); + pthread_kill( thread->pthread, SIGALRM ); usleep( portTICK_RATE_MICROSECONDS ); } From 62220666ba11b038cf0bd2f3f29f54123ba20566 Mon Sep 17 00:00:00 2001 From: Ching-Hsin Lee Date: Wed, 10 Jan 2024 20:30:46 +0800 Subject: [PATCH 11/14] Revert pthread_attr_setstacksize --- portable/ThirdParty/GCC/Posix/port.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 4653d6f1963..51d62b6ffb6 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -169,15 +169,13 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread->pvParams = pvParameters; thread->xDying = pdFALSE; - /* Ensure ulStackSize is at least PTHREAD_STACK_MIN */ - ulStackSize = ( ulStackSize < PTHREAD_STACK_MIN ) ? PTHREAD_STACK_MIN : ulStackSize; - pthread_attr_init( &xThreadAttributes ); - iRet = pthread_attr_setstacksize( &xThreadAttributes, ulStackSize ); + iRet = pthread_attr_setstack( &xThreadAttributes, pxEndOfStack, ulStackSize ); if( iRet != 0 ) { - fprintf( stderr, "[WARN] pthread_attr_setstacksize failed with return value: %d. Default stack size will be used.\n", iRet ); + fprintf( stderr, "[WARN] pthread_attr_setstack failed with return value: %d. Default stack will be used.\n", iRet ); + fprintf( stderr, "[WARN] Increase the stack size to PTHREAD_STACK_MIN.\n" ); } thread->ev = event_create(); From 3baa3dd98bb98ab58337f137747559b4f2cd24db Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Wed, 29 Nov 2023 08:15:50 -0500 Subject: [PATCH 12/14] POSIX port - Switch from allowing the user to specify the stack memory itself, to allowing them to specify the stack size Change from pthread_attr_setstack() to pthread_attr_setstacksize(), and automatically adjust the stack size to be at least PTHREAD_STACK_MIN if it wasn't already, removing the size warning. This permits the user to increase the pthread stack size beyond the PTHREAD_STACK_MIN default of 16384 if desired, without producing a warning in the typical case where stacks are minimized for RAM limited targets. Continue to store thread paramters on the provided stack, for consistency with the MCU targets. Previously pthread_attr_setstack() was used to enable user defined stacks. Note that: 1. The stack size can still be specified by the user. 2. pxPortInitialiseStack(), and pthread_addr_setstack() was failing on stacks of typical size, as these are smaller than PTHREAD_STACK_MIN (16384) bytes, and printing out a series of warnings. Improve usability by having the posix port automatically increase the stack size to be at least PTHREAD_STACK_MIN as posix platforms have enough memory for this not to be a concern. 3. Reuse of stack memory will also result in valgrind 'invalid write' errors to what is demonstrably valid memory. Root cause is that Valgrind is tracking a stack pointer as the stack is used. Reuse of a stack buffer results in the stack being used at its start, in an area that Valgrind thinks is far away from the start of the stack. There are ways to notify Valgrind of these changes however this would require linking against and calling Valgrind functions from the FreeRTOS application using the posix port, https://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq. Also, apparently it isn't permitted by posix to reuse stack memory once its been used in a pthread via pthread_attr_setstack(), see https://stackoverflow.com/a/5422134 --- portable/ThirdParty/GCC/Posix/port.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index 51d62b6ffb6..d5296183aac 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -169,13 +169,15 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, thread->pvParams = pvParameters; thread->xDying = pdFALSE; + /* Ensure ulStackSize is at least PTHREAD_STACK_MIN */ + ulStackSize = (ulStackSize < PTHREAD_STACK_MIN) ? PTHREAD_STACK_MIN : ulStackSize; + pthread_attr_init( &xThreadAttributes ); - iRet = pthread_attr_setstack( &xThreadAttributes, pxEndOfStack, ulStackSize ); + iRet = pthread_attr_setstacksize( &xThreadAttributes, ulStackSize ); if( iRet != 0 ) { - fprintf( stderr, "[WARN] pthread_attr_setstack failed with return value: %d. Default stack will be used.\n", iRet ); - fprintf( stderr, "[WARN] Increase the stack size to PTHREAD_STACK_MIN.\n" ); + fprintf( stderr, "[WARN] pthread_attr_setstacksize failed with return value: %d. Default stack size will be used.\n", iRet ); } thread->ev = event_create(); From c053ffeacc655a8d2610c63f388f2fc1210b096d Mon Sep 17 00:00:00 2001 From: Tony Josi Date: Sat, 13 Jan 2024 21:43:05 +0530 Subject: [PATCH 13/14] Fix -Werror=unused-parameter in GCC posix prvTimerTickHandler() (#949) --- portable/ThirdParty/GCC/Posix/port.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index d5296183aac..d0152169ac5 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -381,6 +381,8 @@ static uint64_t prvGetTimeNs( void ) static void * prvTimerTickHandler( void * arg ) { + ( void ) arg; + while( xTimerTickThreadShouldRun ) { /* From c083af972ab1172aa09938e910ed9451053a9fcf Mon Sep 17 00:00:00 2001 From: IsaacDynamo <61521674+IsaacDynamo@users.noreply.github.com> Date: Wed, 17 Jan 2024 21:22:32 +0100 Subject: [PATCH 14/14] Add mpu_wrappers_v2_asm.c to MPU ports (#951) Co-authored-by: Soren Ptak --- portable/CMakeLists.txt | 80 ++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 25 deletions(-) diff --git a/portable/CMakeLists.txt b/portable/CMakeLists.txt index dea052d9815..9df221d214b 100644 --- a/portable/CMakeLists.txt +++ b/portable/CMakeLists.txt @@ -88,11 +88,13 @@ add_library(freertos_kernel_port STATIC GCC/ARM_CM3/port.c> $<$: - GCC/ARM_CM3_MPU/port.c> + GCC/ARM_CM3_MPU/port.c + GCC/ARM_CM3_MPU/mpu_wrappers_v2_asm.c> # ARMv7E-M ports for GCC $<$: - GCC/ARM_CM4_MPU/port.c> + GCC/ARM_CM4_MPU/port.c + GCC/ARM_CM4_MPU/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM4F/port.c> @@ -103,7 +105,8 @@ add_library(freertos_kernel_port STATIC # ARMv8-M ports for GCC $<$: GCC/ARM_CM23/non_secure/port.c - GCC/ARM_CM23/non_secure/portasm.c> + GCC/ARM_CM23/non_secure/portasm.c + GCC/ARM_CM23/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM23/secure/secure_context_port.c @@ -113,11 +116,13 @@ add_library(freertos_kernel_port STATIC $<$: GCC/ARM_CM23_NTZ/non_secure/port.c - GCC/ARM_CM23_NTZ/non_secure/portasm.c> + GCC/ARM_CM23_NTZ/non_secure/portasm.c + GCC/ARM_CM23_NTZ/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM33/non_secure/port.c - GCC/ARM_CM33/non_secure/portasm.c> + GCC/ARM_CM33/non_secure/portasm.c + GCC/ARM_CM33/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM33/secure/secure_context_port.c @@ -127,16 +132,19 @@ add_library(freertos_kernel_port STATIC $<$: GCC/ARM_CM33_NTZ/non_secure/port.c - GCC/ARM_CM33_NTZ/non_secure/portasm.c> + GCC/ARM_CM33_NTZ/non_secure/portasm.c + GCC/ARM_CM33_NTZ/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM33_NTZ/non_secure/port.c GCC/ARM_CM33_NTZ/non_secure/portasm.c + GCC/ARM_CM33_NTZ/non_secure/mpu_wrappers_v2_asm.c ThirdParty/GCC/ARM_TFM/os_wrapper_freertos.c> $<$: GCC/ARM_CM35P/non_secure/port.c - GCC/ARM_CM35P/non_secure/portasm.c> + GCC/ARM_CM35P/non_secure/portasm.c + GCC/ARM_CM35P/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM35P/secure/secure_context_port.c @@ -146,12 +154,14 @@ add_library(freertos_kernel_port STATIC $<$: GCC/ARM_CM35P_NTZ/non_secure/port.c - GCC/ARM_CM35P_NTZ/non_secure/portasm.c> + GCC/ARM_CM35P_NTZ/non_secure/portasm.c + GCC/ARM_CM35P_NTZ/non_secure/mpu_wrappers_v2_asm.c> # ARMv8.1-M ports for GCC $<$: GCC/ARM_CM55/non_secure/port.c - GCC/ARM_CM55/non_secure/portasm.c> + GCC/ARM_CM55/non_secure/portasm.c + GCC/ARM_CM55/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM55/secure/secure_context_port.c @@ -161,16 +171,19 @@ add_library(freertos_kernel_port STATIC $<$: GCC/ARM_CM55_NTZ/non_secure/port.c - GCC/ARM_CM55_NTZ/non_secure/portasm.c> + GCC/ARM_CM55_NTZ/non_secure/portasm.c + GCC/ARM_CM55_NTZ/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM55_NTZ/non_secure/port.c GCC/ARM_CM55_NTZ/non_secure/portasm.c + GCC/ARM_CM55_NTZ/non_secure/mpu_wrappers_v2_asm.c ThirdParty/GCC/ARM_TFM/os_wrapper_freertos.c> $<$: GCC/ARM_CM85/non_secure/port.c - GCC/ARM_CM85/non_secure/portasm.c> + GCC/ARM_CM85/non_secure/portasm.c + GCC/ARM_CM85/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM85/secure/secure_context_port.c @@ -180,11 +193,13 @@ add_library(freertos_kernel_port STATIC $<$: GCC/ARM_CM85_NTZ/non_secure/port.c - GCC/ARM_CM85_NTZ/non_secure/portasm.c> + GCC/ARM_CM85_NTZ/non_secure/portasm.c + GCC/ARM_CM85_NTZ/non_secure/mpu_wrappers_v2_asm.c> $<$: GCC/ARM_CM85_NTZ/non_secure/port.c GCC/ARM_CM85_NTZ/non_secure/portasm.c + GCC/ARM_CM85_NTZ/non_secure/mpu_wrappers_v2_asm.c ThirdParty/GCC/ARM_TFM/os_wrapper_freertos.c> # ARMv7-R ports for GCC @@ -391,7 +406,8 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM4F_MPU/port.c - IAR/ARM_CM4F_MPU/portasm.s> + IAR/ARM_CM4F_MPU/portasm.s + IAR/ARM_CM4F_MPU/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM7/r0p1/port.c @@ -400,7 +416,8 @@ add_library(freertos_kernel_port STATIC # ARMv8-M Ports for IAR EWARM $<$: IAR/ARM_CM23/non_secure/port.c - IAR/ARM_CM23/non_secure/portasm.s> + IAR/ARM_CM23/non_secure/portasm.s + IAR/ARM_CM23/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM23/secure/secure_context_port_asm.s @@ -410,11 +427,13 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM23_NTZ/non_secure/port.c - IAR/ARM_CM23_NTZ/non_secure/portasm.s> + IAR/ARM_CM23_NTZ/non_secure/portasm.s + IAR/ARM_CM23_NTZ/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM33/non_secure/port.c - IAR/ARM_CM33/non_secure/portasm.s> + IAR/ARM_CM33/non_secure/portasm.s + IAR/ARM_CM33/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM33/secure/secure_context_port_asm.s @@ -424,11 +443,13 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM33_NTZ/non_secure/port.c - IAR/ARM_CM33_NTZ/non_secure/portasm.s> + IAR/ARM_CM33_NTZ/non_secure/portasm.s + IAR/ARM_CM33_NTZ/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM35P/non_secure/port.c - IAR/ARM_CM35P/non_secure/portasm.s> + IAR/ARM_CM35P/non_secure/portasm.s + IAR/ARM_CM35P/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM35P/secure/secure_context_port_asm.s @@ -438,12 +459,14 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM35P_NTZ/non_secure/port.c - IAR/ARM_CM35P_NTZ/non_secure/portasm.s> + IAR/ARM_CM35P_NTZ/non_secure/portasm.s + IAR/ARM_CM35P_NTZ/non_secure/mpu_wrappers_v2_asm.S> # ARMv8.1-M ports for IAR EWARM $<$: IAR/ARM_CM55/non_secure/port.c - IAR/ARM_CM55/non_secure/portasm.s> + IAR/ARM_CM55/non_secure/portasm.s + IAR/ARM_CM55/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM55/secure/secure_context_port_asm.s @@ -453,11 +476,13 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM55_NTZ/non_secure/port.c - IAR/ARM_CM55_NTZ/non_secure/portasm.s> + IAR/ARM_CM55_NTZ/non_secure/portasm.s + IAR/ARM_CM55_NTZ/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM85/non_secure/port.c - IAR/ARM_CM85/non_secure/portasm.s> + IAR/ARM_CM85/non_secure/portasm.s + IAR/ARM_CM85/non_secure/mpu_wrappers_v2_asm.S> $<$: IAR/ARM_CM85/secure/secure_context_port_asm.s @@ -467,7 +492,8 @@ add_library(freertos_kernel_port STATIC $<$: IAR/ARM_CM85_NTZ/non_secure/port.c - IAR/ARM_CM85_NTZ/non_secure/portasm.s> + IAR/ARM_CM85_NTZ/non_secure/portasm.s + IAR/ARM_CM85_NTZ/non_secure/mpu_wrappers_v2_asm.S> # ARMv7-R Ports for IAR EWARM $<$: @@ -659,7 +685,8 @@ add_library(freertos_kernel_port STATIC # ARMv7E-M ports for ARM RVDS / armcc $<$: - RVDS/ARM_CM4_MPU/port.c> + RVDS/ARM_CM4_MPU/port.c + RVDS/ARM_CM4_MPU/mpu_wrappers_v2_asm.c> $<$: RVDS/ARM_CM4F/port.c> @@ -723,7 +750,10 @@ if( FREERTOS_PORT MATCHES "GCC_ARM_CM(3|4)_MPU" OR FREERTOS_PORT MATCHES "IAR_ARM_CM(23|33|55|85)_NTZ_NONSECURE" OR FREERTOS_PORT MATCHES "IAR_ARM_CM(23|33|55|85)_NONSECURE" ) - target_sources(freertos_kernel_port PRIVATE Common/mpu_wrappers.c) + target_sources(freertos_kernel_port PRIVATE + Common/mpu_wrappers.c + Common/mpu_wrappers_v2.c + ) endif() target_include_directories(freertos_kernel_port PUBLIC