Skip to content

Commit

Permalink
Merge branch 'bugfix/clobbering_freertos_base_priority' into 'master'
Browse files Browse the repository at this point in the history
spi_flash: fix issue linked with raising of task priority while priority is already raised

Closes IDFGH-5881

See merge request espressif/esp-idf!19037
  • Loading branch information
KonstantinKondrashov committed Sep 2, 2022
2 parents fb5c710 + aea2fe0 commit 15ec31e
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 3 deletions.
48 changes: 48 additions & 0 deletions components/freertos/esp_additions/include/freertos/idf_additions.h
Expand Up @@ -7,6 +7,7 @@
#include "sdkconfig.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "idf_additions_inc.h"

#if CONFIG_FREERTOS_SMP || __DOXYGEN__

Expand Down Expand Up @@ -141,3 +142,50 @@ BaseType_t xTaskGetAffinity( TaskHandle_t xTask );
#endif // CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS

#endif // CONFIG_FREERTOS_SMP || __DOXYGEN__

#if ( INCLUDE_vTaskPrioritySet == 1 )

/**
* INCLUDE_vTaskPrioritySet must be defined as 1 for this function to be available.
* See the configuration section for more information.
*
* Saves the current priority and current base priority of a task, then raises the tasks
* current and base priority to uxNewPriority if uxNewPriority is of a higher priority.
* Once a task's priority has been raised with this function, the priority can be restored
* by calling prvTaskPriorityRestore()
* - Note that this function differs from vTaskPrioritySet() as the task's current priority
* will be modified even if the task has already inherited a priority.
* - This function is intended for special circumstance where a task must be forced immediately
* to a higher priority.
*
* For configUSE_MUTEXES == 0: A context switch will occur before the function returns if the priority
* being set is higher than the currently executing task.
*
* @note This functions is private is only be called internally within various IDF components.
* Users should never call this function from their application.
*
* @note vTaskPrioritySet() should not be called while a task's priority is already raised via this function
*
* @param pxSavedPriority returns base and current priorities
*
* @param uxNewPriority The priority to which the task will be set.
*/
void prvTaskPriorityRaise( prvTaskSavedPriority_t * pxSavedPriority, UBaseType_t uxNewPriority );

/**
* INCLUDE_vTaskPrioritySet must be defined as 1 for this function to be available.
* See the configuration section for more information.
*
* Restore a task's priority that was previously raised by prvTaskPriorityRaise().
*
* For configUSE_MUTEXES == 0: A context switch will occur before the function returns if the priority
* being set is higher than the currently executing task.
*
* @note This functions is private is only be called internally within various IDF components.
* Users should never call this function from their application.
*
* @param pxSavedPriority previously saved base and current priorities that need to be restored
*/
void prvTaskPriorityRestore( prvTaskSavedPriority_t * pxSavedPriority );

#endif // ( INCLUDE_vTaskPrioritySet == 1)
@@ -0,0 +1,33 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/


#ifndef FREERTOS_ADDITITIONS_INC_H_
#define FREERTOS_ADDITITIONS_INC_H_

#ifdef __cplusplus
extern "C" {
#endif

#include "sdkconfig.h"
#include "freertos/FreeRTOS.h"

#if ( INCLUDE_vTaskPrioritySet == 1 )

typedef struct {
UBaseType_t uxPriority;
#if ( configUSE_MUTEXES == 1 )
UBaseType_t uxBasePriority;
#endif
} prvTaskSavedPriority_t;

#endif // ( INCLUDE_vTaskPrioritySet == 1)

#ifdef __cplusplus
}
#endif

#endif //FREERTOS_ADDITITIONS_INC_H_
Expand Up @@ -7,6 +7,7 @@
#pragma once

#include "sdkconfig.h"
#include "idf_additions_inc.h"

/**
* This file will be included in `tasks.c` file, thus, it must NOT be included
Expand Down Expand Up @@ -393,3 +394,204 @@ void vTaskSetThreadLocalStoragePointerAndDelCallback( TaskHandle_t xTaskToSet, B
#endif // CONFIG_FREERTOS_TLSP_DELETION_CALLBACKS

#endif // CONFIG_FREERTOS_SMP

#if ( INCLUDE_vTaskPrioritySet == 1 )

void prvTaskPriorityRaise( prvTaskSavedPriority_t * pxSavedPriority, UBaseType_t uxNewPriority )
{
TCB_t * pxTCB;
UBaseType_t uxPriorityUsedOnEntry;

configASSERT( ( uxNewPriority < configMAX_PRIORITIES ) );

/* Ensure the new priority is valid. */
if( uxNewPriority >= ( UBaseType_t ) configMAX_PRIORITIES )
{
uxNewPriority = ( UBaseType_t ) configMAX_PRIORITIES - ( UBaseType_t ) 1U;
}

