spi_flash: fix race condition in s_flash_op_complete access

Flash operation complete flag was cleared by the core initiating flash
operation. If the other core was running an ISR, then IPC task could
be late to enter the loop to check s_flash_op_complete by the time next
flash operation started. If the flag is cleared on the CPU waiting on
this flag, then the race condition can not happen.
This commit is contained in:
Ivan Grokhotkov 2017-03-28 01:02:58 +08:00
parent cecdfdb0c0
commit 92436021ab
2 changed files with 84 additions and 2 deletions

View File

@ -69,9 +69,12 @@ void IRAM_ATTR spi_flash_op_block_func(void* arg)
uint32_t cpuid = (uint32_t) arg; uint32_t cpuid = (uint32_t) arg;
// Disable cache so that flash operation can start // Disable cache so that flash operation can start
spi_flash_disable_cache(cpuid, &s_flash_op_cache_state[cpuid]); spi_flash_disable_cache(cpuid, &s_flash_op_cache_state[cpuid]);
// s_flash_op_complete flag is cleared on *this* CPU, otherwise the other
// CPU may reset the flag back to false before IPC task has a chance to check it
// (if it is preempted by an ISR taking non-trivial amount of time)
s_flash_op_complete = false;
s_flash_op_can_start = true; s_flash_op_can_start = true;
while (!s_flash_op_complete) { while (!s_flash_op_complete) {
// until we have a way to use interrupts for inter-CPU communication,
// busy loop here and wait for the other CPU to finish flash operation // busy loop here and wait for the other CPU to finish flash operation
} }
// Flash operation is complete, re-enable cache // Flash operation is complete, re-enable cache
@ -105,7 +108,6 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu()
// Signal to the spi_flash_op_block_task on the other CPU that we need it to // 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. // disable cache there and block other tasks from executing.
s_flash_op_can_start = false; s_flash_op_can_start = false;
s_flash_op_complete = false;
esp_err_t ret = esp_ipc_call(other_cpuid, &spi_flash_op_block_func, (void*) other_cpuid); esp_err_t ret = esp_ipc_call(other_cpuid, &spi_flash_op_block_func, (void*) other_cpuid);
assert(ret == ESP_OK); assert(ret == ESP_OK);
while (!s_flash_op_can_start) { while (!s_flash_op_can_start) {

View File

@ -6,6 +6,8 @@
#include <unity.h> #include <unity.h>
#include <esp_spi_flash.h> #include <esp_spi_flash.h>
#include <esp_attr.h> #include <esp_attr.h>
#include "driver/timer.h"
#include "esp_intr_alloc.h"
struct flash_test_ctx { struct flash_test_ctx {
uint32_t offset; uint32_t offset;
@ -87,3 +89,81 @@ TEST_CASE("flash write and erase work both on PRO CPU and on APP CPU", "[spi_fla
vSemaphoreDelete(done); vSemaphoreDelete(done);
} }
typedef struct {
size_t buf_size;
uint8_t* buf;
size_t flash_addr;
size_t repeat_count;
SemaphoreHandle_t done;
} read_task_arg_t;
typedef struct {
size_t delay_time_us;
size_t repeat_count;
} block_task_arg_t;
static void IRAM_ATTR timer_isr(void* varg) {
block_task_arg_t* arg = (block_task_arg_t*) varg;
TIMERG0.int_clr_timers.t0 = 1;
TIMERG0.hw_timer[0].config.alarm_en = 1;
ets_delay_us(arg->delay_time_us);
arg->repeat_count++;
}
static void read_task(void* varg) {
read_task_arg_t* arg = (read_task_arg_t*) varg;
for (size_t i = 0; i < arg->repeat_count; ++i) {
ESP_ERROR_CHECK( spi_flash_read(arg->flash_addr, arg->buf, arg->buf_size) );
}
xSemaphoreGive(arg->done);
vTaskDelay(1);
vTaskDelete(NULL);
}
TEST_CASE("spi flash functions can run along with IRAM interrupts", "[spi_flash]")
{
const size_t size = 128;
read_task_arg_t read_arg = {
.buf_size = size,
.buf = (uint8_t*) malloc(size),
.flash_addr = 0,
.repeat_count = 1000,
.done = xSemaphoreCreateBinary()
};
timer_config_t config = {
.alarm_en = true,
.counter_en = false,
.intr_type = TIMER_INTR_LEVEL,
.counter_dir = TIMER_COUNT_UP,
.auto_reload = true,
.divider = 80
};
block_task_arg_t block_arg = {
.repeat_count = 0,
.delay_time_us = 100
};
ESP_ERROR_CHECK( timer_init(TIMER_GROUP_0, TIMER_0, &config) );
timer_pause(TIMER_GROUP_0, TIMER_0);
ESP_ERROR_CHECK( timer_set_alarm_value(TIMER_GROUP_0, TIMER_0, 120) );
intr_handle_t handle;
ESP_ERROR_CHECK( timer_isr_register(TIMER_GROUP_0, TIMER_0, &timer_isr, &block_arg, ESP_INTR_FLAG_IRAM, &handle) );
timer_set_counter_value(TIMER_GROUP_0, TIMER_0, 0);
timer_enable_intr(TIMER_GROUP_0, TIMER_0);
timer_start(TIMER_GROUP_0, TIMER_0);
xTaskCreatePinnedToCore(read_task, "r", 2048, &read_arg, 3, NULL, 1);
xSemaphoreTake(read_arg.done, portMAX_DELAY);
timer_pause(TIMER_GROUP_0, TIMER_0);
timer_disable_intr(TIMER_GROUP_0, TIMER_0);
esp_intr_free(handle);
vSemaphoreDelete(read_arg.done);
free(read_arg.buf);
}