A new non volatile storage system #77930

rghaddab · 2024-09-03T13:08:38Z

Implements RFC #77929

This is the first Draft of a new storage system that is designed especially to work on non erasable devices but supports as well erasable devices.

It is intended that some features described in the RFC are not implemented yet to make it easier to review for contributors that are familiar with NVS filesystem.

Already included feature :

Simple Key-Value Storage (i.e. no file/folder abstractions)
32-bit IDs
Metadata entries can also store small (1 to 8 bytes) data entries
Support for no-erase-required flash drivers (i.e. RRAM, MRAM, etc)

Features in progress:

No limits in value length
Support for entries in multiple formats
CRC-24 for entries that require it (part of multiple format entries)
Efficiency for Settings subsystem
Efficiency for Secure Storage subsystem

butok · 2024-09-04T08:04:13Z

There is no a ZMS User Documentation.

rghaddab · 2024-09-04T09:12:49Z

There is no a ZMS User Documentation.

Yes there is no Documentation yet as this could change a lot from its initial proposal.
The PR is still in Draft mode and some work is still in Progress.
I will add some information about what is already included in this PR and what is coming next

subsys/fs/zms/zms_priv.h

subsys/fs/zms/zms.c

frkv · 2024-09-16T10:58:47Z

tests/subsys/fs/zms/src/main.c

+#include <zephyr/sys/crc.h>
+#include "zms_priv.h"
+
+#define TEST_ZMS_FLASH_AREA        storage_partition


I hope we can standardize on more open terms like NVM instead of reiterating FLASH for likely historical reasons. The addition of ZMS should not be tied to any specific type of non volatile memory technology...

Indeed, ZMS is not tied to any specific type of volatile or non volatile memory but as its name indicates (Zephyr Mempry Storage) it is for any Storage device.
In this case, tests are made to work on flash_simulator which is a flash.
I can change these defines by removing FLASH refrence (TEST_ZMS_FLASH_AREA => TEST_ZMS_AREA).
But the references to flash will still be there across the file because we are using flash_simulator and flash_api

List of added tests : - zms.test_delete - zms.test_zms_cache_collission - zms.test_zms_cache_gc - zms.test_zms_cache_hash_quality - zms.test_zms_cache_init - zms.test_zms_corrupted_sector_close_operation - zms.test_zms_corrupted_write - zms.test_zms_full_sector - zms.test_zms_gc - zms.test_zms_gc_3sectors - zms.test_zms_gc_corrupt_ate - zms.test_zms_gc_corrupt_close_ate - zms.test_zms_mount - zms.test_zms_write Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 1f4e92c3da4b738d2dd203348d7a93f5ccdb5b99)

This adds a user application that shows the usage of ZMS The sample app shows three main functions of ZMS: - read/write/delete key/value pairs - fill all storage and delete it - calculate free remaining space Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit f8aaa1eaf15e6607b39c4e36911552a0a2410bc6)

ZMS is the abreviation of Zephyr Memory Storage. It is a storage developed to target especially the non erasable devices. The new memory storage system inherit from the NVS storage multiple features and introduce new ones : * Inherited features : - light key-value based storage - cache for entries - Wear Leveling of flash memory - Resilience to power failures * New features : - cycle counter for non erasable devices (instead of erase emulation) - Keys up to 32-bit - Built-in support of CRC32 for data - Small size data (<= 8 bytes) integrated within entries Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 7bf5d179cc5d586fb3cf4564f92cea7b66b0e964)

List of added tests : - zms.test_delete - zms.test_zms_cache_collission - zms.test_zms_cache_gc - zms.test_zms_cache_hash_quality - zms.test_zms_cache_init - zms.test_zms_corrupted_sector_close_operation - zms.test_zms_corrupted_write - zms.test_zms_full_sector - zms.test_zms_gc - zms.test_zms_gc_3sectors - zms.test_zms_gc_corrupt_ate - zms.test_zms_gc_corrupt_close_ate - zms.test_zms_mount - zms.test_zms_write Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 1f4e92c3da4b738d2dd203348d7a93f5ccdb5b99)

This adds a user application that shows the usage of ZMS The sample app shows three main functions of ZMS: - read/write/delete key/value pairs - fill all storage and delete it - calculate free remaining space Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit f8aaa1eaf15e6607b39c4e36911552a0a2410bc6)

carlescufi · 2024-10-01T13:02:36Z

