nvs: Fix page selection algo to consider free entry counts as well

Current page selection algorithm selects a page for compaction based on just erased counts
and gives up when it does not find any page with erased count greater than 0. This is
problematic since the current allocation procedure skips the active page if there is not
enough room for the item in that page leaving free chunks on the pages. This change modifies
the algorithm to consider both erased as well as free counts on the candidate pages.

Closes TW<20297>

diff --git a/components/nvs_flash/src/nvs_pagemanager.cpp b/components/nvs_flash/src/nvs_pagemanager.cpp
index 943f54f2f8125f9892023bd0db4633c501da9682..cb274619e44747aae956e306a6ccfeacc935d9ea 100644 (file)
--- a/components/nvs_flash/src/nvs_pagemanager.cpp
+++ b/components/nvs_flash/src/nvs_pagemanager.cpp
@@ -125,17 +125,18 @@ esp_err_t PageManager::requestNewPage()
     }
 
     // find the page with the higest number of erased items
-    TPageListIterator maxErasedItemsPageIt;
-    size_t maxErasedItems = 0;
+    TPageListIterator maxUnusedItemsPageIt;
+    size_t maxUnusedItems = 0;
     for (auto it = begin(); it != end(); ++it) {
-        auto erased = it->getErasedEntryCount();
-        if (erased > maxErasedItems) {
-            maxErasedItemsPageIt = it;
-            maxErasedItems = erased;
+
+        auto unused =  Page::ENTRY_COUNT - it->getUsedEntryCount();
+        if (unused > maxUnusedItems) {
+            maxUnusedItemsPageIt = it;
+            maxUnusedItems = unused;
         }
     }
 
-    if (maxErasedItems == 0) {
+    if (maxUnusedItems == 0) {
         return ESP_ERR_NVS_NOT_ENOUGH_SPACE;
     }
 
@@ -146,7 +147,8 @@ esp_err_t PageManager::requestNewPage()
 
     Page* newPage = &mPageList.back();
 
-    Page* erasedPage = maxErasedItemsPageIt;
+    Page* erasedPage = maxUnusedItemsPageIt;
+
 #ifndef NDEBUG
     size_t usedEntries = erasedPage->getUsedEntryCount();
 #endif
@@ -172,7 +174,7 @@ esp_err_t PageManager::requestNewPage()
     assert(usedEntries == newPage->getUsedEntryCount());
 #endif
     
-    mPageList.erase(maxErasedItemsPageIt);
+    mPageList.erase(maxUnusedItemsPageIt);
     mFreePageList.push_back(erasedPage);
 
     return ESP_OK;

diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp
index f419256cf334b9b8c7db3cd0f025c96e7cd53bce..0979bb40da6189a397519eec57a15ae6513a4be5 100644 (file)
--- a/components/nvs_flash/test_nvs_host/test_nvs.cpp
+++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp
@@ -1246,6 +1246,27 @@ TEST_CASE("multiple partitions access check", "[nvs]")
     CHECK(v2 == 0xcafebabe);
 }
 
+TEST_CASE("nvs page selection takes into account free entries also not just erased entries", "[nvs]")
+{
+    const size_t blob_size = Page::BLOB_MAX_SIZE;
+    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) );
+    // Fill second page
+    TEST_ESP_OK( nvs_set_blob(handle, "2a", blob, blob_size) );
+    TEST_ESP_OK( nvs_set_blob(handle, "2b", blob, blob_size) );
+
+    // The item below should be able to fit the first page.
+    TEST_ESP_OK( nvs_set_blob(handle, "3a", blob, 4) );
+    TEST_ESP_OK( nvs_commit(handle) );
+    nvs_close(handle);
+}
+
 TEST_CASE("dump all performance data", "[nvs]")
 {
     std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;