nvs: Check if an item is modified before writing out an identical copy

This prevents wear and tear on the flash, and it also is faster in some cases since the read-out of flash is a cheaper operation than the erasure of flash. Some library modules (such as the esp_wifi) write out to NVS upon every initialization without checking first that the existing value is the same, and this speeds up initialization of modules that make these design choices and moves it into a centralized place. The comparison functions are based on the read-out functions of the same name, and changes out the memcpy(...) operations for memcmp(...) operations. Signed-off-by: Tim Nordell <[email protected]>
espressif · Apr 15, 2019 · 2164e91 · 2164e91
1 parent 70eda3d
commit 2164e91
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 0 deletions.
diff --git a/components/nvs_flash/include/nvs.h b/components/nvs_flash/include/nvs.h
@@ -54,6 +54,7 @@ typedef uint32_t nvs_handle;
 #define ESP_ERR_NVS_KEYS_NOT_INITIALIZED    (ESP_ERR_NVS_BASE + 0x16)  /*!< NVS key partition is uninitialized */
 #define ESP_ERR_NVS_CORRUPT_KEY_PART        (ESP_ERR_NVS_BASE + 0x17)  /*!< NVS key partition is corrupt */
 
+#define ESP_ERR_NVS_COMPARE_IDENTICAL       (ESP_ERR_NVS_BASE + 0x18)  /*!< NVS key is identical in comparison */
 
 #define NVS_DEFAULT_PART_NAME           "nvs"   /*!< Default partition name of the NVS partition in the partition table */
 /**

diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp
@@ -308,6 +308,63 @@ esp_err_t Page::readItem(uint8_t nsIndex, ItemType datatype, const char* key, vo
     return ESP_OK;
 }
 
+esp_err_t Page::cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx, VerOffset chunkStart)
+{
+    size_t index = 0;
+    Item item;
+
+    if (mState == PageState::INVALID) {
+        return ESP_ERR_NVS_INVALID_STATE;
+    }
+
+    esp_err_t rc = findItem(nsIndex, datatype, key, index, item, chunkIdx, chunkStart);
+    if (rc != ESP_OK) {
+        return rc;
+    }
+
+    if (!isVariableLengthType(datatype)) {
+        if (dataSize != getAlignmentForType(datatype)) {
+            return ESP_ERR_NVS_TYPE_MISMATCH;
+        }
+
+        if(!memcmp(data, item.data, dataSize))
+        {
+            return ESP_ERR_NVS_COMPARE_IDENTICAL;
+        }
+        return ESP_OK;
+    }
+
+    if (dataSize < static_cast<size_t>(item.varLength.dataSize)) {
+        return ESP_ERR_NVS_INVALID_LENGTH;
+    }
+
+    bool isSame = true;
+    const uint8_t* dst = reinterpret_cast<const uint8_t*>(data);
+    size_t left = item.varLength.dataSize;
+    for (size_t i = index + 1; i < index + item.span; ++i) {
+        Item ditem;
+        rc = readEntry(i, ditem);
+        if (rc != ESP_OK) {
+            return rc;
+        }
+        size_t willCopy = ENTRY_SIZE;
+        willCopy = (left < willCopy)?left:willCopy;
+        isSame = isSame && !memcmp(dst, ditem.rawData, willCopy);
+        left -= willCopy;
+        dst += willCopy;
+    }
+    if (Item::calculateCrc32(reinterpret_cast<const uint8_t*>(data), item.varLength.dataSize) != item.varLength.dataCrc32) {
+        return ESP_ERR_NVS_NOT_FOUND;
+    }
+
+    if (isSame)
+    {
+        return ESP_ERR_NVS_COMPARE_IDENTICAL;
+    }
+
+    return ESP_OK;
+}
+
 esp_err_t Page::eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx, VerOffset chunkStart)
 {
     size_t index = 0;

diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp
@@ -94,6 +94,8 @@ class Page : public intrusive_list_node<Page>
 
     esp_err_t readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
 
+    esp_err_t cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
+
     esp_err_t eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
 
     esp_err_t findItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
@@ -112,6 +114,12 @@ class Page : public intrusive_list_node<Page>
         return readItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value));
     }
 
+    template<typename T>
+    esp_err_t cmpItem(uint8_t nsIndex, const char* key, const T& value)
+    {
+        return cmpItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value));
+    }
+
     template<typename T>
     esp_err_t eraseItem(uint8_t nsIndex, const char* key)
     {

diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp
@@ -269,6 +269,14 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key
         VerOffset prevStart,  nextStart;
         prevStart = nextStart = VerOffset::VER_0_OFFSET;
         if (findPage) {
+            // Do a sanity check that the item in question is actually being modified.
+            // If it isn't, it is cheaper to purposefully not write out new data.
+            // since it may invoke an erasure of flash.
+            if (ESP_ERR_NVS_COMPARE_IDENTICAL == cmpMultiPageBlob(nsIndex, key, data, dataSize))
+            {
+                return ESP_OK;
+            }
+
             if (findPage->state() == Page::PageState::UNINITIALIZED ||
                     findPage->state() == Page::PageState::INVALID) {
                 ESP_ERROR_CHECK(findItem(nsIndex, datatype, key, findPage, item));
@@ -311,6 +319,14 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key
             }
         }
     } else {
+        // Do a sanity check that the item in question is actually being modified.
+        // If it isn't, it is cheaper to purposefully not write out new data.
+        // since it may invoke an erasure of flash.
+        if (nullptr != findPage &&
+            ESP_ERR_NVS_COMPARE_IDENTICAL == findPage->cmpItem(nsIndex, datatype, key, data, dataSize))
+        {
+            return ESP_OK;
+        }
 
         Page& page = getCurrentPage();
         err = page.writeItem(nsIndex, datatype, key, data, dataSize);
@@ -443,6 +459,50 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat
     return err;
 }
 
+esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize)
+{
+    Item item;
+    Page* findPage = nullptr;
+
+    /* First read the blob index */
+    auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item);
+    if (err != ESP_OK) {
+        return err;
+    }
+
+    uint8_t chunkCount = item.blobIndex.chunkCount;
+    VerOffset chunkStart = item.blobIndex.chunkStart;
+    size_t readSize = item.blobIndex.dataSize;
+    size_t offset = 0;
+
+    if (dataSize != readSize)
+    {
+        return ESP_OK;
+    }
+
+    /* Now read corresponding chunks */
+    for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) {
+        err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast<uint8_t> (chunkStart) + chunkNum);
+        if (err != ESP_OK) {
+            if (err == ESP_ERR_NVS_NOT_FOUND) {
+                break;
+            }
+            return err;
+        }
+        err = findPage->cmpItem(nsIndex, ItemType::BLOB_DATA, key, static_cast<const uint8_t*>(data) + offset, item.varLength.dataSize, static_cast<uint8_t> (chunkStart) + chunkNum);
+        if (ESP_ERR_NVS_COMPARE_IDENTICAL != err) {
+            return ESP_OK;
+        }
+        assert(static_cast<uint8_t> (chunkStart) + chunkNum == item.chunkIndex);
+        offset += item.varLength.dataSize;
+    }
+    if (err == ESP_OK) {
+        assert(offset == dataSize);
+        return ESP_ERR_NVS_COMPARE_IDENTICAL;
+    }
+    return err;
+}
+
 esp_err_t Storage::readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize)
 {
     if (mState != StorageState::ACTIVE) {

diff --git a/components/nvs_flash/src/nvs_storage.hpp b/components/nvs_flash/src/nvs_storage.hpp
@@ -109,6 +109,8 @@ class Storage : public intrusive_list_node<Storage>
 
     esp_err_t readMultiPageBlob(uint8_t nsIndex, const char* key, void* data, size_t dataSize);
 
+    esp_err_t cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize);
+
     esp_err_t eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffset chunkStart = VerOffset::VER_ANY);
 
     void debugDump();