Merge branch 'bugfix/fix_spi_flash_api_concurrency_issue_v5.1' into 'release/v5.1'

spi_flash: fix concurrency issue when concurrently calling esp_flash apis (v5.1)

See merge request espressif/esp-idf!24508
This commit is contained in:
morris 2023-07-10 10:24:51 +08:00
commit 0930b5ca1e
20 changed files with 434 additions and 19 deletions

View File

@ -9,7 +9,7 @@
#include "esp_heap_caps.h"
// load partition table in tests will use memory
#define TEST_MEMORY_LEAK_THRESHOLD (450)
#define TEST_MEMORY_LEAK_THRESHOLD (600)
void setUp(void)
{

View File

@ -47,6 +47,6 @@ TEST_CASE("MSPI: Test mspi timing turning time cost", "[mspi]")
printf("mspi psram speed up cost %ld\n", cost);
}
TEST_ASSERT_LESS_THAN_UINT32(TEST_TIME_LIMIT_US, slow_down_time[TEST_TIME_CNT / 2]);
TEST_ASSERT_LESS_THAN_UINT32(TEST_TIME_LIMIT_US, speed_up_time[TEST_TIME_CNT / 2]);
TEST_ASSERT_LESS_OR_EQUAL_UINT32(TEST_TIME_LIMIT_US, slow_down_time[TEST_TIME_CNT / 2]);
TEST_ASSERT_LESS_OR_EQUAL_UINT32(TEST_TIME_LIMIT_US, speed_up_time[TEST_TIME_CNT / 2]);
}

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -22,9 +22,13 @@
#include "esp_private/spi_common_internal.h"
#define SPI_FLASH_CACHE_NO_DISABLE (CONFIG_SPI_FLASH_AUTO_SUSPEND || (CONFIG_SPIRAM_FETCH_INSTRUCTIONS && CONFIG_SPIRAM_RODATA))
#define SPI_FLASH_CACHE_NO_DISABLE (CONFIG_SPI_FLASH_AUTO_SUSPEND || (CONFIG_SPIRAM_FETCH_INSTRUCTIONS && CONFIG_SPIRAM_RODATA) || CONFIG_APP_BUILD_TYPE_RAM)
static const char TAG[] = "spi_flash";
#if SPI_FLASH_CACHE_NO_DISABLE
static _lock_t s_spi1_flash_mutex;
#endif // #if SPI_FLASH_CACHE_NO_DISABLE
/*
* OS functions providing delay service and arbitration among chips, and with the cache.
*
@ -55,21 +59,19 @@ static inline void on_spi_acquired(app_func_arg_t* ctx);
static inline void on_spi_yielded(app_func_arg_t* ctx);
static inline bool on_spi_check_yield(app_func_arg_t* ctx);
#if !SPI_FLASH_CACHE_NO_DISABLE
IRAM_ATTR static void cache_enable(void* arg)
{
#if !SPI_FLASH_CACHE_NO_DISABLE
spi_flash_enable_interrupts_caches_and_other_cpu();
#endif
}
IRAM_ATTR static void cache_disable(void* arg)
{
#if !SPI_FLASH_CACHE_NO_DISABLE
spi_flash_disable_interrupts_caches_and_other_cpu();
#endif
}
#endif //#if !SPI_FLASH_CACHE_NO_DISABLE
static IRAM_ATTR esp_err_t spi_start(void *arg)
static IRAM_ATTR esp_err_t acquire_spi_bus_lock(void *arg)
{
spi_bus_lock_dev_handle_t dev_lock = ((app_func_arg_t *)arg)->dev_lock;
@ -82,41 +84,58 @@ static IRAM_ATTR esp_err_t spi_start(void *arg)
return ESP_OK;
}
static IRAM_ATTR esp_err_t spi_end(void *arg)
static IRAM_ATTR esp_err_t release_spi_bus_lock(void *arg)
{
return spi_bus_lock_acquire_end(((app_func_arg_t *)arg)->dev_lock);
}
static IRAM_ATTR esp_err_t spi23_start(void *arg){
esp_err_t ret = spi_start(arg);
esp_err_t ret = acquire_spi_bus_lock(arg);
on_spi_acquired((app_func_arg_t*)arg);
return ret;
}
static IRAM_ATTR esp_err_t spi23_end(void *arg){
esp_err_t ret = spi_end(arg);
esp_err_t ret = release_spi_bus_lock(arg);
on_spi_released((app_func_arg_t*)arg);
return ret;
}
static IRAM_ATTR esp_err_t spi1_start(void *arg)
{
esp_err_t ret = ESP_OK;
/**
* There are three ways for ESP Flash API lock:
* 1. spi bus lock, this is used when SPI1 is shared with GPSPI Master Driver
* 2. mutex, this is used when the Cache isn't need to be disabled.
* 3. cache lock (from cache_utils.h), this is used when we need to disable Cache to avoid access from SPI0
*
* From 1 to 3, the lock efficiency decreases.
*/
#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS
//use the lock to disable the cache and interrupts before using the SPI bus
return spi_start(arg);
ret = acquire_spi_bus_lock(arg);
#elif SPI_FLASH_CACHE_NO_DISABLE
_lock_acquire(&s_spi1_flash_mutex);
#else
//directly disable the cache and interrupts when lock is not used
cache_disable(NULL);
on_spi_acquired((app_func_arg_t*)arg);
return ESP_OK;
#endif
on_spi_acquired((app_func_arg_t*)arg);
return ret;
}
static IRAM_ATTR esp_err_t spi1_end(void *arg)
{
esp_err_t ret = ESP_OK;
/**
* There are three ways for ESP Flash API lock, see `spi1_start`
*/
#if CONFIG_SPI_FLASH_SHARE_SPI1_BUS
ret = spi_end(arg);
ret = release_spi_bus_lock(arg);
#elif SPI_FLASH_CACHE_NO_DISABLE
_lock_release(&s_spi1_flash_mutex);
#else
cache_enable(NULL);
#endif

View File

@ -0,0 +1,5 @@
set(srcs "test_flash_utils.c")
idf_component_register(SRCS ${srcs}
INCLUDE_DIRS include
REQUIRES esp_partition)

View File

@ -0,0 +1,27 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once
#include <esp_types.h>
#include "esp_partition.h"
#ifdef __cplusplus
extern "C" {
#endif
/**
* Return the 'flash_test' custom data partition
* defined in the custom partition table.
*
* @return partition handle
*/
const esp_partition_t *get_test_flash_partition(void);
#ifdef __cplusplus
}
#endif

View File

@ -0,0 +1,17 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <string.h>
#include "test_flash_utils.h"
const esp_partition_t *get_test_flash_partition(void)
{
/* This finds "flash_test" partition defined in custom partitions.csv */
const esp_partition_t *result = esp_partition_find_first(ESP_PARTITION_TYPE_DATA,
ESP_PARTITION_SUBTYPE_ANY, "flash_test");
assert(result != NULL); /* means partition table set wrong */
return result;
}

View File

@ -9,7 +9,7 @@
#include "esp_heap_caps.h"
// Some resources are lazy allocated in flash encryption, the threadhold is left for that case
#define TEST_MEMORY_LEAK_THRESHOLD (400)
#define TEST_MEMORY_LEAK_THRESHOLD (600)
void setUp(void)
{

View File

@ -0,0 +1,8 @@
# This is the project CMakeLists.txt file for the test subproject
cmake_minimum_required(VERSION 3.16)
set(EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/components/spi_flash/test_apps/components")
include($ENV{IDF_PATH}/tools/cmake/project.cmake)
project(test_esp_flash_stress)

View File

@ -0,0 +1,2 @@
| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C6 | ESP32-H2 | ESP32-S2 | ESP32-S3 |
| ----------------- | ----- | -------- | -------- | -------- | -------- | -------- | -------- |

View File

@ -0,0 +1,8 @@
set(srcs "test_app_main.c"
"test_esp_flash_stress.c")
# In order for the cases defined by `TEST_CASE` to be linked into the final elf,
# the component can be registered as WHOLE_ARCHIVE
idf_component_register(SRCS ${srcs}
PRIV_REQUIRES test_flash_utils spi_flash unity
WHOLE_ARCHIVE)

View File

@ -0,0 +1,35 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "unity.h"
#include "unity_test_utils.h"
#include "esp_heap_caps.h"
// load partition table in tests will use memory
#define TEST_MEMORY_LEAK_THRESHOLD (600)
void setUp(void)
{
unity_utils_record_free_mem();
}
void tearDown(void)
{
esp_reent_cleanup(); //clean up some of the newlib's lazy allocations
unity_utils_evaluate_leaks_direct(TEST_MEMORY_LEAK_THRESHOLD);
}
void app_main(void)
{
printf(" ______ _____ _____ ______ _ _ _____ _ \n");
printf("| ____|/ ____| __ \\ | ____| | | | / ____| | \n");
printf("| |__ | (___ | |__) | | |__ | | __ _ ___| |__ | (___ | |_ _ __ ___ ___ ___ \n");
printf("| __| \\___ \\| ___/ | __| | |/ _` / __| '_ \\ \\___ \\| __| '__/ _ \\/ __/ __|\n");
printf("| |____ ____) | | | | | | (_| \\__ \\ | | | ____) | |_| | | __/\\__ \\__ \\\n");
printf("|______|_____/|_| |_| |_|\\__,_|___/_| |_| |_____/ \\__|_| \\___||___/___/\n");
unity_run_menu();
}

View File

@ -0,0 +1,200 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <stdio.h>
#include <string.h>
#include "unity.h"
#include "sdkconfig.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "esp_system.h"
#include "esp_check.h"
#include "esp_attr.h"
#include "esp_flash.h"
#include "test_flash_utils.h"
portMUX_TYPE static s_test_spinlock = portMUX_INITIALIZER_UNLOCKED;
/*---------------------------------------------------------------
ESP Flash API Concurrency Pressure Test
---------------------------------------------------------------*/
#define TEST_FLASH_CONCURRENCY_TASK_NUM 12
static int s_test_concurrency_id;
//only used for finite loop for CI test
SemaphoreHandle_t s_test_concurrency_smphr;
typedef struct {
bool is_pressure_test;
const esp_partition_t *part;
} test_concurrency_ctx_t;
static void s_test_flash_ops_task(void *arg)
{
test_concurrency_ctx_t *test_ctx = (test_concurrency_ctx_t *)arg;
bool is_pressure_test = test_ctx->is_pressure_test;;
esp_flash_t *chip = test_ctx->part->flash_chip;
uint32_t offs = test_ctx->part->address;
portENTER_CRITICAL(&s_test_spinlock);
int id = s_test_concurrency_id;
s_test_concurrency_id++;
portEXIT_CRITICAL(&s_test_spinlock);
int flash_block_sz = 4096;
char *buffer = calloc(1, flash_block_sz);
TEST_ASSERT(buffer);
srand(id + 299);
int delay_us = rand() % 1000;
printf("delay_us: %d\n", delay_us);
if (is_pressure_test) {
printf("INFINITE_LOOP\n");
while (1) {
printf("%s %d is running\n", __func__, id);
TEST_ESP_OK(esp_flash_erase_region(chip, id * flash_block_sz + offs, flash_block_sz));
TEST_ESP_OK(esp_flash_write(chip, buffer, id * flash_block_sz + offs, flash_block_sz));
TEST_ESP_OK(esp_flash_read(chip, buffer, id * flash_block_sz + offs, flash_block_sz));
vTaskDelay(pdMS_TO_TICKS(delay_us));
}
} else {
printf("FINITE_LOOP\n");
for (int i = 0; i < 10; i++) {
TEST_ESP_OK(esp_flash_erase_region(chip, id * flash_block_sz + offs, 4096));
TEST_ESP_OK(esp_flash_write(chip, buffer, id * flash_block_sz + offs, flash_block_sz));
TEST_ESP_OK(esp_flash_read(chip, buffer, id * flash_block_sz + offs, flash_block_sz));
vTaskDelay(pdMS_TO_TICKS(delay_us));
}
printf("%s %d finishes\n", __func__, id);
free(buffer);
xSemaphoreGive(s_test_concurrency_smphr);
vTaskDelete(NULL);
}
}
/*-------------------------------------------
Uni Core
-------------------------------------------*/
TEST_CASE("Flash UniCore: Test ESP Flash API Concurrency", "[esp_flash]")
{
const esp_partition_t* part = get_test_flash_partition();
s_test_concurrency_id = 0;
s_test_concurrency_smphr = xSemaphoreCreateCounting(TEST_FLASH_CONCURRENCY_TASK_NUM, 0);
TEST_ASSERT(s_test_concurrency_smphr);
char flash_task_name[32];
test_concurrency_ctx_t ctx = {
.is_pressure_test = false,
.part = part,
};
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM; i++) {
int l = snprintf(flash_task_name, 32, "flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 0);
}
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM; i++) {
TEST_ASSERT(xSemaphoreTake(s_test_concurrency_smphr, portMAX_DELAY) == pdTRUE);
}
vTaskDelay(1);
vSemaphoreDelete(s_test_concurrency_smphr);
}
/**
* esp_flash APIs concurrency pressure test
* This test is for manually test
*/
TEST_CASE("Flash UniCore: Test ESP Flash API Concurrency [Stress]", "[esp_flash][stress][manual][ignore]")
{
const esp_partition_t* part = get_test_flash_partition();
s_test_concurrency_id = 0;
char flash_task_name[32];
test_concurrency_ctx_t ctx = {
.is_pressure_test = true,
.part = part,
};
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM; i++) {
int l = snprintf(flash_task_name, 32, "flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 0);
}
while(1);
}
#if !CONFIG_FREERTOS_UNICORE
/*-------------------------------------------
Dual Core
-------------------------------------------*/
TEST_CASE("Flash DualCore: Test ESP Flash API Concurrency", "[esp_flash]")
{
const esp_partition_t* part = get_test_flash_partition();
s_test_concurrency_id = 0;
s_test_concurrency_smphr = xSemaphoreCreateCounting(TEST_FLASH_CONCURRENCY_TASK_NUM, 0);
TEST_ASSERT(s_test_concurrency_smphr);
char flash_task_name[32];
test_concurrency_ctx_t ctx = {
.is_pressure_test = false,
.part = part,
};
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM / 2; i++) {
int l = snprintf(flash_task_name, 32, "core0_flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 0);
}
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM / 2; i++) {
int l = snprintf(flash_task_name, 32, "core1_flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 1);
}
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM; i++) {
TEST_ASSERT(xSemaphoreTake(s_test_concurrency_smphr, portMAX_DELAY) == pdTRUE);
}
vTaskDelay(1);
vSemaphoreDelete(s_test_concurrency_smphr);
}
/**
* esp_flash APIs concurrency pressure test
* This test is for manually test
*/
TEST_CASE("Flash DualCore: Test ESP Flash API Concurrency [Stress]", "[esp_flash][stress][manual][ignore]")
{
const esp_partition_t* part = get_test_flash_partition();
s_test_concurrency_id = 0;
char flash_task_name[32];
test_concurrency_ctx_t ctx = {
.is_pressure_test = true,
.part = part,
};
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM / 2; i++) {
int l = snprintf(flash_task_name, 32, "core0_flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 0);
}
for (int i = 0; i < TEST_FLASH_CONCURRENCY_TASK_NUM / 2; i++) {
int l = snprintf(flash_task_name, 32, "core1_flash_ops_task%d", i);
flash_task_name[l] = '\0';
xTaskCreatePinnedToCore(&s_test_flash_ops_task, flash_task_name, 4096, (void *)(&ctx), 5, NULL, 1);
}
while(1);
}
#endif //#if !CONFIG_FREERTOS_UNICORE

View File

@ -0,0 +1,6 @@
# Name, Type, SubType, Offset, Size, Flags
# Note: if you have increased the bootloader size, make sure to update the offsets to avoid overlap
nvs, data, nvs, 0x9000, 0x6000,
phy_init, data, phy, 0xf000, 0x1000,
factory, app, factory, 0x10000, 1M,
flash_test, data, fat, , 512K,
1 # Name, Type, SubType, Offset, Size, Flags
2 # Note: if you have increased the bootloader size, make sure to update the offsets to avoid overlap
3 nvs, data, nvs, 0x9000, 0x6000,
4 phy_init, data, phy, 0xf000, 0x1000,
5 factory, app, factory, 0x10000, 1M,
6 flash_test, data, fat, , 512K,

View File

@ -0,0 +1,57 @@
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: Apache-2.0
import pytest
from pytest_embedded import Dut
@pytest.mark.supported_targets
@pytest.mark.generic
@pytest.mark.parametrize(
'config',
[
'release',
],
indirect=True,
)
def test_esp_flash_stress(dut: Dut) -> None:
dut.run_all_single_board_cases(group='esp_flash')
@pytest.mark.esp32s3
@pytest.mark.MSPI_F8R8
@pytest.mark.parametrize(
'config',
[
'esp32s3_f8r8',
],
indirect=True,
)
def test_esp_flash_stress_f8r8(dut: Dut) -> None:
dut.run_all_single_board_cases()
@pytest.mark.esp32s3
@pytest.mark.generic
@pytest.mark.parametrize(
'config',
[
'esp32s3_rom_xip_psram',
],
indirect=True,
)
def test_esp_flash_stress_rom_xip_psram(dut: Dut) -> None:
dut.run_all_single_board_cases()
@pytest.mark.esp32c3
@pytest.mark.flash_suspend
@pytest.mark.parametrize(
'config',
[
'esp32c3_suspend',
],
indirect=True,
)
def test_flash_auto_suspend(dut: Dut) -> None:
dut.run_all_single_board_cases()

View File

@ -0,0 +1,4 @@
# ESP32S3, F8R8, Flash 80M DDR, PSRAM 80M DDR, XIP_PSRAM
CONFIG_IDF_TARGET="esp32c3"
CONFIG_SPI_FLASH_AUTO_SUSPEND=y

View File

@ -0,0 +1,12 @@
# ESP32S3, F8R8, Flash 80M DDR, PSRAM 80M DDR, XIP_PSRAM
CONFIG_IDF_TARGET="esp32s3"
CONFIG_ESPTOOLPY_OCT_FLASH=y
CONFIG_ESPTOOLPY_FLASH_SAMPLE_MODE_DTR=y
CONFIG_ESPTOOLPY_FLASHFREQ_80M=y
CONFIG_ESPTOOLPY_FLASHSIZE_4MB=y
CONFIG_SPIRAM=y
CONFIG_SPIRAM_MODE_OCT=y
CONFIG_SPIRAM_SPEED_80M=y
CONFIG_SPIRAM_FETCH_INSTRUCTIONS=y
CONFIG_SPIRAM_RODATA=y

View File

@ -0,0 +1,8 @@
# ESP32S3, F8R8, ESP Flash ROM Version, XIP_PSRAM
CONFIG_IDF_TARGET="esp32s3"
CONFIG_SPIRAM=y
CONFIG_SPIRAM_FETCH_INSTRUCTIONS=y
CONFIG_SPIRAM_RODATA=y
CONFIG_SPI_FLASH_ROM_IMPL=y

View File

@ -0,0 +1,3 @@
CONFIG_COMPILER_OPTIMIZATION_SIZE=y
CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE=y
CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y

View File

@ -0,0 +1,4 @@
CONFIG_ESP_TASK_WDT_EN=n
CONFIG_PARTITION_TABLE_CUSTOM=y
CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv"
CONFIG_PARTITION_TABLE_FILENAME="partitions.csv"

View File

@ -9,7 +9,7 @@
#include "esp_heap_caps.h"
// Some resources are lazy allocated in flash encryption, the threadhold is left for that case
#define TEST_MEMORY_LEAK_THRESHOLD (-400)
#define TEST_MEMORY_LEAK_THRESHOLD (-600)
static size_t before_free_8bit;
static size_t before_free_32bit;