From 7ae1df1c5e0a21687d494f9a1389d1282631544d Mon Sep 17 00:00:00 2001 From: Sagar Bijwe Date: Fri, 27 Apr 2018 12:40:29 +0530 Subject: [PATCH] nvs: Fix recovery from power-off while page is being freed Currently when page is being freed, items are individually moved from FREEING page to ACTIVE page and erased. If power-off happens during the process, the remaining entries are moved to ACTIVE page during recovery. The problem with this approach is there may not be enough space on ACTIVE page for all items if an item was partially written before power-off and erased during recovery. This change moves all the items from FREEING to ACTIVE page and then erased the FREEING page, If power-off happens during the process, then ACTIVE page is erased and the process is restarted. --- components/nvs_flash/src/nvs_page.cpp | 44 ++++++++++++------- components/nvs_flash/src/nvs_page.hpp | 2 +- components/nvs_flash/src/nvs_pagemanager.cpp | 43 +++++++++--------- .../nvs_flash/test_nvs_host/test_nvs.cpp | 42 ++++++++++++++++++ 4 files changed, 93 insertions(+), 38 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 54d6db7bd..9fc1a6eda 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -382,7 +382,7 @@ void Page::updateFirstUsedEntry(size_t index, size_t span) } } -esp_err_t Page::moveItem(Page& other) +esp_err_t Page::copyItems(Page& other) { if (mFirstUsedEntry == INVALID_ENTRY) { return ESP_ERR_NVS_NOT_FOUND; @@ -396,29 +396,41 @@ esp_err_t Page::moveItem(Page& other) } Item entry; - auto err = readEntry(mFirstUsedEntry, entry); - if (err != ESP_OK) { - return err; - } - other.mHashList.insert(entry, other.mNextFreeEntry); - err = other.writeEntry(entry); - if (err != ESP_OK) { - return err; - } + size_t readEntryIndex = mFirstUsedEntry; - size_t span = entry.span; - size_t end = mFirstUsedEntry + span; + while (readEntryIndex < ENTRY_COUNT) { - assert(mFirstUsedEntry != INVALID_ENTRY || span == 1); + if (mEntryTable.get(readEntryIndex) != EntryState::WRITTEN) { + assert(readEntryIndex != mFirstUsedEntry); + readEntryIndex++; + continue; + } + auto err = readEntry(readEntryIndex, entry); + if (err != ESP_OK) { + return err; + } - for (size_t i = mFirstUsedEntry + 1; i < end; ++i) { - readEntry(i, entry); + other.mHashList.insert(entry, other.mNextFreeEntry); err = other.writeEntry(entry); if (err != ESP_OK) { return err; } + size_t span = entry.span; + size_t end = readEntryIndex + span; + + assert(end <= ENTRY_COUNT); + + for (size_t i = readEntryIndex + 1; i < end; ++i) { + readEntry(i, entry); + err = other.writeEntry(entry); + if (err != ESP_OK) { + return err; + } + } + readEntryIndex = end; + } - return eraseEntryAndSpan(mFirstUsedEntry); + return ESP_OK; } esp_err_t Page::mLoadEntryTable() diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp index 7aa8b9fd8..9baa76dbf 100644 --- a/components/nvs_flash/src/nvs_page.hpp +++ b/components/nvs_flash/src/nvs_page.hpp @@ -127,7 +127,7 @@ public: esp_err_t markFreeing(); - esp_err_t moveItem(Page& other); + esp_err_t copyItems(Page& other); esp_err_t erase(); diff --git a/components/nvs_flash/src/nvs_pagemanager.cpp b/components/nvs_flash/src/nvs_pagemanager.cpp index c7401717f..3f3c5f01c 100644 --- a/components/nvs_flash/src/nvs_pagemanager.cpp +++ b/components/nvs_flash/src/nvs_pagemanager.cpp @@ -67,7 +67,9 @@ esp_err_t PageManager::load(uint32_t baseSector, uint32_t sectorCount) if (lastItemIndex != SIZE_MAX) { auto last = PageManager::TPageListIterator(&lastPage); for (auto it = begin(); it != last; ++it) { - if (it->eraseItem(item.nsIndex, item.datatype, item.key) == ESP_OK) { + + if ((it->state() != Page::PageState::FREEING) && + (it->eraseItem(item.nsIndex, item.datatype, item.key) == ESP_OK)) { break; } } @@ -77,23 +79,26 @@ esp_err_t PageManager::load(uint32_t baseSector, uint32_t sectorCount) for (auto it = begin(); it!= end(); ++it) { if (it->state() == Page::PageState::FREEING) { Page* newPage = &mPageList.back(); - if (newPage->state() != Page::PageState::ACTIVE) { - auto err = activatePage(); + if (newPage->state() == Page::PageState::ACTIVE) { + auto err = newPage->erase(); if (err != ESP_OK) { return err; } - newPage = &mPageList.back(); + mPageList.erase(newPage); + mFreePageList.push_back(newPage); } - while (true) { - auto err = it->moveItem(*newPage); - if (err == ESP_ERR_NVS_NOT_FOUND) { - break; - } else if (err != ESP_OK) { - return err; - } + auto err = activatePage(); + if (err != ESP_OK) { + return err; + } + newPage = &mPageList.back(); + + err = it->copyItems(*newPage); + if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { + return err; } - auto err = it->erase(); + err = it->erase(); if (err != ESP_OK) { return err; } @@ -109,7 +114,7 @@ esp_err_t PageManager::load(uint32_t baseSector, uint32_t sectorCount) if (mFreePageList.size() == 0) { return ESP_ERR_NVS_NO_FREE_PAGES; } - + return ESP_OK; } @@ -156,13 +161,9 @@ esp_err_t PageManager::requestNewPage() if (err != ESP_OK) { return err; } - while (true) { - err = erasedPage->moveItem(*newPage); - if (err == ESP_ERR_NVS_NOT_FOUND) { - break; - } else if (err != ESP_OK) { - return err; - } + err = erasedPage->copyItems(*newPage); + if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { + return err; } err = erasedPage->erase(); @@ -173,7 +174,7 @@ esp_err_t PageManager::requestNewPage() #ifndef NDEBUG assert(usedEntries == newPage->getUsedEntryCount()); #endif - + mPageList.erase(maxUnusedItemsPageIt); mFreePageList.push_back(erasedPage); diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index dd013e24a..6a06658ff 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -1456,6 +1456,48 @@ TEST_CASE("Recovery from power-off when the entry being erased is not on active nvs_close(handle); } +TEST_CASE("Recovery from power-off when page is being freed.", "[nvs]") +{ + const size_t blob_size = (Page::ENTRY_COUNT-3) * Page::ENTRY_SIZE; + size_t read_size = blob_size/2; + uint8_t blob[blob_size] = {0}; + SpiFlashEmulator emu(3); + TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, 0, 3)); + nvs_handle handle; + TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle)); + // Fill first page + TEST_ESP_OK(nvs_set_blob(handle, "1a", blob, blob_size/3)); + TEST_ESP_OK(nvs_set_blob(handle, "1b", blob, blob_size/3)); + TEST_ESP_OK(nvs_set_blob(handle, "1c", blob, blob_size/4)); + // Fill second page + TEST_ESP_OK(nvs_set_blob(handle, "2a", blob, blob_size/2)); + TEST_ESP_OK(nvs_set_blob(handle, "2b", blob, blob_size/2)); + + TEST_ESP_OK(nvs_erase_key(handle, "1c")); + + emu.clearStats(); + emu.failAfter(6 * Page::ENTRY_COUNT); + TEST_ESP_ERR(nvs_set_blob(handle, "1d", blob, blob_size/4), ESP_ERR_FLASH_OP_FAIL); + + TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, 0, 3)); + + read_size = blob_size/3; + TEST_ESP_OK( nvs_get_blob(handle, "1a", blob, &read_size)); + TEST_ESP_OK( nvs_get_blob(handle, "1b", blob, &read_size)); + + read_size = blob_size /4; + TEST_ESP_ERR( nvs_get_blob(handle, "1c", blob, &read_size), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR( nvs_get_blob(handle, "1d", blob, &read_size), ESP_ERR_NVS_NOT_FOUND); + + read_size = blob_size /2; + TEST_ESP_OK( nvs_get_blob(handle, "2a", blob, &read_size)); + TEST_ESP_OK( nvs_get_blob(handle, "2b", blob, &read_size)); + + TEST_ESP_OK(nvs_commit(handle)); + nvs_close(handle); +} + + /* Add new tests above */ /* This test has to be the final one */