Merge branch 'bugfix/nvs_page_selection' into 'master'

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

See merge request idf/esp-idf!2240

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;

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;