Skip to content

Commit

Permalink
Merge branch 'bugfix/nvs_lock_initi_and_multipage_blob' into 'master'
Browse files Browse the repository at this point in the history
Bugfix/nvs Improved handling of BLOB during unreliable power environment and concurrent data access scenarios

Closes IDF-8839

See merge request espressif/esp-idf!28843
  • Loading branch information
pacucha42 committed Feb 26, 2024
2 parents 5eb058e + fc6951e commit 4cb3a75
Show file tree
Hide file tree
Showing 10 changed files with 249 additions and 111 deletions.
5 changes: 3 additions & 2 deletions components/nvs_flash/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ set(srcs "src/nvs_api.cpp"
"src/nvs_partition.cpp"
"src/nvs_partition_lookup.cpp"
"src/nvs_partition_manager.cpp"
"src/nvs_types.cpp")
"src/nvs_types.cpp"
"src/nvs_platform.cpp")

idf_component_register(SRCS "${srcs}"
REQUIRES "esp_partition"
PRIV_REQUIRES spi_flash
PRIV_REQUIRES spi_flash newlib
INCLUDE_DIRS "include"
"../spi_flash/include"
PRIV_INCLUDE_DIRS "private_include")
Expand Down
10 changes: 5 additions & 5 deletions components/nvs_flash/src/nvs_api.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -51,10 +51,6 @@ uint32_t NVSHandleEntry::s_nvs_next_handle;

extern "C" void nvs_dump(const char *partName);

#ifndef LINUX_TARGET
SemaphoreHandle_t nvs::Lock::mSemaphore = nullptr;
#endif // ! LINUX_TARGET

using namespace std;
using namespace nvs;

Expand Down Expand Up @@ -268,6 +264,10 @@ static esp_err_t nvs_find_ns_handle(nvs_handle_t c_handle, NVSHandleSimple** han

extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle)
{
esp_err_t lock_result = Lock::init();
if (lock_result != ESP_OK) {
return lock_result;
}
Lock lock;
ESP_LOGD(TAG, "%s %s %d", __func__, namespace_name, open_mode);

Expand Down
30 changes: 29 additions & 1 deletion components/nvs_flash/src/nvs_page.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,10 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
end = ENTRY_COUNT;
}

