feat(nvs): Optimize read-only NVS loading

Author: adokitkat

components/nvs_flash/host_test/nvs_page_test/main/test_fixtures.hpp

@@ -110,7 +110,7 @@ class PartitionEmulationFixture {
         size_t columns = size / column_size;
         size_t column;
 
-        for(column = 0; column < columns; ++column)
+        for(column = 0; column < columns; column = column + 1)
         {
             // read column
             if((err = esp_partition_read_raw(&esp_partition, dst_offset + (column * column_size), buff, column_size)) != ESP_OK) return err;

components/nvs_flash/src/nvs_pagemanager.cpp

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -56,77 +56,79 @@ esp_err_t PageManager::load(Partition *partition, uint32_t baseSector, uint32_t
 
     // if power went out after a new item for the given key was written,
     // but before the old one was erased, we end up with a duplicate item
-    Page& lastPage = back();
-    size_t lastItemIndex = SIZE_MAX;
-    Item item;
-    size_t itemIndex = 0;
-    while (lastPage.findItem(Page::NS_ANY, ItemType::ANY, nullptr, itemIndex, item) == ESP_OK) {
-        itemIndex += item.span;
-        lastItemIndex = itemIndex;
-    }
-
-    if (lastItemIndex != SIZE_MAX) {
-        auto last = PageManager::TPageListIterator(&lastPage);
-        TPageListIterator it;
+    if (!partition->get_readonly()) {
+        Page& lastPage = back();
+        size_t lastItemIndex = SIZE_MAX;
+        Item item;
+        size_t itemIndex = 0;
+        while (lastPage.findItem(Page::NS_ANY, ItemType::ANY, nullptr, itemIndex, item) == ESP_OK) {
+            itemIndex += item.span;
+            lastItemIndex = itemIndex;
+        }
 
-        for (it = begin(); it != last; ++it) {
+        if (lastItemIndex != SIZE_MAX) {
+            auto last = PageManager::TPageListIterator(&lastPage);
+            TPageListIterator it;
 
-            if ((it->state() != Page::PageState::FREEING) &&
-                    (it->eraseItem(item.nsIndex, item.datatype, item.key, item.chunkIndex) == ESP_OK)) {
-                break;
-            }
-        }
-        if ((it == last) && (item.datatype == ItemType::BLOB_IDX)) {
-            /* Rare case in which the blob was stored using old format, but power went just after writing
-             * blob index during modification. Loop again and delete the old version blob*/
             for (it = begin(); it != last; ++it) {
 
                 if ((it->state() != Page::PageState::FREEING) &&
-                        (it->eraseItem(item.nsIndex, ItemType::BLOB, item.key, item.chunkIndex) == ESP_OK)) {
+                        (it->eraseItem(item.nsIndex, item.datatype, item.key, item.chunkIndex) == ESP_OK)) {
                     break;
                 }
             }
+            if ((it == last) && (item.datatype == ItemType::BLOB_IDX)) {
+                /* Rare case in which the blob was stored using old format, but power went just after writing
+                * blob index during modification. Loop again and delete the old version blob*/
+                for (it = begin(); it != last; ++it) {
+
+                    if ((it->state() != Page::PageState::FREEING) &&
+                            (it->eraseItem(item.nsIndex, ItemType::BLOB, item.key, item.chunkIndex) == ESP_OK)) {
+                        break;
+                    }
+                }
+            }
         }
-    }
 
-    // check if power went out while page was being freed
-    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 = newPage->erase();
+        // check if power went out while page was being freed
+        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 = newPage->erase();
+                    if (err != ESP_OK) {
+                        return err;
+                    }
+                    mPageList.erase(newPage);
+                    mFreePageList.push_back(newPage);
+                }
+                auto err = activatePage();
                 if (err != ESP_OK) {
                     return err;
                 }
-                mPageList.erase(newPage);
-                mFreePageList.push_back(newPage);
-            }
-            auto err = activatePage();
-            if (err != ESP_OK) {
-                return err;
-            }
-            newPage = &mPageList.back();
+                newPage = &mPageList.back();
 
-            err = it->copyItems(*newPage);
-            if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) {
-                return err;
-            }
+                err = it->copyItems(*newPage);
+                if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) {
+                    return err;
+                }
 
-            err = it->erase();
-            if (err != ESP_OK) {
-                return err;
-            }
+                err = it->erase();
+                if (err != ESP_OK) {
+                    return err;
+                }
 
-            Page* p = static_cast<Page*>(it);
-            mPageList.erase(it);
-            mFreePageList.push_back(p);
-            break;
+                Page* p = static_cast<Page*>(it);
+                mPageList.erase(it);
+                mFreePageList.push_back(p);
+                break;
+            }
         }
-    }
 
-    // partition should have at least one free page if it is not read-only
-    if (!partition->get_readonly() && mFreePageList.empty()) {
-        return ESP_ERR_NVS_NO_FREE_PAGES;
+        // partition should have at least one free page if it is not read-only
+        if (mFreePageList.empty()) {
+            return ESP_ERR_NVS_NO_FREE_PAGES;
+        }
     }
 
     return ESP_OK;

examples/storage/parttool/pytest_parttool_example.py

@@ -36,16 +36,12 @@ def test_examples_parttool(dut: Dut) -> None:
     cmds = [
         'read_partition --partition-type=data --partition-subtype=nvs --output custom1.bin',
         'erase_partition --partition-name=custom',
-        'write_partition --partition-name=custom --input custom.bin',
+        'write_partition --partition-name=custom --input custom.bin --ignore-readonly',
         'get_partition_info --partition-boot-default --info size',
     ]
 
     for cmd in cmds:
-        try:
-            subprocess.check_call(BASE_CMD + cmd.split())
-        except subprocess.CalledProcessError as e:
-            print(e.output)
-            raise
+        subprocess.check_call(BASE_CMD + cmd.split())
 
     clean_files = ['custom.bin', 'custom1.bin']
     for clean_file in clean_files: