Skip to content

Commit

Permalink
Merge branch 'bugfix/nvs_lock_initi_and_multipage_blob_v5.1' into 're…
Browse files Browse the repository at this point in the history
…lease/v5.1'

Bugfix/nvs Improved handling of BLOB during unreliable power environment and concurrent data access scenarios (v5.1)

See merge request espressif/esp-idf!29321
  • Loading branch information
pacucha42 committed Mar 13, 2024
2 parents 0c26579 + c3f3358 commit 9706e5a
Show file tree
Hide file tree
Showing 11 changed files with 258 additions and 113 deletions.
14 changes: 10 additions & 4 deletions components/nvs_flash/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
idf_build_get_property(target IDF_TARGET)

if(${target} STREQUAL "linux")
# omit newlib for linux
else()
list(APPEND priv_requires "newlib")
endif()

set(srcs "src/nvs_api.cpp"
"src/nvs_cxx_api.cpp"
"src/nvs_item_hash_list.cpp"
Expand All @@ -11,13 +17,13 @@ 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
REQUIRES "esp_partition" "spi_flash"
PRIV_REQUIRES "${priv_requires}"
INCLUDE_DIRS "include"
"../spi_flash/include"
PRIV_INCLUDE_DIRS "private_include")

# If we use the linux target, we need to redirect the crc functions to the linux
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-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -45,10 +45,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 @@ -272,6 +268,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 @@ -878,7 +878,10 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
end = ENTRY_COUNT;
}

if (nsIndex != NS_ANY && datatype != ItemType::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 @@ -933,6 +936,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

0 comments on commit 9706e5a

Please sign in to comment.