if (nsIndex != NS_ANY && key != NULL) {
// For BLOB_DATA, we may need to search for all chunk indexes, so the hash list won't help
// mHashIndex caluclates hash from nsIndex, key, chunkIdx
// We may not use mHashList if datatype is BLOB_DATA and chunkIdx is CHUNK_ANY as CHUNK_ANY is used by BLOB_INDEX
if (nsIndex != NS_ANY && key != NULL && (datatype == ItemType::BLOB_DATA && chunkIdx != CHUNK_ANY)) {
size_t cachedIndex = mHashList.find(start, Item(nsIndex, datatype, 0, key, chunkIdx));
if (cachedIndex < ENTRY_COUNT) {
start = cachedIndex;
Expand Down Expand Up @@ -934,6 +937,31 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
&& item.chunkIndex != chunkIdx) {
continue;
}

// We may search for any chunk of BLOB_DATA but find BLOB_INDEX or BLOB instead as it
// uses default value of chunkIdx == CHUNK_ANY, then continue searching
if (chunkIdx == CHUNK_ANY
&& datatype == ItemType::BLOB_DATA
&& item.datatype != ItemType::BLOB_DATA) {
continue;
}

// We may search for BLOB but find BLOB_INDEX instead
// In this case it is expected to return ESP_ERR_NVS_TYPE_MISMATCH
if (chunkIdx == CHUNK_ANY
&& datatype == ItemType::BLOB
&& item.datatype == ItemType::BLOB_IDX) {
return ESP_ERR_NVS_TYPE_MISMATCH;
}

// We may search for BLOB but find BLOB_DATA instead
// Then continue
if (chunkIdx == CHUNK_ANY
&& datatype == ItemType::BLOB
&& item.datatype == ItemType::BLOB_DATA) {
continue;
}

/* Blob-index will match the <ns,key> with blob data.
* Skip data chunks when searching for blob index*/
if (datatype == ItemType::BLOB_IDX
Expand Down
4 changes: 2 additions & 2 deletions components/nvs_flash/src/nvs_page.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ class Page : public intrusive_list_node<Page>, public ExceptionlessAllocatable

esp_err_t findItem(uint8_t nsIndex, ItemType datatype, const char* key, size_t &itemIndex, Item& item, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);

esp_err_t eraseEntryAndSpan(size_t index);

template<typename T>
esp_err_t writeItem(uint8_t nsIndex, const char* key, const T& value)
{
Expand Down Expand Up @@ -188,8 +190,6 @@ class Page : public intrusive_list_node<Page>, public ExceptionlessAllocatable

esp_err_t writeEntryData(const uint8_t* data, size_t size);

esp_err_t eraseEntryAndSpan(size_t index);

esp_err_t updateFirstUsedEntry(size_t index, size_t span);

static constexpr size_t getAlignmentForType(ItemType type)
Expand Down
50 changes: 50 additions & 0 deletions components/nvs_flash/src/nvs_platform.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "nvs_platform.hpp"

using namespace nvs;

#ifdef LINUX_TARGET
Lock::Lock() {}
Lock::~Lock() {}
esp_err_t nvs::Lock::init() {return ESP_OK;}
void Lock::uninit() {}
#else

#include "sys/lock.h"

Lock::Lock()
{
// Newlib implementation ensures that even if mSemaphore was 0, it gets initialized.
// Locks mSemaphore
_lock_acquire(&mSemaphore);
}

Lock::~Lock()
{
// Unlocks mSemaphore
_lock_release(&mSemaphore);
}

esp_err_t Lock::init()
{
// Let postpone initialization to the Lock::Lock.
// It is designed to lazy initialize the semaphore in a properly guarded critical section
return ESP_OK;
}

void Lock::uninit()
{
// Uninitializes mSemaphore. Please be aware that uninitialization of semaphore shared and held by another thread
// can cause undefined behavior
if (mSemaphore) {
_lock_close(&mSemaphore);
}
}

_lock_t Lock::mSemaphore = 0;

#endif
92 changes: 17 additions & 75 deletions components/nvs_flash/src/nvs_platform.hpp
Original file line number Diff line number Diff line change
@@ -1,82 +1,24 @@
// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at

// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
/*
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once

#ifdef LINUX_TARGET
namespace nvs
{
class Lock
{
public:
Lock() { }
~Lock() { }
static esp_err_t init()
{
return ESP_OK;
}

static void uninit() {}
};
} // namespace nvs

#else // LINUX_TARGET

#include "freertos/FreeRTOS.h"
#include "freertos/semphr.h"
#include "esp_err.h"

namespace nvs
{

class Lock
{
public:
Lock()
{
if (mSemaphore) {
xSemaphoreTake(mSemaphore, portMAX_DELAY);
}
}

~Lock()
{
if (mSemaphore) {
xSemaphoreGive(mSemaphore);
}
}

static esp_err_t init()
class Lock
{
if (mSemaphore) {
return ESP_OK;
}
mSemaphore = xSemaphoreCreateMutex();
if (!mSemaphore) {
return ESP_ERR_NO_MEM;
}
return ESP_OK;
}

static void uninit()
{
if (mSemaphore) {
vSemaphoreDelete(mSemaphore);
}
mSemaphore = nullptr;
}

static SemaphoreHandle_t mSemaphore;
};
public:
Lock();
~Lock();
static esp_err_t init();
static void uninit();
#ifndef LINUX_TARGET
private:
static _lock_t mSemaphore;
#endif
};
} // namespace nvs

#endif // LINUX_TARGET
Loading

0 comments on commit 4cb3a75

Please sign in to comment.