Hi I was trying to build the ZMS with some example, but the config ZMS does not seem to appear in the menuconfig/guiconfig in any situation while the NVS symbol works fine. I was checking many Kconfig files, but it seems that ZMS is integrated the same way as NVS. My question is - was it tested in some way? And what configuration/project was used? Sorry if I missed something.

Andrej

@andrisk-dev there's a sample now in the PR that you can use.

carlescufi · 2024-10-01T18:13:34Z

@carlescufi the proposed PR does not comply with my suggestion, not by letter, and absolutely not by spirit.

And it likely never will entirely, but it certainly seems to be welcome by several in the community that are facing the lack of a storage system that fulfills their requirements. Those requirements are not artificial, they have been collected by users that are having issues today with Zephyr and storage. So unless you are ready to propose an alternative that also fulfills those, I don't think blocking this PR is constructive at this point.

Laczen · 2024-10-01T19:10:42Z

@carlescufi the proposed PR does not comply with my suggestion, not by letter, and absolutely not by spirit.

And it likely never will entirely, but it certainly seems to be welcome by several in the community that are facing the lack of a storage system that fulfills their requirements. Those requirements are not artificial, they have been collected by users that are having issues today with Zephyr and storage. So unless you are ready to propose an alternative that also fulfills those, I don't think blocking this PR is constructive at this point.

@carlescufi, this is for more than 85% a copy of nvs. There shouldn't be two so equal systems in zephyr. Where will this stop? If someone creates a third option that fulfills their requirements, should this be accepted as well? And a fourth? The blocking is to ensure that zephyr is kept maintainable, if two solutions are so similar they should be integrated. I have given all constructive feedback to achieve this, also taking into account the existing install base.

carlescufi · 2024-10-01T21:29:41Z

@carlescufi the proposed PR does not comply with my suggestion, not by letter, and absolutely not by spirit.

And it likely never will entirely, but it certainly seems to be welcome by several in the community that are facing the lack of a storage system that fulfills their requirements. Those requirements are not artificial, they have been collected by users that are having issues today with Zephyr and storage. So unless you are ready to propose an alternative that also fulfills those, I don't think blocking this PR is constructive at this point.

@carlescufi, this is for more than 85% a copy of nvs. There shouldn't be two so equal systems in zephyr. Where will this stop? If someone creates a third option that fulfills their requirements, should this be accepted as well? And a fourth? The blocking is to ensure that zephyr is kept maintainable, if two solutions are so similar they should be integrated. I have given all constructive feedback to achieve this, also taking into account the existing install base.

There is a huge disconnect between what you write here and the reality of the multiple attempts to enhance NVS that have stalled due to what seems to be a clear lack of either understanding or willingness to make it happen. The inability for others, multiple contributors, to improve NVS is a fact, and it doesn't look like it is going to change. If you want to keep Zephyr maintainable, then please review this new storage system when paired with the requirements and goals it was set out to fulfill, or propose your own solution that solves the problems that people have today when using Zephyr on different hardware and in different conditions.

this is for more than 85% a copy of nvs.

Even if this was completely accurate, which I will leave @rghaddab to comment on, this is establishing the foundations for an advanced memory storage system that can be extended and improved over time. See the list of features that the author plans to include in the description of this PR.

rghaddab · 2024-10-02T07:32:08Z

@carlescufi, this is for more than 85% a copy of nvs. There shouldn't be two so equal systems in zephyr. Where will this stop? If someone creates a third option that fulfills their requirements, should this be accepted as well? And a fourth? The blocking is to ensure that zephyr is kept maintainable,

Again some false statements listed here.

I replaced in zms.c the word zms with nvs to check similarities between zms.c and nvs.c and I found that it has 14% of similarities. Where did you get the 85% ?

ZMS and NVS are not equal. Each of them have different purposes and different requirements. Merging both of them will make both of them behave worse.
Here is a non exhaustive list that explains why:

NVS is optimized for flash devices (with erase) => ZMS is not at all optimized for these devices (performance is less than NVS)
ZMS is optimized for RRAM and MRAM (non erasable devices) => NVS is a life killer for these devices and much more slower.
NVS has limited footprint header and ATE => required by some small memory size devices.
ZMS has a larger header and footprint (ATE size 16 bytes) => optimized especially for usecases where values do not exceed 8 bytes.
NVS cannot evolve a lot from its current version as it is deployed by thousands of systems and the limited free bytes in its entries makes it almost impossible to add a new feature
ZMS have a larger header with more custom fields => Can evolve a lot from its initial version.

Please read the list of features that will be included in ZMS in the future commits before blocking this PR.

