From 82ccd1ef808897c6646dcaab2eb5a0ed8718e2bb Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Wed, 19 Feb 2025 13:27:21 +0100 Subject: [PATCH 01/27] Revert "[nrf noup] settings: zms: use dedicated lookup cache hash function" This reverts commit 14d166ffe235587818ce069c85c5b819c0b170a5. Signed-off-by: Riadh Ghaddab --- subsys/fs/zms/Kconfig | 9 --------- subsys/fs/zms/zms.c | 45 ------------------------------------------- 2 files changed, 54 deletions(-) diff --git a/subsys/fs/zms/Kconfig b/subsys/fs/zms/Kconfig index e1312c57fd8..781da81bf0a 100644 --- a/subsys/fs/zms/Kconfig +++ b/subsys/fs/zms/Kconfig @@ -50,15 +50,6 @@ config ZMS_CUSTOM_BLOCK_SIZE help Changes the internal buffer size of ZMS -config ZMS_LOOKUP_CACHE_FOR_SETTINGS - bool "ZMS Storage lookup cache optimized for settings" - depends on ZMS_LOOKUP_CACHE - help - Use the lookup cache hash function that results in the least number of - collissions and, in turn, the best ZMS performance provided that the ZMS - is used as the settings backend only. This option should NOT be enabled - if the ZMS is also written to directly, outside the settings layer. - config ZMS_NO_DOUBLE_WRITE bool "Avoid writing the same data again in the storage" help diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 99096ab0c17..0e5ca1e5f3b 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -11,10 +11,6 @@ #include #include #include "zms_priv.h" -#ifdef CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS -#include -#include -#endif #include LOG_MODULE_REGISTER(fs_zms, CONFIG_ZMS_LOG_LEVEL); @@ -29,45 +25,6 @@ static int zms_ate_valid_different_sector(struct zms_fs *fs, const struct zms_at #ifdef CONFIG_ZMS_LOOKUP_CACHE -#ifdef CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS - -static inline size_t zms_lookup_cache_pos(uint32_t id) -{ - /* - * 1. The ZMS settings backend uses up to (ZMS_NAME_ID_OFFSET - 1) ZMS IDs to - store keys and equal number of ZMS IDs to store values. - * 2. For each key-value pair, the value is stored at ZMS ID greater by exactly - * ZMS_NAME_ID_OFFSET than ZMS ID that holds the key. - * 3. The backend tries to minimize the range of ZMS IDs used to store keys. - * That is, ZMS IDs are allocated sequentially, and freed ZMS IDs are reused - * before allocating new ones. - * - * Therefore, to assure the least number of collisions in the lookup cache, - * the least significant bit of the hash indicates whether the given ZMS ID - * represents a key or a value, and remaining bits of the hash are set to - * the ordinal number of the key-value pair. Consequently, the hash function - * provides the following mapping: - * - * 1st settings key => hash 0 - * 1st settings value => hash 1 - * 2nd settings key => hash 2 - * 2nd settings value => hash 3 - * ... - */ - BUILD_ASSERT(IS_POWER_OF_TWO(ZMS_NAMECNT_ID), "ZMS_NAMECNT_ID is not power of 2"); - BUILD_ASSERT(IS_POWER_OF_TWO(ZMS_NAME_ID_OFFSET), "ZMS_NAME_ID_OFFSET is not power of 2"); - - uint32_t key_value_bit; - uint32_t key_value_ord; - - key_value_bit = (id >> LOG2(ZMS_NAME_ID_OFFSET)) & 1; - key_value_ord = id & (ZMS_NAME_ID_OFFSET - 1); - - return ((key_value_ord << 1) | key_value_bit) % CONFIG_ZMS_LOOKUP_CACHE_SIZE; -} - -#else /* CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS */ - static inline size_t zms_lookup_cache_pos(uint32_t id) { uint32_t hash; @@ -83,8 +40,6 @@ static inline size_t zms_lookup_cache_pos(uint32_t id) return hash % CONFIG_ZMS_LOOKUP_CACHE_SIZE; } -#endif /* CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS */ - static int zms_lookup_cache_rebuild(struct zms_fs *fs) { int rc; From dd235e5e4319568ccc5689390bfbd72e86daea84 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Wed, 19 Feb 2025 13:27:30 +0100 Subject: [PATCH 02/27] Revert "[nrf fromlist] settings: ZMS: add a backend for ZMS (Zephyr Memory Storage)" This reverts commit 88e9f91ffa5b30f3b20c8fed5dbcd5cc88b1d32e. Signed-off-by: Riadh Ghaddab --- subsys/settings/Kconfig | 41 -- .../settings/include/settings/settings_zms.h | 64 --- subsys/settings/src/CMakeLists.txt | 1 - subsys/settings/src/settings_zms.c | 432 ------------------ tests/subsys/settings/zms/CMakeLists.txt | 7 - tests/subsys/settings/zms/prj.conf | 8 - tests/subsys/settings/zms/src/CMakeLists.txt | 12 - tests/subsys/settings/zms/src/settings_test.h | 42 -- .../settings/zms/src/settings_test_zms.c | 125 ----- tests/subsys/settings/zms/testcase.yaml | 7 - 10 files changed, 739 deletions(-) delete mode 100644 subsys/settings/include/settings/settings_zms.h delete mode 100644 subsys/settings/src/settings_zms.c delete mode 100644 tests/subsys/settings/zms/CMakeLists.txt delete mode 100644 tests/subsys/settings/zms/prj.conf delete mode 100644 tests/subsys/settings/zms/src/CMakeLists.txt delete mode 100644 tests/subsys/settings/zms/src/settings_test.h delete mode 100644 tests/subsys/settings/zms/src/settings_test_zms.c delete mode 100644 tests/subsys/settings/zms/testcase.yaml diff --git a/subsys/settings/Kconfig b/subsys/settings/Kconfig index 48eacf82a8c..bc015cde9c7 100644 --- a/subsys/settings/Kconfig +++ b/subsys/settings/Kconfig @@ -32,7 +32,6 @@ config SETTINGS_ENCODE_LEN choice SETTINGS_BACKEND prompt "Storage back-end" - default SETTINGS_ZMS if ZMS default SETTINGS_NVS if NVS default SETTINGS_FCB if FCB default SETTINGS_FILE if FILE_SYSTEM @@ -40,31 +39,6 @@ choice SETTINGS_BACKEND help Storage back-end to be used by the settings subsystem. -config SETTINGS_ZMS - bool "ZMS (Zephyr Memory Storage)" - depends on ZMS - help - Use ZMS as settings storage backend. - -if SETTINGS_ZMS - -config SETTINGS_ZMS_NAME_CACHE - bool "ZMS name lookup cache" - select SYS_HASH_FUNC32 - help - Enable ZMS name lookup cache, used to reduce the Settings name - lookup time. - -config SETTINGS_ZMS_NAME_CACHE_SIZE - int "ZMS name lookup cache size" - default 128 - range 1 $(UINT32_MAX) - depends on SETTINGS_ZMS_NAME_CACHE - help - Number of entries in Settings ZMS name cache. - -endif # SETTINGS_ZMS - config SETTINGS_FCB bool "FCB" depends on FCB @@ -158,21 +132,6 @@ config SETTINGS_NVS_SECTOR_COUNT help Number of sectors used for the NVS settings area -config SETTINGS_ZMS_SECTOR_SIZE_MULT - int "Sector size of the ZMS settings area" - default 1 - depends on SETTINGS_ZMS - help - The sector size to use for the ZMS settings area as a multiple of - FLASH_ERASE_BLOCK_SIZE. - -config SETTINGS_ZMS_SECTOR_COUNT - int "Sector count of the ZMS settings area" - default 8 - depends on SETTINGS_ZMS - help - Number of sectors used for the ZMS settings area - config SETTINGS_SHELL bool "Settings shell" depends on SHELL diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h deleted file mode 100644 index dd9fb3aba12..00000000000 --- a/subsys/settings/include/settings/settings_zms.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (c) 2024 BayLibre SAS - * - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef __SETTINGS_ZMS_H_ -#define __SETTINGS_ZMS_H_ - -#include -#include - -#ifdef __cplusplus -extern "C" { -#endif - -/* In the ZMS backend, each setting is stored in two ZMS entries: - * 1. setting's name - * 2. setting's value - * - * The ZMS entry ID for the setting's value is determined implicitly based on - * the ID of the ZMS entry for the setting's name, once that is found. The - * difference between name and value ID is constant and equal to - * ZMS_NAME_ID_OFFSET. - * - * Setting's name entries start from ZMS_NAMECNT_ID + 1. - * The entry with ID == ZMS_NAMECNT_ID is used to store the largest name ID in use. - * - * Deleted records will not be found, only the last record will be read. - */ -#define ZMS_NAMECNT_ID 0x80000000 -#define ZMS_NAME_ID_OFFSET 0x40000000 - -struct settings_zms { - struct settings_store cf_store; - struct zms_fs cf_zms; - uint32_t last_name_id; - const struct device *flash_dev; -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - struct { - uint32_t name_hash; - uint32_t name_id; - } cache[CONFIG_SETTINGS_ZMS_NAME_CACHE_SIZE]; - - uint32_t cache_next; - uint32_t cache_total; - bool loaded; -#endif -}; - -/* register zms to be a source of settings */ -int settings_zms_src(struct settings_zms *cf); - -/* register zms to be the destination of settings */ -int settings_zms_dst(struct settings_zms *cf); - -/* Initialize a zms backend. */ -int settings_zms_backend_init(struct settings_zms *cf); - -#ifdef __cplusplus -} -#endif - -#endif /* __SETTINGS_ZMS_H_ */ diff --git a/subsys/settings/src/CMakeLists.txt b/subsys/settings/src/CMakeLists.txt index 4aa797fbb35..d4ec6e76142 100644 --- a/subsys/settings/src/CMakeLists.txt +++ b/subsys/settings/src/CMakeLists.txt @@ -13,4 +13,3 @@ zephyr_sources_ifdef(CONFIG_SETTINGS_FCB settings_fcb.c) zephyr_sources_ifdef(CONFIG_SETTINGS_NVS settings_nvs.c) zephyr_sources_ifdef(CONFIG_SETTINGS_NONE settings_none.c) zephyr_sources_ifdef(CONFIG_SETTINGS_SHELL settings_shell.c) -zephyr_sources_ifdef(CONFIG_SETTINGS_ZMS settings_zms.c) diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c deleted file mode 100644 index 0f1164e4ea1..00000000000 --- a/subsys/settings/src/settings_zms.c +++ /dev/null @@ -1,432 +0,0 @@ -/* - * Copyright (c) 2024 BayLibre SAS - * - * SPDX-License-Identifier: Apache-2.0 - */ - -#include -#include - -#include "settings/settings_zms.h" -#include "settings_priv.h" - -#include -#include -#include -#include -LOG_MODULE_DECLARE(settings, CONFIG_SETTINGS_LOG_LEVEL); - -#if DT_HAS_CHOSEN(zephyr_settings_partition) -#define SETTINGS_PARTITION DT_FIXED_PARTITION_ID(DT_CHOSEN(zephyr_settings_partition)) -#else -#define SETTINGS_PARTITION FIXED_PARTITION_ID(storage_partition) -#endif - -struct settings_zms_read_fn_arg { - struct zms_fs *fs; - uint32_t id; -}; - -static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg); -static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, - size_t val_len); -static void *settings_zms_storage_get(struct settings_store *cs); - -static struct settings_store_itf settings_zms_itf = {.csi_load = settings_zms_load, - .csi_save = settings_zms_save, - .csi_storage_get = settings_zms_storage_get}; - -static ssize_t settings_zms_read_fn(void *back_end, void *data, size_t len) -{ - struct settings_zms_read_fn_arg *rd_fn_arg; - - rd_fn_arg = (struct settings_zms_read_fn_arg *)back_end; - - return zms_read(rd_fn_arg->fs, rd_fn_arg->id, data, len); -} - -int settings_zms_src(struct settings_zms *cf) -{ - cf->cf_store.cs_itf = &settings_zms_itf; - settings_src_register(&cf->cf_store); - - return 0; -} - -int settings_zms_dst(struct settings_zms *cf) -{ - cf->cf_store.cs_itf = &settings_zms_itf; - settings_dst_register(&cf->cf_store); - - return 0; -} - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE -#define SETTINGS_ZMS_CACHE_OVFL(cf) ((cf)->cache_total > ARRAY_SIZE((cf)->cache)) - -static void settings_zms_cache_add(struct settings_zms *cf, const char *name, uint32_t name_id) -{ - uint32_t name_hash = sys_hash32(name, strlen(name)); - - cf->cache[cf->cache_next].name_hash = name_hash; - cf->cache[cf->cache_next++].name_id = name_id; - - cf->cache_next %= CONFIG_SETTINGS_ZMS_NAME_CACHE_SIZE; -} - -static uint32_t settings_zms_cache_match(struct settings_zms *cf, const char *name, char *rdname, - size_t len) -{ - uint32_t name_hash = sys_hash32(name, strlen(name)); - int rc; - - for (int i = 0; i < CONFIG_SETTINGS_ZMS_NAME_CACHE_SIZE; i++) { - if (cf->cache[i].name_hash != name_hash) { - continue; - } - - if (cf->cache[i].name_id <= ZMS_NAMECNT_ID) { - continue; - } - - rc = zms_read(&cf->cf_zms, cf->cache[i].name_id, rdname, len); - if (rc < 0) { - continue; - } - - rdname[rc] = '\0'; - - if (strcmp(name, rdname)) { - continue; - } - - return cf->cache[i].name_id; - } - - return ZMS_NAMECNT_ID; -} -#endif /* CONFIG_SETTINGS_ZMS_NAME_CACHE */ - -static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg) -{ - int ret = 0; - struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - struct settings_zms_read_fn_arg read_fn_arg; - char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; - ssize_t rc1, rc2; - uint32_t name_id = ZMS_NAMECNT_ID; - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - uint32_t cached = 0; - - cf->loaded = false; -#endif - - name_id = cf->last_name_id + 1; - - while (1) { - - name_id--; - if (name_id == ZMS_NAMECNT_ID) { -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - cf->loaded = true; - cf->cache_total = cached; -#endif - break; - } - - /* In the ZMS backend, each setting item is stored in two ZMS - * entries one for the setting's name and one with the - * setting's value. - */ - rc1 = zms_read(&cf->cf_zms, name_id, &name, sizeof(name)); - /* get the length of data and verify that it exists */ - rc2 = zms_get_data_length(&cf->cf_zms, name_id + ZMS_NAME_ID_OFFSET); - - if ((rc1 <= 0) && (rc2 <= 0)) { - /* Settings largest ID in use is invalid due to - * reset, power failure or partition overflow. - * Decrement it and check the next ID in subsequent - * iteration. - */ - if (name_id == cf->last_name_id) { - cf->last_name_id--; - zms_write(&cf->cf_zms, ZMS_NAMECNT_ID, &cf->last_name_id, - sizeof(uint32_t)); - } - - continue; - } - - if ((rc1 <= 0) || (rc2 <= 0)) { - /* Settings item is not stored correctly in the ZMS. - * ZMS entry for its name or value is either missing - * or deleted. Clean dirty entries to make space for - * future settings item. - */ - zms_delete(&cf->cf_zms, name_id); - zms_delete(&cf->cf_zms, name_id + ZMS_NAME_ID_OFFSET); - - if (name_id == cf->last_name_id) { - cf->last_name_id--; - zms_write(&cf->cf_zms, ZMS_NAMECNT_ID, &cf->last_name_id, - sizeof(uint32_t)); - } - - continue; - } - - /* Found a name, this might not include a trailing \0 */ - name[rc1] = '\0'; - read_fn_arg.fs = &cf->cf_zms; - read_fn_arg.id = name_id + ZMS_NAME_ID_OFFSET; - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - settings_zms_cache_add(cf, name, name_id); - cached++; -#endif - - ret = settings_call_set_handler(name, rc2, settings_zms_read_fn, &read_fn_arg, - (void *)arg); - if (ret) { - break; - } - } - return ret; -} - -static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, - size_t val_len) -{ - struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - char rdname[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; - uint32_t name_id, write_name_id; - bool delete, write_name; - int rc = 0; - - if (!name) { - return -EINVAL; - } - - /* Find out if we are doing a delete */ - delete = ((value == NULL) || (val_len == 0)); - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - bool name_in_cache = false; - - name_id = settings_zms_cache_match(cf, name, rdname, sizeof(rdname)); - if (name_id != ZMS_NAMECNT_ID) { - write_name_id = name_id; - write_name = false; - name_in_cache = true; - goto found; - } -#endif - - /* No entry with "name" is in cache, let's find if it exists in the storage */ - name_id = cf->last_name_id + 1; - write_name_id = cf->last_name_id + 1; - write_name = true; - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - /* We can skip reading ZMS if we know that the cache wasn't overflowed. */ - if (cf->loaded && !SETTINGS_ZMS_CACHE_OVFL(cf)) { - goto found; - } -#endif - - /* Let's find if we already have an ID within storage */ - while (1) { - name_id--; - if (name_id == ZMS_NAMECNT_ID) { - break; - } - - rc = zms_read(&cf->cf_zms, name_id, &rdname, sizeof(rdname)); - - if (rc < 0) { - /* Error or entry not found */ - if (rc == -ENOENT) { - /* This is a free ID let's keep it */ - write_name_id = name_id; - } - continue; - } - - rdname[rc] = '\0'; - - if (strcmp(name, rdname)) { - /* ID exists but the name is different, that's not the ID - * we are looking for. - */ - continue; - } - - /* At this step we found the ID that corresponds to name */ - if (!delete) { - write_name_id = name_id; - write_name = false; - } - - goto found; - } - -found: - if (delete) { - if (name_id == ZMS_NAMECNT_ID) { - return 0; - } - - rc = zms_delete(&cf->cf_zms, name_id); - if (rc >= 0) { - rc = zms_delete(&cf->cf_zms, name_id + ZMS_NAME_ID_OFFSET); - } - - if (rc < 0) { - return rc; - } - - if (name_id == cf->last_name_id) { - cf->last_name_id--; - rc = zms_write(&cf->cf_zms, ZMS_NAMECNT_ID, &cf->last_name_id, - sizeof(uint32_t)); - if (rc < 0) { - /* Error: can't to store - * the largest name ID in use. - */ - return rc; - } - } - - return 0; - } - - /* No free IDs left. */ - if (write_name_id == ZMS_NAMECNT_ID + ZMS_NAME_ID_OFFSET - 1) { - return -ENOMEM; - } - - /* update the last_name_id and write to flash if required*/ - if (write_name_id > cf->last_name_id) { - cf->last_name_id = write_name_id; - rc = zms_write(&cf->cf_zms, ZMS_NAMECNT_ID, &cf->last_name_id, sizeof(uint32_t)); - if (rc < 0) { - return rc; - } - } - - /* write the value */ - rc = zms_write(&cf->cf_zms, write_name_id + ZMS_NAME_ID_OFFSET, value, val_len); - if (rc < 0) { - return rc; - } - - /* write the name if required */ - if (write_name) { - rc = zms_write(&cf->cf_zms, write_name_id, name, strlen(name)); - if (rc < 0) { - return rc; - } - } - -#if CONFIG_SETTINGS_ZMS_NAME_CACHE - if (!name_in_cache) { - settings_zms_cache_add(cf, name, write_name_id); - if (cf->loaded && !SETTINGS_ZMS_CACHE_OVFL(cf)) { - cf->cache_total++; - } - } -#endif - - return 0; -} - -/* Initialize the zms backend. */ -int settings_zms_backend_init(struct settings_zms *cf) -{ - int rc; - uint32_t last_name_id; - - cf->cf_zms.flash_device = cf->flash_dev; - if (cf->cf_zms.flash_device == NULL) { - return -ENODEV; - } - - rc = zms_mount(&cf->cf_zms); - if (rc) { - return rc; - } - - rc = zms_read(&cf->cf_zms, ZMS_NAMECNT_ID, &last_name_id, sizeof(last_name_id)); - if (rc < 0) { - cf->last_name_id = ZMS_NAMECNT_ID; - } else { - cf->last_name_id = last_name_id; - } - - LOG_DBG("Initialized"); - return 0; -} - -int settings_backend_init(void) -{ - static struct settings_zms default_settings_zms; - int rc; - uint32_t cnt = 0; - size_t zms_sector_size, zms_size = 0; - const struct flash_area *fa; - struct flash_sector hw_flash_sector; - uint32_t sector_cnt = 1; - - rc = flash_area_open(SETTINGS_PARTITION, &fa); - if (rc) { - return rc; - } - - rc = flash_area_get_sectors(SETTINGS_PARTITION, §or_cnt, &hw_flash_sector); - if (rc != 0 && rc != -ENOMEM) { - return rc; - } - - zms_sector_size = CONFIG_SETTINGS_ZMS_SECTOR_SIZE_MULT * hw_flash_sector.fs_size; - - if (zms_sector_size > UINT32_MAX) { - return -EDOM; - } - - while (cnt < CONFIG_SETTINGS_ZMS_SECTOR_COUNT) { - zms_size += zms_sector_size; - if (zms_size > fa->fa_size) { - break; - } - cnt++; - } - - /* define the zms file system using the page_info */ - default_settings_zms.cf_zms.sector_size = zms_sector_size; - default_settings_zms.cf_zms.sector_count = cnt; - default_settings_zms.cf_zms.offset = fa->fa_off; - default_settings_zms.flash_dev = fa->fa_dev; - - rc = settings_zms_backend_init(&default_settings_zms); - if (rc) { - return rc; - } - - rc = settings_zms_src(&default_settings_zms); - - if (rc) { - return rc; - } - - rc = settings_zms_dst(&default_settings_zms); - - return rc; -} - -static void *settings_zms_storage_get(struct settings_store *cs) -{ - struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - - return &cf->cf_zms; -} diff --git a/tests/subsys/settings/zms/CMakeLists.txt b/tests/subsys/settings/zms/CMakeLists.txt deleted file mode 100644 index 218451c840e..00000000000 --- a/tests/subsys/settings/zms/CMakeLists.txt +++ /dev/null @@ -1,7 +0,0 @@ -# SPDX-License-Identifier: Apache-2.0 - -cmake_minimum_required(VERSION 3.20.0) -find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) -project(test_settings_zms_raw) - -add_subdirectory(./src zms_test_bindir) diff --git a/tests/subsys/settings/zms/prj.conf b/tests/subsys/settings/zms/prj.conf deleted file mode 100644 index 3de961c7c72..00000000000 --- a/tests/subsys/settings/zms/prj.conf +++ /dev/null @@ -1,8 +0,0 @@ -CONFIG_ZTEST=y -CONFIG_FLASH=y -CONFIG_FLASH_MAP=y -CONFIG_ZMS=y - -CONFIG_SETTINGS=y -CONFIG_SETTINGS_RUNTIME=y -CONFIG_SETTINGS_ZMS=y diff --git a/tests/subsys/settings/zms/src/CMakeLists.txt b/tests/subsys/settings/zms/src/CMakeLists.txt deleted file mode 100644 index 220a42849a8..00000000000 --- a/tests/subsys/settings/zms/src/CMakeLists.txt +++ /dev/null @@ -1,12 +0,0 @@ -# SPDX-License-Identifier: Apache-2.0 -# Copyright (c) 2024 BayLibre SAS - -zephyr_include_directories( - ${ZEPHYR_BASE}/subsys/settings/include - ${ZEPHYR_BASE}/subsys/settings/src - ${ZEPHYR_BASE}/tests/subsys/settings/zms/src - ) - -target_sources(app PRIVATE settings_test_zms.c) - -add_subdirectory(../../src settings_test_bindir) diff --git a/tests/subsys/settings/zms/src/settings_test.h b/tests/subsys/settings/zms/src/settings_test.h deleted file mode 100644 index 472ff71f59b..00000000000 --- a/tests/subsys/settings/zms/src/settings_test.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (c) 2024 BayLibre SAS - * - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef _SETTINGS_TEST_ZMS_H -#define _SETTINGS_TEST_ZMS_H - -#include -#include -#include - -#include - -#ifdef __cplusplus -extern "C" { -#endif - -extern uint8_t val8; -extern uint8_t val8_un; -extern uint32_t val32; -extern uint64_t val64; - -extern int test_get_called; -extern int test_set_called; -extern int test_commit_called; -extern int test_export_block; - -extern struct settings_handler c_test_handlers[]; - -void ctest_clear_call_state(void); -int ctest_get_call_state(void); - -void config_wipe_srcs(void); -void *settings_config_setup(void); -void settings_config_teardown(void *fixture); - -#ifdef __cplusplus -} -#endif -#endif /* _SETTINGS_TEST_ZMS_H */ diff --git a/tests/subsys/settings/zms/src/settings_test_zms.c b/tests/subsys/settings/zms/src/settings_test_zms.c deleted file mode 100644 index 17a5ccd56fe..00000000000 --- a/tests/subsys/settings/zms/src/settings_test_zms.c +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright (c) 2024 BayLibre SAS - * - * SPDX-License-Identifier: Apache-2.0 - */ -#include -#include - -#include "settings_priv.h" -#include "settings_test.h" - -uint8_t val8; -uint8_t val8_un; -uint32_t val32; -uint64_t val64; - -int test_get_called; -int test_set_called; -int test_commit_called; -int test_export_block; - -int c1_handle_get(const char *name, char *val, int val_len_max); -int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); -int c1_handle_commit(void); -int c1_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)); - -struct settings_handler c_test_handlers[] = { - {.name = "myfoo", - .h_get = c1_handle_get, - .h_set = c1_handle_set, - .h_commit = c1_handle_commit, - .h_export = c1_handle_export}, -}; - -int c1_handle_get(const char *name, char *val, int val_len_max) -{ - const char *next; - - test_get_called = 1; - - if (settings_name_steq(name, "mybar", &next) && !next) { - val_len_max = MIN(val_len_max, sizeof(val8)); - memcpy(val, &val8, MIN(val_len_max, sizeof(val8))); - return val_len_max; - } - - if (settings_name_steq(name, "mybar64", &next) && !next) { - val_len_max = MIN(val_len_max, sizeof(val64)); - memcpy(val, &val64, MIN(val_len_max, sizeof(val64))); - return val_len_max; - } - - return -ENOENT; -} - -int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) -{ - size_t val_len; - int rc; - const char *next; - - test_set_called = 1; - if (settings_name_steq(name, "mybar", &next) && !next) { - rc = read_cb(cb_arg, &val8, sizeof(val8)); - zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); - return 0; - } - - if (settings_name_steq(name, "mybar64", &next) && !next) { - rc = read_cb(cb_arg, &val64, sizeof(val64)); - zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); - return 0; - } - - if (settings_name_steq(name, "unaligned", &next) && !next) { - val_len = len; - zassert_equal(val_len, sizeof(val8_un), "value length: %d, ought equal 1", val_len); - rc = read_cb(cb_arg, &val8_un, sizeof(val8_un)); - zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); - return 0; - } - - return -ENOENT; -} - -int c1_handle_commit(void) -{ - test_commit_called = 1; - return 0; -} - -int c1_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)) -{ - if (test_export_block) { - return 0; - } - - (void)cb("myfoo/mybar", &val8, sizeof(val8)); - - (void)cb("myfoo/mybar64", &val64, sizeof(val64)); - - (void)cb("myfoo/unaligned", &val8_un, sizeof(val8_un)); - - return 0; -} - -void ctest_clear_call_state(void) -{ - test_get_called = 0; - test_set_called = 0; - test_commit_called = 0; -} - -int ctest_get_call_state(void) -{ - return test_get_called + test_set_called + test_commit_called; -} - -void config_wipe_srcs(void) -{ - sys_slist_init(&settings_load_srcs); - settings_save_dst = NULL; -} - -ZTEST_SUITE(settings_config, NULL, settings_config_setup, NULL, NULL, settings_config_teardown); diff --git a/tests/subsys/settings/zms/testcase.yaml b/tests/subsys/settings/zms/testcase.yaml deleted file mode 100644 index 2234fd4cd78..00000000000 --- a/tests/subsys/settings/zms/testcase.yaml +++ /dev/null @@ -1,7 +0,0 @@ -tests: - settings.zms: - depends_on: zms - min_ram: 32 - tags: - - settings - - zms From 860da44ad9dc89e9bcc16159e089147b35797386 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 17 Sep 2024 11:16:28 +0200 Subject: [PATCH 03/27] [nrf fromtree] settings: ZMS: add a backend for ZMS (Zephyr Memory Storage) This adds the initial backend support for the ZMS storage system. Signed-off-by: Riadh Ghaddab (cherry picked from commit ef4e8dd5c3f8186ab62d28d92bd3f31ddefade61) --- subsys/settings/Kconfig | 39 ++ .../settings/include/settings/settings_zms.h | 89 ++++ subsys/settings/src/CMakeLists.txt | 1 + subsys/settings/src/settings_zms.c | 463 ++++++++++++++++++ tests/subsys/settings/zms/CMakeLists.txt | 7 + tests/subsys/settings/zms/prj.conf | 8 + tests/subsys/settings/zms/src/CMakeLists.txt | 12 + tests/subsys/settings/zms/src/settings_test.h | 42 ++ .../settings/zms/src/settings_test_zms.c | 125 +++++ tests/subsys/settings/zms/testcase.yaml | 7 + 10 files changed, 793 insertions(+) create mode 100644 subsys/settings/include/settings/settings_zms.h create mode 100644 subsys/settings/src/settings_zms.c create mode 100644 tests/subsys/settings/zms/CMakeLists.txt create mode 100644 tests/subsys/settings/zms/prj.conf create mode 100644 tests/subsys/settings/zms/src/CMakeLists.txt create mode 100644 tests/subsys/settings/zms/src/settings_test.h create mode 100644 tests/subsys/settings/zms/src/settings_test_zms.c create mode 100644 tests/subsys/settings/zms/testcase.yaml diff --git a/subsys/settings/Kconfig b/subsys/settings/Kconfig index bc015cde9c7..a59b618759e 100644 --- a/subsys/settings/Kconfig +++ b/subsys/settings/Kconfig @@ -32,6 +32,7 @@ config SETTINGS_ENCODE_LEN choice SETTINGS_BACKEND prompt "Storage back-end" + default SETTINGS_ZMS if ZMS default SETTINGS_NVS if NVS default SETTINGS_FCB if FCB default SETTINGS_FILE if FILE_SYSTEM @@ -39,6 +40,13 @@ choice SETTINGS_BACKEND help Storage back-end to be used by the settings subsystem. +config SETTINGS_ZMS + bool "ZMS (Zephyr Memory Storage)" + depends on ZMS + select SYS_HASH_FUNC32 + help + Use ZMS as settings storage backend. + config SETTINGS_FCB bool "FCB" depends on FCB @@ -132,6 +140,37 @@ config SETTINGS_NVS_SECTOR_COUNT help Number of sectors used for the NVS settings area +config SETTINGS_ZMS_SECTOR_SIZE_MULT + int "Sector size of the ZMS settings area" + default 1 + depends on SETTINGS_ZMS + help + The sector size to use for the ZMS settings area as a multiple of + FLASH_ERASE_BLOCK_SIZE. + +config SETTINGS_ZMS_CUSTOM_SECTOR_COUNT + bool "Customize the sector count of the ZMS settings partition" + depends on SETTINGS_ZMS + help + The number of sectors used by default is the maximum value that can + fit in the settings storage partition. + Enabling this config allows to customize the number of used sectors. + +config SETTINGS_ZMS_SECTOR_COUNT + int "Sector count of the ZMS settings area" + default 8 + depends on SETTINGS_ZMS && SETTINGS_ZMS_CUSTOM_SECTOR_COUNT + help + Number of sectors used for the ZMS settings area + +config SETTINGS_ZMS_MAX_COLLISIONS_BITS + int "number of bits reserved to handle collisions between hash numbers" + default 4 + depends on SETTINGS_ZMS + help + The maximum number of hash collisions needs to be well sized depending + on the data that is going to be stored in ZMS and its hash values + config SETTINGS_SHELL bool "Settings shell" depends on SHELL diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h new file mode 100644 index 00000000000..5c8db6c8673 --- /dev/null +++ b/subsys/settings/include/settings/settings_zms.h @@ -0,0 +1,89 @@ +/* Copyright (c) 2024 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef __SETTINGS_ZMS_H_ +#define __SETTINGS_ZMS_H_ + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/* In the ZMS backend, each setting is stored in two ZMS entries: + * 1. setting's name + * 2. setting's value + * + * The ZMS entry ID for the setting's value is determined implicitly based on + * the ID of the ZMS entry for the setting's name, once that is found. The + * difference between name and value ID is constant and equal to + * ZMS_NAME_ID_OFFSET. + * + * Setting's name is hashed into 29 bits minus hash_collisions_bits. + * The 2 MSB_bits have always the same value 10, the LL_bit for the name's hash is 0 + * and the hash_collisions_bits is configurable through CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS. + * The resulted 32 bits is the ZMS_ID of the Setting's name. + * If we detect a collision between ZMS_IDs we increment the value within hash_collision_bits + * until we find a free ZMS_ID. + * Separately, we store a linked list using the Setting's name ZMS_ID but setting the lsb to 1. + * + * The linked list is used to maintain a relation between all ZMS_IDs. This is necessary to load + * all settings at initialization. + * The linked list contains at least a header followed by multiple linked list elements that + * we can refer to as LL_x (where x is the order of that element in that list). + * This is a representation of the Linked List that is stored in the storage. + * LL_header <--> LL_0 <--> LL_1 <--> LL_2. + * The "next_hash" pointer of each LL element refers to the next element in the linked list. + * The "previous_hash" pointer is referring the previous element in the linked list. + * + * The bit representation of the 32 bits ZMS_ID is the following: + * -------------------------------------------------------------- + * | MSB_bits | hash (truncated) | hash_collision_bits | LL_bit | + * -------------------------------------------------------------- + * Where: + * MSB_bits (2 bits width) : = 10 for Name IDs + * = 11 for Data IDs + * hash (29 bits - hash_collision_bits) : truncated hash obtained from sys_hash32 + * hash_collision_bits (configurable width) : used to handle hash collisions + * LL_bit : = 0 when this is a name's ZMS_ID + * = 1 when this is the linked list ZMS_ID corresponding to the name + * + * if a settings element is deleted it won't be found. + */ + +#define ZMS_LL_HEAD_HASH_ID 0x80000000 +#define ZMS_DATA_ID_OFFSET 0x40000000 +#define ZMS_HASH_MASK GENMASK(29, CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS + 1) +#define ZMS_COLLISIONS_MASK GENMASK(CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS, 1) +#define ZMS_HASH_TOTAL_MASK GENMASK(29, 1) +#define ZMS_MAX_COLLISIONS (BIT(CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS) - 1) +#define ZMS_HEADER_HASH 0x80000000 + +/* some useful macros */ +#define ZMS_NAME_HASH_ID(x) (x & ZMS_HASH_TOTAL_MASK) +#define ZMS_UPDATE_COLLISION_NUM(x, y) \ + ((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK)) +#define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) + +struct settings_zms { + struct settings_store cf_store; + struct zms_fs cf_zms; + const struct device *flash_dev; + uint32_t last_hash_id; + uint32_t second_to_last_hash_id; + uint8_t hash_collision_num; +}; + +struct settings_hash_linked_list { + uint32_t previous_hash; + uint32_t next_hash; +}; + +#ifdef __cplusplus +} +#endif + +#endif /* __SETTINGS_ZMS_H_ */ diff --git a/subsys/settings/src/CMakeLists.txt b/subsys/settings/src/CMakeLists.txt index d4ec6e76142..4aa797fbb35 100644 --- a/subsys/settings/src/CMakeLists.txt +++ b/subsys/settings/src/CMakeLists.txt @@ -13,3 +13,4 @@ zephyr_sources_ifdef(CONFIG_SETTINGS_FCB settings_fcb.c) zephyr_sources_ifdef(CONFIG_SETTINGS_NVS settings_nvs.c) zephyr_sources_ifdef(CONFIG_SETTINGS_NONE settings_none.c) zephyr_sources_ifdef(CONFIG_SETTINGS_SHELL settings_shell.c) +zephyr_sources_ifdef(CONFIG_SETTINGS_ZMS settings_zms.c) diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c new file mode 100644 index 00000000000..abb8989c203 --- /dev/null +++ b/subsys/settings/src/settings_zms.c @@ -0,0 +1,463 @@ +/* Copyright (c) 2024 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +#include "settings/settings_zms.h" +#include "settings_priv.h" + +#include +#include +#include +#include +LOG_MODULE_DECLARE(settings, CONFIG_SETTINGS_LOG_LEVEL); + +#if DT_HAS_CHOSEN(zephyr_settings_partition) +#define SETTINGS_PARTITION DT_FIXED_PARTITION_ID(DT_CHOSEN(zephyr_settings_partition)) +#else +#define SETTINGS_PARTITION FIXED_PARTITION_ID(storage_partition) +#endif + +struct settings_zms_read_fn_arg { + struct zms_fs *fs; + uint32_t id; +}; + +static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg); +static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, + size_t val_len); +static void *settings_zms_storage_get(struct settings_store *cs); + +static struct settings_store_itf settings_zms_itf = {.csi_load = settings_zms_load, + .csi_save = settings_zms_save, + .csi_storage_get = settings_zms_storage_get}; + +static ssize_t settings_zms_read_fn(void *back_end, void *data, size_t len) +{ + struct settings_zms_read_fn_arg *rd_fn_arg; + + rd_fn_arg = (struct settings_zms_read_fn_arg *)back_end; + + return zms_read(rd_fn_arg->fs, rd_fn_arg->id, data, len); +} + +static int settings_zms_src(struct settings_zms *cf) +{ + cf->cf_store.cs_itf = &settings_zms_itf; + settings_src_register(&cf->cf_store); + + return 0; +} + +static int settings_zms_dst(struct settings_zms *cf) +{ + cf->cf_store.cs_itf = &settings_zms_itf; + settings_dst_register(&cf->cf_store); + + return 0; +} + +static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_hash) +{ + int rc = 0; + struct settings_hash_linked_list settings_element; + struct settings_hash_linked_list settings_update_element; + + /* let's update the linked list */ + rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + /* update the next element */ + if (settings_element.next_hash) { + rc = zms_read(&cf->cf_zms, settings_element.next_hash, &settings_update_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + settings_update_element.previous_hash = settings_element.previous_hash; + rc = zms_write(&cf->cf_zms, settings_element.next_hash, &settings_update_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + if (!settings_update_element.next_hash) { + /* update second_to_last_hash_id */ + cf->second_to_last_hash_id = settings_element.previous_hash; + } + } else { + /* we are deleting the last element of the linked list + * let's update the last_hash_id. + */ + cf->last_hash_id = settings_element.previous_hash; + } + /* update the previous element */ + if (settings_element.previous_hash) { + rc = zms_read(&cf->cf_zms, settings_element.previous_hash, &settings_update_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + if (!settings_element.next_hash) { + /* we are deleting the last element of the linked list, + * let's update the second_to_last_hash_id + */ + cf->second_to_last_hash_id = settings_update_element.previous_hash; + } + settings_update_element.next_hash = settings_element.next_hash; + rc = zms_write(&cf->cf_zms, settings_element.previous_hash, + &settings_update_element, sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + } + + return rc; +} + +static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) +{ + int rc = 0; + + rc = zms_delete(&cf->cf_zms, name_hash); + if (rc >= 0) { + rc = zms_delete(&cf->cf_zms, name_hash + ZMS_DATA_ID_OFFSET); + } + if (rc < 0) { + return rc; + } + + rc = settings_zms_unlink_ll_node(cf, name_hash); + if (rc < 0) { + return rc; + } + + /* Now delete the current linked list element */ + rc = zms_delete(&cf->cf_zms, name_hash | 1); + if (rc < 0) { + return rc; + } + + return rc; +} + +static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg) +{ + int ret = 0; + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + struct settings_zms_read_fn_arg read_fn_arg; + struct settings_hash_linked_list settings_element; + char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + ssize_t rc1; + ssize_t rc2; + uint32_t ll_hash_id; + + ret = zms_read(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } + ll_hash_id = settings_element.next_hash; + + while (ll_hash_id) { + + /* In the ZMS backend, each setting item is stored in two ZMS + * entries one for the setting's name and one with the + * setting's value. + */ + rc1 = zms_read(&cf->cf_zms, ZMS_NAME_HASH_ID(ll_hash_id), &name, sizeof(name) - 1); + /* get the length of data and verify that it exists */ + rc2 = zms_get_data_length(&cf->cf_zms, + ZMS_NAME_HASH_ID(ll_hash_id) + ZMS_DATA_ID_OFFSET); + + if ((rc1 <= 0) || (rc2 <= 0)) { + /* Settings item is not stored correctly in the ZMS. + * ZMS entry for its name or value is either missing + * or deleted. Clean dirty entries to make space for + * future settings item. + */ + ret = settings_zms_delete(cf, ZMS_NAME_HASH_ID(ll_hash_id)); + if (ret < 0) { + return ret; + } + continue; + } + + /* Found a name, this might not include a trailing \0 */ + name[rc1] = '\0'; + read_fn_arg.fs = &cf->cf_zms; + read_fn_arg.id = ZMS_NAME_HASH_ID(ll_hash_id) + ZMS_DATA_ID_OFFSET; + + ret = settings_call_set_handler(name, rc2, settings_zms_read_fn, &read_fn_arg, + (void *)arg); + if (ret) { + break; + } + + /* update next ll_hash_id */ + ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } + ll_hash_id = settings_element.next_hash; + } + + return ret; +} + +static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, + size_t val_len) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + struct settings_hash_linked_list settings_element; + char rdname[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + uint32_t name_hash; + uint32_t collision_num = 0; + bool delete; + bool write_name; + bool hash_collision; + int rc = 0; + int first_available_hash_index = -1; + + if (!name) { + return -EINVAL; + } + + /* Find out if we are doing a delete */ + delete = ((value == NULL) || (val_len == 0)); + + name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + + /* Let's find out if there is no hash collisions in the storage */ + write_name = true; + hash_collision = true; + + for (int i = 0; i <= cf->hash_collision_num; i++) { + rc = zms_read(&cf->cf_zms, name_hash + i * LSB_GET(ZMS_COLLISIONS_MASK), &rdname, + sizeof(rdname)); + if (rc == -ENOENT) { + if (first_available_hash_index < 0) { + first_available_hash_index = i; + } + continue; + } else if (rc < 0) { + /* error while reading */ + return rc; + } + /* Settings entry exist, let's verify if this is the same + * name + */ + rdname[rc] = '\0'; + if (!strcmp(name, rdname)) { + /* Hash exist and the names are equal, we should + * not write the names again. + */ + write_name = false; + name_hash += i * LSB_GET(ZMS_COLLISIONS_MASK); + goto no_hash_collision; + } + /* At this step a Hash collision exists and names are different. + * If we are in the middle of the loop, we should continue checking + * all other possible hash collisions. + * If we reach the end of the loop, either we should select the first + * free hash value otherwise we increment it to the next free value and + * update hash_collision_num + */ + collision_num++; + } + + if (collision_num <= cf->hash_collision_num) { + /* At this step there is a free hash found */ + name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, first_available_hash_index); + goto no_hash_collision; + } else if (collision_num > cf->hash_collision_num) { + /* We must create a new hash based on incremented collision_num */ + if (collision_num > ZMS_MAX_COLLISIONS) { + /* At this step there is no more space to store hash values */ + LOG_ERR("Maximum hash collisions reached"); + return -ENOSPC; + } + cf->hash_collision_num = collision_num; + name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, collision_num); + } + +no_hash_collision: + if (delete) { + if (write_name) { + /* hash doesn't exist, do not write anything here */ + return 0; + } + + rc = settings_zms_delete(cf, name_hash); + return rc; + } + + /* write the value */ + rc = zms_write(&cf->cf_zms, name_hash + ZMS_DATA_ID_OFFSET, value, val_len); + if (rc < 0) { + return rc; + } + + /* write the name if required */ + if (write_name) { + rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name)); + if (rc < 0) { + return rc; + } + /* write linked list structure element */ + settings_element.next_hash = 0; + settings_element.previous_hash = cf->last_hash_id; + rc = zms_write(&cf->cf_zms, name_hash | 1, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + /* Now update the previous linked list element */ + settings_element.next_hash = name_hash | 1; + settings_element.previous_hash = cf->second_to_last_hash_id; + rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + cf->second_to_last_hash_id = cf->last_hash_id; + cf->last_hash_id = name_hash | 1; + } + + return 0; +} + +static int settings_zms_get_last_hash_ids(struct settings_zms *cf) +{ + struct settings_hash_linked_list settings_element; + uint32_t ll_last_hash_id = ZMS_LL_HEAD_HASH_ID; + int rc = 0; + + cf->hash_collision_num = 0; + do { + rc = zms_read(&cf->cf_zms, ll_last_hash_id, &settings_element, + sizeof(settings_element)); + if (rc) { + return rc; + } + /* increment hash collision number if necessary */ + if (ZMS_COLLISION_NUM(ll_last_hash_id) > cf->hash_collision_num) { + cf->hash_collision_num = ZMS_COLLISION_NUM(ll_last_hash_id); + } + cf->last_hash_id = ll_last_hash_id; + cf->second_to_last_hash_id = settings_element.previous_hash; + ll_last_hash_id = settings_element.next_hash; + } while (settings_element.next_hash); + + return 0; +} + +/* Initialize the zms backend. */ +static int settings_zms_backend_init(struct settings_zms *cf) +{ + int rc; + + cf->cf_zms.flash_device = cf->flash_dev; + if (cf->cf_zms.flash_device == NULL) { + return -ENODEV; + } + + rc = zms_mount(&cf->cf_zms); + if (rc) { + return rc; + } + + cf->hash_collision_num = 0; + + rc = settings_zms_get_last_hash_ids(cf); + if (rc == -ENOENT) { + /* header doesn't exist or linked list broken, reinitialize the header */ + const struct settings_hash_linked_list settings_element = {.previous_hash = 0, + .next_hash = 0}; + rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; + cf->second_to_last_hash_id = 0; + } else if (rc < 0) { + return rc; + } + + LOG_DBG("ZMS backend initialized"); + return 0; +} + +int settings_backend_init(void) +{ + static struct settings_zms default_settings_zms; + int rc; + uint32_t cnt = 0; + size_t zms_sector_size; + const struct flash_area *fa; + struct flash_sector hw_flash_sector; + uint32_t sector_cnt = 1; + + rc = flash_area_open(SETTINGS_PARTITION, &fa); + if (rc) { + return rc; + } + + rc = flash_area_get_sectors(SETTINGS_PARTITION, §or_cnt, &hw_flash_sector); + if (rc != 0 && rc != -ENOMEM) { + return rc; + } + + zms_sector_size = CONFIG_SETTINGS_ZMS_SECTOR_SIZE_MULT * hw_flash_sector.fs_size; + + if (zms_sector_size > UINT32_MAX) { + return -EDOM; + } + +#if defined(CONFIG_SETTINGS_ZMS_CUSTOM_SECTOR_COUNT) + size_t zms_size = 0; + + while (cnt < CONFIG_SETTINGS_ZMS_SECTOR_COUNT) { + zms_size += zms_sector_size; + if (zms_size > fa->fa_size) { + break; + } + cnt++; + } +#else + cnt = fa->fa_size / zms_sector_size; +#endif + /* initialize the zms file system structure using the page_info */ + default_settings_zms.cf_zms.sector_size = zms_sector_size; + default_settings_zms.cf_zms.sector_count = cnt; + default_settings_zms.cf_zms.offset = fa->fa_off; + default_settings_zms.flash_dev = fa->fa_dev; + + rc = settings_zms_backend_init(&default_settings_zms); + if (rc) { + return rc; + } + + rc = settings_zms_src(&default_settings_zms); + + if (rc) { + return rc; + } + + rc = settings_zms_dst(&default_settings_zms); + + return rc; +} + +static void *settings_zms_storage_get(struct settings_store *cs) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + + return &cf->cf_zms; +} diff --git a/tests/subsys/settings/zms/CMakeLists.txt b/tests/subsys/settings/zms/CMakeLists.txt new file mode 100644 index 00000000000..218451c840e --- /dev/null +++ b/tests/subsys/settings/zms/CMakeLists.txt @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(test_settings_zms_raw) + +add_subdirectory(./src zms_test_bindir) diff --git a/tests/subsys/settings/zms/prj.conf b/tests/subsys/settings/zms/prj.conf new file mode 100644 index 00000000000..3de961c7c72 --- /dev/null +++ b/tests/subsys/settings/zms/prj.conf @@ -0,0 +1,8 @@ +CONFIG_ZTEST=y +CONFIG_FLASH=y +CONFIG_FLASH_MAP=y +CONFIG_ZMS=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y +CONFIG_SETTINGS_ZMS=y diff --git a/tests/subsys/settings/zms/src/CMakeLists.txt b/tests/subsys/settings/zms/src/CMakeLists.txt new file mode 100644 index 00000000000..220a42849a8 --- /dev/null +++ b/tests/subsys/settings/zms/src/CMakeLists.txt @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2024 BayLibre SAS + +zephyr_include_directories( + ${ZEPHYR_BASE}/subsys/settings/include + ${ZEPHYR_BASE}/subsys/settings/src + ${ZEPHYR_BASE}/tests/subsys/settings/zms/src + ) + +target_sources(app PRIVATE settings_test_zms.c) + +add_subdirectory(../../src settings_test_bindir) diff --git a/tests/subsys/settings/zms/src/settings_test.h b/tests/subsys/settings/zms/src/settings_test.h new file mode 100644 index 00000000000..472ff71f59b --- /dev/null +++ b/tests/subsys/settings/zms/src/settings_test.h @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2024 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef _SETTINGS_TEST_ZMS_H +#define _SETTINGS_TEST_ZMS_H + +#include +#include +#include + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +extern uint8_t val8; +extern uint8_t val8_un; +extern uint32_t val32; +extern uint64_t val64; + +extern int test_get_called; +extern int test_set_called; +extern int test_commit_called; +extern int test_export_block; + +extern struct settings_handler c_test_handlers[]; + +void ctest_clear_call_state(void); +int ctest_get_call_state(void); + +void config_wipe_srcs(void); +void *settings_config_setup(void); +void settings_config_teardown(void *fixture); + +#ifdef __cplusplus +} +#endif +#endif /* _SETTINGS_TEST_ZMS_H */ diff --git a/tests/subsys/settings/zms/src/settings_test_zms.c b/tests/subsys/settings/zms/src/settings_test_zms.c new file mode 100644 index 00000000000..17a5ccd56fe --- /dev/null +++ b/tests/subsys/settings/zms/src/settings_test_zms.c @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2024 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include + +#include "settings_priv.h" +#include "settings_test.h" + +uint8_t val8; +uint8_t val8_un; +uint32_t val32; +uint64_t val64; + +int test_get_called; +int test_set_called; +int test_commit_called; +int test_export_block; + +int c1_handle_get(const char *name, char *val, int val_len_max); +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); +int c1_handle_commit(void); +int c1_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)); + +struct settings_handler c_test_handlers[] = { + {.name = "myfoo", + .h_get = c1_handle_get, + .h_set = c1_handle_set, + .h_commit = c1_handle_commit, + .h_export = c1_handle_export}, +}; + +int c1_handle_get(const char *name, char *val, int val_len_max) +{ + const char *next; + + test_get_called = 1; + + if (settings_name_steq(name, "mybar", &next) && !next) { + val_len_max = MIN(val_len_max, sizeof(val8)); + memcpy(val, &val8, MIN(val_len_max, sizeof(val8))); + return val_len_max; + } + + if (settings_name_steq(name, "mybar64", &next) && !next) { + val_len_max = MIN(val_len_max, sizeof(val64)); + memcpy(val, &val64, MIN(val_len_max, sizeof(val64))); + return val_len_max; + } + + return -ENOENT; +} + +int c1_handle_set(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg) +{ + size_t val_len; + int rc; + const char *next; + + test_set_called = 1; + if (settings_name_steq(name, "mybar", &next) && !next) { + rc = read_cb(cb_arg, &val8, sizeof(val8)); + zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); + return 0; + } + + if (settings_name_steq(name, "mybar64", &next) && !next) { + rc = read_cb(cb_arg, &val64, sizeof(val64)); + zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); + return 0; + } + + if (settings_name_steq(name, "unaligned", &next) && !next) { + val_len = len; + zassert_equal(val_len, sizeof(val8_un), "value length: %d, ought equal 1", val_len); + rc = read_cb(cb_arg, &val8_un, sizeof(val8_un)); + zassert_true(rc >= 0, "SETTINGS_VALUE_SET callback"); + return 0; + } + + return -ENOENT; +} + +int c1_handle_commit(void) +{ + test_commit_called = 1; + return 0; +} + +int c1_handle_export(int (*cb)(const char *name, const void *value, size_t val_len)) +{ + if (test_export_block) { + return 0; + } + + (void)cb("myfoo/mybar", &val8, sizeof(val8)); + + (void)cb("myfoo/mybar64", &val64, sizeof(val64)); + + (void)cb("myfoo/unaligned", &val8_un, sizeof(val8_un)); + + return 0; +} + +void ctest_clear_call_state(void) +{ + test_get_called = 0; + test_set_called = 0; + test_commit_called = 0; +} + +int ctest_get_call_state(void) +{ + return test_get_called + test_set_called + test_commit_called; +} + +void config_wipe_srcs(void) +{ + sys_slist_init(&settings_load_srcs); + settings_save_dst = NULL; +} + +ZTEST_SUITE(settings_config, NULL, settings_config_setup, NULL, NULL, settings_config_teardown); diff --git a/tests/subsys/settings/zms/testcase.yaml b/tests/subsys/settings/zms/testcase.yaml new file mode 100644 index 00000000000..2234fd4cd78 --- /dev/null +++ b/tests/subsys/settings/zms/testcase.yaml @@ -0,0 +1,7 @@ +tests: + settings.zms: + depends_on: zms + min_ram: 32 + tags: + - settings + - zms From 6affc2ac7c1e78f2d9d72b7ebe8fb3cdffbf5381 Mon Sep 17 00:00:00 2001 From: Omkar Kulkarni Date: Wed, 23 Oct 2024 10:50:36 +0200 Subject: [PATCH 04/27] [nrf fromtree] tests: settings: Performance test for settings This adds a on target performance test for Settings SS. Using this test performance of the Setting SS + NVS/ZMS backend can be benchmarked. The test repeatedly write 128 settings entries. Each setting entry has a size of 4 bytes, and path length of 16 bytes (excluding the null-terminator). The test has two variants, with and without Bluetooth scan running. This is useful to benchmark performance along with some component of BT subsystem activated. The test could be enhanced in future to include or create combinations of different functionalities running, when agreesive store operations are happening. Signed-off-by: Omkar Kulkarni (cherry picked from commit 97166a5c1115c34544df5c031f4ccaba22a6b403) --- .../settings/performance/CMakeLists.txt | 13 ++ tests/subsys/settings/performance/prj.conf | 7 + .../settings/performance/settings_test_perf.c | 133 ++++++++++++++++++ .../subsys/settings/performance/testcase.yaml | 66 +++++++++ 4 files changed, 219 insertions(+) create mode 100644 tests/subsys/settings/performance/CMakeLists.txt create mode 100644 tests/subsys/settings/performance/prj.conf create mode 100644 tests/subsys/settings/performance/settings_test_perf.c create mode 100644 tests/subsys/settings/performance/testcase.yaml diff --git a/tests/subsys/settings/performance/CMakeLists.txt b/tests/subsys/settings/performance/CMakeLists.txt new file mode 100644 index 00000000000..b3e4780d3aa --- /dev/null +++ b/tests/subsys/settings/performance/CMakeLists.txt @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(test_settings_perf) + +zephyr_include_directories( + ${ZEPHYR_BASE}/subsys/settings/include + ${ZEPHYR_BASE}/subsys/settings/src + ) + +target_sources(app PRIVATE + settings_test_perf.c) diff --git a/tests/subsys/settings/performance/prj.conf b/tests/subsys/settings/performance/prj.conf new file mode 100644 index 00000000000..27c208baa0b --- /dev/null +++ b/tests/subsys/settings/performance/prj.conf @@ -0,0 +1,7 @@ +CONFIG_ZTEST=y +CONFIG_FLASH=y +CONFIG_FLASH_MAP=y +CONFIG_ZMS=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y diff --git a/tests/subsys/settings/performance/settings_test_perf.c b/tests/subsys/settings/performance/settings_test_perf.c new file mode 100644 index 00000000000..a5de17e3475 --- /dev/null +++ b/tests/subsys/settings/performance/settings_test_perf.c @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include + +#include "settings_priv.h" +#include +#include +#include + +/* This is a test suite for performance testing of settings subsystem by writing + * many small setting values repeatedly. Ideally, this should consume as small + * amount of time as possible for best possible UX. + */ + +static struct k_work_q settings_work_q; +static K_THREAD_STACK_DEFINE(settings_work_stack, 2024); +static struct k_work_delayable pending_store; + +#define TEST_SETTINGS_COUNT (128) +#define TEST_STORE_ITR (5) +#define TEST_TIMEOUT_SEC (60) +#define TEST_SETTINGS_WORKQ_PRIO (1) + +static void bt_scan_cb(const bt_addr_le_t *addr, int8_t rssi, uint8_t adv_type, + struct net_buf_simple *buf) +{ + printk("len %u\n", buf->len); +} + +struct test_setting { + uint32_t val; +} test_settings[TEST_SETTINGS_COUNT]; + +K_SEM_DEFINE(waitfor_work, 0, 1); + +static void store_pending(struct k_work *work) +{ + int err; + char path[20]; + struct test_stats { + uint32_t total_calculated; + uint32_t total_measured; + uint32_t single_entry_max; + uint32_t single_entry_min; + } stats = {0, 0, 0, UINT32_MAX}; + + int64_t ts1 = k_uptime_get(); + + /* benchmark storage performance */ + for (int j = 0; j < TEST_STORE_ITR; j++) { + for (int i = 0; i < TEST_SETTINGS_COUNT; i++) { + test_settings[i].val = TEST_SETTINGS_COUNT*j + i; + + int64_t ts2 = k_uptime_get(); + + snprintk(path, sizeof(path), "ab/cdef/ghi/%04x", i); + err = settings_save_one(path, &test_settings[i], + sizeof(struct test_setting)); + zassert_equal(err, 0, "settings_save_one failed %d", err); + + int64_t delta2 = k_uptime_delta(&ts2); + + if (stats.single_entry_max < delta2) { + stats.single_entry_max = delta2; + } + if (stats.single_entry_min > delta2) { + stats.single_entry_min = delta2; + } + stats.total_calculated += delta2; + } + } + + int64_t delta1 = k_uptime_delta(&ts1); + + stats.total_measured = delta1; + + printk("*** storing of %u entries completed ***\n", ARRAY_SIZE(test_settings)); + printk("total calculated: %u, total measured: %u\n", stats.total_calculated, + stats.total_measured); + printk("entry max: %u, entry min: %u\n", stats.single_entry_max, + stats.single_entry_min); + + k_sem_give(&waitfor_work); +} + +ZTEST_SUITE(settings_perf, NULL, NULL, NULL, NULL, NULL); + +ZTEST(settings_perf, test_performance) +{ + int err; + + if (IS_ENABLED(CONFIG_NVS)) { + printk("Testing with NVS\n"); + } else if (IS_ENABLED(CONFIG_ZMS)) { + printk("Testing with ZMS\n"); + } + + k_work_queue_start(&settings_work_q, settings_work_stack, + K_THREAD_STACK_SIZEOF(settings_work_stack), + K_PRIO_COOP(TEST_SETTINGS_WORKQ_PRIO), NULL); + k_thread_name_set(&settings_work_q.thread, "Settings workq"); + k_work_init_delayable(&pending_store, store_pending); + + if (IS_ENABLED(CONFIG_BT)) { + /* enable one of the major subsystems, and start scanning. */ + err = bt_enable(NULL); + zassert_equal(err, 0, "Bluetooth init failed (err %d)\n", err); + + err = bt_le_scan_start(BT_LE_SCAN_ACTIVE, bt_scan_cb); + zassert_equal(err, 0, "Scanning failed to start (err %d)\n", err); + } + + err = settings_subsys_init(); + zassert_equal(err, 0, "settings_backend_init failed %d", err); + + /* fill with values */ + for (int i = 0; i < TEST_SETTINGS_COUNT; i++) { + test_settings[i].val = i; + } + + k_work_reschedule_for_queue(&settings_work_q, &pending_store, K_NO_WAIT); + + err = k_sem_take(&waitfor_work, K_SECONDS(TEST_TIMEOUT_SEC)); + zassert_equal(err, 0, "k_sem_take failed %d", err); + + if (IS_ENABLED(CONFIG_BT)) { + err = bt_le_scan_stop(); + zassert_equal(err, 0, "Scanning failed to stop (err %d)\n", err); + } +} diff --git a/tests/subsys/settings/performance/testcase.yaml b/tests/subsys/settings/performance/testcase.yaml new file mode 100644 index 00000000000..7ae65c3c556 --- /dev/null +++ b/tests/subsys/settings/performance/testcase.yaml @@ -0,0 +1,66 @@ +tests: + settings.performance.zms: + extra_configs: + - CONFIG_SETTINGS_ZMS=y + - CONFIG_ZMS_LOOKUP_CACHE=y + - CONFIG_ZMS_LOOKUP_CACHE_SIZE=512 + platform_allow: + - nrf52840dk/nrf52840 + - nrf54l15dk/nrf54l15/cpuapp + min_ram: 32 + tags: + - settings + - zms + + settings.performance.nvs: + extra_configs: + - CONFIG_ZMS=n + - CONFIG_NVS=y + - CONFIG_NVS_LOOKUP_CACHE=y + - CONFIG_NVS_LOOKUP_CACHE_SIZE=512 + - CONFIG_SETTINGS_NVS_NAME_CACHE=y + - CONFIG_SETTINGS_NVS_NAME_CACHE_SIZE=512 + platform_allow: + - nrf52840dk/nrf52840 + - nrf54l15dk/nrf54l15/cpuapp + min_ram: 32 + tags: + - settings + - nvs + + settings.performance.zms_bt: + extra_configs: + - CONFIG_BT=y + - CONFIG_BT_OBSERVER=y + - CONFIG_BT_PERIPHERAL=y + - CONFIG_SETTINGS_ZMS=y + - CONFIG_ZMS_LOOKUP_CACHE=y + - CONFIG_ZMS_LOOKUP_CACHE_SIZE=512 + platform_allow: nrf52840dk/nrf52840 + platform_exclude: + - native_sim + - qemu_x86 + min_ram: 32 + tags: + - settings + - zms + + settings.performance.nvs_bt: + extra_configs: + - CONFIG_BT=y + - CONFIG_BT_OBSERVER=y + - CONFIG_BT_PERIPHERAL=y + - CONFIG_ZMS=n + - CONFIG_NVS=y + - CONFIG_NVS_LOOKUP_CACHE=y + - CONFIG_NVS_LOOKUP_CACHE_SIZE=512 + - CONFIG_SETTINGS_NVS_NAME_CACHE=y + - CONFIG_SETTINGS_NVS_NAME_CACHE_SIZE=512 + platform_allow: nrf52840dk/nrf52840 + platform_exclude: + - native_sim + - qemu_x86 + min_ram: 32 + tags: + - settings + - nvs From 1ee81c6c9be408798e3c9074c6da6ebf0ca15692 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Wed, 30 Oct 2024 14:45:27 +0100 Subject: [PATCH 05/27] [nrf fromtree] fs: zms: multiple style fixes from previous PR review This resolves some addressed comments in this PR zephyrproject-rtos#77930 as well as this PR zephyrproject-rtos#80407 Signed-off-by: Riadh Ghaddab (cherry picked from commit 5f7cda5e0608bd027f759736aef78bf6f03070d0) --- doc/services/storage/zms/zms.rst | 302 ++++++++++++++----------------- doc/zephyr.doxyfile.in | 1 + include/zephyr/fs/zms.h | 73 ++++++-- subsys/fs/zms/CMakeLists.txt | 2 +- subsys/fs/zms/Kconfig | 25 ++- subsys/fs/zms/zms.c | 6 +- subsys/fs/zms/zms_priv.h | 60 +++--- 7 files changed, 242 insertions(+), 227 deletions(-) diff --git a/doc/services/storage/zms/zms.rst b/doc/services/storage/zms/zms.rst index 3523bfc5eae..8a94e6d5992 100644 --- a/doc/services/storage/zms/zms.rst +++ b/doc/services/storage/zms/zms.rst @@ -15,15 +15,15 @@ pairs until it is full. The key-value pair is divided into two parts: - The key part is written in an ATE (Allocation Table Entry) called "ID-ATE" which is stored - starting from the bottom of the sector -- The value part is defined as "DATA" and is stored raw starting from the top of the sector + starting from the bottom of the sector. +- The value part is defined as "data" and is stored raw starting from the top of the sector. -Additionally, for each sector we store at the last positions Header-ATEs which are ATEs that +Additionally, for each sector we store at the last positions header ATEs which are ATEs that are needed for the sector to describe its status (closed, open) and the current version of ZMS. When the current sector is full we verify first that the following sector is empty, we garbage -collect the N+2 sector (where N is the current sector number) by moving the valid ATEs to the -N+1 empty sector, we erase the garbage collected sector and then we close the current sector by +collect the sector N+2 (where N is the current sector number) by moving the valid ATEs to the +N+1 empty sector, we erase the garbage-collected sector and then we close the current sector by writing a garbage_collect_done ATE and the close ATE (one of the header entries). Afterwards we move forward to the next sector and start writing entries again. @@ -60,50 +60,50 @@ A sector is organized in this form (example with 3 sectors): - . - . * - . - - ATE_b2 - - ATE_c2 - * - ATE_a2 - - ATE_b1 - - ATE_c1 - * - ATE_a1 - - ATE_b0 - - ATE_c0 - * - ATE_a0 - - GC_done - - GC_done - * - Close (cyc=1) - - Close (cyc=1) - - Close (cyc=1) - * - Empty (cyc=1) - - Empty (cyc=2) - - Empty (cyc=2) + - ID ATE_b2 + - ID ATE_c2 + * - ID ATE_a2 + - ID ATE_b1 + - ID ATE_c1 + * - ID ATE_a1 + - ID ATE_b0 + - ID ATE_c0 + * - ID ATE_a0 + - GC_done ATE + - GC_done ATE + * - Close ATE (cyc=1) + - Close ATE (cyc=1) + - Close ATE (cyc=1) + * - Empty ATE (cyc=1) + - Empty ATE (cyc=2) + - Empty ATE (cyc=2) Definition of each element in the sector ======================================== -``Empty ATE:`` is written when erasing a sector (last position of the sector). +``Empty ATE`` is written when erasing a sector (last position of the sector). -``Close ATE:`` is written when closing a sector (second to last position of the sector). +``Close ATE`` is written when closing a sector (second to last position of the sector). -``GC_done ATE:`` is written to indicate that the next sector has been already garbage -collected. This ATE could be in any position of the sector. +``GC_done ATE`` is written to indicate that the next sector has already been garbage-collected. +This ATE could be at any position of the sector. -``ID-ATE:`` are entries that contain a 32 bits Key and describe where the data is stored, its -size and its crc32 +``ID ATE`` are entries that contain a 32-bit key and describe where the data is stored, its +size and its CRC32. -``Data:`` is the actual value associated to the ID-ATE +``Data`` is the actual value associated to the ID-ATE. How does ZMS work? ****************** -Mounting the Storage system +Mounting the storage system =========================== -Mounting the storage starts by getting the flash parameters, checking that the file system +Mounting the storage system starts by getting the flash parameters, checking that the file system properties are correct (sector_size, sector_count ...) then calling the zms_init function to make the storage ready. -To mount the filesystem some elements in the zms_fs structure must be initialized. +To mount the filesystem the following elements in the ``zms_fs`` structure must be initialized: .. code-block:: c @@ -125,43 +125,44 @@ To mount the filesystem some elements in the zms_fs structure must be initialize Initialization ============== -As ZMS has a fast-forward write mechanism, we must find the last sector and the last pointer of +As ZMS has a fast-forward write mechanism, it must find the last sector and the last pointer of the entry where it stopped the last time. It must look for a closed sector followed by an open one, then within the open sector, it finds -(recover) the last written ATE (Allocation Table Entry). +(recovers) the last written ATE. After that, it checks that the sector after this one is empty, or it will erase it. -ZMS ID-Data write +ZMS ID/data write =================== -To avoid rewriting the same data with the same ID again, it must look in all the sectors if the -same ID exist then compares its data, if the data is identical no write is performed. -If we must perform a write, then an ATE and Data (if not a delete) are written in the sector. -If the sector is full (cannot hold the current data + ATE) we have to move to the next sector, +To avoid rewriting the same data with the same ID again, ZMS must look in all the sectors if the +same ID exists and then compares its data. If the data is identical, no write is performed. +If it must perform a write, then an ATE and the data (if the operation is not a delete) are written +in the sector. +If the sector is full (cannot hold the current data + ATE), ZMS has to move to the next sector, garbage collect the sector after the newly opened one then erase it. -Data size that is smaller or equal to 8 bytes are written within the ATE. +Data whose size is smaller or equal to 8 bytes are written within the ATE. ZMS ID/data read (with history) =============================== -By default it looks for the last data with the same ID by browsing through all stored ATEs from +By default ZMS looks for the last data with the same ID by browsing through all stored ATEs from the most recent ones to the oldest ones. If it finds a valid ATE with a matching ID it retrieves its data and returns the number of bytes that were read. -If history count is provided that is different than 0, older data with same ID is retrieved. +If a history count is provided and different than 0, older data with same ID is retrieved. ZMS free space calculation ========================== ZMS can also return the free space remaining in the partition. -However, this operation is very time consuming and needs to browse all valid ATEs in all sectors -of the partition and for each valid ATE try to find if an older one exist. -It is not recommended for application to use this function often, as it is time consuming and +However, this operation is very time-consuming as it needs to browse through all valid ATEs +in all sectors of the partition and for each valid ATE try to find if an older one exists. +It is not recommended for applications to use this function often, as it is time-consuming and could slow down the calling thread. The cycle counter ================= -Each sector has a lead cycle counter which is a uin8_t that is used to validate all the other +Each sector has a lead cycle counter which is a ``uin8_t`` that is used to validate all the other ATEs. The lead cycle counter is stored in the empty ATE. To become valid, an ATE must have the same cycle counter as the one stored in the empty ATE. @@ -179,88 +180,68 @@ counter as the empty ATE. When closing a sector, all the remaining space that has not been used is filled with garbage data to avoid having old ATEs with a valid cycle counter. -Triggering Garbage collection +Triggering garbage collection ============================= Some applications need to make sure that storage writes have a maximum defined latency. -When calling a ZMS write, the current sector could be almost full and we need to trigger the GC -to switch to the next sector. -This operation is time consuming and it will cause some applications to not meet their real time +When calling ZMS to make a write, the current sector could be almost full such that ZMS needs to +trigger the GC to switch to the next sector. +This operation is time-consuming and will cause some applications to not meet their real time constraints. ZMS adds an API for the application to get the current remaining free space in a sector. -The application could then decide when needed to switch to the next sector if the current one is -almost full and of course it will trigger the garbage collection on the next sector. +The application could then decide when to switch to the next sector if the current one is almost +full. This will of course trigger the garbage collection operation on the next sector. This will guarantee the application that the next write won't trigger the garbage collection. ATE (Allocation Table Entry) structure ====================================== -An entry has 16 bytes divided between these variables : +An entry has 16 bytes divided between these fields: -.. code-block:: c +See the :c:struct:`zms_ate` structure. - struct zms_ate { - uint8_t crc8; /* crc8 check of the entry */ - uint8_t cycle_cnt; /* cycle counter for non-erasable devices */ - uint16_t len; /* data len within sector */ - uint32_t id; /* data id */ - union { - uint8_t data[8]; /* used to store small size data */ - struct { - uint32_t offset; /* data offset within sector */ - union { - uint32_t data_crc; /* crc for data */ - uint32_t metadata; /* Used to store metadata information - * such as storage version. - */ - }; - }; - }; - } __packed; - -.. note:: The CRC of the data is checked only when the whole the element is read. +.. note:: The CRC of the data is checked only when a full read of the data is made. The CRC of the data is not checked for a partial read, as it is computed for the whole element. -.. note:: Enabling the CRC feature on previously existing ZMS content without CRC enabled - will make all existing data invalid. - -.. _free-space: +.. warning:: Enabling the CRC feature on previously existing ZMS content that did not have it + enabled will make all existing data invalid. Available space for user data (key-value pairs) *********************************************** -For both scenarios ZMS should always have an empty sector to be able to perform the -garbage collection (GC). +ZMS always needs an empty sector to be able to perform the garbage collection (GC). So, if we suppose that 4 sectors exist in a partition, ZMS will only use 3 sectors to store -Key-value pairs and keep one sector empty to be able to launch GC. +key-value pairs and keep one sector empty to be able to perform GC. The empty sector will rotate between the 4 sectors in the partition. -.. note:: The maximum single data length that could be written at once in a sector is 64K - (This could change in future versions of ZMS) +.. note:: The maximum single data length that can be written at once in a sector is 64K + (this could change in future versions of ZMS). Small data values ================= -Values smaller than 8 bytes will be stored within the entry (ATE) itself, without writing data -at the top of the sector. +Values smaller than or equal to 8 bytes will be stored within the entry (ATE) itself, without +writing data at the top of the sector. ZMS has an entry size of 16 bytes which means that the maximum available space in a partition to -store data is computed in this scenario as : +store data is computed in this scenario as: .. math:: - \small\frac{(NUM\_SECTORS - 1) \times (SECTOR\_SIZE - (5 \times ATE\_SIZE))}{2} + \small\frac{(NUM\_SECTORS - 1) \times (SECTOR\_SIZE - (5 \times ATE\_SIZE)) \times (DATA\_SIZE)}{ATE\_SIZE} Where: -``NUM_SECTOR:`` Total number of sectors +``NUM_SECTOR``: Total number of sectors + +``SECTOR_SIZE``: Size of the sector -``SECTOR_SIZE:`` Size of the sector +``ATE_SIZE``: 16 bytes -``ATE_SIZE:`` 16 bytes +``(5 * ATE_SIZE)``: Reserved ATEs for header and delete items -``(5 * ATE_SIZE):`` Reserved ATEs for header and delete items +``DATA_SIZE``: Size of the small data values (range from 1 to 8) -For example for 4 sectors of 1024 bytes, free space for data is :math:`\frac{3 \times 944}{2} = 1416 \, \text{ bytes}`. +For example for 4 sectors of 1024 bytes, free space for 8-byte length data is :math:`\frac{3 \times 944 \times 8}{16} = 1416 \, \text{ bytes}`. Large data values ================= @@ -274,67 +255,66 @@ Let's take an example: For a partition that has 4 sectors of 1024 bytes and for data size of 64 bytes. Only 3 sectors are available for writes with a capacity of 944 bytes each. -Each Key-value pair needs an extra 16 bytes for ATE which makes it possible to store 11 pairs -in each sectors (:math:`\frac{944}{80}`). -Total data that could be stored in this partition for this case is :math:`11 \times 3 \times 64 = 2112 \text{ bytes}` - -.. _wear-leveling: +Each key-value pair needs an extra 16 bytes for the ATE, which makes it possible to store 11 pairs +in each sector (:math:`\frac{944}{80}`). +Total data that could be stored in this partition for this case is :math:`11 \times 3 \times 64 = 2112 \text{ bytes}`. Wear leveling ************* This storage system is optimized for devices that do not require an erase. -Using storage systems that rely on an erase-value (NVS as an example) will need to emulate the -erase with write operations. This will cause a significant decrease in the life expectancy of -these devices and will cause more delays for write operations and for initialization. -ZMS uses a cycle count mechanism that avoids emulating erase operation for these devices. +Storage systems that rely on an erase value (NVS as an example) need to emulate the erase with +write operations. This causes a significant decrease in the life expectancy of these devices +as well as more delays for write operations and initialization of the device when it is empty. +ZMS uses a cycle count mechanism that avoids emulating erase operations for these devices. It also guarantees that every memory location is written only once for each cycle of sector write. -As an example, to erase a 4096 bytes sector on a non-erasable device using NVS, 256 flash writes -must be performed (supposing that write-block-size=16 bytes), while using ZMS only 1 write of -16 bytes is needed. This operation is 256 times faster in this case. +As an example, to erase a 4096-byte sector on devices that do not require an erase operation +using NVS, 256 flash writes must be performed (supposing that ``write-block-size`` = 16 bytes), while +using ZMS, only 1 write of 16 bytes is needed. This operation is 256 times faster in this case. -Garbage collection operation is also adding some writes to the memory cell life expectancy as it -is moving some blocks from one sector to another. +The garbage collection operation also reduces the memory cell life expectancy as it performs write +operations when moving blocks from one sector to another. To make the garbage collector not affect the life expectancy of the device it is recommended -to correctly dimension the partition size. Its size should be the double of the maximum size of -data (including extra headers) that could be written in the storage. +to dimension the partition appropriately. Its size should be the double of the maximum size of +data (including headers) that could be written in the storage. -See :ref:`free-space`. +See `Available space for user data <#available-space-for-user-data-key-value-pairs>`_. Device lifetime calculation =========================== -Storage devices whether they are classical Flash or new technologies like RRAM/MRAM has a limited -life expectancy which is determined by the number of times memory cells can be erased/written. +Storage devices, whether they are classical flash or new technologies like RRAM/MRAM, have a +limited life expectancy which is determined by the number of times memory cells can be +erased/written. Flash devices are erased one page at a time as part of their functional behavior (otherwise -memory cells cannot be overwritten) and for non-erasable storage devices memory cells can be -overwritten directly. +memory cells cannot be overwritten), and for storage devices that do not require an erase +operation, memory cells can be overwritten directly. A typical scenario is shown here to calculate the life expectancy of a device: -Let's suppose that we store an 8 bytes variable using the same ID but its content changes every +Let's suppose that we store an 8-byte variable using the same ID but its content changes every minute. The partition has 4 sectors with 1024 bytes each. Each write of the variable requires 16 bytes of storage. As we have 944 bytes available for ATEs for each sector, and because ZMS is a fast-forward storage system, we are going to rewrite the first location of the first sector after :math:`\frac{(944 \times 4)}{16} = 236 \text{ minutes}`. -In addition to the normal writes, garbage collector will move the still valid data from old -sectors to new ones. +In addition to the normal writes, the garbage collector will move the data that is still valid +from old sectors to new ones. As we are using the same ID and a big partition size, no data will be moved by the garbage collector in this case. -For storage devices that could be written 20000 times, the storage will last about -4.720.000 minutes (~9 years). +For storage devices that can be written 20 000 times, the storage will last about +4 720 000 minutes (~9 years). To make a more general formula we must first compute the effective used size in ZMS by our typical set of data. -For id/data pair with data <= 8 bytes, effective_size is 16 bytes -For id/data pair with data > 8 bytes, effective_size is 16 bytes + sizeof(data) -Let's suppose that total_effective_size is the total size of the set of data that is written in -the storage and that the partition is well dimensioned (double of the effective size) to avoid +For ID/data pairs with data <= 8 bytes, ``effective_size`` is 16 bytes. +For ID/data pairs with data > 8 bytes, ``effective_size`` is ``16 + sizeof(data)`` bytes. +Let's suppose that ``total_effective_size`` is the total size of the data that is written in +the storage and that the partition is sized appropriately (double of the effective size) to avoid having the garbage collector moving blocks all the time. -The expected life of the device in minutes is computed as : +The expected lifetime of the device in minutes is computed as: .. math:: @@ -342,11 +322,11 @@ The expected life of the device in minutes is computed as : Where: -``SECTOR_EFFECTIVE_SIZE``: is the size sector - header_size(80 bytes) +``SECTOR_EFFECTIVE_SIZE``: The sector size - header size (80 bytes) -``SECTOR_NUMBER``: is the number of sectors +``SECTOR_NUMBER``: The number of sectors -``MAX_NUM_WRITES``: is the life expectancy of the storage device in number of writes +``MAX_NUM_WRITES``: The life expectancy of the storage device in number of writes ``TOTAL_EFFECTIVE_SIZE``: Total effective size of the set of written data @@ -360,15 +340,16 @@ such as low latency and bigger storage space. Existing features ================= -Version1 --------- -- Supports non-erasable devices (only one write operation to erase a sector) -- Supports large partition size and sector size (64 bits address space) -- Supports 32-bit IDs to store ID/Value pairs -- Small sized data ( <= 8 bytes) are stored in the ATE itself -- Built-in Data CRC32 (included in the ATE) -- Versioning of ZMS (to handle future evolution) -- Supports large write-block-size (Only for platforms that need this) +Version 1 +--------- +- Supports storage devices that do not require an erase operation (only one write operation + to invalidate a sector) +- Supports large partition and sector sizes (64-bit address space) +- Supports 32-bit IDs +- Small-sized data (<= 8 bytes) are stored in the ATE itself +- Built-in data CRC32 (included in the ATE) +- Versioning of ZMS (to handle future evolutions) +- Supports large ``write-block-size`` (only for platforms that need it) Future features =============== @@ -395,10 +376,10 @@ functionality: :ref:`NVS ` and :ref:`FCB `. Which one to use in your application will depend on your needs and the hardware you are using, and this section provides information to help make a choice. -- If you are using a non-erasable technology device like RRAM or MRAM, :ref:`ZMS ` is definitely the +- If you are using devices that do not require an erase operation like RRAM or MRAM, :ref:`ZMS ` is definitely the best fit for your storage subsystem as it is designed to avoid emulating erase operation using large block writes for these devices and replaces it with a single write call. -- For devices with large write_block_size and/or needs a sector size that is different than the +- For devices that have a large ``write_block_size`` and/or need a sector size that is different than the classical flash page size (equal to erase_block_size), :ref:`ZMS ` is also the best fit as there is the possibility to customize these parameters and add the support of these devices in ZMS. - For classical flash technology devices, :ref:`NVS ` is recommended as it has low footprint (smaller @@ -413,7 +394,7 @@ and this section provides information to help make a choice. More generally to make the right choice between NVS and ZMS, all the blockers should be first verified to make sure that the application could work with one subsystem or the other, then if both solutions could be implemented, the best choice should be based on the calculations of the -life expectancy of the device described in this section: :ref:`wear-leveling`. +life expectancy of the device described in this section: `Wear leveling <#wear-leveling>`_. Recommendations to increase performance *************************************** @@ -421,44 +402,41 @@ Recommendations to increase performance Sector size and count ===================== -- The total size of the storage partition should be well dimensioned to achieve the best - performance for ZMS. +- The total size of the storage partition should be set appropriately to achieve the best + performance with ZMS. All the information regarding the effectively available free space in ZMS can be found - in the documentation. See :ref:`free-space`. - We recommend choosing a storage partition that can hold double the size of the key-value pairs + in the documentation. See `Available space for user data <#available-space-for-user-data-key-value-pairs>`_. + It's recommended to choose a storage partition size that is double the size of the key-value pairs that will be written in the storage. -- The size of a sector needs to be dimensioned to hold the maximum data length that will be stored. - Increasing the size of a sector will slow down the garbage collection operation which will - occur less frequently. - Decreasing its size, in the opposite, will make the garbage collection operation faster - which will occur more frequently. +- The sector size needs to be set such that a sector can fit the maximum data size that will be + stored. + Increasing the sector size will slow down the garbage collection operation and make it occur + less frequently. + Decreasing its size, on the opposite, will make the garbage collection operation faster but also + occur more frequently. - For some subsystems like :ref:`Settings `, all path-value pairs are split into two ZMS entries (ATEs). - The header needed by the two entries should be accounted when computing the needed storage space. -- Using small data to store in the ZMS entries can increase the performance, as this data is - written within the entry header. + The headers needed by the two entries should be accounted for when computing the needed storage + space. +- Storing small data (<= 8 bytes) in ZMS entries can increase the performance, as this data is + written within the entry. For example, for the :ref:`Settings ` subsystem, choosing a path name that is less than or equal to 8 bytes can make reads and writes faster. -Dimensioning cache -================== +Cache size +========== -- When using ZMS API directly, the recommended cache size should be, at least, equal to - the number of different entries that will be written in the storage. +- When using the ZMS API directly, the recommendation for the cache size is to make it at least + equal to the number of different entries that will be written in the storage. - Each additional cache entry will add 8 bytes to your RAM usage. Cache size should be carefully chosen. - If you use ZMS through :ref:`Settings `, you have to take into account that each Settings entry is - divided into two ZMS entries. The recommended cache size should be, at least, twice the number - of Settings entries. - -Sample -****** - -A sample of how ZMS can be used is supplied in :zephyr:code-sample:`zms`. + divided into two ZMS entries. The recommendation for the cache size is to make it at least + twice the number of Settings entries. API Reference ************* -The ZMS subsystem APIs are provided by ``zms.h``: +The ZMS API is provided by ``zms.h``: .. doxygengroup:: zms_data_structures diff --git a/doc/zephyr.doxyfile.in b/doc/zephyr.doxyfile.in index 036bef82c66..dfd9251bb7b 100644 --- a/doc/zephyr.doxyfile.in +++ b/doc/zephyr.doxyfile.in @@ -980,6 +980,7 @@ INPUT = @ZEPHYR_BASE@/doc/_doxygen/mainpage.md \ @ZEPHYR_BASE@/subsys/testsuite/include/ \ @ZEPHYR_BASE@/subsys/testsuite/ztest/include/ \ @ZEPHYR_BASE@/subsys/secure_storage/include/ \ + @ZEPHYR_BASE@/subsys/fs/zms/zms_priv.h \ # This tag can be used to specify the character encoding of the source files # that Doxygen parses. Internally Doxygen uses the UTF-8 encoding. Doxygen uses diff --git a/include/zephyr/fs/zms.h b/include/zephyr/fs/zms.h index 0f0fbb82cc9..9a514d65818 100644 --- a/include/zephyr/fs/zms.h +++ b/include/zephyr/fs/zms.h @@ -80,8 +80,13 @@ struct zms_fs { * @brief Mount a ZMS file system onto the device specified in `fs`. * * @param fs Pointer to the file system. - * @retval 0 Success - * @retval -ERRNO Negative errno code on error + * + * @retval 0 on success. + * @retval -ENOTSUP if the detected file system is not ZMS. + * @retval -EPROTONOSUPPORT if the ZMS version is not supported. + * @retval -EINVAL if any of the flash parameters or the sector layout is invalid. + * @retval -ENXIO if there is a device error. + * @retval -EIO if there is a memory read/write error. */ int zms_mount(struct zms_fs *fs); @@ -89,8 +94,11 @@ int zms_mount(struct zms_fs *fs); * @brief Clear the ZMS file system from device. * * @param fs Pointer to the file system. - * @retval 0 Success - * @retval -ERRNO Negative errno code on error + * + * @retval 0 on success. + * @retval -EACCES if `fs` is not mounted. + * @retval -ENXIO if there is a device error. + * @retval -EIO if there is a memory read/write error. */ int zms_clear(struct zms_fs *fs); @@ -102,14 +110,20 @@ int zms_clear(struct zms_fs *fs); * entry and an entry with data of length 0. * * @param fs Pointer to the file system. - * @param id ID of the entry to be written - * @param data Pointer to the data to be written - * @param len Number of bytes to be written (maximum 64 KiB) + * @param id ID of the entry to be written. + * @param data Pointer to the data to be written. + * @param len Number of bytes to be written (maximum 64 KiB). * * @return Number of bytes written. On success, it will be equal to the number of bytes requested * to be written or 0. * When a rewrite of the same data already stored is attempted, nothing is written to flash, * thus 0 is returned. On error, returns negative value of error codes defined in `errno.h`. + * @retval Number of bytes written (`len` or 0) on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -ENXIO if there is a device error. + * @retval -EIO if there is a memory read/write error. + * @retval -EINVAL if `len` is invalid. + * @retval -ENOSPC if no space is left on the device. */ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len); @@ -117,9 +131,12 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len); * @brief Delete an entry from the file system * * @param fs Pointer to the file system. - * @param id ID of the entry to be deleted - * @retval 0 Success - * @retval -ERRNO Negative errno code on error + * @param id ID of the entry to be deleted. + * + * @retval 0 on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -ENXIO if there is a device error. + * @retval -EIO if there is a memory read/write error. */ int zms_delete(struct zms_fs *fs, uint32_t id); @@ -127,13 +144,17 @@ int zms_delete(struct zms_fs *fs, uint32_t id); * @brief Read an entry from the file system. * * @param fs Pointer to the file system. - * @param id ID of the entry to be read - * @param data Pointer to data buffer - * @param len Number of bytes to read at most + * @param id ID of the entry to be read. + * @param data Pointer to data buffer. + * @param len Number of bytes to read at most. * * @return Number of bytes read. On success, it will be equal to the number of bytes requested * to be read or less than that if the stored data has a smaller size than the requested one. * On error, returns negative value of error codes defined in `errno.h`. + * @retval Number of bytes read (> 0) on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -EIO if there is a memory read/write error. + * @retval -ENOENT if there is no entry with the given `id`. */ ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len); @@ -141,26 +162,34 @@ ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len); * @brief Read a history entry from the file system. * * @param fs Pointer to the file system. - * @param id ID of the entry to be read - * @param data Pointer to data buffer - * @param len Number of bytes to be read + * @param id ID of the entry to be read. + * @param data Pointer to data buffer. + * @param len Number of bytes to be read. * @param cnt History counter: 0: latest entry, 1: one before latest ... * * @return Number of bytes read. On success, it will be equal to the number of bytes requested * to be read. When the return value is larger than the number of bytes requested to read this * indicates not all bytes were read, and more data is available. On error, returns negative * value of error codes defined in `errno.h`. + * @retval Number of bytes read (> 0) on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -EIO if there is a memory read/write error. + * @retval -ENOENT if there is no entry with the given `id` and history counter. */ ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt); /** - * @brief Gets the length of the data that is stored in an entry with a given ID + * @brief Gets the length of the data that is stored in an entry with a given `id` * * @param fs Pointer to the file system. * @param id ID of the entry whose data length to retrieve. * * @return Data length contained in the ATE. On success, it will be equal to the number of bytes * in the ATE. On error, returns negative value of error codes defined in `errno.h`. + * @retval Length of the entry with the given `id` (> 0) on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -EIO if there is a memory read/write error. + * @retval -ENOENT if there is no entry with the given id and history counter. */ ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id); @@ -173,6 +202,9 @@ ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id); * still be written to the file system. * Calculating the free space is a time-consuming operation, especially on SPI flash. * On error, returns negative value of error codes defined in `errno.h`. + * @retval Number of free bytes (>= 0) on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -EIO if there is a memory read/write error. */ ssize_t zms_calc_free_space(struct zms_fs *fs); @@ -181,7 +213,8 @@ ssize_t zms_calc_free_space(struct zms_fs *fs); * * @param fs Pointer to the file system. * - * @return Number of free bytes. + * @retval >=0 Number of free bytes in the currently active sector + * @retval -EACCES if ZMS is still not initialized. */ size_t zms_active_sector_free_space(struct zms_fs *fs); @@ -196,7 +229,9 @@ size_t zms_active_sector_free_space(struct zms_fs *fs); * * @param fs Pointer to the file system. * - * @return 0 on success. On error, returns negative value of error codes defined in `errno.h`. + * @retval 0 on success. + * @retval -EACCES if ZMS is still not initialized. + * @retval -EIO if there is a memory read/write error. */ int zms_sector_use_next(struct zms_fs *fs); diff --git a/subsys/fs/zms/CMakeLists.txt b/subsys/fs/zms/CMakeLists.txt index b6db8a3f57f..91e4651c3f6 100644 --- a/subsys/fs/zms/CMakeLists.txt +++ b/subsys/fs/zms/CMakeLists.txt @@ -1,3 +1,3 @@ -#SPDX-License-Identifier: Apache-2.0 +# SPDX-License-Identifier: Apache-2.0 zephyr_sources(zms.c) diff --git a/subsys/fs/zms/Kconfig b/subsys/fs/zms/Kconfig index 781da81bf0a..9be94d17354 100644 --- a/subsys/fs/zms/Kconfig +++ b/subsys/fs/zms/Kconfig @@ -1,14 +1,16 @@ -#Copyright (c) 2024 BayLibre SAS +# Copyright (c) 2024 BayLibre SAS -#SPDX-License-Identifier: Apache-2.0 +# SPDX-License-Identifier: Apache-2.0 -#Zephyr Memory Storage ZMS +# Zephyr Memory Storage ZMS config ZMS bool "Zephyr Memory Storage" select CRC help - Enable support of Zephyr Memory Storage. + Enable Zephyr Memory Storage, which is a key-value storage system designed to work with + all types of non-volatile storage technologies. + It supports classical on-chip NOR flash as well as new technologies like RRAM and MRAM. if ZMS @@ -20,19 +22,16 @@ config ZMS_LOOKUP_CACHE table entry (ATE) for all ZMS IDs that fall into that cache position. config ZMS_LOOKUP_CACHE_SIZE - int "ZMS Storage lookup cache size" + int "ZMS lookup cache size" default 128 range 1 65536 depends on ZMS_LOOKUP_CACHE help - Number of entries in ZMS lookup cache. - It is recommended that it should be a power of 2. - Every additional entry in cache will add 8 bytes in RAM + Number of entries in the ZMS lookup cache. + Every additional entry in cache will use 8 bytes of RAM. config ZMS_DATA_CRC - bool "ZMS DATA CRC" - help - Enables DATA CRC + bool "ZMS data CRC" config ZMS_CUSTOMIZE_BLOCK_SIZE bool "Customize the size of the buffer used internally for reads and writes" @@ -40,8 +39,8 @@ config ZMS_CUSTOMIZE_BLOCK_SIZE ZMS uses an internal buffer to read/write and compare stored data. Increasing the size of this buffer should be done carefully in order to not overflow the stack. - Increasing this buffer means as well that ZMS could work with storage devices - that have larger write-block-size which decreases ZMS performance + Increasing it makes ZMS able to work with storage devices + that have a larger `write-block-size` (which decreases the performance of ZMS). config ZMS_CUSTOM_BLOCK_SIZE int "ZMS internal buffer size" diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 0e5ca1e5f3b..dd859e20479 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -1101,7 +1101,7 @@ static int zms_init(struct zms_fs *fs) /* Let's check that we support this ZMS version */ if (ZMS_GET_VERSION(empty_ate.metadata) != ZMS_DEFAULT_VERSION) { LOG_ERR("ZMS Version is not supported"); - rc = -ENOEXEC; + rc = -EPROTONOSUPPORT; goto end; } } @@ -1125,7 +1125,7 @@ static int zms_init(struct zms_fs *fs) } /* all sectors are closed, and zms magic number not found. This is not a zms fs */ if ((closed_sectors == fs->sector_count) && !zms_magic_exist) { - rc = -EDEADLK; + rc = -ENOTSUP; goto end; } /* TODO: add a recovery mechanism here if the ZMS magic number exist but all @@ -1157,7 +1157,7 @@ static int zms_init(struct zms_fs *fs) /* Let's check the version */ if (ZMS_GET_VERSION(empty_ate.metadata) != ZMS_DEFAULT_VERSION) { LOG_ERR("ZMS Version is not supported"); - rc = -ENOEXEC; + rc = -EPROTONOSUPPORT; goto end; } } diff --git a/subsys/fs/zms/zms_priv.h b/subsys/fs/zms/zms_priv.h index 428ff6babca..e2cbf5f08bb 100644 --- a/subsys/fs/zms/zms_priv.h +++ b/subsys/fs/zms/zms_priv.h @@ -8,15 +8,11 @@ #ifndef __ZMS_PRIV_H_ #define __ZMS_PRIV_H_ -#ifdef __cplusplus -extern "C" { -#endif - /* - * MASKS AND SHIFT FOR ADDRESSES - * an address in zms is an uint64_t where: - * high 4 bytes represent the sector number - * low 4 bytes represent the offset in a sector + * MASKS AND SHIFT FOR ADDRESSES. + * An address in zms is an uint64_t where: + * - high 4 bytes represent the sector number + * - low 4 bytes represent the offset in a sector */ #define ADDR_SECT_MASK GENMASK64(63, 32) #define ADDR_SECT_SHIFT 32 @@ -44,34 +40,40 @@ extern "C" { #define ZMS_INVALID_SECTOR_NUM -1 #define ZMS_DATA_IN_ATE_SIZE 8 +/** + * @ingroup zms_data_structures + * ZMS Allocation Table Entry (ATE) structure + */ struct zms_ate { - uint8_t crc8; /* crc8 check of the entry */ - uint8_t cycle_cnt; /* cycle counter for non erasable devices */ - uint16_t len; /* data len within sector */ - uint32_t id; /* data id */ + /** crc8 check of the entry */ + uint8_t crc8; + /** cycle counter for non erasable devices */ + uint8_t cycle_cnt; + /** data len within sector */ + uint16_t len; + /** data id */ + uint32_t id; union { - uint8_t data[8]; /* used to store small size data */ + /** data field used to store small sized data */ + uint8_t data[8]; struct { - uint32_t offset; /* data offset within sector */ + /** data offset within sector */ + uint32_t offset; union { - uint32_t data_crc; /* - * crc for data: The data CRC is checked only - * when the whole data of the element is read. - * The data CRC is not checked for a partial - * read, as it is computed for the complete - * set of data. - */ - uint32_t metadata; /* - * Used to store metadata information - * such as storage version. - */ + /** + * crc for data: The data CRC is checked only when the whole data + * of the element is read. + * The data CRC is not checked for a partial read, as it is computed + * for the complete set of data. + */ + uint32_t data_crc; + /** + * Used to store metadata information such as storage version. + */ + uint32_t metadata; }; }; }; } __packed; -#ifdef __cplusplus -} -#endif - #endif /* __ZMS_PRIV_H_ */ From 04dc9c30136f9c6beee64fe7f853c89eb39debef Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Tue, 25 Feb 2025 11:57:59 +0100 Subject: [PATCH 06/27] [nrf fromtree] samples/subsys/fs/zms: run on native_sim instead of native_posix native_posix is deprecated and will be removed in 4.2 Run this sample in native_sim instead. Signed-off-by: Alberto Escolar Piedras (cherry picked from commit 4a91a55ae24f4afbd4893661637f263560636c0d) --- samples/subsys/fs/zms/sample.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/subsys/fs/zms/sample.yaml b/samples/subsys/fs/zms/sample.yaml index 802dabcf0f1..96d353cb0d3 100644 --- a/samples/subsys/fs/zms/sample.yaml +++ b/samples/subsys/fs/zms/sample.yaml @@ -7,4 +7,4 @@ tests: depends_on: zms platform_allow: - qemu_x86 - - native_posix + - native_sim From 347c8ef85a3b61ddda100c8b7e5ab882b8f6aea2 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Tue, 25 Feb 2025 12:29:16 +0100 Subject: [PATCH 07/27] [nrf fromtree] samples/subsys/fs/zms: Allow it to run and add check This sample could not run on any target as it depedend on zms which is set by no board. But it seems to run just fine in both allowed platforms, so let's add a trivial twister check (so it actually passes instead of waiting for ever), and remove that `depends on`. Signed-off-by: Alberto Escolar Piedras (cherry picked from commit cc2d31f4da1446c54baef7688fb41761658e44b9) --- samples/subsys/fs/zms/sample.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/samples/subsys/fs/zms/sample.yaml b/samples/subsys/fs/zms/sample.yaml index 96d353cb0d3..c28770ec78a 100644 --- a/samples/subsys/fs/zms/sample.yaml +++ b/samples/subsys/fs/zms/sample.yaml @@ -4,7 +4,11 @@ sample: tests: sample.zms.basic: tags: zms - depends_on: zms platform_allow: - qemu_x86 - native_sim + harness: console + harness_config: + type: one_line + regex: + - "Sample code finished Successfully" From bad257bb19ac255e383bad428f6b8fd0e270b1e6 Mon Sep 17 00:00:00 2001 From: Andrej Butok Date: Tue, 25 Feb 2025 15:22:43 +0100 Subject: [PATCH 08/27] [nrf fromtree] sample: zms: add ZMS sample Kconfig - adds ability to change the ZMS sample MAX_ITERATIONS and DELETE_ITERATION parameters via Kconfig without manually modifying the source code. - adds ability to reduce flash memory wear on real devices by reducing the number of write iterations. Signed-off-by: Andrej Butok (cherry picked from commit c887d5e235478cdf59381759a081c15f68a12869) --- samples/subsys/fs/zms/Kconfig | 16 ++++++++++++++++ samples/subsys/fs/zms/README.rst | 2 +- samples/subsys/fs/zms/src/main.c | 11 ++++------- 3 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 samples/subsys/fs/zms/Kconfig diff --git a/samples/subsys/fs/zms/Kconfig b/samples/subsys/fs/zms/Kconfig new file mode 100644 index 00000000000..0184fbdc4ef --- /dev/null +++ b/samples/subsys/fs/zms/Kconfig @@ -0,0 +1,16 @@ +# Copyright 2025 NXP +# SPDX-License-Identifier: Apache-2.0 + +mainmenu "ZMS sample configuration" + +config MAX_ITERATIONS + int "The number of iterations that the sample writes the all set of data." + default 300 + range 1 300 + +config DELETE_ITERATION + int "The number of iterations after the sample delete all set of data and verify that it has been deleted." + default 10 + range 1 MAX_ITERATIONS + +source "Kconfig.zephyr" diff --git a/samples/subsys/fs/zms/README.rst b/samples/subsys/fs/zms/README.rst index f05d1fa0838..98deead06d8 100644 --- a/samples/subsys/fs/zms/README.rst +++ b/samples/subsys/fs/zms/README.rst @@ -20,7 +20,7 @@ Overview A loop is executed where we mount the storage system, and then write all set of data. - Each DELETE_ITERATION period, we delete all set of data and verify that it has been deleted. + Each CONFIG_DELETE_ITERATION period, we delete all set of data and verify that it has been deleted. We generate as well incremented ID/value pairs, we store them until storage is full, then we delete them and verify that storage is empty. diff --git a/samples/subsys/fs/zms/src/main.c b/samples/subsys/fs/zms/src/main.c index 959d5ac5f3e..3c49542ace7 100644 --- a/samples/subsys/fs/zms/src/main.c +++ b/samples/subsys/fs/zms/src/main.c @@ -26,9 +26,6 @@ static struct zms_fs fs; #define CNT_ID 2 #define LONG_DATA_ID 3 -#define MAX_ITERATIONS 300 -#define DELETE_ITERATION 10 - static int delete_and_verify_items(struct zms_fs *fs, uint32_t id) { int rc = 0; @@ -112,7 +109,7 @@ int main(void) fs.sector_size = info.size; fs.sector_count = 3U; - for (i = 0; i < MAX_ITERATIONS; i++) { + for (i = 0; i < CONFIG_MAX_ITERATIONS; i++) { rc = zms_mount(&fs); if (rc) { printk("Storage Init failed, rc=%d\n", rc); @@ -195,8 +192,8 @@ int main(void) break; } - /* Each DELETE_ITERATION delete all basic items */ - if (!(i % DELETE_ITERATION) && (i)) { + /* Each CONFIG_DELETE_ITERATION delete all basic items */ + if (!(i % CONFIG_DELETE_ITERATION) && (i)) { rc = delete_basic_items(&fs); if (rc) { break; @@ -204,7 +201,7 @@ int main(void) } } - if (i != MAX_ITERATIONS) { + if (i != CONFIG_MAX_ITERATIONS) { printk("Error: Something went wrong at iteration %u rc=%d\n", i, rc); return 0; } From 4134baf8954de508e2bf58df2ed51850aa6f9482 Mon Sep 17 00:00:00 2001 From: Andrej Butok Date: Mon, 3 Mar 2025 13:54:41 +0100 Subject: [PATCH 09/27] [nrf fromtree] samples: zms: fix the loop_cnt iteration 0 check - fix unwanted "Error: Something went wrong at iteration 0". - the loop_cnt check has no sense for iteration 0, comparing with wrong -1. Signed-off-by: Andrej Butok (cherry picked from commit 2766c788230f54bd30d3b4f1d95af4e87f0c3de8) --- samples/subsys/fs/zms/src/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samples/subsys/fs/zms/src/main.c b/samples/subsys/fs/zms/src/main.c index 3c49542ace7..a9615823608 100644 --- a/samples/subsys/fs/zms/src/main.c +++ b/samples/subsys/fs/zms/src/main.c @@ -161,7 +161,8 @@ int main(void) rc = zms_read(&fs, CNT_ID, &i_cnt, sizeof(i_cnt)); if (rc > 0) { /* item was found, show it */ printk("Id: %d, loop_cnt: %u\n", CNT_ID, i_cnt); - if (i_cnt != (i - 1)) { + if ((i > 0) && (i_cnt != (i - 1))) { + printk("Error loop_cnt %u must be %d\n", i_cnt, i - 1); break; } } From 3b68dd89e10640df5167a7aa4ba5270c99d90c39 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Sat, 22 Feb 2025 02:11:54 +0100 Subject: [PATCH 10/27] [nrf fromtree] settings: zms: fix some bugs related to the name's ID To avoid collisions between IDs used by settings and IDs used directly by subsystems using ZMS API, the MSB is always set to 1 for Setting's name ID written to ZMS backend Add as well a recovery path if the hash linked list is broken. Signed-off-by: Riadh Ghaddab (cherry picked from commit 333faddd43ff4958bbcb441fd3aa4e7e2f7cf371) --- .../settings/include/settings/settings_zms.h | 3 +- subsys/settings/src/settings_zms.c | 57 ++++++++++++------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h index 5c8db6c8673..9b9939815c2 100644 --- a/subsys/settings/include/settings/settings_zms.h +++ b/subsys/settings/include/settings/settings_zms.h @@ -60,10 +60,9 @@ extern "C" { #define ZMS_COLLISIONS_MASK GENMASK(CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS, 1) #define ZMS_HASH_TOTAL_MASK GENMASK(29, 1) #define ZMS_MAX_COLLISIONS (BIT(CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS) - 1) -#define ZMS_HEADER_HASH 0x80000000 /* some useful macros */ -#define ZMS_NAME_HASH_ID(x) (x & ZMS_HASH_TOTAL_MASK) +#define ZMS_NAME_ID_FROM_LL_NODE(x) (x & ~BIT(0)) #define ZMS_UPDATE_COLLISION_NUM(x, y) \ ((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK)) #define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index abb8989c203..9d588bc588d 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -30,6 +30,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); static void *settings_zms_storage_get(struct settings_store *cs); +static int settings_zms_get_last_hash_ids(struct settings_zms *cf); static struct settings_store_itf settings_zms_itf = {.csi_load = settings_zms_load, .csi_save = settings_zms_save, @@ -169,10 +170,11 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo * entries one for the setting's name and one with the * setting's value. */ - rc1 = zms_read(&cf->cf_zms, ZMS_NAME_HASH_ID(ll_hash_id), &name, sizeof(name) - 1); + rc1 = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id), &name, + sizeof(name) - 1); /* get the length of data and verify that it exists */ - rc2 = zms_get_data_length(&cf->cf_zms, - ZMS_NAME_HASH_ID(ll_hash_id) + ZMS_DATA_ID_OFFSET); + rc2 = zms_get_data_length(&cf->cf_zms, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id) + + ZMS_DATA_ID_OFFSET); if ((rc1 <= 0) || (rc2 <= 0)) { /* Settings item is not stored correctly in the ZMS. @@ -180,7 +182,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo * or deleted. Clean dirty entries to make space for * future settings item. */ - ret = settings_zms_delete(cf, ZMS_NAME_HASH_ID(ll_hash_id)); + ret = settings_zms_delete(cf, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id)); if (ret < 0) { return ret; } @@ -190,7 +192,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo /* Found a name, this might not include a trailing \0 */ name[rc1] = '\0'; read_fn_arg.fs = &cf->cf_zms; - read_fn_arg.id = ZMS_NAME_HASH_ID(ll_hash_id) + ZMS_DATA_ID_OFFSET; + read_fn_arg.id = ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id) + ZMS_DATA_ID_OFFSET; ret = settings_call_set_handler(name, rc2, settings_zms_read_fn, &read_fn_arg, (void *)arg); @@ -232,8 +234,10 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const delete = ((value == NULL) || (val_len == 0)); name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + /* MSB is always 1 */ + name_hash |= BIT(31); - /* Let's find out if there is no hash collisions in the storage */ + /* Let's find out if there are hash collisions in the storage */ write_name = true; hash_collision = true; @@ -311,6 +315,16 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const } /* write linked list structure element */ settings_element.next_hash = 0; + /* Verify first that the linked list last element is not broken. + * Settings subsystem uses ID that starts from ZMS_LL_HEAD_HASH_ID. + */ + if (cf->last_hash_id < ZMS_LL_HEAD_HASH_ID) { + LOG_WRN("Linked list for hashes is broken, Trying to recover"); + rc = settings_zms_get_last_hash_ids(cf); + if (rc < 0) { + return rc; + } + } settings_element.previous_hash = cf->last_hash_id; rc = zms_write(&cf->cf_zms, name_hash | 1, &settings_element, sizeof(struct settings_hash_linked_list)); @@ -342,9 +356,22 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) do { rc = zms_read(&cf->cf_zms, ll_last_hash_id, &settings_element, sizeof(settings_element)); - if (rc) { + if (rc == -ENOENT) { + /* header doesn't exist or linked list broken, reinitialize the header */ + const struct settings_hash_linked_list settings_element = { + .previous_hash = 0, .next_hash = 0}; + rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; + cf->second_to_last_hash_id = 0; + return 0; + } else if (rc < 0) { return rc; } + /* increment hash collision number if necessary */ if (ZMS_COLLISION_NUM(ll_last_hash_id) > cf->hash_collision_num) { cf->hash_collision_num = ZMS_COLLISION_NUM(ll_last_hash_id); @@ -375,23 +402,9 @@ static int settings_zms_backend_init(struct settings_zms *cf) cf->hash_collision_num = 0; rc = settings_zms_get_last_hash_ids(cf); - if (rc == -ENOENT) { - /* header doesn't exist or linked list broken, reinitialize the header */ - const struct settings_hash_linked_list settings_element = {.previous_hash = 0, - .next_hash = 0}; - rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; - } - cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; - cf->second_to_last_hash_id = 0; - } else if (rc < 0) { - return rc; - } LOG_DBG("ZMS backend initialized"); - return 0; + return rc; } int settings_backend_init(void) From fbacc7672b4b5969c07e194313132d2fdc00ad44 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Sat, 22 Feb 2025 09:20:37 +0100 Subject: [PATCH 11/27] [nrf fromtree] doc: settings: add reference to ZMS backend Add reference to the new settings backend ZMS and add more details about how it works. Signed-off-by: Riadh Ghaddab (cherry picked from commit f3630157e03fa322787f264b1047f91da5c2af8c) --- doc/services/settings/index.rst | 45 ++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/doc/services/settings/index.rst b/doc/services/settings/index.rst index 66c99d050a3..4f8831e3a11 100644 --- a/doc/services/settings/index.rst +++ b/doc/services/settings/index.rst @@ -5,9 +5,9 @@ Settings The settings subsystem gives modules a way to store persistent per-device configuration and runtime state. A variety of storage implementations are -provided behind a common API using FCB, NVS, or a file system. These different -implementations give the application developer flexibility to select an -appropriate storage medium, and even change it later as needs change. This +provided behind a common API using FCB, NVS, ZMS or a file system. These +different implementations give the application developer flexibility to select +an appropriate storage medium, and even change it later as needs change. This subsystem is used by various Zephyr components and can be used simultaneously by user applications. @@ -23,8 +23,8 @@ For an example of the settings subsystem refer to :zephyr:code-sample:`settings` .. note:: - As of Zephyr release 2.1 the recommended backend for non-filesystem - storage is :ref:`NVS `. + As of Zephyr release 4.1 the recommended backends for non-filesystem + storage are :ref:`NVS ` and :ref:`ZMS `. Handlers ******** @@ -39,7 +39,7 @@ for static handlers. :c:func:`settings_runtime_get()` from the runtime backend. **h_set** - This gets called when the value is loaded from persisted storage with + This gets called when the value is loaded from persistent storage with :c:func:`settings_load()`, or when using :c:func:`settings_runtime_set()` from the runtime backend. @@ -93,10 +93,12 @@ backend. Zephyr Storage Backends *********************** -Zephyr has three storage backends: a Flash Circular Buffer -(:kconfig:option:`CONFIG_SETTINGS_FCB`), a file in the filesystem -(:kconfig:option:`CONFIG_SETTINGS_FILE`), or non-volatile storage -(:kconfig:option:`CONFIG_SETTINGS_NVS`). +Zephyr offers the following storage backends: + +* Flash Circular Buffer (:kconfig:option:`CONFIG_SETTINGS_FCB`). +* A file in the filesystem (:kconfig:option:`CONFIG_SETTINGS_FILE`). +* Non-Volatile Storage (:kconfig:option:`CONFIG_SETTINGS_NVS`). +* Zephyr Memory Storage (:kconfig:option:`CONFIG_SETTINGS_ZMS`). You can declare multiple sources for settings; settings from all of these are restored when :c:func:`settings_load()` is called. @@ -109,14 +111,27 @@ using :c:func:`settings_fcb_dst()`. As a side-effect, :c:func:`settings_fcb_src initializes the FCB area, so it must be called before calling :c:func:`settings_fcb_dst()`. File read target is registered using :c:func:`settings_file_src()`, and write target by using :c:func:`settings_file_dst()`. + Non-volatile storage read target is registered using :c:func:`settings_nvs_src()`, and write target by using :c:func:`settings_nvs_dst()`. +Zephyr Memory Storage (ZMS) read target is registered using :c:func:`settings_zms_src()`, +and write target is registered using :c:func:`settings_zms_dst()`. + +ZMS backend has the particularity of using hash functions to hash the settings +key before storing it to the persistent storage. This implementation implies +that some collisions between key's hashes could occur if a big number of +different keys are stored. This number depends on the selected hash function. + +ZMS backend can handle :math:`2^n` maximum collisions where n is defined by +(:kconfig:option:`SETTINGS_ZMS_MAX_COLLISIONS_BITS`). + + Storage Location **************** -The FCB and non-volatile storage (NVS) backends both look for a fixed +The FCB, non-volatile storage (NVS) and ZMS backends look for a fixed partition with label "storage" by default. A different partition can be selected by setting the ``zephyr,settings-partition`` property of the chosen node in the devicetree. @@ -124,8 +139,8 @@ chosen node in the devicetree. The file path used by the file backend to store settings is selected via the option :kconfig:option:`CONFIG_SETTINGS_FILE_PATH`. -Loading data from persisted storage -*********************************** +Loading data from persistent storage +************************************ A call to :c:func:`settings_load()` uses an ``h_set`` implementation to load settings data from storage to volatile memory. @@ -146,7 +161,7 @@ A call to :c:func:`settings_save_one()` uses a backend implementation to store settings data to the storage medium. A call to :c:func:`settings_save()` uses an ``h_export`` implementation to store different data in one operation using :c:func:`settings_save_one()`. -A key need to be covered by a ``h_export`` only if it is supposed to be stored +A key needs to be covered by a ``h_export`` only if it is supposed to be stored by :c:func:`settings_save()` call. For both FCB and file back-end only storage requests with data which @@ -227,7 +242,7 @@ Example: Persist Runtime State This is a simple example showing how to persist runtime state. In this example, only ``h_set`` is defined, which is used when restoring value from -persisted storage. +persistent storage. In this example, the ``main`` function increments ``foo_val``, and then persists the latest number. When the system restarts, the application calls From b975c2cfaf5d5c907a3d3cefda093b9a43d47ac8 Mon Sep 17 00:00:00 2001 From: Andrej Butok Date: Wed, 26 Feb 2025 14:21:24 +0100 Subject: [PATCH 12/27] [nrf fromtree] tests: zms: allow to build & run on real platforms - Allows to build and run ZMS tests on real platforms. - Enables ZMS tests designed for a flash-simulator only when built for qemu_x86 or native_sim targets. Signed-off-by: Andrej Butok (cherry picked from commit b826d4663226909d49ea85bab707ee31dcdd86f8) --- tests/subsys/fs/zms/Kconfig | 16 ++++++++++++++++ tests/subsys/fs/zms/src/main.c | 21 ++++++++++----------- 2 files changed, 26 insertions(+), 11 deletions(-) create mode 100644 tests/subsys/fs/zms/Kconfig diff --git a/tests/subsys/fs/zms/Kconfig b/tests/subsys/fs/zms/Kconfig new file mode 100644 index 00000000000..9c8f49ff59b --- /dev/null +++ b/tests/subsys/fs/zms/Kconfig @@ -0,0 +1,16 @@ +# Copyright 2025 NXP +# SPDX-License-Identifier: Apache-2.0 + +mainmenu "ZMS test configuration" + +config TEST_ZMS_SIMULATOR + bool "Enable ZMS tests designed to be run using a flash-simulator" + default y if BOARD_QEMU_X86 || ARCH_POSIX + help + If y, enables ZMS tests designed to be run using a flash-simulator, + which provide functionality for flash property customization + and emulating errors in flash operation in parallel to + the regular flash API. + The tests must be run only on qemu_x86 or native_sim target. + +source "Kconfig.zephyr" diff --git a/tests/subsys/fs/zms/src/main.c b/tests/subsys/fs/zms/src/main.c index 500ad758d68..6d170ce1f9f 100644 --- a/tests/subsys/fs/zms/src/main.c +++ b/tests/subsys/fs/zms/src/main.c @@ -4,17 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -/* - * This test is designed to be run using flash-simulator which provide - * functionality for flash property customization and emulating errors in - * flash operation in parallel to regular flash API. - * Test should be run on qemu_x86 or native_sim target. - */ - -#if !defined(CONFIG_BOARD_QEMU_X86) && !defined(CONFIG_ARCH_POSIX) -#error "Run only on qemu_x86 or a posix architecture based target (for ex. native_sim)" -#endif - #include #include #include @@ -37,8 +26,10 @@ static const struct device *const flash_dev = TEST_ZMS_AREA_DEV; struct zms_fixture { struct zms_fs fs; +#ifdef CONFIG_TEST_ZMS_SIMULATOR struct stats_hdr *sim_stats; struct stats_hdr *sim_thresholds; +#endif /* CONFIG_TEST_ZMS_SIMULATOR */ }; static void *setup(void) @@ -66,22 +57,26 @@ static void *setup(void) static void before(void *data) { +#ifdef CONFIG_TEST_ZMS_SIMULATOR struct zms_fixture *fixture = (struct zms_fixture *)data; fixture->sim_stats = stats_group_find("flash_sim_stats"); fixture->sim_thresholds = stats_group_find("flash_sim_thresholds"); +#endif /* CONFIG_TEST_ZMS_SIMULATOR */ } static void after(void *data) { struct zms_fixture *fixture = (struct zms_fixture *)data; +#ifdef CONFIG_TEST_ZMS_SIMULATOR if (fixture->sim_stats) { stats_reset(fixture->sim_stats); } if (fixture->sim_thresholds) { stats_reset(fixture->sim_thresholds); } +#endif /* CONFIG_TEST_ZMS_SIMULATOR */ /* Clear ZMS */ if (fixture->fs.ready) { @@ -137,6 +132,7 @@ ZTEST_F(zms, test_zms_write) execute_long_pattern_write(TEST_DATA_ID, &fixture->fs); } +#ifdef CONFIG_TEST_ZMS_SIMULATOR static int flash_sim_write_calls_find(struct stats_hdr *hdr, void *arg, const char *name, uint16_t off) { @@ -453,6 +449,7 @@ ZTEST_F(zms, test_zms_corrupted_sector_close_operation) /* Ensure that the ZMS is able to store new content. */ execute_long_pattern_write(max_id, &fixture->fs); } +#endif /* CONFIG_TEST_ZMS_SIMULATOR */ /** * @brief Test case when storage become full, so only deletion is possible. @@ -562,6 +559,7 @@ ZTEST_F(zms, test_delete) #endif } +#ifdef CONFIG_TEST_ZMS_SIMULATOR /* * Test that garbage-collection can recover all ate's even when the last ate, * ie close_ate, is corrupt. In this test the close_ate is set to point to the @@ -639,6 +637,7 @@ ZTEST_F(zms, test_zms_gc_corrupt_close_ate) zassert_true(len == sizeof(data), "zms_read should have read %d bytes", sizeof(data)); zassert_true(data == 0xaa55aa55, "unexpected value %d", data); } +#endif /* CONFIG_TEST_ZMS_SIMULATOR */ /* * Test that garbage-collection correctly handles corrupt ate's. From 1bb29ebad695f7836620733ab200d8d47828eea2 Mon Sep 17 00:00:00 2001 From: Andrej Butok Date: Tue, 4 Mar 2025 13:55:55 +0100 Subject: [PATCH 13/27] [nrf fromtree] samples: zms: fix 9999 messages dropped - Sets CONFIG_LOG_BLOCK_IN_THREAD for the zms sample. - Fixes "--- 9999 messages dropped ---". Signed-off-by: Andrej Butok (cherry picked from commit ac7790e452ddae08fc4d9c72d7f74ba50c7e1dec) --- samples/subsys/fs/zms/prj.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/samples/subsys/fs/zms/prj.conf b/samples/subsys/fs/zms/prj.conf index 343c5021899..195027ea287 100644 --- a/samples/subsys/fs/zms/prj.conf +++ b/samples/subsys/fs/zms/prj.conf @@ -3,3 +3,4 @@ CONFIG_FLASH_MAP=y CONFIG_ZMS=y CONFIG_LOG=y +CONFIG_LOG_BLOCK_IN_THREAD=y From 66f838ab7c896126eecec2bab9f49e3d1995868e Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Thu, 27 Feb 2025 16:22:29 +0100 Subject: [PATCH 14/27] [nrf fromlist] settings: zms: add option to disable updating linked list When deleting a settings entry the linked list is updated by removing the deleted node. This operation is time consuming and add some delays. For applications that use almost the same set of Setting entries, add an option to make this operation faster at a cost of having some empty nodes in the linked list. These empty nodes can be used later when the deleted entry is written again. Each empty node occupies 16B of storage. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 02ab54e077e090e4c9364e4d4954a8c3508e513e) --- subsys/settings/Kconfig | 11 +++++++++ subsys/settings/src/settings_zms.c | 39 ++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/subsys/settings/Kconfig b/subsys/settings/Kconfig index a59b618759e..24564841ca0 100644 --- a/subsys/settings/Kconfig +++ b/subsys/settings/Kconfig @@ -171,6 +171,17 @@ config SETTINGS_ZMS_MAX_COLLISIONS_BITS The maximum number of hash collisions needs to be well sized depending on the data that is going to be stored in ZMS and its hash values +config SETTINGS_ZMS_NO_LL_DELETE + bool "Disable deletion of Linked list hashes" + help + For some applications, the Settings delete operation is too long for + ZMS because of the linked list update. + As a tradeoff for performance the linked list is not updated. As a + result, some nodes will be unused and will occupy some space in the + storage. + These nodes will be used again when the same Settings element that has + been deleted is created again. + config SETTINGS_SHELL bool "Settings shell" depends on SHELL diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index 9d588bc588d..8ba72bedafd 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -61,6 +61,7 @@ static int settings_zms_dst(struct settings_zms *cf) return 0; } +#ifndef CONFIG_SETTINGS_ZMS_NO_LL_DELETE static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_hash) { int rc = 0; @@ -119,6 +120,7 @@ static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_ha return rc; } +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) { @@ -132,6 +134,7 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) return rc; } +#ifndef CONFIG_SETTINGS_ZMS_NO_LL_DELETE rc = settings_zms_unlink_ll_node(cf, name_hash); if (rc < 0) { return rc; @@ -143,6 +146,7 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) return rc; } +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ return rc; } @@ -177,15 +181,27 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo ZMS_DATA_ID_OFFSET); if ((rc1 <= 0) || (rc2 <= 0)) { - /* Settings item is not stored correctly in the ZMS. - * ZMS entry for its name or value is either missing - * or deleted. Clean dirty entries to make space for - * future settings item. + /* In case we are not updating the linked list, this is an empty mode + * Just continue + */ +#ifndef CONFIG_SETTINGS_ZMS_NO_LL_DELETE + /* Otherwise, Settings item is not stored correctly in the ZMS. + * ZMS entry's name or value is either missing or deleted. + * Clean dirty entries to make space for future settings items. */ ret = settings_zms_delete(cf, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id)); if (ret < 0) { return ret; } +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ + + ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } + /* update next ll_hash_id */ + ll_hash_id = settings_element.next_hash; continue; } @@ -313,6 +329,17 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const if (rc < 0) { return rc; } +#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE + /* verify that the ll_node doesn't exist otherwise do not update it */ + rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc >= 0) { + goto no_ll_update; + } else if (rc != -ENOENT) { + return rc; + } + /* else the LL node doesn't exist let's update it */ +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ /* write linked list structure element */ settings_element.next_hash = 0; /* Verify first that the linked list last element is not broken. @@ -342,7 +369,9 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const cf->second_to_last_hash_id = cf->last_hash_id; cf->last_hash_id = name_hash | 1; } - +#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE +no_ll_update: +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ return 0; } From 1582ac7bc0230f45bfe4c2accfc96ff09bd6d5f6 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Fri, 7 Mar 2025 16:22:13 +0100 Subject: [PATCH 15/27] [nrf fromlist] zms: remove non needed lookup cache before writing When the CONFIG_ZMS_NO_DOUBLE_WRITE is not enabled there is no need to search in the cache for matching ID Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 546ed2dbe817794bedb16bae0c81fd4af22675ea) --- subsys/fs/zms/zms.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index dd859e20479..b4512b4312b 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -1392,8 +1392,6 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) { int rc; size_t data_size; - uint64_t wlk_addr; - uint64_t rd_addr; uint32_t gc_count; uint32_t required_space = 0U; /* no space, appropriate for delete ate */ @@ -1414,19 +1412,19 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) return -EINVAL; } +#ifdef CONFIG_ZMS_NO_DOUBLE_WRITE /* find latest entry with same id */ #ifdef CONFIG_ZMS_LOOKUP_CACHE - wlk_addr = fs->lookup_cache[zms_lookup_cache_pos(id)]; + uint64_t wlk_addr = fs->lookup_cache[zms_lookup_cache_pos(id)]; if (wlk_addr == ZMS_LOOKUP_CACHE_NO_ADDR) { goto no_cached_entry; } #else wlk_addr = fs->ate_wra; -#endif - rd_addr = wlk_addr; +#endif /* CONFIG_ZMS_LOOKUP_CACHE */ + uint64_t rd_addr = wlk_addr; -#ifdef CONFIG_ZMS_NO_DOUBLE_WRITE /* Search for a previous valid ATE with the same ID */ struct zms_ate wlk_ate; int prev_found = zms_find_ate_with_id(fs, id, wlk_addr, fs->ate_wra, &wlk_ate, &rd_addr); @@ -1470,11 +1468,11 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) return 0; } } -#endif - #ifdef CONFIG_ZMS_LOOKUP_CACHE no_cached_entry: -#endif +#endif /* CONFIG_ZMS_LOOKUP_CACHE */ +#endif /* CONFIG_ZMS_NO_DOUBLE_WRITE */ + /* calculate required space if the entry contains data */ if (data_size) { /* Leave space for delete ate */ From 71c548e0bcc0d70e15266231e70d2368d6ca08d2 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Fri, 7 Mar 2025 16:42:27 +0100 Subject: [PATCH 16/27] [nrf fromlist] zms: optimize cache for settings subsystem Settings subsystem is storing the name ID and the linked list node ID with only one bit difference at BIT(0). Settings subsystem is also storing the name ID and the data ID in two different ZMS entries at an exact offset of ZMS_DATA_ID_OFFSET. Using the regular lookup function could result in many cache misses. Therefore, to assure the least number of collisions in the lookup cache, the BIT(0) of the hash indicates whether the given ZMS ID represents a linked list entry or not, the BIT(1) indicates whether the ZMS ID is a name or data and the remaining bits of the hash are set to a truncated part of the original hash generated by Settings. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit ad2581fede5f6efa570e3e8157ff37290d2dbe21) --- subsys/fs/zms/Kconfig | 9 +++++++++ subsys/fs/zms/zms.c | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/subsys/fs/zms/Kconfig b/subsys/fs/zms/Kconfig index 9be94d17354..c2b1d6f0fef 100644 --- a/subsys/fs/zms/Kconfig +++ b/subsys/fs/zms/Kconfig @@ -49,6 +49,15 @@ config ZMS_CUSTOM_BLOCK_SIZE help Changes the internal buffer size of ZMS +config ZMS_LOOKUP_CACHE_FOR_SETTINGS + bool "ZMS Storage lookup cache optimized for settings" + depends on ZMS_LOOKUP_CACHE && SETTINGS_ZMS + help + Use the lookup cache hash function that results in the least number of + collissions and, in turn, the best ZMS performance provided that the ZMS + is used as the settings backend only. This option should NOT be enabled + if the ZMS is also written to directly, outside the settings layer. + config ZMS_NO_DOUBLE_WRITE bool "Avoid writing the same data again in the storage" help diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index b4512b4312b..4336d90805b 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -11,6 +11,9 @@ #include #include #include "zms_priv.h" +#ifdef CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS +#include +#endif #include LOG_MODULE_REGISTER(fs_zms, CONFIG_ZMS_LOG_LEVEL); @@ -27,15 +30,41 @@ static int zms_ate_valid_different_sector(struct zms_fs *fs, const struct zms_at static inline size_t zms_lookup_cache_pos(uint32_t id) { - uint32_t hash; + uint32_t hash = id; + +#ifdef CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS + /* + * 1. Settings subsystem is storing the name ID and the linked list node ID + * with only one bit difference at BIT(0). + * 2. Settings subsystem is also storing the name ID and the data ID in two + * different ZMS entries at an exact offset of ZMS_DATA_ID_OFFSET. + * + * Therefore, to assure the least number of collisions in the lookup cache, + * the BIT(0) of the hash indicates whether the given ZMS ID represents a + * linked list entry or not, the BIT(1) indicates whether the ZMS ID is a name + * or data and the remaining bits of the hash are set to a truncated part of the + * original hash generated by Settings. + */ + BUILD_ASSERT(IS_POWER_OF_TWO(ZMS_DATA_ID_OFFSET), "ZMS_NAME_ID_OFFSET is not power of 2"); + + uint32_t key_value_bit; + uint32_t key_value_hash; + uint32_t key_value_ll; + + key_value_bit = (id >> LOG2(ZMS_DATA_ID_OFFSET)) & 1; + key_value_hash = (id & ZMS_HASH_MASK) >> (CONFIG_SETTINGS_ZMS_MAX_COLLISIONS_BITS + 1); + key_value_ll = id & BIT(0); + + hash = (key_value_hash << 2) | (key_value_bit << 1) | key_value_ll; +#else /* 32-bit integer hash function found by https://github.com/skeeto/hash-prospector. */ - hash = id; hash ^= hash >> 16; hash *= 0x7feb352dU; hash ^= hash >> 15; hash *= 0x846ca68bU; hash ^= hash >> 16; +#endif /* CONFIG_ZMS_LOOKUP_CACHE_FOR_SETTINGS */ return hash % CONFIG_ZMS_LOOKUP_CACHE_SIZE; } From 60cc27172de9a93172e700deaba3894def1205e0 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 11 Mar 2025 12:22:30 +0100 Subject: [PATCH 17/27] [nrf fromlist] settings: zms: load only subtree in the argument If the subtree argument is not NULL in the settings_load function, load only the setting entry that corresponds to that subtree. If the subtree argument is NULL or it is not found in the storage, load all the existing entries in the storage. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 86132feb5c59e8a071d9782c5482ab346cfbf6f3) --- subsys/settings/Kconfig | 9 +++ .../settings/include/settings/settings_zms.h | 3 +- subsys/settings/src/settings_zms.c | 60 +++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/subsys/settings/Kconfig b/subsys/settings/Kconfig index 24564841ca0..7df37dc284f 100644 --- a/subsys/settings/Kconfig +++ b/subsys/settings/Kconfig @@ -182,6 +182,15 @@ config SETTINGS_ZMS_NO_LL_DELETE These nodes will be used again when the same Settings element that has been deleted is created again. +config SETTINGS_ZMS_LOAD_SUBTREE_PATH + bool "Load only subtree path if provided" + help + Loads first the key defined by the subtree path. + If the callback handler returns a zero value it will + continue to look for all the keys under that subtree path. + If the callback handler returns a non negative value, it + returns immeditaley. + config SETTINGS_SHELL bool "Settings shell" depends on SHELL diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h index 9b9939815c2..16059d4420e 100644 --- a/subsys/settings/include/settings/settings_zms.h +++ b/subsys/settings/include/settings/settings_zms.h @@ -65,7 +65,8 @@ extern "C" { #define ZMS_NAME_ID_FROM_LL_NODE(x) (x & ~BIT(0)) #define ZMS_UPDATE_COLLISION_NUM(x, y) \ ((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK)) -#define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) +#define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) +#define ZMS_NAME_ID_FROM_HASH(x) ((x & ZMS_HASH_TOTAL_MASK) | BIT(31)) struct settings_zms { struct settings_store cf_store; diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index 8ba72bedafd..59f38d65fb1 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -150,6 +150,56 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) return rc; } +#ifdef CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH +/* Loads first the key which is defined by the name found in "subtree" root. + * If the key is not found or further keys under the same subtree are needed + * by the caller, returns 0. + */ +static int settings_zms_load_subtree(struct settings_store *cs, const struct settings_load_arg *arg) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + struct settings_zms_read_fn_arg read_fn_arg; + char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + ssize_t rc1; + ssize_t rc2; + uint32_t name_hash; + + name_hash = sys_hash32(arg->subtree, strlen(arg->subtree)) & ZMS_HASH_MASK; + for (int i = 0; i <= cf->hash_collision_num; i++) { + name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); + /* Get the name entry from ZMS */ + rc1 = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_HASH(name_hash), &name, + sizeof(name) - 1); + /* get the length of data and verify that it exists */ + rc2 = zms_get_data_length(&cf->cf_zms, + ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET); + if ((rc1 <= 0) || (rc2 <= 0)) { + /* Name or data doesn't exist */ + continue; + } + /* Found a name, this might not include a trailing \0 */ + name[rc1] = '\0'; + if (strcmp(arg->subtree, name)) { + /* Names are not equal let's continue to the next collision hash + * if it exists. + */ + continue; + } + /* At this steps the names are equal, let's set the handler */ + read_fn_arg.fs = &cf->cf_zms; + read_fn_arg.id = ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET; + + /* We should return here as there is no need to look for the next + * hash collision + */ + return settings_call_set_handler(arg->subtree, rc2, settings_zms_read_fn, &read_fn_arg, + (void *)arg); + } + + return 0; +} +#endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ + static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg) { int ret = 0; @@ -160,6 +210,15 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo ssize_t rc1; ssize_t rc2; uint32_t ll_hash_id; +#ifdef CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH + /* If arg->subtree is not null we must first load settings in that subtree */ + if (arg->subtree != NULL) { + ret = settings_zms_load_subtree(cs, arg); + if (ret) { + return ret; + } + } +#endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ ret = zms_read(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, sizeof(struct settings_hash_linked_list)); @@ -168,6 +227,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo } ll_hash_id = settings_element.next_hash; + /* If subtree is NULL then we must load all found Settings */ while (ll_hash_id) { /* In the ZMS backend, each setting item is stored in two ZMS From de45a6344c86f46f2ac19451842f3b4ccd6fbeb4 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Thu, 13 Mar 2025 11:22:30 +0100 Subject: [PATCH 18/27] [nrf fromlist] settings: zms: add cache for linked list hash Increase the load performance by adding an optional cache for the linked list hashes. This is used only when the settings_load is called with NULL parameter and we need to load all Settings that exist in the persistent storage. Cache is enabled using SETTINGS_ZMS_LL_CACHE and the size of the cache is set using SETTINGS_ZMS_LL_CACHE_SIZE. Each cache entry will add 8 bytes of RAM usage. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 50c099243557a49b53d793c839299e0a55370f42) --- subsys/settings/Kconfig | 19 +++++ .../settings/include/settings/settings_zms.h | 15 ++-- subsys/settings/src/settings_zms.c | 79 ++++++++++++++++--- 3 files changed, 97 insertions(+), 16 deletions(-) diff --git a/subsys/settings/Kconfig b/subsys/settings/Kconfig index 7df37dc284f..fdb3c4bb3ce 100644 --- a/subsys/settings/Kconfig +++ b/subsys/settings/Kconfig @@ -47,6 +47,25 @@ config SETTINGS_ZMS help Use ZMS as settings storage backend. +if SETTINGS_ZMS + +config SETTINGS_ZMS_LL_CACHE + bool "ZMS linked list lookup cache" + help + Enable ZMS lookup cache for linked list, used to reduce the + Settings load time by having most linked list elements already + in cache. + +config SETTINGS_ZMS_LL_CACHE_SIZE + int "ZMS linked list lookup cache size" + default 128 + range 1 $(UINT32_MAX) + depends on SETTINGS_ZMS_LL_CACHE + help + Number of entries in Settings ZMS linked list cache. + +endif # SETTINGS_ZMS + config SETTINGS_FCB bool "FCB" depends on FCB diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h index 16059d4420e..8a3817fd3a0 100644 --- a/subsys/settings/include/settings/settings_zms.h +++ b/subsys/settings/include/settings/settings_zms.h @@ -68,20 +68,25 @@ extern "C" { #define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) #define ZMS_NAME_ID_FROM_HASH(x) ((x & ZMS_HASH_TOTAL_MASK) | BIT(31)) +struct settings_hash_linked_list { + uint32_t previous_hash; + uint32_t next_hash; +}; + struct settings_zms { struct settings_store cf_store; struct zms_fs cf_zms; const struct device *flash_dev; +#if CONFIG_SETTINGS_ZMS_LL_CACHE + struct settings_hash_linked_list ll_cache[CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE]; + uint32_t ll_cache_next; + bool ll_has_changed; +#endif /* CONFIG_SETTINGS_ZMS_LL_CACHE */ uint32_t last_hash_id; uint32_t second_to_last_hash_id; uint8_t hash_collision_num; }; -struct settings_hash_linked_list { - uint32_t previous_hash; - uint32_t next_hash; -}; - #ifdef __cplusplus } #endif diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index 59f38d65fb1..ed4e96bcae8 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -135,6 +135,9 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) } #ifndef CONFIG_SETTINGS_ZMS_NO_LL_DELETE +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + cf->ll_has_changed = true; +#endif rc = settings_zms_unlink_ll_node(cf, name_hash); if (rc < 0) { return rc; @@ -220,16 +223,28 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo } #endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + uint32_t ll_cache_index = 0; + + if (cf->ll_has_changed) { + /* reload the linked list in cache */ + ret = settings_zms_get_last_hash_ids(cf); + if (ret < 0) { + return ret; + } + } + settings_element = cf->ll_cache[ll_cache_index++]; +#else ret = zms_read(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, sizeof(struct settings_hash_linked_list)); if (ret < 0) { return ret; } +#endif /* CONFIG_SETTINGS_ZMS_LL_CACHE */ ll_hash_id = settings_element.next_hash; /* If subtree is NULL then we must load all found Settings */ while (ll_hash_id) { - /* In the ZMS backend, each setting item is stored in two ZMS * entries one for the setting's name and one with the * setting's value. @@ -254,12 +269,24 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo return ret; } #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ - - ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (ret < 0) { - return ret; +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + if (ll_cache_index < cf->ll_cache_next) { + settings_element = cf->ll_cache[ll_cache_index++]; + } else if (ll_hash_id == cf->second_to_last_hash_id) { + /* The last ll node is not stored in the cache as it is already + * in the cf->last_hash_id. + */ + settings_element.next_hash = cf->last_hash_id; + } else { +#endif + ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE } +#endif /* update next ll_hash_id */ ll_hash_id = settings_element.next_hash; continue; @@ -276,12 +303,22 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo break; } - /* update next ll_hash_id */ - ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (ret < 0) { - return ret; +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + if (ll_cache_index < cf->ll_cache_next) { + settings_element = cf->ll_cache[ll_cache_index++]; + } else if (ll_hash_id == cf->second_to_last_hash_id) { + settings_element.next_hash = cf->last_hash_id; + } else { +#endif + ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE } +#endif + /* update next ll_hash_id */ ll_hash_id = settings_element.next_hash; } @@ -418,6 +455,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const if (rc < 0) { return rc; } + /* Now update the previous linked list element */ settings_element.next_hash = name_hash | 1; settings_element.previous_hash = cf->second_to_last_hash_id; @@ -428,6 +466,12 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const } cf->second_to_last_hash_id = cf->last_hash_id; cf->last_hash_id = name_hash | 1; +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + if (cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) { + cf->ll_cache[cf->ll_cache_next] = settings_element; + cf->ll_cache_next = cf->ll_cache_next + 1; + } +#endif } #ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE no_ll_update: @@ -441,6 +485,9 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) uint32_t ll_last_hash_id = ZMS_LL_HEAD_HASH_ID; int rc = 0; +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + cf->ll_cache_next = 0; +#endif cf->hash_collision_num = 0; do { rc = zms_read(&cf->cf_zms, ll_last_hash_id, &settings_element, @@ -461,6 +508,13 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) return rc; } +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + if ((cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) && + (settings_element.next_hash)) { + cf->ll_cache[cf->ll_cache_next] = settings_element; + cf->ll_cache_next = cf->ll_cache_next + 1; + } +#endif /* increment hash collision number if necessary */ if (ZMS_COLLISION_NUM(ll_last_hash_id) > cf->hash_collision_num) { cf->hash_collision_num = ZMS_COLLISION_NUM(ll_last_hash_id); @@ -470,6 +524,9 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) ll_last_hash_id = settings_element.next_hash; } while (settings_element.next_hash); +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + cf->ll_has_changed = false; +#endif return 0; } From 3b747c5a87aa01d68be834c2f65869898faa7f1b Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 18 Mar 2025 14:31:51 +0100 Subject: [PATCH 19/27] [nrf fromlist] settings: add an API function to load only one settings entry Add a a function settings_load_one that allows to load only one settings entry instead of a complete subtree. Add as well its implementation for ZMS backend. This will allow a faster return if the Settings entry doesn't exist in the persistent storage. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 98e544b4b838cd96012bb4bd5adfb584e67a52f1) --- include/zephyr/settings/settings.h | 22 ++++++++++ subsys/settings/src/settings_store.c | 61 ++++++++++++++++++++++++++++ subsys/settings/src/settings_zms.c | 46 +++++++++++++++++++++ 3 files changed, 129 insertions(+) diff --git a/include/zephyr/settings/settings.h b/include/zephyr/settings/settings.h index f22f1aba118..9a810b5920f 100644 --- a/include/zephyr/settings/settings.h +++ b/include/zephyr/settings/settings.h @@ -278,6 +278,17 @@ int settings_load(void); */ int settings_load_subtree(const char *subtree); +/** + * Load one serialized item from registered persistence sources. + * + * @param[in] name Name/key of the settings item. + * @param[out] buf Pointer to the buffer where the data is going to be loaded + * @param[in] buf_len Length of the allocated buffer. + * @return actual size of value that corresponds to name on success, negative + * value on failure. + */ +ssize_t settings_load_one(const char *name, void *buf, size_t buf_len); + /** * Callback function used for direct loading. * Used by @ref settings_load_subtree_direct function. @@ -457,6 +468,17 @@ struct settings_store_itf { * load callback only on the final entity. */ + ssize_t (*csi_load_one)(struct settings_store *cs, const char *name, + char *buf, size_t buf_len); + /**< Loads one value from storage that corresponds to the key defined by name. + * + * Parameters: + * - cs - Corresponding backend handler node. + * - name - Key in string format. + * - buf - Buffer where data should be copied. + * - buf_len - Length of buf. + */ + int (*csi_save_start)(struct settings_store *cs); /**< Handler called before an export operation. * diff --git a/subsys/settings/src/settings_store.c b/subsys/settings/src/settings_store.c index b697f993d94..26b14f5aa8b 100644 --- a/subsys/settings/src/settings_store.c +++ b/subsys/settings/src/settings_store.c @@ -88,6 +88,67 @@ int settings_load_subtree_direct( return 0; } +struct default_param { + void *buf; + size_t buf_len; + size_t *val_len; +}; + +static int settings_set_default_cb(const char *name, size_t len, settings_read_cb read_cb, + void *cb_arg, void *param) +{ + int rc = 0; + const char *next; + size_t name_len; + struct default_param *dest = (struct default_param *)param; + + name_len = settings_name_next(name, &next); + if (name_len == 0) { + rc = read_cb(cb_arg, dest->buf, MIN(dest->buf_len, len)); + *dest->val_len = len; + } + + return rc; +} + +/* Load a single key/value from persistent storage */ +ssize_t settings_load_one(const char *name, void *buf, size_t buf_len) +{ + struct settings_store *cs; + size_t val_len = 0; + int rc = 0; + + /* + * for every config store that supports this function + * load config + */ + k_mutex_lock(&settings_lock, K_FOREVER); + SYS_SLIST_FOR_EACH_CONTAINER(&settings_load_srcs, cs, cs_next) { + if (cs->cs_itf->csi_load_one) { + rc = cs->cs_itf->csi_load_one(cs, name, (char *)buf, buf_len); + val_len = (rc >= 0) ? rc : 0; + } else { + struct default_param param = { + .buf = buf, + .buf_len = buf_len, + .val_len = &val_len + }; + const struct settings_load_arg arg = { + .subtree = name, + .cb = &settings_set_default_cb, + .param = ¶m + }; + rc = cs->cs_itf->csi_load(cs, &arg); + } + } + k_mutex_unlock(&settings_lock); + + if (rc >= 0) { + return val_len; + } + return rc; +} + /* * Append a single value to persisted config. Don't store duplicate value. */ diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index ed4e96bcae8..f853ce0a946 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -27,12 +27,15 @@ struct settings_zms_read_fn_arg { }; static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg); +static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name, char *buf, + size_t buf_len); static int settings_zms_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); static void *settings_zms_storage_get(struct settings_store *cs); static int settings_zms_get_last_hash_ids(struct settings_zms *cf); static struct settings_store_itf settings_zms_itf = {.csi_load = settings_zms_load, + .csi_load_one = settings_zms_load_one, .csi_save = settings_zms_save, .csi_storage_get = settings_zms_storage_get}; @@ -203,6 +206,49 @@ static int settings_zms_load_subtree(struct settings_store *cs, const struct set } #endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ +static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name, char *buf, + size_t buf_len) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + ssize_t rc = 0; + uint32_t name_hash; + uint32_t value_id; + + /* verify that name is not NULL */ + if (!name || !buf) { + return -EINVAL; + } + + name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + for (int i = 0; i <= cf->hash_collision_num; i++) { + name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); + /* Get the name entry from ZMS */ + rc = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_HASH(name_hash), r_name, + sizeof(r_name) - 1); + if (rc <= 0) { + /* Name doesn't exist */ + continue; + } + /* Found a name, this might not include a trailing \0 */ + r_name[rc] = '\0'; + if (strcmp(name, r_name)) { + /* Names are not equal let's continue to the next collision hash + * if it exists. + */ + continue; + } + + /* At this steps the names are equal, let's read the data */ + value_id = ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET; + rc = zms_read(&cf->cf_zms, value_id, buf, buf_len); + + return rc == buf_len ? zms_get_data_length(&cf->cf_zms, value_id) : rc; + } + + return rc; +} + static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg) { int ret = 0; From b5fceed81bcc3192a347efd4a20a7c9bb90a6932 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Mon, 24 Mar 2025 13:56:18 +0100 Subject: [PATCH 20/27] [nrf fromlist] tests: settings: add a functional test for settings_load_one Add a test for the new API settings_load_one that loads only one path from the persistent storage. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 168ae245bdce756fd5db22dd391469ad17eeb9b2) --- .../settings/functional/src/settings_basic_test.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/subsys/settings/functional/src/settings_basic_test.c b/tests/subsys/settings/functional/src/settings_basic_test.c index f77c87db71d..2766e7ea97d 100644 --- a/tests/subsys/settings/functional/src/settings_basic_test.c +++ b/tests/subsys/settings/functional/src/settings_basic_test.c @@ -345,6 +345,17 @@ ZTEST(settings_functional, test_register_and_loading) err = (!data.en1) && (data.en2) && (!data.en3); zassert_true(err, "wrong data enable found"); + memset(&data, 0, sizeof(struct stored_data)); + /* test load_one: path "ps/ss/ss/val2". Only data.val2 should + * receive a value + */ + val = 2; + settings_save_one("ps/ss/ss/val2", &val, sizeof(uint8_t)); + rc = settings_load_one("ps/ss/ss/val2", &data.val2, sizeof(uint8_t)); + zassert_true(rc >= 0, "settings_load_one failed"); + err = (data.val1 == 0) && (data.val2 == 2) && (data.val3 == 0); + zassert_true(err, "wrong data value found %u != 2", data.val2); + /* clean up by deregistering settings_handler */ rc = settings_deregister(&val1_settings); zassert_true(rc, "deregistering val1_settings failed"); From 6f7b016f3bf3143b9cc07689bac70b9352532edd Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Mon, 24 Mar 2025 14:12:44 +0100 Subject: [PATCH 21/27] [nrf fromlist] tests: settings: add functional test for ZMS Add the functional test for the new backend of ZMS Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 18f7736ce3c73f6f10e037aa627293104dc55b9d) --- .../functional/src/settings_basic_test.c | 2 +- .../settings/functional/zms/CMakeLists.txt | 10 ++++++++ tests/subsys/settings/functional/zms/prj.conf | 8 ++++++ .../functional/zms/settings_test_zms.c | 25 +++++++++++++++++++ .../settings/functional/zms/testcase.yaml | 9 +++++++ 5 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/subsys/settings/functional/zms/CMakeLists.txt create mode 100644 tests/subsys/settings/functional/zms/prj.conf create mode 100644 tests/subsys/settings/functional/zms/settings_test_zms.c create mode 100644 tests/subsys/settings/functional/zms/testcase.yaml diff --git a/tests/subsys/settings/functional/src/settings_basic_test.c b/tests/subsys/settings/functional/src/settings_basic_test.c index 2766e7ea97d..1d8349941b9 100644 --- a/tests/subsys/settings/functional/src/settings_basic_test.c +++ b/tests/subsys/settings/functional/src/settings_basic_test.c @@ -16,7 +16,7 @@ #include LOG_MODULE_REGISTER(settings_basic_test); -#if defined(CONFIG_SETTINGS_FCB) || defined(CONFIG_SETTINGS_NVS) +#if defined(CONFIG_SETTINGS_FCB) || defined(CONFIG_SETTINGS_NVS) || defined(CONFIG_SETTINGS_ZMS) #include #if DT_HAS_CHOSEN(zephyr_settings_partition) #define TEST_FLASH_AREA_ID DT_FIXED_PARTITION_ID(DT_CHOSEN(zephyr_settings_partition)) diff --git a/tests/subsys/settings/functional/zms/CMakeLists.txt b/tests/subsys/settings/functional/zms/CMakeLists.txt new file mode 100644 index 00000000000..e29cd6c417e --- /dev/null +++ b/tests/subsys/settings/functional/zms/CMakeLists.txt @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(functional_zms) + +# The code is in the library common to several tests. +target_sources(app PRIVATE settings_test_zms.c) + +add_subdirectory(../src func_test_bindir) diff --git a/tests/subsys/settings/functional/zms/prj.conf b/tests/subsys/settings/functional/zms/prj.conf new file mode 100644 index 00000000000..3de961c7c72 --- /dev/null +++ b/tests/subsys/settings/functional/zms/prj.conf @@ -0,0 +1,8 @@ +CONFIG_ZTEST=y +CONFIG_FLASH=y +CONFIG_FLASH_MAP=y +CONFIG_ZMS=y + +CONFIG_SETTINGS=y +CONFIG_SETTINGS_RUNTIME=y +CONFIG_SETTINGS_ZMS=y diff --git a/tests/subsys/settings/functional/zms/settings_test_zms.c b/tests/subsys/settings/functional/zms/settings_test_zms.c new file mode 100644 index 00000000000..2a2ef9a5d59 --- /dev/null +++ b/tests/subsys/settings/functional/zms/settings_test_zms.c @@ -0,0 +1,25 @@ +/* Copyright (c) 2024 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include + +ZTEST(settings_functional, test_setting_storage_get) +{ + int rc; + void *storage; + uint32_t data = 0xdeadbeef; + + rc = settings_storage_get(&storage); + zassert_equal(0, rc, "Can't fetch storage reference (err=%d)", rc); + zassert_not_null(storage, "Null reference."); + + rc = zms_write((struct zms_fs *)storage, 512, &data, sizeof(data)); + zassert_true(rc >= 0, "Can't write ZMS entry (err=%d).", rc); +} +ZTEST_SUITE(settings_functional, NULL, NULL, NULL, NULL, NULL); diff --git a/tests/subsys/settings/functional/zms/testcase.yaml b/tests/subsys/settings/functional/zms/testcase.yaml new file mode 100644 index 00000000000..8b7fcdf576f --- /dev/null +++ b/tests/subsys/settings/functional/zms/testcase.yaml @@ -0,0 +1,9 @@ +tests: + settings.functional.zms: + platform_allow: + - qemu_x86 + - native_sim + - native_sim/native/64 + tags: + - settings + - zms From 69fe25bfadcd872fd850ed8b5c1fd08cb4aaaf44 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 18 Mar 2025 17:41:20 +0100 Subject: [PATCH 22/27] [nrf fromlist] settings: zms: add more robustness to the save function When a power off happens after writing the settings name and before writing the linked list node we cannot write the settings name again in the future. Fix this by writing the linked list node before writing the name. When loading all settings, we already delete linked list node that do not have any name or value written. Adds as well a recover path if a power down happens in the middle of unlinking an LL node after a delete. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit bf4d15316b1a3bd9239ddbe179b11006eea1e3b1) --- .../settings/include/settings/settings_zms.h | 1 + subsys/settings/src/settings_zms.c | 101 +++++++++++------- 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h index 8a3817fd3a0..96697edc196 100644 --- a/subsys/settings/include/settings/settings_zms.h +++ b/subsys/settings/include/settings/settings_zms.h @@ -63,6 +63,7 @@ extern "C" { /* some useful macros */ #define ZMS_NAME_ID_FROM_LL_NODE(x) (x & ~BIT(0)) +#define ZMS_LL_NODE_FROM_NAME_ID(x) (x | BIT(0)) #define ZMS_UPDATE_COLLISION_NUM(x, y) \ ((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK)) #define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index f853ce0a946..af7b9e89985 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -72,11 +72,39 @@ static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_ha struct settings_hash_linked_list settings_update_element; /* let's update the linked list */ - rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element, + rc = zms_read(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element, sizeof(struct settings_hash_linked_list)); if (rc < 0) { return rc; } + + /* update the previous element */ + if (settings_element.previous_hash) { + rc = zms_read(&cf->cf_zms, settings_element.previous_hash, &settings_update_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + if (!settings_element.next_hash) { + /* we are deleting the last element of the linked list, + * let's update the second_to_last_hash_id + */ + cf->second_to_last_hash_id = settings_update_element.previous_hash; + } + settings_update_element.next_hash = settings_element.next_hash; + rc = zms_write(&cf->cf_zms, settings_element.previous_hash, + &settings_update_element, sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + } + + /* Now delete the current linked list element */ + rc = zms_delete(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash)); + if (rc < 0) { + return rc; + } + /* update the next element */ if (settings_element.next_hash) { rc = zms_read(&cf->cf_zms, settings_element.next_hash, &settings_update_element, @@ -100,28 +128,8 @@ static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_ha */ cf->last_hash_id = settings_element.previous_hash; } - /* update the previous element */ - if (settings_element.previous_hash) { - rc = zms_read(&cf->cf_zms, settings_element.previous_hash, &settings_update_element, - sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; - } - if (!settings_element.next_hash) { - /* we are deleting the last element of the linked list, - * let's update the second_to_last_hash_id - */ - cf->second_to_last_hash_id = settings_update_element.previous_hash; - } - settings_update_element.next_hash = settings_element.next_hash; - rc = zms_write(&cf->cf_zms, settings_element.previous_hash, - &settings_update_element, sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; - } - } - return rc; + return 0; } #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ @@ -146,12 +154,6 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) return rc; } - /* Now delete the current linked list element */ - rc = zms_delete(&cf->cf_zms, name_hash | 1); - if (rc < 0) { - return rc; - } - #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ return rc; } @@ -468,13 +470,10 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const /* write the name if required */ if (write_name) { - rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name)); - if (rc < 0) { - return rc; - } + /* First let's update the linked list */ #ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE /* verify that the ll_node doesn't exist otherwise do not update it */ - rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element, + rc = zms_read(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element, sizeof(struct settings_hash_linked_list)); if (rc >= 0) { goto no_ll_update; @@ -496,14 +495,14 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const } } settings_element.previous_hash = cf->last_hash_id; - rc = zms_write(&cf->cf_zms, name_hash | 1, &settings_element, + rc = zms_write(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element, sizeof(struct settings_hash_linked_list)); if (rc < 0) { return rc; } /* Now update the previous linked list element */ - settings_element.next_hash = name_hash | 1; + settings_element.next_hash = ZMS_LL_NODE_FROM_NAME_ID(name_hash); settings_element.previous_hash = cf->second_to_last_hash_id; rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element, sizeof(struct settings_hash_linked_list)); @@ -511,13 +510,21 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const return rc; } cf->second_to_last_hash_id = cf->last_hash_id; - cf->last_hash_id = name_hash | 1; + cf->last_hash_id = ZMS_LL_NODE_FROM_NAME_ID(name_hash); #ifdef CONFIG_SETTINGS_ZMS_LL_CACHE if (cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) { cf->ll_cache[cf->ll_cache_next] = settings_element; cf->ll_cache_next = cf->ll_cache_next + 1; } #endif +#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE +no_ll_update: +#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ + /* Now let's write the name */ + rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name)); + if (rc < 0) { + return rc; + } } #ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE no_ll_update: @@ -529,6 +536,7 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) { struct settings_hash_linked_list settings_element; uint32_t ll_last_hash_id = ZMS_LL_HEAD_HASH_ID; + uint32_t previous_ll_hash_id = 0; int rc = 0; #ifdef CONFIG_SETTINGS_ZMS_LL_CACHE @@ -554,6 +562,27 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) return rc; } + if (settings_element.previous_hash != previous_ll_hash_id) { + /* This is a special case that can happen when a power down occurred + * when deleting a linked list node. + * If the power down occurred after updating the previous linked list node, + * then we would end up with a state where the previous_hash of the linked + * list is broken. Let's recover from this + */ + rc = zms_delete(&cf->cf_zms, settings_element.previous_hash); + if (rc < 0) { + return rc; + } + /* Now recover the linked list */ + settings_element.previous_hash = previous_ll_hash_id; + zms_write(&cf->cf_zms, ll_last_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + } + previous_ll_hash_id = ll_last_hash_id; + #ifdef CONFIG_SETTINGS_ZMS_LL_CACHE if ((cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) && (settings_element.next_hash)) { From 7100cecd339d06280555451ba35b1adb66185afa Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 18 Mar 2025 18:37:56 +0100 Subject: [PATCH 23/27] [nrf fromlist] settings: zms: recover linked list if broken When the linked list is broken, recover it instead of reinitializing it. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 057c1d65fa5822d9145330c87183d0b652707ab2) --- subsys/settings/src/settings_zms.c | 36 ++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index af7b9e89985..2a4047bb51d 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -526,9 +526,6 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const return rc; } } -#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE -no_ll_update: -#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ return 0; } @@ -547,16 +544,31 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) rc = zms_read(&cf->cf_zms, ll_last_hash_id, &settings_element, sizeof(settings_element)); if (rc == -ENOENT) { - /* header doesn't exist or linked list broken, reinitialize the header */ - const struct settings_hash_linked_list settings_element = { - .previous_hash = 0, .next_hash = 0}; - rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; + /* header doesn't exist or linked list broken, reinitialize the header + * if it doesn't exist and recover it if it is broken + */ + if (ll_last_hash_id == ZMS_LL_HEAD_HASH_ID) { + /* header doesn't exist */ + const struct settings_hash_linked_list settings_element = { + .previous_hash = 0, .next_hash = 0}; + rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } + cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; + cf->second_to_last_hash_id = 0; + } else { + /* let's recover it by keeping all nodes until the last one */ + const struct settings_hash_linked_list settings_element = { + .previous_hash = cf->second_to_last_hash_id, + .next_hash = 0}; + rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; + } } - cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; - cf->second_to_last_hash_id = 0; return 0; } else if (rc < 0) { return rc; From 9aa00672cd8eabbc0c67ba330bd2e618ee58df32 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Tue, 25 Mar 2025 18:52:50 +0100 Subject: [PATCH 24/27] [nrf fromlist] settings: add new API function settings_get_val_len() Add a function to get the value's length of a Key. If it doesn't exist returns 0. Add ZMS implementation for csi_get_val_len() and a default implementation for the other storage systems. Add some functional tests to verify it. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 5a730f71f49e87fb4da35120826e8d45bc62fef6) --- include/zephyr/settings/settings.h | 17 ++++++ subsys/settings/src/settings_store.c | 55 +++++++++++++++++++ subsys/settings/src/settings_zms.c | 42 +++++++++++++- .../functional/src/settings_basic_test.c | 27 +++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/include/zephyr/settings/settings.h b/include/zephyr/settings/settings.h index 9a810b5920f..ac8d6fc4bb4 100644 --- a/include/zephyr/settings/settings.h +++ b/include/zephyr/settings/settings.h @@ -289,6 +289,14 @@ int settings_load_subtree(const char *subtree); */ ssize_t settings_load_one(const char *name, void *buf, size_t buf_len); +/** + * Get the data length of the value relative to the key + * + * @param[in] key Name/key of the settings item. + * @return length of value if item exists, 0 if not and negative value on failure. + */ +ssize_t settings_get_val_len(const char *key); + /** * Callback function used for direct loading. * Used by @ref settings_load_subtree_direct function. @@ -479,6 +487,15 @@ struct settings_store_itf { * - buf_len - Length of buf. */ + ssize_t (*csi_get_val_len)(struct settings_store *cs, const char *name); + /**< Gets the value's length associated to the Key defined by name. + * It returns 0 if the Key/Value doesn't exist. + * + * Parameters: + * - cs - Corresponding backend handler node. + * - name - Key in string format. + */ + int (*csi_save_start)(struct settings_store *cs); /**< Handler called before an export operation. * diff --git a/subsys/settings/src/settings_store.c b/subsys/settings/src/settings_store.c index 26b14f5aa8b..b8f8d847541 100644 --- a/subsys/settings/src/settings_store.c +++ b/subsys/settings/src/settings_store.c @@ -94,6 +94,7 @@ struct default_param { size_t *val_len; }; +/* Default callback to set a Key/Value pair */ static int settings_set_default_cb(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg, void *param) { @@ -111,6 +112,60 @@ static int settings_set_default_cb(const char *name, size_t len, settings_read_c return rc; } +/* Default callback to get the value's length of the Key defined by name. Returns 0 if Key + * doesn't exist. + */ +static int settings_get_val_len_default_cb(const char *name, size_t len, + [[maybe_unused]] settings_read_cb read_cb, + [[maybe_unused]] void *cb_arg, void *param) +{ + const char *next; + size_t name_len; + size_t *val_len = (size_t *)param; + + name_len = settings_name_next(name, &next); + if (name_len == 0) { + *val_len = len; + } + + return 0; +} + +/* Gets the value's size if the Key defined by name is in the persistent storage, + * if not found returns 0. + */ +ssize_t settings_get_val_len(const char *name) +{ + struct settings_store *cs; + int rc = 0; + size_t val_len = 0; + + /* + * for every config store that supports this function + * get the value's length. + */ + k_mutex_lock(&settings_lock, K_FOREVER); + SYS_SLIST_FOR_EACH_CONTAINER(&settings_load_srcs, cs, cs_next) { + if (cs->cs_itf->csi_get_val_len) { + val_len = cs->cs_itf->csi_get_val_len(cs, name); + } else { + const struct settings_load_arg arg = { + .subtree = name, + .cb = &settings_get_val_len_default_cb, + .param = &val_len + }; + rc = cs->cs_itf->csi_load(cs, &arg); + } + } + k_mutex_unlock(&settings_lock); + + if (rc >= 0) { + return val_len; + } + + return rc; +} + /* Load a single key/value from persistent storage */ ssize_t settings_load_one(const char *name, void *buf, size_t buf_len) { diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index 2a4047bb51d..becca2275b1 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -33,11 +33,13 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const size_t val_len); static void *settings_zms_storage_get(struct settings_store *cs); static int settings_zms_get_last_hash_ids(struct settings_zms *cf); +static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *name); static struct settings_store_itf settings_zms_itf = {.csi_load = settings_zms_load, .csi_load_one = settings_zms_load_one, .csi_save = settings_zms_save, - .csi_storage_get = settings_zms_storage_get}; + .csi_storage_get = settings_zms_storage_get, + .csi_get_val_len = settings_zms_get_val_len}; static ssize_t settings_zms_read_fn(void *back_end, void *data, size_t len) { @@ -529,6 +531,44 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const return 0; } +static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *name) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + ssize_t rc = 0; + uint32_t name_hash; + + /* verify that name is not NULL */ + if (!name) { + return -EINVAL; + } + + name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + for (int i = 0; i <= cf->hash_collision_num; i++) { + name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); + /* Get the name entry from ZMS */ + rc = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_HASH(name_hash), r_name, + sizeof(r_name) - 1); + if (rc <= 0) { + /* Name doesn't exist */ + continue; + } + /* Found a name, this might not include a trailing \0 */ + r_name[rc] = '\0'; + if (strcmp(name, r_name)) { + /* Names are not equal let's continue to the next collision hash + * if it exists. + */ + continue; + } + /* At this steps the names are equal, let's read the data size*/ + return zms_get_data_length(&cf->cf_zms, + ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET); + } + + return 0; +} + static int settings_zms_get_last_hash_ids(struct settings_zms *cf) { struct settings_hash_linked_list settings_element; diff --git a/tests/subsys/settings/functional/src/settings_basic_test.c b/tests/subsys/settings/functional/src/settings_basic_test.c index 1d8349941b9..03ff3eba8a8 100644 --- a/tests/subsys/settings/functional/src/settings_basic_test.c +++ b/tests/subsys/settings/functional/src/settings_basic_test.c @@ -240,13 +240,22 @@ ZTEST(settings_functional, test_register_and_loading) { int rc, err; uint8_t val = 0; + ssize_t val_len = 0; rc = settings_subsys_init(); zassert_true(rc == 0, "subsys init failed"); + /* Check that key that corresponds to val2 do not exist in storage */ + val_len = settings_get_val_len("ps/ss/ss/val2"); + zassert_true((val_len == 0), "Failure: key should not exist"); + settings_save_one("ps/ss/ss/val2", &val, sizeof(uint8_t)); + /* Check that the key that corresponds to val2 exists in storage */ + val_len = settings_get_val_len("ps/ss/ss/val2"); + zassert_true((val_len == 1), "Failure: key should exist"); + memset(&data, 0, sizeof(struct stored_data)); rc = settings_register(&val1_settings); @@ -279,7 +288,16 @@ ZTEST(settings_functional, test_register_and_loading) err = (data.en1) && (data.en2) && (!data.en3); zassert_true(err, "wrong data enable found"); + /* Check that key that corresponds to val3 do not exist in storage */ + val_len = settings_get_val_len("ps/ss/val3"); + zassert_true((val_len == 0), "Failure: key should not exist"); + settings_save_one("ps/ss/val3", &val, sizeof(uint8_t)); + + /* Check that the key that corresponds to val3 exists in storage */ + val_len = settings_get_val_len("ps/ss/val3"); + zassert_true((val_len == 1), "Failure: key should exist"); + memset(&data, 0, sizeof(struct stored_data)); /* when we load settings now data.val2 and data.val1 should receive a * value @@ -310,7 +328,16 @@ ZTEST(settings_functional, test_register_and_loading) err = (data.en1) && (data.en2) && (data.en3); zassert_true(err, "wrong data enable found"); + /* Check that key that corresponds to val1 do not exist in storage */ + val_len = settings_get_val_len("ps/val1"); + zassert_true((val_len == 0), "Failure: key should not exist"); + settings_save_one("ps/val1", &val, sizeof(uint8_t)); + + /* Check that the key that corresponds to val1 exists in storage */ + val_len = settings_get_val_len("ps/val1"); + zassert_true((val_len == 1), "Failure: key should exist"); + memset(&data, 0, sizeof(struct stored_data)); /* when we load settings all data should receive a value loaded */ rc = settings_load(); From e1cf30fd71c985a4167a184526b34df9993bdbb8 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Thu, 20 Mar 2025 11:00:11 +0100 Subject: [PATCH 25/27] [nrf fromlist] settings: zms: use the safe function strnlen instead of strlen if the provided name in argument is not null this could lead to un undefined behavior. Use strnlen to make this safe Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit e9ac2ef95bd34a8c4756e46e8f2ddf66e0964dbc) --- include/zephyr/settings/settings.h | 3 +++ subsys/settings/src/settings_zms.c | 24 ++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/include/zephyr/settings/settings.h b/include/zephyr/settings/settings.h index ac8d6fc4bb4..b5b89fbe199 100644 --- a/include/zephyr/settings/settings.h +++ b/include/zephyr/settings/settings.h @@ -45,6 +45,9 @@ extern "C" { */ #define SETTINGS_EXTRA_LEN ((SETTINGS_MAX_DIR_DEPTH - 1) + 2) +/* Maximum Settings name length including separators */ +#define SETTINGS_FULL_NAME_LEN SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1 + /** * Function used to read the data from the settings storage in * h_set handler implementations. diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index becca2275b1..fdef9aff5d3 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -3,6 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +#undef _POSIX_C_SOURCE +#define _POSIX_C_SOURCE 200809L /* for strnlen() */ + #include #include @@ -169,12 +172,13 @@ static int settings_zms_load_subtree(struct settings_store *cs, const struct set { struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); struct settings_zms_read_fn_arg read_fn_arg; - char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + char name[SETTINGS_FULL_NAME_LEN]; ssize_t rc1; ssize_t rc2; uint32_t name_hash; - name_hash = sys_hash32(arg->subtree, strlen(arg->subtree)) & ZMS_HASH_MASK; + name_hash = sys_hash32(arg->subtree, strnlen(arg->subtree, SETTINGS_FULL_NAME_LEN)) & + ZMS_HASH_MASK; for (int i = 0; i <= cf->hash_collision_num; i++) { name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); /* Get the name entry from ZMS */ @@ -214,7 +218,7 @@ static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name size_t buf_len) { struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + char r_name[SETTINGS_FULL_NAME_LEN]; ssize_t rc = 0; uint32_t name_hash; uint32_t value_id; @@ -224,7 +228,7 @@ static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name return -EINVAL; } - name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + name_hash = sys_hash32(name, strnlen(name, SETTINGS_FULL_NAME_LEN)) & ZMS_HASH_MASK; for (int i = 0; i <= cf->hash_collision_num; i++) { name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); /* Get the name entry from ZMS */ @@ -259,7 +263,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); struct settings_zms_read_fn_arg read_fn_arg; struct settings_hash_linked_list settings_element; - char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + char name[SETTINGS_FULL_NAME_LEN]; ssize_t rc1; ssize_t rc2; uint32_t ll_hash_id; @@ -380,7 +384,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const { struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); struct settings_hash_linked_list settings_element; - char rdname[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + char rdname[SETTINGS_FULL_NAME_LEN]; uint32_t name_hash; uint32_t collision_num = 0; bool delete; @@ -396,7 +400,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const /* Find out if we are doing a delete */ delete = ((value == NULL) || (val_len == 0)); - name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + name_hash = sys_hash32(name, strnlen(name, SETTINGS_FULL_NAME_LEN)) & ZMS_HASH_MASK; /* MSB is always 1 */ name_hash |= BIT(31); @@ -523,7 +527,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const no_ll_update: #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ /* Now let's write the name */ - rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name)); + rc = zms_write(&cf->cf_zms, name_hash, name, strnlen(name, SETTINGS_FULL_NAME_LEN)); if (rc < 0) { return rc; } @@ -534,7 +538,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *name) { struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; + char r_name[SETTINGS_FULL_NAME_LEN]; ssize_t rc = 0; uint32_t name_hash; @@ -543,7 +547,7 @@ static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *n return -EINVAL; } - name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK; + name_hash = sys_hash32(name, strnlen(name, SETTINGS_FULL_NAME_LEN)) & ZMS_HASH_MASK; for (int i = 0; i <= cf->hash_collision_num; i++) { name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); /* Get the name entry from ZMS */ From 10f199a63f4d0200470044f143440df65c86cd21 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Thu, 27 Mar 2025 13:40:49 +0100 Subject: [PATCH 26/27] [nrf fromlist] doc: settings: new API functions Add references to the new API functions that were added "csi_load_one" and "csi_get_val_len" Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit b26ed3a3ec5dae5017ecd4e40164e3b38ca88235) --- doc/services/settings/index.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/services/settings/index.rst b/doc/services/settings/index.rst index 4f8831e3a11..68b1faed548 100644 --- a/doc/services/settings/index.rst +++ b/doc/services/settings/index.rst @@ -78,6 +78,14 @@ backend. This gets called when loading values from persistent storage using :c:func:`settings_load()`. +**csi_load_one** + This gets called when loading only one item from persistent storage using + :c:func:`settings_load_one()`. + +**csi_get_val_len** + This gets called when getting a value's length from persistent storage using + :c:func:`settings_get_val_len()`. + **csi_save** This gets called when saving a single setting to persistent storage using :c:func:`settings_save_one()`. @@ -148,6 +156,12 @@ After all data is loaded, the ``h_commit`` handler is issued, signalling the application that the settings were successfully retrieved. +Alternatively, a call to :c:func:`settings_load_one()` will load only one +Settings entry and store it in the provided buffer. + +To get the value's length associated with the Settings entry, a call to +:c:func:`settings_get_val_len()` should be performed + Technically FCB and file backends may store some history of the entities. This means that the newest data entity is stored after any older existing data entities. From a08cf11de0135af2ee4d12ad5f7bcd0e86b4c607 Mon Sep 17 00:00:00 2001 From: Riadh Ghaddab Date: Fri, 4 Apr 2025 12:37:03 +0200 Subject: [PATCH 27/27] [nrf fromlist] settings: zms: code style clean up Clean some parts of the code and refactor it to avoid multiple nested conditions. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab (cherry picked from commit 1699785ff3fd7d7e260133898729afff634eea73) --- .../settings/include/settings/settings_zms.h | 3 + subsys/settings/src/settings_zms.c | 258 +++++++++--------- .../settings/performance/settings_test_perf.c | 25 +- 3 files changed, 148 insertions(+), 138 deletions(-) diff --git a/subsys/settings/include/settings/settings_zms.h b/subsys/settings/include/settings/settings_zms.h index 96697edc196..76ca92f6d11 100644 --- a/subsys/settings/include/settings/settings_zms.h +++ b/subsys/settings/include/settings/settings_zms.h @@ -68,6 +68,9 @@ extern "C" { ((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK)) #define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1) #define ZMS_NAME_ID_FROM_HASH(x) ((x & ZMS_HASH_TOTAL_MASK) | BIT(31)) +#define ZMS_DATA_ID_FROM_HASH(x) (ZMS_NAME_ID_FROM_HASH(x) + ZMS_DATA_ID_OFFSET) +#define ZMS_DATA_ID_FROM_NAME(x) (x + ZMS_DATA_ID_OFFSET) +#define ZMS_DATA_ID_FROM_LL_NODE(x) (ZMS_NAME_ID_FROM_LL_NODE(x) + ZMS_DATA_ID_OFFSET) struct settings_hash_linked_list { uint32_t previous_hash; diff --git a/subsys/settings/src/settings_zms.c b/subsys/settings/src/settings_zms.c index fdef9aff5d3..b3250074486 100644 --- a/subsys/settings/src/settings_zms.c +++ b/subsys/settings/src/settings_zms.c @@ -144,7 +144,7 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) rc = zms_delete(&cf->cf_zms, name_hash); if (rc >= 0) { - rc = zms_delete(&cf->cf_zms, name_hash + ZMS_DATA_ID_OFFSET); + rc = zms_delete(&cf->cf_zms, ZMS_DATA_ID_FROM_NAME(name_hash)); } if (rc < 0) { return rc; @@ -155,11 +155,8 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash) cf->ll_has_changed = true; #endif rc = settings_zms_unlink_ll_node(cf, name_hash); - if (rc < 0) { - return rc; - } - #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ + return rc; } @@ -185,8 +182,7 @@ static int settings_zms_load_subtree(struct settings_store *cs, const struct set rc1 = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_HASH(name_hash), &name, sizeof(name) - 1); /* get the length of data and verify that it exists */ - rc2 = zms_get_data_length(&cf->cf_zms, - ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET); + rc2 = zms_get_data_length(&cf->cf_zms, ZMS_DATA_ID_FROM_HASH(name_hash)); if ((rc1 <= 0) || (rc2 <= 0)) { /* Name or data doesn't exist */ continue; @@ -201,32 +197,28 @@ static int settings_zms_load_subtree(struct settings_store *cs, const struct set } /* At this steps the names are equal, let's set the handler */ read_fn_arg.fs = &cf->cf_zms; - read_fn_arg.id = ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET; + read_fn_arg.id = ZMS_DATA_ID_FROM_HASH(name_hash); /* We should return here as there is no need to look for the next * hash collision */ - return settings_call_set_handler(arg->subtree, rc2, settings_zms_read_fn, &read_fn_arg, - (void *)arg); + return settings_call_set_handler(arg->subtree, rc2, settings_zms_read_fn, + &read_fn_arg, arg); } return 0; } #endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ -static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name, char *buf, - size_t buf_len) +/* Search for the name_hash that corresponds to name. + * If no hash that corresponds to name is found in the persistent storage, + * returns 0. + */ +static uint32_t settings_zms_find_hash_from_name(struct settings_zms *cf, const char *name) { - struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + uint32_t name_hash = 0; + int rc = 0; char r_name[SETTINGS_FULL_NAME_LEN]; - ssize_t rc = 0; - uint32_t name_hash; - uint32_t value_id; - - /* verify that name is not NULL */ - if (!name || !buf) { - return -EINVAL; - } name_hash = sys_hash32(name, strnlen(name, SETTINGS_FULL_NAME_LEN)) & ZMS_HASH_MASK; for (int i = 0; i <= cf->hash_collision_num; i++) { @@ -246,15 +238,71 @@ static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name */ continue; } + /* At this step names are equal, we found the corresponding hash */ + return name_hash; + } + + return 0; +} + +static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name, char *buf, + size_t buf_len) +{ + struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); + uint32_t name_hash = 0; + ssize_t rc = 0; + uint32_t value_id; - /* At this steps the names are equal, let's read the data */ - value_id = ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET; + /* verify that name is not NULL */ + if (!name || !buf) { + return -EINVAL; + } + + name_hash = settings_zms_find_hash_from_name(cf, name); + if (name_hash) { + /* we found a name_hash corresponding to name */ + value_id = ZMS_DATA_ID_FROM_HASH(name_hash); rc = zms_read(&cf->cf_zms, value_id, buf, buf_len); - return rc == buf_len ? zms_get_data_length(&cf->cf_zms, value_id) : rc; + return (rc == buf_len) ? zms_get_data_length(&cf->cf_zms, value_id) : rc; } - return rc; + return 0; +} + +/* Gets the next linked list node either from cache (if enabled) or from persistent + * storage if cache is full or cache is not enabled. + * It updates as well the next cache index and the next linked list node ID. + */ +static int settings_zms_get_next_ll(struct settings_zms *cf, uint32_t *ll_hash_id, + [[maybe_unused]] uint32_t *ll_cache_index) +{ + struct settings_hash_linked_list settings_element; + int ret = 0; + +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + if (*ll_cache_index < cf->ll_cache_next) { + settings_element = cf->ll_cache[*ll_cache_index]; + *ll_cache_index = *ll_cache_index + 1; + } else if (*ll_hash_id == cf->second_to_last_hash_id) { + /* The last ll node is not stored in the cache as it is already + * in the cf->last_hash_id. + */ + settings_element.next_hash = cf->last_hash_id; + } else { +#endif + ret = zms_read(&cf->cf_zms, *ll_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (ret < 0) { + return ret; + } +#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + } +#endif + /* update next ll_hash_id */ + *ll_hash_id = settings_element.next_hash; + + return 0; } static int settings_zms_load(struct settings_store *cs, const struct settings_load_arg *arg) @@ -262,11 +310,13 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo int ret = 0; struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); struct settings_zms_read_fn_arg read_fn_arg; - struct settings_hash_linked_list settings_element; char name[SETTINGS_FULL_NAME_LEN]; ssize_t rc1; ssize_t rc2; uint32_t ll_hash_id; + uint32_t prev_ll_hash_id; + uint32_t ll_cache_index = 0; + #ifdef CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH /* If arg->subtree is not null we must first load settings in that subtree */ if (arg->subtree != NULL) { @@ -278,8 +328,6 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo #endif /* CONFIG_SETTINGS_ZMS_LOAD_SUBTREE_PATH */ #ifdef CONFIG_SETTINGS_ZMS_LL_CACHE - uint32_t ll_cache_index = 0; - if (cf->ll_has_changed) { /* reload the linked list in cache */ ret = settings_zms_get_last_hash_ids(cf); @@ -287,17 +335,15 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo return ret; } } - settings_element = cf->ll_cache[ll_cache_index++]; -#else - ret = zms_read(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, - sizeof(struct settings_hash_linked_list)); +#endif + + /* If subtree is NULL then we must load all found Settings */ + ll_hash_id = ZMS_LL_HEAD_HASH_ID; + ret = settings_zms_get_next_ll(cf, &ll_hash_id, &ll_cache_index); if (ret < 0) { return ret; } -#endif /* CONFIG_SETTINGS_ZMS_LL_CACHE */ - ll_hash_id = settings_element.next_hash; - /* If subtree is NULL then we must load all found Settings */ while (ll_hash_id) { /* In the ZMS backend, each setting item is stored in two ZMS * entries one for the setting's name and one with the @@ -306,8 +352,16 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo rc1 = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id), &name, sizeof(name) - 1); /* get the length of data and verify that it exists */ - rc2 = zms_get_data_length(&cf->cf_zms, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id) + - ZMS_DATA_ID_OFFSET); + rc2 = zms_get_data_length(&cf->cf_zms, ZMS_DATA_ID_FROM_LL_NODE(ll_hash_id)); + + /* updated the next linked list node in case the called handler will + * delete this settings entry. + */ + prev_ll_hash_id = ll_hash_id; + ret = settings_zms_get_next_ll(cf, &ll_hash_id, &ll_cache_index); + if (ret < 0) { + return ret; + } if ((rc1 <= 0) || (rc2 <= 0)) { /* In case we are not updating the linked list, this is an empty mode @@ -318,62 +372,23 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo * ZMS entry's name or value is either missing or deleted. * Clean dirty entries to make space for future settings items. */ - ret = settings_zms_delete(cf, ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id)); + ret = settings_zms_delete(cf, ZMS_NAME_ID_FROM_LL_NODE(prev_ll_hash_id)); if (ret < 0) { return ret; } #endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */ -#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE - if (ll_cache_index < cf->ll_cache_next) { - settings_element = cf->ll_cache[ll_cache_index++]; - } else if (ll_hash_id == cf->second_to_last_hash_id) { - /* The last ll node is not stored in the cache as it is already - * in the cf->last_hash_id. - */ - settings_element.next_hash = cf->last_hash_id; - } else { -#endif - ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (ret < 0) { - return ret; - } -#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE - } -#endif - /* update next ll_hash_id */ - ll_hash_id = settings_element.next_hash; continue; } /* Found a name, this might not include a trailing \0 */ name[rc1] = '\0'; read_fn_arg.fs = &cf->cf_zms; - read_fn_arg.id = ZMS_NAME_ID_FROM_LL_NODE(ll_hash_id) + ZMS_DATA_ID_OFFSET; + read_fn_arg.id = ZMS_DATA_ID_FROM_LL_NODE(prev_ll_hash_id); - ret = settings_call_set_handler(name, rc2, settings_zms_read_fn, &read_fn_arg, - (void *)arg); + ret = settings_call_set_handler(name, rc2, settings_zms_read_fn, &read_fn_arg, arg); if (ret) { - break; - } - -#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE - if (ll_cache_index < cf->ll_cache_next) { - settings_element = cf->ll_cache[ll_cache_index++]; - } else if (ll_hash_id == cf->second_to_last_hash_id) { - settings_element.next_hash = cf->last_hash_id; - } else { -#endif - ret = zms_read(&cf->cf_zms, ll_hash_id, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (ret < 0) { - return ret; - } -#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE + return ret; } -#endif - /* update next ll_hash_id */ - ll_hash_id = settings_element.next_hash; } return ret; @@ -469,7 +484,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const } /* write the value */ - rc = zms_write(&cf->cf_zms, name_hash + ZMS_DATA_ID_OFFSET, value, val_len); + rc = zms_write(&cf->cf_zms, ZMS_DATA_ID_FROM_NAME(name_hash), value, val_len); if (rc < 0) { return rc; } @@ -538,36 +553,49 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *name) { struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store); - char r_name[SETTINGS_FULL_NAME_LEN]; - ssize_t rc = 0; - uint32_t name_hash; + uint32_t name_hash = 0; /* verify that name is not NULL */ if (!name) { return -EINVAL; } - name_hash = sys_hash32(name, strnlen(name, SETTINGS_FULL_NAME_LEN)) & ZMS_HASH_MASK; - for (int i = 0; i <= cf->hash_collision_num; i++) { - name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i); - /* Get the name entry from ZMS */ - rc = zms_read(&cf->cf_zms, ZMS_NAME_ID_FROM_HASH(name_hash), r_name, - sizeof(r_name) - 1); - if (rc <= 0) { - /* Name doesn't exist */ - continue; + name_hash = settings_zms_find_hash_from_name(cf, name); + if (name_hash) { + return zms_get_data_length(&cf->cf_zms, ZMS_DATA_ID_FROM_HASH(name_hash)); + } + + return 0; +} + +/* This function inits the linked list head if it doesn't exist or recover it + * if the ll_last_hash_id is different than the head hash ID + */ +static int settings_zms_init_or_recover_ll(struct settings_zms *cf, uint32_t ll_last_hash_id) +{ + struct settings_hash_linked_list settings_element; + int rc = 0; + + if (ll_last_hash_id == ZMS_LL_HEAD_HASH_ID) { + /* header doesn't exist */ + settings_element.previous_hash = 0; + settings_element.next_hash = 0; + rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; } - /* Found a name, this might not include a trailing \0 */ - r_name[rc] = '\0'; - if (strcmp(name, r_name)) { - /* Names are not equal let's continue to the next collision hash - * if it exists. - */ - continue; + cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; + cf->second_to_last_hash_id = 0; + } else { + /* let's recover it by keeping all nodes until the last one */ + settings_element.previous_hash = cf->second_to_last_hash_id; + settings_element.next_hash = 0; + rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element, + sizeof(struct settings_hash_linked_list)); + if (rc < 0) { + return rc; } - /* At this steps the names are equal, let's read the data size*/ - return zms_get_data_length(&cf->cf_zms, - ZMS_NAME_ID_FROM_HASH(name_hash) + ZMS_DATA_ID_OFFSET); } return 0; @@ -591,29 +619,7 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf) /* header doesn't exist or linked list broken, reinitialize the header * if it doesn't exist and recover it if it is broken */ - if (ll_last_hash_id == ZMS_LL_HEAD_HASH_ID) { - /* header doesn't exist */ - const struct settings_hash_linked_list settings_element = { - .previous_hash = 0, .next_hash = 0}; - rc = zms_write(&cf->cf_zms, ZMS_LL_HEAD_HASH_ID, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; - } - cf->last_hash_id = ZMS_LL_HEAD_HASH_ID; - cf->second_to_last_hash_id = 0; - } else { - /* let's recover it by keeping all nodes until the last one */ - const struct settings_hash_linked_list settings_element = { - .previous_hash = cf->second_to_last_hash_id, - .next_hash = 0}; - rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element, - sizeof(struct settings_hash_linked_list)); - if (rc < 0) { - return rc; - } - } - return 0; + return settings_zms_init_or_recover_ll(cf, ll_last_hash_id); } else if (rc < 0) { return rc; } diff --git a/tests/subsys/settings/performance/settings_test_perf.c b/tests/subsys/settings/performance/settings_test_perf.c index a5de17e3475..3efe0fcaa52 100644 --- a/tests/subsys/settings/performance/settings_test_perf.c +++ b/tests/subsys/settings/performance/settings_test_perf.c @@ -19,20 +19,21 @@ static struct k_work_q settings_work_q; static K_THREAD_STACK_DEFINE(settings_work_stack, 2024); static struct k_work_delayable pending_store; -#define TEST_SETTINGS_COUNT (128) -#define TEST_STORE_ITR (5) -#define TEST_TIMEOUT_SEC (60) +#define TEST_SETTINGS_COUNT (128) +#define TEST_STORE_ITR (5) +#define TEST_TIMEOUT_SEC (60) #define TEST_SETTINGS_WORKQ_PRIO (1) -static void bt_scan_cb(const bt_addr_le_t *addr, int8_t rssi, uint8_t adv_type, - struct net_buf_simple *buf) +static void bt_scan_cb([[maybe_unused]] const bt_addr_le_t *addr, [[maybe_unused]] int8_t rssi, + [[maybe_unused]] uint8_t adv_type, struct net_buf_simple *buf) { printk("len %u\n", buf->len); } struct test_setting { uint32_t val; -} test_settings[TEST_SETTINGS_COUNT]; +}; +struct test_setting test_settings[TEST_SETTINGS_COUNT]; K_SEM_DEFINE(waitfor_work, 0, 1); @@ -45,14 +46,15 @@ static void store_pending(struct k_work *work) uint32_t total_measured; uint32_t single_entry_max; uint32_t single_entry_min; - } stats = {0, 0, 0, UINT32_MAX}; + }; + struct test_stats stats = {0, 0, 0, UINT32_MAX}; int64_t ts1 = k_uptime_get(); /* benchmark storage performance */ for (int j = 0; j < TEST_STORE_ITR; j++) { for (int i = 0; i < TEST_SETTINGS_COUNT; i++) { - test_settings[i].val = TEST_SETTINGS_COUNT*j + i; + test_settings[i].val = TEST_SETTINGS_COUNT * j + i; int64_t ts2 = k_uptime_get(); @@ -80,8 +82,7 @@ static void store_pending(struct k_work *work) printk("*** storing of %u entries completed ***\n", ARRAY_SIZE(test_settings)); printk("total calculated: %u, total measured: %u\n", stats.total_calculated, stats.total_measured); - printk("entry max: %u, entry min: %u\n", stats.single_entry_max, - stats.single_entry_min); + printk("entry max: %u, entry min: %u\n", stats.single_entry_max, stats.single_entry_min); k_sem_give(&waitfor_work); } @@ -99,8 +100,8 @@ ZTEST(settings_perf, test_performance) } k_work_queue_start(&settings_work_q, settings_work_stack, - K_THREAD_STACK_SIZEOF(settings_work_stack), - K_PRIO_COOP(TEST_SETTINGS_WORKQ_PRIO), NULL); + K_THREAD_STACK_SIZEOF(settings_work_stack), + K_PRIO_COOP(TEST_SETTINGS_WORKQ_PRIO), NULL); k_thread_name_set(&settings_work_q.thread, "Settings workq"); k_work_init_delayable(&pending_store, store_pending);