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 5840fadda..658fdb53f 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -1458,6 +1458,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 */