From 1ba3202c1852a3dfa1e0ede94066a1745e14b779 Mon Sep 17 00:00:00 2001 From: Dong Heng Date: Tue, 22 Jan 2019 11:53:48 +0800 Subject: [PATCH] fix(nvs_flash): 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. esp-idf commit ID: 7ae1df1c --- components/nvs_flash/src/nvs_page.cpp | 44 +++++++++++++------- components/nvs_flash/src/nvs_page.hpp | 2 +- components/nvs_flash/src/nvs_pagemanager.cpp | 43 +++++++++---------- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index bf9007ae..efadc185 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -386,7 +386,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; @@ -400,29 +400,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 7731e403..413da458 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 cb274619..31240d98 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);