diff --git a/doc/services/storage/zms/zms.rst b/doc/services/storage/zms/zms.rst index d1125e0505d..02fed3cf77c 100644 --- a/doc/services/storage/zms/zms.rst +++ b/doc/services/storage/zms/zms.rst @@ -201,9 +201,9 @@ An entry has 16 bytes divided between these variables : struct zms_ate { uint8_t crc8; /* crc8 check of the entry */ - uint8_t cycle_cnt; /* cycle counter for non erasable devices */ - uint32_t id; /* data id */ + 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 { @@ -218,21 +218,22 @@ An entry has 16 bytes divided between these variables : }; } __packed; -.. note:: 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. +.. note:: The CRC of the data is checked only when the whole the element is read. + The CRC of the data is not checked for a partial read, as it is computed for the whole element. -.. note:: Enabling the data CRC feature on a previously existing ZMS content without - data CRC will make all existing data invalid. +.. note:: Enabling the CRC feature on previously existing ZMS content without CRC enabled + will make all existing data invalid. .. _free-space: Available space for user data (key-value pairs) *********************************************** -For both scenarios ZMS should have always an empty sector to be able to perform the garbage -collection. -So if we suppose that 4 sectors exist in a partition, ZMS will only use 3 sectors to store -Key-value pairs and keep always one (rotating sector) empty to be able to launch GC. +For both scenarios ZMS should always have 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. +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) @@ -240,8 +241,8 @@ Key-value pairs and keep always one (rotating sector) empty to be able to launch Small data values ================= -For small data values (<= 8 bytes), the data is stored within the entry (ATE) itself and no data -is written at the top of the sector. +Values smaller than 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 : @@ -265,7 +266,7 @@ Large data values ================= Large data values ( > 8 bytes) are stored separately at the top of the sector. -In this case it is hard to estimate the free available space as this depends on the size of +In this case, it is hard to estimate the free available space, as this depends on the size of the data. But we can take into account that for N bytes of data (N > 8 bytes) an additional 16 bytes of ATE must be added at the bottom of the sector. @@ -286,17 +287,17 @@ 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 introduces a cycle count mechanism that avoids emulating erase operation for these devices. +ZMS uses a cycle count mechanism that avoids emulating erase operation 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 +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. 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. To make the garbage collector not affect the life expectancy of the device it is recommended -to dimension correctly the partition size. Its size should be the double of the maximum size of +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. See :ref:`free-space`. @@ -307,10 +308,10 @@ 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. 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 +memory cells cannot be overwritten) and for non-erasable storage devices memory cells can be overwritten directly. -A typical scenario is shown here to calculate the life expectancy of a device. +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 minute. The partition has 4 sectors with 1024 bytes each. Each write of the variable requires 16 bytes of storage. @@ -361,9 +362,9 @@ Existing features ================= Version1 -------- -- Supports non erasable devices (only one write operation to erase a sector) +- Supports non-erasable devices (only one write operation to erase a sector) - Supports large partition size and sector size (64 bits address space) -- Supports large IDs width (32 bits) to store ID/Value pairs +- 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) - Versionning of ZMS (to handle future evolution) @@ -375,7 +376,7 @@ Future features - Add multiple format ATE support to be able to use ZMS with different ATE formats that satisfies requirements from application - Add the possibility to skip garbage collector for some application usage where ID/value pairs - are written periodically and do not exceed half of the partition size (ther is always an old + are written periodically and do not exceed half of the partition size (there is always an old entry with the same ID). - Divide IDs into namespaces and allocate IDs on demand from application to handle collisions between IDs used by different subsystems or samples. @@ -394,9 +395,9 @@ 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 - best fit for your storage subsystem as it is designed very well to avoid emulating erase for - these devices and replace it by a single write call. +- If you are using a non-erasable technology device 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 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. @@ -414,6 +415,41 @@ verified to make sure that the application could work with one subsystem or the 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`. +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. + 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 + 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. +- 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. + 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 +================== + +- 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. +- 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 ****** diff --git a/include/zephyr/fs/zms.h b/include/zephyr/fs/zms.h index 1155319d792..0f0fbb82cc9 100644 --- a/include/zephyr/fs/zms.h +++ b/include/zephyr/fs/zms.h @@ -1,14 +1,14 @@ -/* ZMS: Zephyr Memory Storage - * - * Copyright (c) 2024 BayLibre SAS +/* Copyright (c) 2024 BayLibre SAS * * SPDX-License-Identifier: Apache-2.0 + * + * ZMS: Zephyr Memory Storage */ #ifndef ZEPHYR_INCLUDE_FS_ZMS_H_ #define ZEPHYR_INCLUDE_FS_ZMS_H_ -#include #include +#include #include #include #include @@ -18,7 +18,6 @@ extern "C" { #endif /** - * @brief Zephyr Memory Storage (ZMS) * @defgroup zms Zephyr Memory Storage (ZMS) * @ingroup file_system_storage * @{ @@ -26,37 +25,34 @@ extern "C" { */ /** - * @brief Zephyr Memory Storage Data Structures - * @defgroup zms_data_structures Zephyr Memory Storage Data Structures + * @defgroup zms_data_structures ZMS data structures * @ingroup zms * @{ */ -/** - * @brief Zephyr Memory Storage File system structure - */ +/** Zephyr Memory Storage file system structure */ struct zms_fs { - /** File system offset in flash **/ + /** File system offset in flash */ off_t offset; - /** Allocation table entry write address. - * Addresses are stored as uint64_t: + /** Allocation Table Entry (ATE) write address. + * Addresses are stored as `uint64_t`: * - high 4 bytes correspond to the sector * - low 4 bytes are the offset in the sector */ uint64_t ate_wra; /** Data write address */ uint64_t data_wra; - /** Storage system is split into sectors, each sector size must be multiple of erase-blocks - * if the device has erase capabilities + /** Storage system is split into sectors. The sector size must be a multiple of + * `erase-block-size` if the device has erase capabilities */ uint32_t sector_size; /** Number of sectors in the file system */ uint32_t sector_count; - /** Current cycle counter of the active sector (pointed by ate_wra)*/ + /** Current cycle counter of the active sector (pointed to by `ate_wra`) */ uint8_t sector_cycle; /** Flag indicating if the file system is initialized */ bool ready; - /** Mutex */ + /** Mutex used to lock flash writes */ struct k_mutex zms_lock; /** Flash device runtime structure */ const struct device *flash_device; @@ -65,7 +61,7 @@ struct zms_fs { /** Size of an Allocation Table Entry */ size_t ate_size; #if CONFIG_ZMS_LOOKUP_CACHE - /** Lookup table used to cache ATE address of a written ID */ + /** Lookup table used to cache ATE addresses of written IDs */ uint64_t lookup_cache[CONFIG_ZMS_LOOKUP_CACHE_SIZE]; #endif }; @@ -75,78 +71,77 @@ struct zms_fs { */ /** - * @brief Zephyr Memory Storage APIs - * @defgroup zms_high_level_api Zephyr Memory Storage APIs + * @defgroup zms_high_level_api ZMS API * @ingroup zms * @{ */ /** - * @brief Mount a ZMS file system onto the device specified in @p fs. + * @brief Mount a ZMS file system onto the device specified in `fs`. * - * @param fs Pointer to file system + * @param fs Pointer to the file system. * @retval 0 Success - * @retval -ERRNO errno code if error + * @retval -ERRNO Negative errno code on error */ int zms_mount(struct zms_fs *fs); /** * @brief Clear the ZMS file system from device. * - * @param fs Pointer to file system + * @param fs Pointer to the file system. * @retval 0 Success - * @retval -ERRNO errno code if error + * @retval -ERRNO Negative errno code on error */ int zms_clear(struct zms_fs *fs); /** * @brief Write an entry to the file system. * - * @note When @p len parameter is equal to @p 0 then entry is effectively removed (it is - * equivalent to calling of zms_delete). It is not possible to distinguish between a deleted + * @note When the `len` parameter is equal to `0` the entry is effectively removed (it is + * equivalent to calling @ref zms_delete()). It is not possible to distinguish between a deleted * entry and an entry with data of length 0. * - * @param fs Pointer to file system - * @param id Id of the entry to be written + * @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 KB) + * @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. 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 errno.h defined error codes. + * 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`. */ 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 file system - * @param id Id of the entry to be deleted + * @param fs Pointer to the file system. + * @param id ID of the entry to be deleted * @retval 0 Success - * @retval -ERRNO errno code if error + * @retval -ERRNO Negative errno code on error */ int zms_delete(struct zms_fs *fs, uint32_t id); /** * @brief Read an entry from the file system. * - * @param fs Pointer to file system - * @param id Id of the entry to be read + * @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 (or size of the allocated read 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. When the return value is less than the number of bytes requested to read this - * indicates that ATE contain less data than requested. On error, returns negative value of - * errno.h defined error codes. + * 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`. */ 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 file system - * @param id Id of the entry to be read + * @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 cnt History counter: 0: latest entry, 1: one before latest ... @@ -154,40 +149,41 @@ ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len); * @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 errno.h defined error codes. + * value of error codes defined in `errno.h`. */ ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt); /** - * @brief Gets the data size 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 file system - * @param id Id of the entry that we want to get its data length + * @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 errno.h defined error codes. + * in the ATE. On error, returns negative value of error codes defined in `errno.h`. */ ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id); + /** * @brief Calculate the available free space in the file system. * - * @param fs Pointer to file system + * @param fs Pointer to the file system. * - * @return Number of bytes free. On success, it will be equal to the number of bytes that can + * @return Number of free bytes. On success, it will be equal to the number of bytes that can * 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 errno.h defined error codes. + * 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`. */ ssize_t zms_calc_free_space(struct zms_fs *fs); /** - * @brief Tell how many contiguous free space remains in the currently active ZMS sector. + * @brief Tell how much contiguous free space remains in the currently active ZMS sector. * * @param fs Pointer to the file system. * * @return Number of free bytes. */ -size_t zms_sector_max_data_size(struct zms_fs *fs); +size_t zms_active_sector_free_space(struct zms_fs *fs); /** * @brief Close the currently active sector and switch to the next one. @@ -195,12 +191,12 @@ size_t zms_sector_max_data_size(struct zms_fs *fs); * @note The garbage collector is called on the new sector. * * @warning This routine is made available for specific use cases. - * It collides with the ZMS goal of avoiding any unnecessary flash erase operations. + * It collides with ZMS's goal of avoiding any unnecessary flash erase operations. * Using this routine extensively can result in premature failure of the flash device. * * @param fs Pointer to the file system. * - * @return 0 on success. On error, returns negative value of errno.h defined error codes. + * @return 0 on success. On error, returns negative value of error codes defined in `errno.h`. */ int zms_sector_use_next(struct zms_fs *fs); diff --git a/samples/subsys/fs/zms/src/main.c b/samples/subsys/fs/zms/src/main.c index a2166392724..959d5ac5f3e 100644 --- a/samples/subsys/fs/zms/src/main.c +++ b/samples/subsys/fs/zms/src/main.c @@ -83,7 +83,8 @@ int main(void) int rc = 0; char buf[16]; uint8_t key[8] = {0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF}, longarray[128]; - uint32_t i_cnt = 0U, i; + uint32_t i_cnt = 0U; + uint32_t i; uint32_t id = 0; ssize_t free_space = 0; struct flash_pages_info info; @@ -144,7 +145,7 @@ int main(void) rc = zms_read(&fs, KEY_VALUE_ID, &key, sizeof(key)); if (rc > 0) { /* item was found, show it */ printk("Id: %x, Key: ", KEY_VALUE_ID); - for (int n = 0; n < 8; n++) { + for (uint8_t n = 0; n < 8; n++) { printk("%x ", key[n]); } printk("\n"); @@ -181,7 +182,7 @@ int main(void) if (rc > 0) { /* item was found, show it */ printk("Id: %d, Longarray: ", LONG_DATA_ID); - for (int n = 0; n < sizeof(longarray); n++) { + for (uint16_t n = 0; n < sizeof(longarray); n++) { printk("%x ", longarray[n]); } printk("\n"); @@ -204,7 +205,7 @@ int main(void) } if (i != MAX_ITERATIONS) { - printk("Error: Something went wrong at iteration %u rc=%d\n", i - 1, rc); + printk("Error: Something went wrong at iteration %u rc=%d\n", i, rc); return 0; } @@ -249,7 +250,7 @@ int main(void) * Let's compute free space in storage. But before doing that let's Garbage collect * all sectors where we deleted all entries and then compute the free space */ - for (uint32_t i = 0; i < fs.sector_count; i++) { + for (i = 0; i < fs.sector_count; i++) { rc = zms_sector_use_next(&fs); if (rc) { printk("Error while changing sector rc=%d\n", rc); @@ -261,6 +262,13 @@ int main(void) return 0; } printk("Free space in storage is %u bytes\n", free_space); + + /* Let's clean the storage now */ + rc = zms_clear(&fs); + if (rc < 0) { + printk("Error while cleaning the storage, rc=%d\n", rc); + } + printk("Sample code finished Successfully\n"); return 0; diff --git a/subsys/fs/zms/Kconfig b/subsys/fs/zms/Kconfig index 42f23905e09..a2b060dfb4b 100644 --- a/subsys/fs/zms/Kconfig +++ b/subsys/fs/zms/Kconfig @@ -1,9 +1,9 @@ -#Zephyr Memory Storage ZMS - #Copyright (c) 2024 BayLibre SAS #SPDX-License-Identifier: Apache-2.0 +#Zephyr Memory Storage ZMS + config ZMS bool "Zephyr Memory Storage" select CRC @@ -34,19 +34,19 @@ config ZMS_DATA_CRC help Enables DATA CRC -config ZMS_CUSTOM_BLOCK_SIZE - bool "Custom buffer size used by ZMS for reads and writes" +config ZMS_CUSTOMIZE_BLOCK_SIZE + bool "Customize the size of the buffer used internally for reads and writes" help - ZMS uses internal buffers to read/write and compare stored data. - Increasing the size of these buffers should be done carefully in order to not + 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 -config ZMS_MAX_BLOCK_SIZE +config ZMS_CUSTOM_BLOCK_SIZE int "ZMS internal buffer size" default 32 - depends on ZMS_CUSTOM_BLOCK_SIZE + depends on ZMS_CUSTOMIZE_BLOCK_SIZE help Changes the internal buffer size of ZMS diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 814a08a8839..f0f22c59735 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -1,8 +1,8 @@ -/* ZMS: Zephyr Memory Storage - * - * Copyright (c) 2024 BayLibre SAS +/* Copyright (c) 2024 BayLibre SAS * * SPDX-License-Identifier: Apache-2.0 + * + * ZMS: Zephyr Memory Storage */ #include @@ -87,8 +87,10 @@ static inline size_t zms_lookup_cache_pos(uint32_t id) static int zms_lookup_cache_rebuild(struct zms_fs *fs) { - int rc, previous_sector_num = ZMS_INVALID_SECTOR_NUM; - uint64_t addr, ate_addr; + int rc; + int previous_sector_num = ZMS_INVALID_SECTOR_NUM; + uint64_t addr; + uint64_t ate_addr; uint64_t *cache_entry; uint8_t current_cycle; struct zms_ate ate; @@ -155,6 +157,19 @@ static inline off_t zms_addr_to_offset(struct zms_fs *fs, uint64_t addr) return fs->offset + (fs->sector_size * SECTOR_NUM(addr)) + SECTOR_OFFSET(addr); } +/* Helper to round down len to the closest multiple of write_block_size */ +static inline size_t zms_round_down_write_block_size(struct zms_fs *fs, size_t len) +{ + return len & ~(fs->flash_parameters->write_block_size - 1U); +} + +/* Helper to round up len to multiple of write_block_size */ +static inline size_t zms_round_up_write_block_size(struct zms_fs *fs, size_t len) +{ + return (len + (fs->flash_parameters->write_block_size - 1U)) & + ~(fs->flash_parameters->write_block_size - 1U); +} + /* zms_al_size returns size aligned to fs->write_block_size */ static inline size_t zms_al_size(struct zms_fs *fs, size_t len) { @@ -163,7 +178,8 @@ static inline size_t zms_al_size(struct zms_fs *fs, size_t len) if (write_block_size <= 1U) { return len; } - return (len + (write_block_size - 1U)) & ~(write_block_size - 1U); + + return zms_round_up_write_block_size(fs, len); } /* Helper to get empty ATE address */ @@ -194,7 +210,7 @@ static int zms_flash_al_wrt(struct zms_fs *fs, uint64_t addr, const void *data, offset = zms_addr_to_offset(fs, addr); - blen = len & ~(fs->flash_parameters->write_block_size - 1U); + blen = zms_round_down_write_block_size(fs, len); if (blen > 0) { rc = flash_write(fs->flash_device, offset, data8, blen); if (rc) { @@ -276,10 +292,11 @@ static int zms_flash_block_cmp(struct zms_fs *fs, uint64_t addr, const void *dat { const uint8_t *data8 = (const uint8_t *)data; int rc; - size_t bytes_to_cmp, block_size; + size_t bytes_to_cmp; + size_t block_size; uint8_t buf[ZMS_BLOCK_SIZE]; - block_size = ZMS_BLOCK_SIZE & ~(fs->flash_parameters->write_block_size - 1U); + block_size = zms_round_down_write_block_size(fs, ZMS_BLOCK_SIZE); while (len) { bytes_to_cmp = MIN(block_size, len); @@ -305,10 +322,11 @@ static int zms_flash_block_cmp(struct zms_fs *fs, uint64_t addr, const void *dat static int zms_flash_cmp_const(struct zms_fs *fs, uint64_t addr, uint8_t value, size_t len) { int rc; - size_t bytes_to_cmp, block_size; + size_t bytes_to_cmp; + size_t block_size; uint8_t cmp[ZMS_BLOCK_SIZE]; - block_size = ZMS_BLOCK_SIZE & ~(fs->flash_parameters->write_block_size - 1U); + block_size = zms_round_down_write_block_size(fs, ZMS_BLOCK_SIZE); (void)memset(cmp, value, block_size); while (len) { @@ -329,10 +347,11 @@ static int zms_flash_cmp_const(struct zms_fs *fs, uint64_t addr, uint8_t value, static int zms_flash_block_move(struct zms_fs *fs, uint64_t addr, size_t len) { int rc; - size_t bytes_to_copy, block_size; + size_t bytes_to_copy; + size_t block_size; uint8_t buf[ZMS_BLOCK_SIZE]; - block_size = ZMS_BLOCK_SIZE & ~(fs->flash_parameters->write_block_size - 1U); + block_size = zms_round_down_write_block_size(fs, ZMS_BLOCK_SIZE); while (len) { bytes_to_copy = MIN(block_size, len); @@ -416,17 +435,17 @@ static int zms_ate_crc8_check(const struct zms_ate *entry) return 1; } -/* zms_ate_valid validates an ate: - * return 1 if crc8 and cycle_cnt valid, - * 0 otherwise +/* zms_ate_valid validates an ate in the current sector by checking if the ate crc is valid + * and its cycle cnt matches the cycle cnt of the active sector + * + * return 1 if ATE is valid, + * 0 otherwise + * + * see: zms_ate_valid_different_sector */ static int zms_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) { - if ((fs->sector_cycle != entry->cycle_cnt) || zms_ate_crc8_check(entry)) { - return 0; - } - - return 1; + return zms_ate_valid_different_sector(fs, entry, fs->sector_cycle); } /* zms_ate_valid_different_sector validates an ate that is in a different @@ -467,10 +486,11 @@ static inline int zms_get_cycle_on_sector_change(struct zms_fs *fs, uint64_t add return 0; } -/* zms_close_ate_valid validates an sector close ate: a valid sector close ate: - * - valid ate - * - len = 0 and id = ZMS_HEAD_ID - * - offset points to location at ate multiple from sector size +/* zms_close_ate_valid validates a sector close ate. + * A valid sector close ate should be: + * - a valid ate + * - with len = 0 and id = ZMS_HEAD_ID + * - and offset points to location at ate multiple from sector size * return true if valid, false otherwise */ static bool zms_close_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) @@ -479,9 +499,10 @@ static bool zms_close_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) (entry->id == ZMS_HEAD_ID) && !((fs->sector_size - entry->offset) % fs->ate_size)); } -/* zms_empty_ate_valid validates an sector empty ate: a valid sector empty ate: - * - valid ate - * - len = 0xffff and id = 0xffffffff +/* zms_empty_ate_valid validates an sector empty ate. + * A valid sector empty ate should be: + * - a valid ate + * - with len = 0xffff and id = 0xffffffff * return true if valid, false otherwise */ static bool zms_empty_ate_valid(struct zms_fs *fs, const struct zms_ate *entry) @@ -576,7 +597,8 @@ static int zms_flash_write_entry(struct zms_fs *fs, uint32_t id, const void *dat */ static int zms_recover_last_ate(struct zms_fs *fs, uint64_t *addr, uint64_t *data_wra) { - uint64_t data_end_addr, ate_end_addr; + uint64_t data_end_addr; + uint64_t ate_end_addr; struct zms_ate end_ate; int rc; @@ -614,7 +636,8 @@ static int zms_recover_last_ate(struct zms_fs *fs, uint64_t *addr, uint64_t *dat static int zms_compute_prev_addr(struct zms_fs *fs, uint64_t *addr) { int sec_closed; - struct zms_ate empty_ate, close_ate; + struct zms_ate empty_ate; + struct zms_ate close_ate; *addr += fs->ate_size; if ((SECTOR_OFFSET(*addr)) != (fs->sector_size - 2 * fs->ate_size)) { @@ -677,7 +700,8 @@ static void zms_sector_advance(struct zms_fs *fs, uint64_t *addr) static int zms_sector_close(struct zms_fs *fs) { int rc; - struct zms_ate close_ate, garbage_ate; + struct zms_ate close_ate; + struct zms_ate garbage_ate; close_ate.id = ZMS_HEAD_ID; close_ate.len = 0U; @@ -851,7 +875,8 @@ static int zms_find_ate_with_id(struct zms_fs *fs, uint32_t id, uint64_t start_a { int rc; int previous_sector_num = ZMS_INVALID_SECTOR_NUM; - uint64_t wlk_prev_addr, wlk_addr; + uint64_t wlk_prev_addr; + uint64_t wlk_addr; int prev_found = 0; struct zms_ate wlk_ate; uint8_t current_cycle; @@ -893,9 +918,19 @@ static int zms_find_ate_with_id(struct zms_fs *fs, uint32_t id, uint64_t start_a */ static int zms_gc(struct zms_fs *fs) { - int rc, sec_closed; - struct zms_ate close_ate, gc_ate, wlk_ate, empty_ate; - uint64_t sec_addr, gc_addr, gc_prev_addr, wlk_addr, wlk_prev_addr, data_addr, stop_addr; + int rc; + int sec_closed; + struct zms_ate close_ate; + struct zms_ate gc_ate; + struct zms_ate wlk_ate; + struct zms_ate empty_ate; + uint64_t sec_addr; + uint64_t gc_addr; + uint64_t gc_prev_addr; + uint64_t wlk_addr; + uint64_t wlk_prev_addr; + uint64_t data_addr; + uint64_t stop_addr; uint8_t previous_cycle = 0; rc = zms_get_sector_cycle(fs, fs->ate_wra, &fs->sector_cycle); @@ -1072,14 +1107,16 @@ int zms_clear(struct zms_fs *fs) static int zms_init(struct zms_fs *fs) { - int rc, sec_closed; - struct zms_ate last_ate, first_ate, close_ate, empty_ate; - /* Initialize addr to 0 for the case fs->sector_count == 0. This - * should never happen as this is verified in zms_mount() but both - * Coverity and GCC believe the contrary. - */ - uint64_t addr = 0U, data_wra = 0U; - uint32_t i, closed_sectors = 0; + int rc; + int sec_closed; + struct zms_ate last_ate; + struct zms_ate first_ate; + struct zms_ate close_ate; + struct zms_ate empty_ate; + uint64_t addr = 0U; + uint64_t data_wra = 0U; + uint32_t i; + uint32_t closed_sectors = 0; bool zms_magic_exist = false; k_mutex_lock(&fs->zms_lock, K_FOREVER); @@ -1330,7 +1367,6 @@ static int zms_init(struct zms_fs *fs) int zms_mount(struct zms_fs *fs) { - int rc; struct flash_pages_info info; size_t write_block_size; @@ -1344,7 +1380,7 @@ int zms_mount(struct zms_fs *fs) } fs->ate_size = zms_al_size(fs, sizeof(struct zms_ate)); - write_block_size = flash_get_write_block_size(fs->flash_device); + write_block_size = fs->flash_parameters->write_block_size; /* check that the write block size is supported */ if (write_block_size > ZMS_BLOCK_SIZE || write_block_size == 0) { @@ -1402,8 +1438,10 @@ ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len) int rc; size_t data_size; struct zms_ate wlk_ate; - uint64_t wlk_addr, rd_addr; - uint32_t gc_count, required_space = 0U; /* no space, appropriate for delete ate */ + uint64_t wlk_addr; + uint64_t rd_addr; + uint32_t gc_count; + uint32_t required_space = 0U; /* no space, appropriate for delete ate */ int prev_found = 0; if (!fs->ready) { @@ -1543,8 +1581,11 @@ int zms_delete(struct zms_fs *fs, uint32_t id) ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt) { - int rc, prev_found = 0; - uint64_t wlk_addr, rd_addr = 0, wlk_prev_addr = 0; + int rc; + int prev_found = 0; + uint64_t wlk_addr; + uint64_t rd_addr = 0; + uint64_t wlk_prev_addr = 0; uint32_t cnt_his; struct zms_ate wlk_ate; #ifdef CONFIG_ZMS_DATA_CRC @@ -1659,12 +1700,22 @@ ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id) ssize_t zms_calc_free_space(struct zms_fs *fs) { - - int rc, previous_sector_num = ZMS_INVALID_SECTOR_NUM, prev_found = 0, sec_closed; - struct zms_ate step_ate, wlk_ate, empty_ate, close_ate; - uint64_t step_addr, wlk_addr, step_prev_addr, wlk_prev_addr, data_wra = 0U; + int rc; + int previous_sector_num = ZMS_INVALID_SECTOR_NUM; + int prev_found = 0; + int sec_closed; + struct zms_ate step_ate; + struct zms_ate wlk_ate; + struct zms_ate empty_ate; + struct zms_ate close_ate; + uint64_t step_addr; + uint64_t wlk_addr; + uint64_t step_prev_addr; + uint64_t wlk_prev_addr; + uint64_t data_wra = 0U; uint8_t current_cycle; ssize_t free_space = 0; + const uint32_t second_to_last_offset = (2 * fs->ate_size); if (!fs->ready) { LOG_ERR("zms not initialized"); @@ -1728,9 +1779,8 @@ ssize_t zms_calc_free_space(struct zms_fs *fs) /* Let's look now for special cases where some sectors have only ATEs with * small data size. */ - const uint32_t second_to_last_offset = (2 * fs->ate_size); - for (uint32_t i = 0; i < fs->sector_count; i++) { + for (int i = 0; i < fs->sector_count; i++) { step_addr = zms_close_ate_addr(fs, ((uint64_t)i << ADDR_SECT_SHIFT)); /* verify if the sector is closed */ @@ -1763,7 +1813,7 @@ ssize_t zms_calc_free_space(struct zms_fs *fs) return free_space; } -size_t zms_sector_max_data_size(struct zms_fs *fs) +size_t zms_active_sector_free_space(struct zms_fs *fs) { if (!fs->ready) { LOG_ERR("ZMS not initialized"); diff --git a/subsys/fs/zms/zms_priv.h b/subsys/fs/zms/zms_priv.h index 6594048ea0f..428ff6babca 100644 --- a/subsys/fs/zms/zms_priv.h +++ b/subsys/fs/zms/zms_priv.h @@ -1,9 +1,10 @@ -/* ZMS: Zephyr Memory Storage - * - * Copyright (c) 2024 BayLibre SAS +/* Copyright (c) 2024 BayLibre SAS * * SPDX-License-Identifier: Apache-2.0 + * + * ZMS: Zephyr Memory Storage */ + #ifndef __ZMS_PRIV_H_ #define __ZMS_PRIV_H_ @@ -23,8 +24,8 @@ extern "C" { #define SECTOR_NUM(x) FIELD_GET(ADDR_SECT_MASK, x) #define SECTOR_OFFSET(x) FIELD_GET(ADDR_OFFS_MASK, x) -#if defined(CONFIG_ZMS_CUSTOM_BLOCK_SIZE) -#define ZMS_BLOCK_SIZE CONFIG_ZMS_MAX_BLOCK_SIZE +#if defined(CONFIG_ZMS_CUSTOMIZE_BLOCK_SIZE) +#define ZMS_BLOCK_SIZE CONFIG_ZMS_CUSTOM_BLOCK_SIZE #else #define ZMS_BLOCK_SIZE 32 #endif @@ -46,8 +47,8 @@ extern "C" { struct zms_ate { uint8_t crc8; /* crc8 check of the entry */ uint8_t cycle_cnt; /* cycle counter for non erasable devices */ - uint32_t id; /* data id */ uint16_t len; /* data len within sector */ + uint32_t id; /* data id */ union { uint8_t data[8]; /* used to store small size data */ struct { diff --git a/tests/subsys/fs/zms/src/main.c b/tests/subsys/fs/zms/src/main.c index 80866687dba..bd4e21dd745 100644 --- a/tests/subsys/fs/zms/src/main.c +++ b/tests/subsys/fs/zms/src/main.c @@ -233,8 +233,7 @@ ZTEST_F(zms, test_zms_gc) int len; uint8_t buf[32]; uint8_t rd_buf[32]; - - const uint16_t max_id = 10; + const uint8_t max_id = 10; /* 21st write will trigger GC. */ const uint16_t max_writes = 21; @@ -243,7 +242,7 @@ ZTEST_F(zms, test_zms_gc) err = zms_mount(&fixture->fs); zassert_true(err == 0, "zms_mount call failure: %d", err); - for (uint32_t i = 0; i < max_writes; i++) { + for (int i = 0; i < max_writes; i++) { uint8_t id = (i % max_id); uint8_t id_data = id + max_id * (i / max_id); @@ -253,11 +252,11 @@ ZTEST_F(zms, test_zms_gc) zassert_true(len == sizeof(buf), "zms_write failed: %d", len); } - for (uint32_t id = 0; id < max_id; id++) { + for (int id = 0; id < max_id; id++) { len = zms_read(&fixture->fs, id, rd_buf, sizeof(buf)); zassert_true(len == sizeof(rd_buf), "zms_read unexpected failure: %d", len); - for (uint16_t i = 0; i < sizeof(rd_buf); i++) { + for (int i = 0; i < sizeof(rd_buf); i++) { rd_buf[i] = rd_buf[i] % max_id; buf[i] = id; } @@ -268,11 +267,11 @@ ZTEST_F(zms, test_zms_gc) err = zms_mount(&fixture->fs); zassert_true(err == 0, "zms_mount call failure: %d", err); - for (uint32_t id = 0; id < max_id; id++) { + for (int id = 0; id < max_id; id++) { len = zms_read(&fixture->fs, id, rd_buf, sizeof(buf)); zassert_true(len == sizeof(rd_buf), "zms_read unexpected failure: %d", len); - for (uint16_t i = 0; i < sizeof(rd_buf); i++) { + for (int i = 0; i < sizeof(rd_buf); i++) { rd_buf[i] = rd_buf[i] % max_id; buf[i] = id; } @@ -286,7 +285,7 @@ static void write_content(uint32_t max_id, uint32_t begin, uint32_t end, struct uint8_t buf[32]; ssize_t len; - for (uint32_t i = begin; i < end; i++) { + for (int i = begin; i < end; i++) { uint8_t id = (i % max_id); uint8_t id_data = id + max_id * (i / max_id); @@ -303,11 +302,11 @@ static void check_content(uint32_t max_id, struct zms_fs *fs) uint8_t buf[32]; ssize_t len; - for (uint32_t id = 0; id < max_id; id++) { + for (int id = 0; id < max_id; id++) { len = zms_read(fs, id, rd_buf, sizeof(buf)); zassert_true(len == sizeof(rd_buf), "zms_read unexpected failure: %d", len); - for (uint16_t i = 0; i < ARRAY_SIZE(rd_buf); i++) { + for (int i = 0; i < ARRAY_SIZE(rd_buf); i++) { rd_buf[i] = rd_buf[i] % max_id; buf[i] = id; } @@ -322,7 +321,6 @@ static void check_content(uint32_t max_id, struct zms_fs *fs) ZTEST_F(zms, test_zms_gc_3sectors) { int err; - const uint16_t max_id = 10; /* 41st write will trigger 1st GC. */ const uint16_t max_writes = 41; @@ -410,7 +408,6 @@ ZTEST_F(zms, test_zms_corrupted_sector_close_operation) uint32_t *flash_write_stat; uint32_t *flash_max_write_calls; uint32_t *flash_max_len; - const uint16_t max_id = 10; /* 21st write will trigger GC. */ const uint16_t max_writes = 21; @@ -423,7 +420,7 @@ ZTEST_F(zms, test_zms_corrupted_sector_close_operation) err = zms_mount(&fixture->fs); zassert_true(err == 0, "zms_mount call failure: %d", err); - for (uint32_t i = 0; i < max_writes; i++) { + for (int i = 0; i < max_writes; i++) { uint8_t id = (i % max_id); uint8_t id_data = id + max_id * (i / max_id); @@ -465,7 +462,7 @@ ZTEST_F(zms, test_zms_full_sector) int err; ssize_t len; uint32_t filling_id = 0; - uint32_t i, data_read; + uint32_t data_read; fixture->fs.sector_count = 3; @@ -493,7 +490,7 @@ ZTEST_F(zms, test_zms_full_sector) zassert_true(len == sizeof(filling_id), "zms_write failed: %d", len); /* sanitycheck on ZMS content */ - for (i = 0; i <= filling_id; i++) { + for (int i = 0; i <= filling_id; i++) { len = zms_read(&fixture->fs, i, &data_read, sizeof(data_read)); if (i == 1) { zassert_true(len == -ENOENT, "zms_read shouldn't found the entry: %d", len); @@ -511,8 +508,10 @@ ZTEST_F(zms, test_delete) { int err; ssize_t len; - uint32_t filling_id, data_read; - uint32_t ate_wra, data_wra; + uint32_t filling_id; + uint32_t data_read; + uint32_t ate_wra; + uint32_t data_wra; fixture->fs.sector_count = 3; @@ -570,7 +569,9 @@ ZTEST_F(zms, test_delete) */ ZTEST_F(zms, test_zms_gc_corrupt_close_ate) { - struct zms_ate ate, close_ate, empty_ate; + struct zms_ate ate; + struct zms_ate close_ate; + struct zms_ate empty_ate; uint32_t data; ssize_t len; int err; @@ -642,7 +643,8 @@ ZTEST_F(zms, test_zms_gc_corrupt_close_ate) */ ZTEST_F(zms, test_zms_gc_corrupt_ate) { - struct zms_ate corrupt_ate, close_ate; + struct zms_ate corrupt_ate; + struct zms_ate close_ate; int err; close_ate.id = 0xffffffff; @@ -685,10 +687,10 @@ ZTEST_F(zms, test_zms_gc_corrupt_ate) #ifdef CONFIG_ZMS_LOOKUP_CACHE static size_t num_matching_cache_entries(uint64_t addr, bool compare_sector_only, struct zms_fs *fs) { - size_t i, num = 0; + size_t num = 0; uint64_t mask = compare_sector_only ? ADDR_SECT_MASK : UINT64_MAX; - for (i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { + for (int i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { if ((fs->lookup_cache[i] & mask) == addr) { num++; } @@ -759,20 +761,19 @@ ZTEST_F(zms, test_zms_cache_collission) { #ifdef CONFIG_ZMS_LOOKUP_CACHE int err; - uint32_t id; uint16_t data; fixture->fs.sector_count = 4; err = zms_mount(&fixture->fs); zassert_true(err == 0, "zms_mount call failure: %d", err); - for (id = 0; id < CONFIG_ZMS_LOOKUP_CACHE_SIZE + 1; id++) { + for (int id = 0; id < CONFIG_ZMS_LOOKUP_CACHE_SIZE + 1; id++) { data = id; err = zms_write(&fixture->fs, id, &data, sizeof(data)); zassert_equal(err, sizeof(data), "zms_write call failure: %d", err); } - for (id = 0; id < CONFIG_ZMS_LOOKUP_CACHE_SIZE + 1; id++) { + for (int id = 0; id < CONFIG_ZMS_LOOKUP_CACHE_SIZE + 1; id++) { err = zms_read(&fixture->fs, id, &data, sizeof(data)); zassert_equal(err, sizeof(data), "zms_read call failure: %d", err); zassert_equal(data, id, "incorrect data read"); @@ -846,7 +847,7 @@ ZTEST_F(zms, test_zms_cache_hash_quality) /* Write ZMS IDs from 0 to CONFIG_ZMS_LOOKUP_CACHE_SIZE - 1 */ - for (uint16_t i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { + for (int i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { id = i; data = 0; @@ -869,7 +870,7 @@ ZTEST_F(zms, test_zms_cache_hash_quality) /* Write CONFIG_ZMS_LOOKUP_CACHE_SIZE ZMS IDs that form the following series: 0, 4, 8... */ - for (uint16_t i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { + for (int i = 0; i < CONFIG_ZMS_LOOKUP_CACHE_SIZE; i++) { id = i * 4; data = 0;