#if CONFIG_FREERTOS_SMP
taskENTER_CRITICAL();
#else
taskENTER_CRITICAL( &xKernelLock );
#endif
{
pxTCB = prvGetTCBFromHandle( NULL );

#if ( configUSE_MUTEXES == 1 )
{
pxSavedPriority->uxPriority = pxTCB->uxPriority;
pxSavedPriority->uxBasePriority = pxTCB->uxBasePriority;

/* If uxNewPriority < uxBasePriority, then there is nothing else to
* do, as uxBasePriority is always <= uxPriority. */
if( uxNewPriority > pxTCB->uxBasePriority )
{
pxTCB->uxBasePriority = uxNewPriority;

/* Remember the task's current priority before attempting to
* change it. If the task's current priority is changed, it must
* be done so before moving the task between task lists) in order
* for the taskRESET_READY_PRIORITY() macro to function correctly. */
uxPriorityUsedOnEntry = pxTCB->uxPriority;

if( uxNewPriority > pxTCB->uxPriority )
{
pxTCB->uxPriority = uxNewPriority;

/* Only reset the event list item value if the value is not
* being used for anything else. */
if( ( listGET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ) ) & taskEVENT_LIST_ITEM_VALUE_IN_USE ) == 0UL )
{
listSET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ), ( ( TickType_t ) configMAX_PRIORITIES - ( TickType_t ) uxNewPriority ) ); /*lint !e961 MISRA exception as the casts are only redundant for some ports. */
}

/* If the task is in the blocked or suspended list we need do
* nothing more than change its priority variable. However, if
* the task is in a ready list it needs to be removed and placed
* in the list appropriate to its new priority. */
if( listIS_CONTAINED_WITHIN( &( pxReadyTasksLists[ uxPriorityUsedOnEntry ] ), &( pxTCB->xStateListItem ) ) != pdFALSE )
{
/* The task is currently in its ready list - remove before
* adding it to its new ready list. As we are in a critical
* section we can do this even if the scheduler is suspended. */
if( uxListRemove( &( pxTCB->xStateListItem ) ) == ( UBaseType_t ) 0 )
{
/* It is known that the task is in its ready list so
* there is no need to check again and the port level
* reset macro can be called directly. */
portRESET_READY_PRIORITY( uxPriorityUsedOnEntry, uxTopReadyPriority );
}
prvAddTaskToReadyList( pxTCB );
}
}
}
}
#else /* if ( configUSE_MUTEXES == 1 ) */
{
pxSavedPriority->uxPriority = pxTCB->uxPriority;
if ( uxNewPriority > pxTCB->uxPriority)
{
vTaskPrioritySet( NULL, uxNewPriority );
}
}
#endif
}
#if CONFIG_FREERTOS_SMP
taskEXIT_CRITICAL();
#else
taskEXIT_CRITICAL( &xKernelLock );
#endif
}

