spi_flash: Remove 16KB free internal heap limit for esp_flash_read() into PSRAM

Allocation of the temporary internal buffer will now repeat until a small enough buffer can be
allocated, and only fail if less than a 256 byte block of internal RAM is free.

Adds unit test for the same, and generic test utility for creating memory pressure.

components/spi_flash/esp_flash_api.c

@@ -497,11 +497,25 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
     bool direct_read = chip->host->supports_direct_read(chip->host, buffer);
     uint8_t* temp_buffer = NULL;
 
+    //each time, we at most read this length
+    //after that, we release the lock to allow some other operations
+    size_t read_chunk_size = MIN(MAX_READ_CHUNK, length);
+
     if (!direct_read) {
-        uint32_t length_to_allocate = MIN(MAX_READ_CHUNK, length);
+        /* Allocate temporary internal buffer to use for the actual read. If the preferred size
-        length_to_allocate = (length_to_allocate+3)&(~3);
+           doesn't fit in free internal memory, allocate the largest available free block.
-        temp_buffer = heap_caps_malloc(length_to_allocate, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+
-        ESP_LOGV(TAG, "allocate temp buffer: %p (%d)", temp_buffer, length_to_allocate);
+           (May need to shrink read_chunk_size and retry due to race conditions with other tasks
+           also allocating from the heap.)
+        */
+        unsigned retries = 5;
+        while(temp_buffer == NULL && retries--) {
+            read_chunk_size = MIN(read_chunk_size, heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT));
+            read_chunk_size = (read_chunk_size + 3) & ~3;
+            temp_buffer = heap_caps_malloc(read_chunk_size, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+        }
+        ESP_LOGV(TAG, "allocate temp buffer: %p (%d)", temp_buffer, read_chunk_size);
+
         if (temp_buffer == NULL) {
             return ESP_ERR_NO_MEM;
         }
@@ -516,9 +530,9 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
         }
         //if required (dma buffer allocated), read to the buffer instead of the original buffer
         uint8_t* buffer_to_read = (temp_buffer)? temp_buffer : buffer;