About maintainability of two storage systems => that's not a problem as I will maintain this new storage system and I will keep it updated. Unless you want to remain the only maintainer of storage systems.

Should there be a third, fourth or even a 10th storage system ? Why not ? As long as they serve a certain purpose and they are maintained by people that developed/used them. Do we have a limited number of maintainers/contributor in Zephyr for each subsystem ? I am not aware if such policy exists

rghaddab · 2024-10-02T10:33:09Z

@Damian-Nordic, @de-nordic and @RICCIARDI-Adrien I added a sample code and fixed some corner cases bugs.
Can you review the latest changes please ?

andrisk-dev · 2024-10-03T12:27:20Z

Hi I was trying to build the ZMS with some example, but the config ZMS does not seem to appear in the menuconfig/guiconfig in any situation while the NVS symbol works fine. I was checking many Kconfig files, but it seems that ZMS is integrated the same way as NVS. My question is - was it tested in some way? And what configuration/project was used? Sorry if I missed something.
Andrej

@andrisk-dev there's a sample now in the PR that you can use.

@carlescufi many thanks for the update.

I tried to build the example but the result was the same problem I had when I tried to modify NVS example to use ZMS:

C:/_/zms2/zephyr/samples/subsys/fs/zms/prj.conf:4: warning: attempt to assign the value 'y' to the undefined symbol ZMS

I did the following to run the example:

git clone https://github.com/rghaddab/zephyr
cd zephyr
git checkout rghaddab/zms
git pull
west init
west update
west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always

Could there be some problem with the Kconfigs? Or is some configuration for the board also needed?

Andrej

carlescufi · 2024-10-03T12:33:50Z

@andrisk-dev

This builds just fine for me:
west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always

Can you try cleaning your Git folder (git clean -fdx)?

Edit:
You don't need a west init after the git clone at all, and I wouldn't recommend cloning @rghaddab repo. Instead, what I do is to go to my local copy of Zephyr and use:

cd ~/zephyrproject/zephyr
git fetch https://github.com/zephyrproject-rtos/zephyr.git pull/77930/head:zms-branch
git checkout zms-branch
west update
west build ...

alternatively you can use GitHub's gh tool:

cd ~/zephyrproject/zephyr
gh pr checkout 77930
west update
west build ...

rghaddab · 2024-10-03T12:58:09Z

git clone https://github.com/rghaddab/zephyr
cd zephyr
git checkout rghaddab/zms
git pull
west init
west update
west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always
Could there be some problem with the Kconfigs? Or is some configuration for the board also needed?

Andrej

I think doing these commands west will create another zephyr folder under the zephyr folder that you checked out. (zephyr/zephyr/...)
And then it will use the one created with origin/main branch, that's why west don't find CONFIG_ZMS

butok · 2024-10-03T13:14:58Z

git clone https://github.com/rghaddab/zephyr
cd zephyr
git checkout rghaddab/zms
git pull
west init
west update
west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always

It can be:

 git clone https://github.com/rghaddab/zephyr
 west init -l zephyr
 cd zephyr
 git checkout rghaddab/zms
 west update
 west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always

The result:

-- Zephyr version: 3.7.99 (C:/_ddm/ZEPHYR/review/zms/zephyr), build: f8aaa1eaf15e
[15/159] Building C object CMakeFiles/app.dir/src/main.c.obj
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c: In function 'main':
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c:239:71: warning: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' {aka 'long long unsigned int'} [-Wformat=]
  239 |                         printk("Error while computing free space, rc=%d\n", free_space);
      |                                                                      ~^     ~~~~~~~~~~
      |                                                                       |     |
      |                                                                       int   uint64_t {aka long long unsigned int}
      |                                                                      %lld
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c:275:63: warning: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' {aka 'long long unsigned int'} [-Wformat=]
  275 |                 printk("Error while computing free space, rc=%d\n", free_space);
      |                                                              ~^     ~~~~~~~~~~
      |                                                               |     |
      |                                                               int   uint64_t {aka long long unsigned int}
      |                                                              %lld
[159/159] Linking C executable zephyr\zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       36168 B       160 KB     22.08%
             RAM:        6528 B       192 KB      3.32%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%
Generating files from C:/_ddm/ZEPHYR/review/zms/zephyr/build/zephyr/zephyr.elf for board: lpcxpresso55s69

carlescufi · 2024-10-03T13:19:12Z

The result:

@butok right, but those are warnings (@rghaddab can you please fix them) but not a Kconfig error like @andrisk-dev was reporting.

andrisk-dev · 2024-10-03T13:24:18Z

