Settings ZMS backend optimizations #2615
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 13, 2025
|
|
||
| ret = settings_call_set_handler(arg->subtree, rc2, settings_zms_read_fn, | ||
| &read_fn_arg, (void *)arg); | ||
| return ret; |
There was a problem hiding this comment.
Unfortunately, you cannot return here. If subtree == "abc", the code is expected to return not only "abc" but also "abc/def" etc. You can only break if ret != 0.
There was a problem hiding this comment.
@Damian-Nordic you are right I will change that.
I actually didn't see any application using this feature of settings where you can load subtree "abc" and it will load abc/*
There was a problem hiding this comment.
E.g. OpenThread uses it.
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
5 times, most recently
from
44521ba to
934e9d4
Compare
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
6 times, most recently
from
d5f2ca0 to
b2c11b2
Compare
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
2 times, most recently
from
8aadee1 to
3022716
Compare
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
from
3022716 to
9fec112
Compare
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
2 times, most recently
from
67900bf to
0b14b25
Compare
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
3 times, most recently
from
e166a69 to
3a0a404
Compare
omkar3141
previously requested changes
Apr 3, 2025
subsys/settings/src/settings_store.c
Outdated
| /* 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, settings_read_cb read_cb, |
There was a problem hiding this comment.
Looks like read_cb and cb_arg is unused here.
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
from
3a0a404 to
329de8f
Compare
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 <[email protected]> (cherry picked from commit 333fadd)
Add reference to the new settings backend ZMS and add more details about how it works. Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit f363015)
- 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 <[email protected]> (cherry picked from commit b826d46)
- Sets CONFIG_LOG_BLOCK_IN_THREAD for the zms sample. - Fixes "--- 9999 messages dropped ---". Signed-off-by: Andrej Butok <[email protected]> (cherry picked from commit ac7790e)
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 <[email protected]> (cherry picked from commit 02ab54e077e090e4c9364e4d4954a8c3508e513e)
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 <[email protected]> (cherry picked from commit 546ed2dbe817794bedb16bae0c81fd4af22675ea)
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 <[email protected]> (cherry picked from commit ad2581fede5f6efa570e3e8157ff37290d2dbe21)
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 <[email protected]> (cherry picked from commit 86132feb5c59e8a071d9782c5482ab346cfbf6f3)
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 <[email protected]> (cherry picked from commit 50c099243557a49b53d793c839299e0a55370f42)
…s 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 <[email protected]> (cherry picked from commit 98e544b4b838cd96012bb4bd5adfb584e67a52f1)
…ad_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 <[email protected]> (cherry picked from commit 168ae245bdce756fd5db22dd391469ad17eeb9b2)
Add the functional test for the new backend of ZMS Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 18f7736ce3c73f6f10e037aa627293104dc55b9d)
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 <[email protected]> (cherry picked from commit bf4d15316b1a3bd9239ddbe179b11006eea1e3b1)
When the linked list is broken, recover it instead of reinitializing it. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 057c1d65fa5822d9145330c87183d0b652707ab2)
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 <[email protected]> (cherry picked from commit 5a730f71f49e87fb4da35120826e8d45bc62fef6)
…f 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 <[email protected]> (cherry picked from commit e9ac2ef95bd34a8c4756e46e8f2ddf66e0964dbc)
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 <[email protected]> (cherry picked from commit b26ed3a3ec5dae5017ecd4e40164e3b38ca88235)
Clean some parts of the code and refactor it to avoid multiple nested conditions. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 1699785ff3fd7d7e260133898729afff634eea73)
rghaddab
force-pushed
the
rghaddab/settings-zms-optimizations
branch
from
0992192 to
a08cf11
Compare
|
rghaddab
requested review from
Damian-Nordic,
carlescufi,
doublemis1,
nordicjm and
omkar3141
|
@omkar3141 @Damian-Nordic please take another look |
carlescufi
approved these changes
Apr 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are series of optimization for the new backend + some commits to syncronize ZMS to the upstream version