-        //each time, we at most read this length
+
-        //after that, we release the lock to allow some other operations
+        // Length we will read this iteration is either the chunk size or the remaining length, whichever is smaller
-        uint32_t length_to_read = MIN(MAX_READ_CHUNK, length);
+        size_t length_to_read = MIN(read_chunk_size, length);
 
         if (err == ESP_OK) {
             err = chip->chip_drv->read(chip, buffer_to_read, address, length_to_read);

components/spi_flash/test/test_esp_flash.c

@@ -3,7 +3,7 @@
 #include <freertos/FreeRTOS.h>
 #include <freertos/task.h>
 #include <freertos/semphr.h>
-
+#include <sys/param.h>
 #include <unity.h>
 #include "esp_flash.h"
 #include "driver/spi_common_internal.h"
@@ -597,11 +597,26 @@ static void test_write_large_buffer(esp_flash_t *chip, const uint8_t *source, si
 
 #ifdef CONFIG_SPIRAM_USE_MALLOC
 
+/* Utility: Read into a small internal RAM buffer using esp_flash_read() and compare what
+   we read with 'buffer' */
+static void s_test_compare_flash_contents_small_reads(esp_flash_t *chip, const uint8_t *buffer, size_t offs, size_t len)
+{
+    const size_t INTERNAL_BUF_SZ = 1024; // Should fit in internal RAM
+    uint8_t *ibuf = heap_caps_malloc(INTERNAL_BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_INTERNAL);
+    TEST_ASSERT_NOT_NULL(ibuf);
+
+    for (int i = 0; i < len; i += INTERNAL_BUF_SZ) {
+        size_t to_read = MIN(INTERNAL_BUF_SZ, len - i);
+        ESP_ERROR_CHECK( esp_flash_read(chip, ibuf, offs + i, to_read) );
+        TEST_ASSERT_EQUAL_HEX8_ARRAY(buffer + i, ibuf, to_read);
+    }
+
+    free(ibuf);
+}
+
 static void test_flash_read_large_psram_buffer(esp_flash_t *chip)
 {
     const size_t BUF_SZ = 256 * 1024;    // Too large for internal RAM
-    const size_t INTERNAL_BUF_SZ = 1024; // Should fit in internal RAM
-    _Static_assert(BUF_SZ % INTERNAL_BUF_SZ == 0, "should be a multiple");
     const size_t TEST_OFFS = 0x1000; // Can be any offset, really
 
     uint8_t *buf = heap_caps_malloc(BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_SPIRAM);
@@ -610,18 +625,41 @@ static void test_flash_read_large_psram_buffer(esp_flash_t *chip)
     ESP_ERROR_CHECK( esp_flash_read(chip, buf, TEST_OFFS, BUF_SZ) );
 
     // Read back the same into smaller internal memory buffer and check it all matches
-    uint8_t *ibuf = heap_caps_malloc(INTERNAL_BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_INTERNAL);
+    s_test_compare_flash_contents_small_reads(chip, buf, TEST_OFFS, BUF_SZ);
-    TEST_ASSERT_NOT_NULL(ibuf);
 
-    for (int i = 0; i < BUF_SZ; i += INTERNAL_BUF_SZ) {
-        ESP_ERROR_CHECK( esp_flash_read(chip, ibuf, TEST_OFFS + i, INTERNAL_BUF_SZ) );
-        TEST_ASSERT_EQUAL_HEX8_ARRAY(buf + i, ibuf, INTERNAL_BUF_SZ);
-    }
-
-    free(ibuf);
     free(buf);
 }
 
 FLASH_TEST_CASE("esp_flash_read large PSRAM buffer", test_flash_read_large_psram_buffer);
 
+
+/* similar to above test, but perform it under memory pressure */
+static void test_flash_read_large_psram_buffer_low_internal_mem(esp_flash_t *chip)
+{
+    const size_t BUF_SZ = 256 * 1024;    // Too large for internal RAM
+    const size_t REMAINING_INTERNAL = 1024; // Exhaust internal memory until maximum free block is less than this
+    const size_t TEST_OFFS = 0x8000;
+
+    /* Exhaust the available free internal memory */
+    test_utils_exhaust_memory_rec erec = test_utils_exhaust_memory(MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT, REMAINING_INTERNAL);
+
+    uint8_t *buf = heap_caps_malloc(BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_SPIRAM);
+    TEST_ASSERT_NOT_NULL(buf);
+
+    /* Calling esp_flash_read() here will need to allocate a small internal buffer,
+       so check it works. */
+    ESP_ERROR_CHECK( esp_flash_read(chip, buf, TEST_OFFS, BUF_SZ) );
+
+    test_utils_free_exhausted_memory(erec);
+
+    // Read back the same into smaller internal memory buffer and check it all matches
+    s_test_compare_flash_contents_small_reads(chip, buf, TEST_OFFS, BUF_SZ);
+
+    free(buf);
+}
+
+FLASH_TEST_CASE("esp_flash_read large PSRAM buffer low memory", test_flash_read_large_psram_buffer_low_internal_mem);
+
+
+
 #endif

tools/unit-test-app/components/test_utils/include/test_utils.h

@@ -236,3 +236,31 @@ esp_err_t test_utils_set_leak_level(size_t leak_level, esp_type_leak_t type, esp
  * return Leak level
  */
 size_t test_utils_get_leak_level(esp_type_leak_t type, esp_comp_leak_t component);
+
+
+
+typedef struct test_utils_exhaust_memory_record_s *test_utils_exhaust_memory_rec;
+
+/**
+ * Limit the largest free block of memory with a particular capability set to
+ * 'limit' bytes (meaning an allocation of 'limit' should succeed at least once,
+ * but any allocation of more bytes will fail.)
+ *
+ * Returns a record pointer which needs to be passed back in to test_utils_free_exhausted_memory
+ * before the test completes, to avoid a major memory leak.
+ *
+ * @param caps Capabilities of memory to exhause
+ * @param limit The size to limit largest free block to
+ * @return Record pointer to pass to test_utils_free_exhausted_memory() once done
+ */
+test_utils_exhaust_memory_rec test_utils_exhaust_memory(uint32_t caps, size_t limit);
+
+
+/**
+ * Call to free memory which was taken up by test_utils_exhaust_memory() call
+ *
+ * @param rec Result previously returned from test_utils_exhaust_memory()
+ */
+void test_utils_free_exhausted_memory(test_utils_exhaust_memory_rec rec);
+
+

tools/unit-test-app/components/test_utils/test_utils.c

@@ -143,3 +143,41 @@ size_t test_utils_get_leak_level(esp_type_leak_t type_of_leak, esp_comp_leak_t c
     }
     return leak_level;
 }
+
+
+#define EXHAUST_MEMORY_ENTRIES 100
+
+struct test_utils_exhaust_memory_record_s {
+    int *entries[EXHAUST_MEMORY_ENTRIES];
+};
+
+test_utils_exhaust_memory_rec test_utils_exhaust_memory(uint32_t caps, size_t limit)
+{
+    int idx = 0;
+    test_utils_exhaust_memory_rec rec = calloc(1, sizeof(struct test_utils_exhaust_memory_record_s));
+    TEST_ASSERT_NOT_NULL_MESSAGE(rec, "test_utils_exhaust_memory: not enough free memory to allocate record structure!");
+
+    while (idx < EXHAUST_MEMORY_ENTRIES) {
+        size_t free_caps = heap_caps_get_largest_free_block(caps);
+        if (free_caps <= limit) {
+            return rec; // done!
+        }
+        rec->entries[idx] = heap_caps_malloc(free_caps - limit, caps);
+        TEST_ASSERT_NOT_NULL_MESSAGE(rec->entries[idx],
+                                     "test_utils_exhaust_memory: something went wrong while freeing up memory, is another task using heap?");
+        heap_caps_check_integrity_all(true);
+        idx++;
+    }
+
+    TEST_FAIL_MESSAGE("test_utils_exhaust_memory: The heap with the requested caps is too fragmented, increase EXHAUST_MEMORY_ENTRIES or defrag the heap!");
+    abort();
+}
+
+void test_utils_free_exhausted_memory(test_utils_exhaust_memory_rec rec)
+{
+    for (int i = 0; i < EXHAUST_MEMORY_ENTRIES; i++) {
+        free(rec->entries[i]);
+    }
+    free(rec);
+}
+