spi_flash: raise priority of the task performing spi_flash operation

The fix is for the situation when cache disabling mechanism causes
a deadlock with user tasks. Situation is as follows:

1. spi_flash operation is started from low-priority task on CPU0
2. It uses IPC to wake up high-priority IPC1 task on CPU1, preventing
   all other tasks on CPU1 from running. This is needed to safely
   disable the cache.
3. While the task which started spi_flash operation is waiting for IPC1
   task to acknowledge that CPU1 is not using cache anymore, it is
   preempted by a higher priority application task ("app0").
4. Task app0 busy-waits for some operation on CPU1 to complete. But
   since application tasks are blocked out by IPC1 task, this never
   happens. Since app0 is busy-waiting, the task doing spi flash
   operation never runs.

The more or less logical soltion to the problem would be to also do
cache disabling on CPU0 and the SPI flash operation itself from IPC0
task. However IPC0 task stack would need to be increased to allow doing
SPI flash operation (and IPC1 stack as well). This would waste some
memory. An alternative approach adopted in this fix is to call FreeRTOS
functions to temporary increase the priority of SPI flash operation task
to the same level as the IPC task.

Fixes https://github.com/espressif/arduino-esp32/issues/740
Fixes https://github.com/espressif/esp-idf/issues/1157
This commit is contained in:
Ivan Grokhotkov 2017-10-30 18:53:39 +08:00
parent abbccb7acd
commit f9f2937694
2 changed files with 64 additions and 0 deletions

View File

@ -110,6 +110,10 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
assert(other_cpuid == 1);
spi_flash_disable_cache(other_cpuid, &s_flash_op_cache_state[other_cpuid]);
} 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);
// 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;
@ -121,6 +125,8 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
}
// Disable scheduler on the current CPU
vTaskSuspendAll();
// Can now set the priority back to the normal one
vTaskPrioritySet(NULL, old_prio);
// This is guaranteed to run on CPU <cpuid> because the other CPU is now
// occupied by highest priority task
assert(xPortGetCoreID() == cpuid);

View File

@ -167,3 +167,61 @@ TEST_CASE("spi flash functions can run along with IRAM interrupts", "[spi_flash]
free(read_arg.buf);
}
#if portNUM_PROCESSORS > 1
TEST_CASE("spi_flash deadlock with high priority busy-waiting task", "[spi_flash]")
{
typedef struct {
QueueHandle_t queue;
volatile bool done;
} deadlock_test_arg_t;
/* Create two tasks: high-priority consumer on CPU0, low-priority producer on CPU1.
* Consumer polls the queue until it gets some data, then yields.
* Run flash operation on CPU0. Check that when IPC1 task blocks out the producer,
* the task which does flash operation does not get blocked by the consumer.
*/
void producer_task(void* varg)
{
int dummy = 0;
deadlock_test_arg_t* arg = (deadlock_test_arg_t*) varg;
while (!arg->done) {
xQueueSend(arg->queue, &dummy, 0);
vTaskDelay(1);
}
vTaskDelete(NULL);
}
void consumer_task(void* varg)
{
int dummy;
deadlock_test_arg_t* arg = (deadlock_test_arg_t*) varg;
while (!arg->done) {
if (xQueueReceive(arg->queue, &dummy, 0) == pdTRUE) {
vTaskDelay(1);
}
}
vTaskDelete(NULL);
}
deadlock_test_arg_t arg = {
.queue = xQueueCreate(32, sizeof(int)),
.done = false
};
TEST_ASSERT(xTaskCreatePinnedToCore(&producer_task, "producer", 4096, &arg, 5, NULL, 1));
TEST_ASSERT(xTaskCreatePinnedToCore(&consumer_task, "consumer", 4096, &arg, 10, NULL, 0));
for (int i = 0; i < 1000; i++) {
uint32_t dummy;
TEST_ESP_OK(spi_flash_read(0, &dummy, sizeof(dummy)));
}
arg.done = true;
vTaskDelay(5);
vQueueDelete(arg.queue);
/* Check that current task priority is still correct */
TEST_ASSERT_EQUAL_INT(uxTaskPriorityGet(NULL), UNITY_FREERTOS_PRIORITY);
}
#endif // portNUM_PROCESSORS > 1