Skip to content

Fix SMP task self void run state change #984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 6, 2024
Merged

Fix SMP task self void run state change #984

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ba47502
Request task to yield after been suspended or deleted
chinglee-iot Feb 6, 2024
da4f2a3
Fix volatile variable access order
chinglee-iot Feb 6, 2024
ad1a178
Merge branch 'main' into fix-vTaskSuspend-vTaskDelete-self-add-back
chinglee-iot Feb 6, 2024
5ed50e5
Fix uncrustify
chinglee-iot Feb 6, 2024
68a172c
Add more comment to fix MISRA
chinglee-iot Feb 6, 2024
dc288d6
Merge branch 'fix-vTaskSuspend-vTaskDelete-self-add-back' of https://…
chinglee-iot Feb 6, 2024
6907d1d
Update comments
aggarg Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 87 additions & 76 deletions tasks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,6 +2191,7 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
{
TCB_t * pxTCB;
BaseType_t xDeleteTCBInIdleTask = pdFALSE;
BaseType_t xTaskIsRunningOrYielding;

traceENTER_vTaskDelete( xTaskToDelete );

Expand Down Expand Up @@ -2226,10 +2227,15 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
* not return. */
uxTaskNumber++;

/* Use temp variable as distinct sequence points for reading volatile
* variables prior to a logical operator to ensure compliance with
* MISRA C 2012 Rule 13.5. */
xTaskIsRunningOrYielding = taskTASK_IS_RUNNING_OR_SCHEDULED_TO_YIELD( pxTCB );

/* If the task is running (or yielding), we must add it to the
* termination list so that an idle task can delete it when it is
* no longer running. */
if( ( xSchedulerRunning != pdFALSE ) && ( taskTASK_IS_RUNNING_OR_SCHEDULED_TO_YIELD( pxTCB ) != pdFALSE ) )
if( ( xSchedulerRunning != pdFALSE ) && ( xTaskIsRunningOrYielding != pdFALSE ) )
{
/* A running task or a task which is scheduled to yield is being
* deleted. This cannot complete when the task is still running
Expand Down Expand Up @@ -2261,6 +2267,30 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
#else
portPRE_TASK_DELETE_HOOK( pxTCB, &( xYieldPendings[ pxTCB->xTaskRunState ] ) );
#endif

/* In the case of SMP, it is possible that the task being deleted
* is running on another core. We must evict the task before
* exiting the critical section to ensure that the task cannot
* take an action which puts it back on ready/state/event list,
* thereby nullifying the delete operation. Once evicted, the
* task won't be scheduled ever as it will no longer be on the
* ready list. */
#if ( configNUMBER_OF_CORES > 1 )
{
if( taskTASK_IS_RUNNING( pxTCB ) == pdTRUE )
{
if( pxTCB->xTaskRunState == ( BaseType_t ) portGET_CORE_ID() )
{
configASSERT( uxSchedulerSuspended == 0 );
taskYIELD_WITHIN_API();
}
else
{
prvYieldCore( pxTCB->xTaskRunState );
}
}
}
#endif /* #if ( configNUMBER_OF_CORES > 1 ) */
}
else
{
Expand All @@ -2284,9 +2314,9 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,

/* Force a reschedule if it is the currently running task that has just
* been deleted. */
if( xSchedulerRunning != pdFALSE )
#if ( configNUMBER_OF_CORES == 1 )
{
#if ( configNUMBER_OF_CORES == 1 )
if( xSchedulerRunning != pdFALSE )
{
if( pxTCB == pxCurrentTCB )
{
Expand All @@ -2298,30 +2328,8 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
mtCOVERAGE_TEST_MARKER();
}
}
#else /* #if ( configNUMBER_OF_CORES == 1 ) */
{
/* It is important to use critical section here because
* checking run state of a task must be done inside a
* critical section. */
taskENTER_CRITICAL();
{
if( taskTASK_IS_RUNNING( pxTCB ) == pdTRUE )
{
if( pxTCB->xTaskRunState == ( BaseType_t ) portGET_CORE_ID() )
{
configASSERT( uxSchedulerSuspended == 0 );
taskYIELD_WITHIN_API();
}
else
{
prvYieldCore( pxTCB->xTaskRunState );
}
}
}
taskEXIT_CRITICAL();
}
#endif /* #if ( configNUMBER_OF_CORES == 1 ) */
}
#endif /* #if ( configNUMBER_OF_CORES == 1 ) */

traceRETURN_vTaskDelete();
}
Expand Down Expand Up @@ -3155,26 +3163,66 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
}
}
#endif /* if ( configUSE_TASK_NOTIFICATIONS == 1 ) */
}
taskEXIT_CRITICAL();

