From abea6c50f15f74c43f467a1179166ad031443b0a Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 31 Oct 2016 21:10:47 +0800 Subject: [PATCH] nvs_flash: delete all duplicate entries in a page while loading Due to previous flash write bug it was possible to create multiple duplicate entries in a single page. Recovery logic detected that case and bailed out with an assert. This change adds graceful recovery from this condition. Tests included. --- components/nvs_flash/src/nvs_page.cpp | 40 +++++++++++------- components/nvs_flash/test/test_nvs.cpp | 56 ++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index a10b88c976..23cefd1aad 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -301,6 +301,8 @@ esp_err_t Page::eraseEntryAndSpan(size_t index) } if (item.calculateCrc32() != item.crc32) { rc = alterEntryState(index, EntryState::ERASED); + --mUsedEntryCount; + ++mErasedEntryCount; if (rc != ESP_OK) { return rc; } @@ -473,7 +475,9 @@ esp_err_t Page::mLoadEntryTable() if (end > ENTRY_COUNT) { end = ENTRY_COUNT; } - for (size_t i = 0; i < end; ++i) { + size_t span; + for (size_t i = 0; i < end; i += span) { + span = 1; if (mEntryTable.get(i) == EntryState::ERASED) { lastItemIndex = INVALID_ENTRY; continue; @@ -488,6 +492,9 @@ esp_err_t Page::mLoadEntryTable() } mHashList.insert(item, i); + + // search for potential duplicate item + size_t duplicateIndex = mHashList.find(0, item); if (item.crc32 != item.calculateCrc32()) { err = eraseEntryAndSpan(i); @@ -498,23 +505,26 @@ esp_err_t Page::mLoadEntryTable() continue; } - if (item.datatype != ItemType::BLOB && item.datatype != ItemType::SZ) { - continue; - } - - size_t span = item.span; - bool needErase = false; - for (size_t j = i; j < i + span; ++j) { - if (mEntryTable.get(j) != EntryState::WRITTEN) { - needErase = true; - lastItemIndex = INVALID_ENTRY; - break; + + if (item.datatype == ItemType::BLOB || item.datatype == ItemType::SZ) { + span = item.span; + bool needErase = false; + for (size_t j = i; j < i + span; ++j) { + if (mEntryTable.get(j) != EntryState::WRITTEN) { + needErase = true; + lastItemIndex = INVALID_ENTRY; + break; + } + } + if (needErase) { + eraseEntryAndSpan(i); + continue; } } - if (needErase) { - eraseEntryAndSpan(i); + + if (duplicateIndex < i) { + eraseEntryAndSpan(duplicateIndex); } - i += span - 1; } // check that last item is not duplicate diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index b70801f848..528c9df686 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -963,22 +963,27 @@ TEST_CASE("duplicate items are removed", "[nvs][dupes]") p.writeItem(1, "opmode", 3); } { - // add another one without deleting the first one + // add another two without deleting the first one nvs::Item item(1, ItemType::U8, 1, "opmode"); item.data[0] = 2; item.crc32 = item.calculateCrc32(); emu.write(3 * 32, reinterpret_cast(&item), sizeof(item)); - uint32_t mask = 0xFFFFFFFA; + emu.write(4 * 32, reinterpret_cast(&item), sizeof(item)); + uint32_t mask = 0xFFFFFFEA; emu.write(32, &mask, 4); } { // load page and check that second item persists - nvs::Page p; - p.load(0); + nvs::Storage s; + s.init(0, 3); uint8_t val; - p.readItem(1, "opmode", val); + ESP_ERROR_CHECK(s.readItem(1, "opmode", val)); CHECK(val == 2); - CHECK(p.getErasedEntryCount() == 1); + } + { + Page p; + p.load(0); + CHECK(p.getErasedEntryCount() == 2); CHECK(p.getUsedEntryCount() == 1); } } @@ -986,8 +991,7 @@ TEST_CASE("duplicate items are removed", "[nvs][dupes]") TEST_CASE("recovery after failure to write data", "[nvs]") { SpiFlashEmulator emu(3); - // TODO: remove explicit alignment - const char str[] __attribute__((aligned(4))) = "value 0123456789abcdef012345678value 0123456789abcdef012345678"; + const char str[] = "value 0123456789abcdef012345678value 0123456789abcdef012345678"; // make flash write fail exactly in Page::writeEntryData emu.failAfter(17); @@ -1015,6 +1019,42 @@ TEST_CASE("recovery after failure to write data", "[nvs]") } } +TEST_CASE("crc error in variable length item is handled", "[nvs]") +{ + SpiFlashEmulator emu(3); + const uint64_t before_val = 0xbef04e; + const uint64_t after_val = 0xaf7e4; + // write some data + { + Page p; + p.load(0); + TEST_ESP_OK(p.writeItem(0, "before", before_val)); + const char* str = "foobar"; + TEST_ESP_OK(p.writeItem(0, ItemType::SZ, "key", str, strlen(str))); + TEST_ESP_OK(p.writeItem(0, "after", after_val)); + } + // corrupt some data + uint32_t w; + CHECK(emu.read(&w, 32 * 3 + 8, sizeof(w))); + w &= 0xf000000f; + CHECK(emu.write(32 * 3 + 8, &w, sizeof(w))); + // load and check + { + Page p; + p.load(0); + CHECK(p.getUsedEntryCount() == 2); + CHECK(p.getErasedEntryCount() == 2); + + uint64_t val; + TEST_ESP_OK(p.readItem(0, "before", val)); + CHECK(val == before_val); + TEST_ESP_ERR(p.findItem(0, ItemType::SZ, "key"), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(p.readItem(0, "after", val)); + CHECK(val == after_val); + } +} + + TEST_CASE("dump all performance data", "[nvs]") { std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;