Merge pull request #14483 from LDong-Arm/tdbstore_whitebox_low_ram

Improve HeapBlockDevice, TDBStore and tests

Author: 0xc0170

storage/blockdevice/include/blockdevice/HeapBlockDevice.h

@@ -28,9 +28,11 @@
 
 namespace mbed {
 
-/** Lazily allocated heap-backed block device
+/** Lazily allocated heap-backed block device.
  *
- * Useful for simulating a block device and tests
+ * Useful for simulating a block device and tests.
+ *
+ * @note Each block is allocated when used, and freed when erased.
  *
  * @code
  * #include "mbed.h"

storage/blockdevice/source/HeapBlockDevice.cpp

@@ -40,11 +40,11 @@ HeapBlockDevice::~HeapBlockDevice()
 {
     if (_blocks) {
         for (size_t i = 0; i < _count; i++) {
-            free(_blocks[i]);
+            delete[] _blocks[i];
         }
 
         delete[] _blocks;
-        _blocks = 0;
+        _blocks = nullptr;
     }
 }
 
@@ -57,9 +57,13 @@ int HeapBlockDevice::init()
     }
 
     if (!_blocks) {
-        _blocks = new uint8_t *[_count];
+        _blocks = new (std::nothrow) uint8_t *[_count];
+        if (!_blocks) {
+            return BD_ERROR_DEVICE_ERROR;
+        }
+
         for (size_t i = 0; i < _count; i++) {
-            _blocks[i] = 0;
+            _blocks[i] = nullptr;
         }
     }
 
@@ -156,7 +160,7 @@ int HeapBlockDevice::program(const void *b, bd_addr_t addr, bd_size_t size)
         bd_addr_t lo = addr % _erase_size;
 
         if (!_blocks[hi]) {
-            _blocks[hi] = (uint8_t *)malloc(_erase_size);
+            _blocks[hi] = new (std::nothrow) uint8_t[_erase_size];
             if (!_blocks[hi]) {
                 return BD_ERROR_DEVICE_ERROR;
             }
@@ -180,6 +184,13 @@ int HeapBlockDevice::erase(bd_addr_t addr, bd_size_t size)
     if (!is_valid_erase(addr, size)) {
         return BD_ERROR_DEVICE_ERROR;
     }
+
+    for (size_t i = 0; i < (size / _erase_size); i++) {
+        size_t index = addr / _erase_size + i;
+        delete[] _blocks[index];
+        _blocks[index] = nullptr;
+    }
+
     return 0;
 }
 

storage/kvstore/filesystemstore/tests/UNITTESTS/FileSystemStore/moduletest.cpp

@@ -67,21 +67,6 @@ TEST_F(FileSystemStoreModuleTest, set_get)
     EXPECT_STREQ("data", buf);
 }
 
-TEST_F(FileSystemStoreModuleTest, erased_set_get)
-{
-    EXPECT_EQ(store->deinit(), MBED_SUCCESS);
-    EXPECT_EQ(heap.init(), MBED_SUCCESS);
-    EXPECT_EQ(heap.erase(0, heap.size()), MBED_SUCCESS);
-    EXPECT_EQ(heap.deinit(), MBED_SUCCESS);
-    EXPECT_EQ(store->init(), MBED_SUCCESS);
-    char buf[100];
-    size_t size;
-    EXPECT_EQ(store->set("key", "data", 5, 0), MBED_SUCCESS);
-    EXPECT_EQ(store->get("key", buf, 100, &size), MBED_SUCCESS);
-    EXPECT_EQ(size, 5);
-    EXPECT_STREQ("data", buf);
-}
-
 TEST_F(FileSystemStoreModuleTest, set_deinit_init_get)
 {
     char buf[100];

storage/kvstore/tdbstore/include/tdbstore/TDBStore.h

@@ -346,14 +346,15 @@ class TDBStore : public KVStore {
     int reset_area(uint8_t area);
 
     /**
-     * @brief Erase an erase unit.
+     * @brief Erase an area.
      *
      * @param[in]  area                   Area.
      * @param[in]  offset                 Offset in area.
+     * @param[in]  size                   Number of bytes to erase.
      *
      * @returns 0 for success, nonzero for failure.
      */
-    int erase_erase_unit(uint8_t area, uint32_t offset);
+    int erase_area(uint8_t area, uint32_t offset, uint32_t size);
 
     /**
      * @brief Calculate addresses and sizes of areas.

storage/kvstore/tdbstore/source/TDBStore.cpp

@@ -170,24 +170,31 @@ int TDBStore::write_area(uint8_t area, uint32_t offset, uint32_t size, const voi
     return MBED_SUCCESS;
 }
 
-int TDBStore::erase_erase_unit(uint8_t area, uint32_t offset)
+int TDBStore::erase_area(uint8_t area, uint32_t offset, uint32_t size)
 {
     uint32_t bd_offset = _area_params[area].address + offset;
-    uint32_t eu_size = _buff_bd->get_erase_size(bd_offset);
 
-    if (_buff_bd->get_erase_value() != -1) {
-        return _buff_bd->erase(bd_offset, eu_size);
-    } else {
-        // We need to simulate erase, as our block device
-        // does not do it. We can do this one byte at a time
-        // because we use BufferedBlockDevice that has page buffers
-        uint8_t val = 0xff;
-        int ret;
-        for (; eu_size; --eu_size) {
-            ret = _buff_bd->program(&val, bd_offset++, 1);
+    int ret = _buff_bd->erase(bd_offset, size);
+    if (ret) {
+        return ret;
+    }
+
+    if (_buff_bd->get_erase_value() == -1) {
+        // We need to simulate erase to wipe records, as our block device
+        // may not do it. Program in chunks of _work_buf_size if the minimum
+        // program size is too small (e.g. one-byte) to avoid performance
+        // issues.
+        MBED_ASSERT(_work_buf != nullptr);
+        MBED_ASSERT(_work_buf_size != 0);
+        memset(_work_buf, 0xFF, _work_buf_size);
+        while (size) {
+            uint32_t chunk = std::min<uint32_t>(_work_buf_size, size);
+            ret = _buff_bd->program(_work_buf, bd_offset, chunk);
             if (ret) {
                 return ret;
             }
+            size -= chunk;
+            bd_offset += chunk;
         }
     }
     return MBED_SUCCESS;
@@ -1458,33 +1465,47 @@ void TDBStore::offset_in_erase_unit(uint8_t area, uint32_t offset,
                                     uint32_t &offset_from_start, uint32_t &dist_to_end)
 {
     uint32_t bd_offset = _area_params[area].address + offset;
-    uint32_t agg_offset = 0;
 
-    while (bd_offset >= agg_offset + _buff_bd->get_erase_size(agg_offset)) {
-        agg_offset += _buff_bd->get_erase_size(agg_offset);
-    }
-    offset_from_start = bd_offset - agg_offset;
-    dist_to_end = _buff_bd->get_erase_size(agg_offset) - offset_from_start;
+    // The parameter of `BlockDevice::get_erase_size(bd_addr_t addr)`
+    // does not need to be aligned.
+    uint32_t erase_unit = _buff_bd->get_erase_size(bd_offset);
+
+    // Even on a flash device with multiple regions, the start address of
+    // an erase unit is aligned to the current region's unit size.
+    offset_from_start = bd_offset % erase_unit;
+    dist_to_end = erase_unit - offset_from_start;
 }
 
 int TDBStore::check_erase_before_write(uint8_t area, uint32_t offset, uint32_t size, bool force_check)
 {
     // In order to save init time, we don't check that the entire area is erased.
     // Instead, whenever reaching an erase unit start erase it.
+    bool erase = false;
+    uint32_t start_offset;
+    uint32_t end_offset;
     while (size) {
         uint32_t dist, offset_from_start;
         int ret;
         offset_in_erase_unit(area, offset, offset_from_start, dist);
         uint32_t chunk = std::min(size, dist);
 
         if (offset_from_start == 0 || force_check) {
-            ret = erase_erase_unit(area, offset - offset_from_start);
-            if (ret != MBED_SUCCESS) {
-                return MBED_ERROR_WRITE_FAILED;
+            if (!erase) {
+                erase = true;
+                start_offset = offset - offset_from_start;
             }
+            end_offset = offset + dist;
         }
         offset += chunk;
         size -= chunk;
     }
+
+    if (erase) {
+        int ret = erase_area(area, start_offset, end_offset - start_offset);
+        if (ret != MBED_SUCCESS) {
+            return MBED_ERROR_WRITE_FAILED;
+        }
+    }
+
     return MBED_SUCCESS;
 }

storage/kvstore/tdbstore/tests/TESTS/tdbstore/whitebox/main.cpp

@@ -132,11 +132,14 @@ static void white_box_test()
         } else {
             test_bd = &heap_bd;
             // We need to skip the test if we don't have enough memory for the heap block device.
-            // However, this device allocates the erase units on the fly, so "erase" it via the flash
-            // simulator. A failure here means we haven't got enough memory.
-            heap_bd.init();
-            result = heap_bd.erase(0, heap_bd.size());
-            TEST_SKIP_UNLESS_MESSAGE(!result, "Not enough heap to run test");
+            // However, this device allocates the blocks on the fly when programmed. A failure
+            // here means we haven't got enough memory.
+            result = heap_bd.init();
+            TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test")
+            for (bd_addr_t addr = 0; addr < heap_bd.size(); addr += heap_bd.get_program_size()) {
+                result = heap_bd.program(dummy, addr, heap_bd.get_program_size());
+                TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test")
+            }
             heap_bd.deinit();
         }
 
@@ -345,11 +348,14 @@ static void multi_set_test()
 
 #ifdef USE_HEAP_BD
     // We need to skip the test if we don't have enough memory for the heap block device.
-    // However, this device allocates the erase units on the fly, so "erase" it via the flash
-    // simulator. A failure here means we haven't got enough memory.
-    flash_bd.init();
-    result = flash_bd.erase(0, flash_bd.size());
-    TEST_SKIP_UNLESS_MESSAGE(!result, "Not enough heap to run test");
+    // However, this device allocates the blocks on the fly when programmed. A failure
+    // here means we haven't got enough memory.
+    result = flash_bd.init();
+    TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test")
+    for (bd_addr_t addr = 0; addr < flash_bd.size(); addr += flash_bd.get_program_size()) {
+        result = flash_bd.program(dummy, addr, flash_bd.get_program_size());
+        TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test")
+    }
     flash_bd.deinit();
 #endif