if( xSchedulerRunning != pdFALSE )
{
/* Reset the next expected unblock time in case it referred to the
* task that is now in the Suspended state. */
taskENTER_CRITICAL();
/* In the case of SMP, it is possible that the task being suspended
* is running on another core. We must evict the task before
* exiting the critical section to ensure that the task cannot
* take an action which puts it back on ready/state/event list,
* thereby nullifying the suspend operation. Once evicted, the
* task won't be scheduled before it is resumed as it will no longer
* be on the ready list. */
#if ( configNUMBER_OF_CORES > 1 )
{
prvResetNextTaskUnblockTime();
if( xSchedulerRunning != pdFALSE )
{
/* Reset the next expected unblock time in case it referred to the
* task that is now in the Suspended state. */
prvResetNextTaskUnblockTime();

if( taskTASK_IS_RUNNING( pxTCB ) == pdTRUE )
{
if( pxTCB->xTaskRunState == ( BaseType_t ) portGET_CORE_ID() )
{
/* The current task has just been suspended. */
configASSERT( uxSchedulerSuspended == 0 );
vTaskYieldWithinAPI();
}
else
{
prvYieldCore( pxTCB->xTaskRunState );
}
}
else
{
mtCOVERAGE_TEST_MARKER();
}
}
else
{
mtCOVERAGE_TEST_MARKER();
}
}
taskEXIT_CRITICAL();
}
else
{
mtCOVERAGE_TEST_MARKER();
#endif /* #if ( configNUMBER_OF_CORES > 1 ) */
}
taskEXIT_CRITICAL();

#if ( configNUMBER_OF_CORES == 1 )
{
if( xSchedulerRunning != pdFALSE )
{
/* Reset the next expected unblock time in case it referred to the
* task that is now in the Suspended state. */
taskENTER_CRITICAL();
{
prvResetNextTaskUnblockTime();
}
taskEXIT_CRITICAL();
}
else
{
mtCOVERAGE_TEST_MARKER();
}

if( pxTCB == pxCurrentTCB )
{
if( xSchedulerRunning != pdFALSE )
Expand Down Expand Up @@ -3207,43 +3255,6 @@ static void prvInitialiseNewTask( TaskFunction_t pxTaskCode,
mtCOVERAGE_TEST_MARKER();
}
}
#else /* #if ( configNUMBER_OF_CORES == 1 ) */
{
/* Enter critical section here to check run state of a task. */
taskENTER_CRITICAL();
{
if( taskTASK_IS_RUNNING( pxTCB ) == pdTRUE )
{
if( xSchedulerRunning != pdFALSE )
{
if( pxTCB->xTaskRunState == ( BaseType_t ) portGET_CORE_ID() )
{
/* The current task has just been suspended. */
configASSERT( uxSchedulerSuspended == 0 );
vTaskYieldWithinAPI();
}
else
{
prvYieldCore( pxTCB->xTaskRunState );
}
}
else
{
/* This code path is not possible because only Idle tasks are
* assigned a core before the scheduler is started ( i.e.
* taskTASK_IS_RUNNING is only true for idle tasks before
* the scheduler is started ) and idle tasks cannot be
* suspended. */
mtCOVERAGE_TEST_MARKER();
}
}
else
{
mtCOVERAGE_TEST_MARKER();
}
}
taskEXIT_CRITICAL();
}
#endif /* #if ( configNUMBER_OF_CORES == 1 ) */

traceRETURN_vTaskSuspend();
Expand Down