void prvTaskPriorityRestore( prvTaskSavedPriority_t * pxSavedPriority )
{
TCB_t * pxTCB;
UBaseType_t uxNewPriority;
UBaseType_t uxPriorityUsedOnEntry;
UBaseType_t uxBasePriorityUsedOnEntry;
BaseType_t xYieldRequired = pdFALSE;

#if CONFIG_FREERTOS_SMP
taskENTER_CRITICAL();
#else
taskENTER_CRITICAL( &xKernelLock );
#endif
{
pxTCB = prvGetTCBFromHandle( NULL );

#if ( configUSE_MUTEXES == 1 )
{
/* If the saved uxBasePriority == the task's uxBasePriority, it means
* that prvTaskPriorityRaise() never raised the task's uxBasePriority.
* In that case, there is nothing else to do. */
if( pxSavedPriority->uxBasePriority != pxTCB->uxBasePriority )
{
uxBasePriorityUsedOnEntry = pxTCB->uxBasePriority;
pxTCB->uxBasePriority = pxSavedPriority->uxBasePriority;

/* Remember the task's current priority before attempting to
* change it. If the task's current priority is changed, it must
* be done so before moving the task between task lists in order
* for the taskRESET_READY_PRIORITY() macro to function correctly. */
uxPriorityUsedOnEntry = pxTCB->uxPriority;

/* Check if the task inherited a priority after prvTaskPriorityRaise().
* If this is the case, there is nothing else to do. The priority
* will be restored when the task disinherits its priority. */
if( pxTCB->uxPriority == uxBasePriorityUsedOnEntry )
{
if( pxTCB->uxMutexesHeld == 0 )
{
/* The task may have inherited a priority before prvTaskPriorityRaise()
* then disinherited a priority after prvTaskPriorityRaise().
* Thus we need set the uxPriority to the saved base priority
* so that the task's priority gets restored to the priority
* before any inheritance or raising. */
pxTCB->uxPriority = pxSavedPriority->uxBasePriority;
}
else
{
/* The task may have inherited a priority before prvTaskPriorityRaise()
* was called. Thus, we need to restore uxPriority to the
* "saved uxPriority" so that the task still retains that
* inherited priority. */
pxTCB->uxPriority = pxSavedPriority->uxPriority;
}
uxNewPriority = pxTCB->uxPriority;

if( uxNewPriority < uxPriorityUsedOnEntry )
{
/* Setting the priority of the running task down means
* there may now be another task of higher priority that
* is ready to execute. */
xYieldRequired = pdTRUE;
}

/* Only reset the event list item value if the value is not
* being used for anything else. */
if( ( listGET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ) ) & taskEVENT_LIST_ITEM_VALUE_IN_USE ) == 0UL )
{
listSET_LIST_ITEM_VALUE( &( pxTCB->xEventListItem ), ( ( TickType_t ) configMAX_PRIORITIES - ( TickType_t ) uxNewPriority ) ); /*lint !e961 MISRA exception as the casts are only redundant for some ports. */
}

/* If the task is in the blocked or suspended list we need do
* nothing more than change its priority variable. However, if
* the task is in a ready list it needs to be removed and placed
* in the list appropriate to its new priority. */
if( listIS_CONTAINED_WITHIN( &( pxReadyTasksLists[ uxPriorityUsedOnEntry ] ), &( pxTCB->xStateListItem ) ) != pdFALSE )
{
/* The task is currently in its ready list - remove before
* adding it to its new ready list. As we are in a critical
* section we can do this even if the scheduler is suspended. */
if( uxListRemove( &( pxTCB->xStateListItem ) ) == ( UBaseType_t ) 0 )
{
/* It is known that the task is in its ready list so
* there is no need to check again and the port level
* reset macro can be called directly. */
portRESET_READY_PRIORITY( uxPriorityUsedOnEntry, uxTopReadyPriority );
}
prvAddTaskToReadyList( pxTCB );
}

if( xYieldRequired != pdFALSE )
{
taskYIELD_IF_USING_PREEMPTION();
}
}
}
}
#else /* if ( configUSE_MUTEXES == 1 ) */
{
vTaskPrioritySet( NULL, pxSavedPriority->uxPriority );
}
#endif
}
#if CONFIG_FREERTOS_SMP
taskEXIT_CRITICAL();
#else
taskEXIT_CRITICAL( &xKernelLock );
#endif
}

#endif // ( INCLUDE_vTaskPrioritySet == 1 )
2 changes: 2 additions & 0 deletions components/freertos/linker.lf
Expand Up @@ -34,6 +34,8 @@ entries:
tasks: vTaskRemoveFromUnorderedEventList (default)
tasks: uxTaskPriorityGet (default)
tasks: vTaskPrioritySet (default)
tasks: prvTaskPriorityRaise (default)
tasks: prvTaskPriorityRestore (default)
tasks: vTaskSetThreadLocalStoragePointerAndDelCallback (default)
tasks: pvTaskGetThreadLocalStoragePointer (default)
tasks: xTaskGetCurrentTaskHandleForCPU (default)
Expand Down
2 changes: 2 additions & 0 deletions components/freertos/linker_smp.lf
Expand Up @@ -76,6 +76,8 @@ entries:
tasks: eTaskGetState (default)
tasks: uxTaskPriorityGet (default)
tasks: vTaskPrioritySet (default)
tasks: prvTaskPriorityRaise (default)
tasks: prvTaskPriorityRestore (default)
tasks: vTaskSuspend (default)
tasks: vTaskResume (default)
tasks: prvCreateIdleTasks (default)
Expand Down
8 changes: 5 additions & 3 deletions components/spi_flash/cache_utils.c
Expand Up @@ -11,6 +11,7 @@

#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
#include <freertos/idf_additions.h>
#include <freertos/semphr.h>
#if CONFIG_IDF_TARGET_ESP32
#include "soc/dport_reg.h"
Expand Down Expand Up @@ -170,8 +171,9 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
} else {
// Temporarily raise current task priority to prevent a deadlock while
// waiting for IPC task to start on the other CPU
int old_prio = uxTaskPriorityGet(NULL);
vTaskPrioritySet(NULL, configMAX_PRIORITIES - 1);
prvTaskSavedPriority_t SavedPriority;
prvTaskPriorityRaise(&SavedPriority, configMAX_PRIORITIES - 1);

// Signal to the spi_flash_op_block_task on the other CPU that we need it to
// disable cache there and block other tasks from executing.
s_flash_op_can_start = false;
Expand All @@ -189,7 +191,7 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
vTaskSuspendAll();
#endif // CONFIG_FREERTOS_SMP
// Can now set the priority back to the normal one
vTaskPrioritySet(NULL, old_prio);
prvTaskPriorityRestore(&SavedPriority);
// This is guaranteed to run on CPU <cpuid> because the other CPU is now
// occupied by highest priority task
assert(xPortGetCoreID() == cpuid);
Expand Down

0 comments on commit 15ec31e

Please sign in to comment.