git clone https://github.com/rghaddab/zephyr
cd zephyr
git checkout rghaddab/zms
git pull
west init
west update
west build -b lpcxpresso55s69/lpc55s69/cpu0 samples/subsys/fs/zms -p always
Could there be some problem with the Kconfigs? Or is some configuration for the board also needed?

Andrej
I think doing these commands west will create another zephyr folder under the zephyr folder that you checked out. (zephyr/zephyr/...) And then it will use the one created with origin/main branch, that's why west don't find CONFIG_ZMS

I think you are right. So the problem was on my side.

Sorry

Andrej

This adds a user application that shows the usage of ZMS The sample app shows three main functions of ZMS: - read/write/delete key/value pairs - fill all storage and delete it - calculate free remaining space Signed-off-by: Riadh Ghaddab <[email protected]>

rghaddab · 2024-10-03T13:35:21Z

-- Zephyr version: 3.7.99 (C:/_ddm/ZEPHYR/review/zms/zephyr), build: f8aaa1eaf15e
[15/159] Building C object CMakeFiles/app.dir/src/main.c.obj
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c: In function 'main':
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c:239:71: warning: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' {aka 'long long unsigned int'} [-Wformat=]
  239 |                         printk("Error while computing free space, rc=%d\n", free_space);
      |                                                                      ~^     ~~~~~~~~~~
      |                                                                       |     |
      |                                                                       int   uint64_t {aka long long unsigned int}
      |                                                                      %lld
C:/_ddm/ZEPHYR/review/zms/zephyr/samples/subsys/fs/zms/src/main.c:275:63: warning: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' {aka 'long long unsigned int'} [-Wformat=]
  275 |                 printk("Error while computing free space, rc=%d\n", free_space);
      |                                                              ~^     ~~~~~~~~~~
      |                                                               |     |
      |                                                               int   uint64_t {aka long long unsigned int}
      |                                                              %lld
[159/159] Linking C executable zephyr\zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       36168 B       160 KB     22.08%
             RAM:        6528 B       192 KB      3.32%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%
Generating files from C:/_ddm/ZEPHYR/review/zms/zephyr/build/zephyr/zephyr.elf for board: lpcxpresso55s69

These warnings are fixed now

ZMS is the abreviation of Zephyr Memory Storage. It is a storage developed to target especially the non erasable devices. The new memory storage system inherit from the NVS storage multiple features and introduce new ones : * Inherited features : - light key-value based storage - cache for entries - Wear Leveling of flash memory - Resilience to power failures * New features : - cycle counter for non erasable devices (instead of erase emulation) - Keys up to 32-bit - Built-in support of CRC32 for data - Small size data (<= 8 bytes) integrated within entries Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 7bf5d179cc5d586fb3cf4564f92cea7b66b0e964)

List of added tests : - zms.test_delete - zms.test_zms_cache_collission - zms.test_zms_cache_gc - zms.test_zms_cache_hash_quality - zms.test_zms_cache_init - zms.test_zms_corrupted_sector_close_operation - zms.test_zms_corrupted_write - zms.test_zms_full_sector - zms.test_zms_gc - zms.test_zms_gc_3sectors - zms.test_zms_gc_corrupt_ate - zms.test_zms_gc_corrupt_close_ate - zms.test_zms_mount - zms.test_zms_write Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit 1f4e92c3da4b738d2dd203348d7a93f5ccdb5b99)

This adds a user application that shows the usage of ZMS The sample app shows three main functions of ZMS: - read/write/delete key/value pairs - fill all storage and delete it - calculate free remaining space Upstream PR: zephyrproject-rtos/zephyr#77930 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit c479fadad817a84da18ea2f2e1873cebbd9c1f12)

nordicjm linked an issue Sep 4, 2024 that may be closed by this pull request

New non-volatile storage system #77929

Open

rghaddab force-pushed the rghaddab/zms branch 9 times, most recently from 363962a to 013a303 Compare September 9, 2024 07:59

rghaddab marked this pull request as ready for review September 9, 2024 13:17

zephyrbot added the area: File System label Sep 9, 2024

zephyrbot requested review from de-nordic, Laczen and nashif September 9, 2024 13:18

zephyrbot assigned de-nordic Sep 9, 2024

de-nordic requested a review from tomi-font September 9, 2024 14:39

rghaddab force-pushed the rghaddab/zms branch 2 times, most recently from 716277d to acdc1b8 Compare September 10, 2024 18:40

carlescufi requested a review from Damian-Nordic September 12, 2024 11:58