RFC: Make NVS filesystem NVS_BLOCK_SIZE parameter configurable

[andrisk-dev]

Introduction

I propose to update the NVS filesystem code to have the NVS_BLOCK_SIZE configurable in its Kconfig.

Problem description

The NVS_BLOCK_SIZE is currently is set as a predefined value in nvs source code. Both the smaller and bigger NVS_BLOCK_SIZE have its advantages and disadvantages and it currently cannot be changed without patching the NVS source code.

Proposed change

Add NVS_BLOCK_SIZE_LIMIT configuration to Kconfig and if it is defined, the NVS_BLOCK_SIZE would be equal to that value.

Detailed RFC

The NVS_BLOCK_SIZE prevents us from using flash memory which would have write_block_size larger than its value. Additionally the parameter also decides the size of buffers in the NVS functions - so it has an effect on the required stack size. Therefore larger value of NVS_BLOCK_SIZE wastes stack memory and smaller value limits the compatibility with some flash memories. I personally need this change for making the NVS example work on NXP lpc55Sxx internal flash (both write_block_size and erase_block_size = 512 bytes).

Proposed change (Detailed)

I plan to make following changes:

1. add new configuration to subsys/fs/nvs/Kconfig:

    config NVS_BLOCK_SIZE_LIMIT
        int "Sets maximum supported nvs block size"
        default 32
        range 8 512
        help
        Smaller number can decrease size requirements for buffers in nvs routines.
	  It can lower stack size requirements. The large value might be needed for
	  some flash memories - e.g. if using flash memory with larger write block size.

2. change the definition of NVS_BLOCK_SIZE in nvs_priv.c:

#ifdef CONFIG_NVS_BLOCK_SIZE_LIMIT
#define NVS_BLOCK_SIZE CONFIG_NVS_BLOCK_SIZE_LIMIT
#else
#define NVS_BLOCK_SIZE 32
#endif

3. improve the nvs example (samples\subsys\nvs\src\main.c) to handle correctly situation when write_block_size is the same as erase_block_size for example like this:

fs.sector_size = 5 * write_block_size > info.size ? 16 * info.size : info.size;

Dependencies

A fix in PR #77569 is needed for write_block_size larger than 255.

Concerns and Unresolved Questions

In general I created this RFC to find out id Zephyr team would consider this change as acceptable

I am aware that large write_block_size will cause the NVS to use flash memory inefficiently and the flash wear would increase significantly. But I think that even in case of as large size as 512 there are some limited use cases.

In case I was about to submit such PR I am unsure if I need to also create some new tests to cover possible block sizes not tested currently.

Alternatives

I did not find better solution yet - I think setting the value to 512 for all projects is unreasonable so from my point of view configurable value is the only solution for my use case (NXP lpc55sXX) and Kconfig seems to be the most suitable way for configuring the parameter.

[github-actions[bot]]

Hi @andrisk-dev! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[butok]

Just do it! ;)

[Laczen]

The NVS_BLOCK_SIZE is intentionally a non-configurable option to prevent users from creating a non efficient storage solution (it is already very inefficient when used with a allowed 32 byte write-block-size). Making it configurable with the intent to increase should not be done.

[butok]

A comment about non-efficiency can be added to the help, so a user will be informed. Its default value can be changed in a platform specific code. We need it for the LPC and MCXN family support.

[dleach02]

I understand trying to protect an uninformed user from configuring something that impacts their performance. I think we should be careful though in constraining designs because we don't trust the users to do what is right for their platform/products. With the current definition being expressed here, the NVS cannot be used on our chip. If you have an alternative solution we should consider then please present it.

We are not uninformed users here. We know the features/capabilities of our part and what we need to enable this.

If we are so concerned that we can't trust users with this kconfig then consider making it a hidden config that won't come up in the GUI. The SOC platform maintainer can set it as part of the default configuration for the SOC. I suspect that the change being proposed here should have zero impact on existing implementations.

[butok]

If we are so concerned that we can't trust users with this kconfig then consider making it a hidden config that won't come up in the GUI. The SOC platform maintainer can set it as part of the default configuration for the SOC. I suspect that the change being proposed here should have zero impact on existing implementations.

Hi @Laczen, do you agree with the suggested compromise of using it as a hidden configuration?

[Laczen]

I am not trying to protect an uninformed users (but yes they will be the first to complain about it). NVS was designed for flashes with small write-block-sizes (< 32 byte) and "small" erase-block-sizes (< 64kB sectors). These design limit have been part of NVS since it was introduced.

These design limits have been disturbing other platforms as well (e.g. ST devices with large erase-block-size).

Enabling configuration (hidden or not) of a design limit that has been carefully chosen is in effect ignoring this design limit. On top of ignoring the design limit enabling the configuration of the maximum write-block-size also results in shifting the problem from the SOC to NVS: complaints that nvs is inefficiently using flash will arise quickly (and as said in PR #77569 they cannot be solved).

A RFC can be made to request NVS to support larger write-block-sizes, from my side there are no plans to do that.

Does this mean that these platforms with large write-block-size have no alternative ? No, as said in PR #77569 these platforms are better off using littlefs as a storage solution. With this alternative you are better off than the devices with large erase-block-size as they even cannot use this.

[butok]

These design limits have been disturbing other platforms as well (e.g. ST devices with large erase-block-size).

We are able to fix these limits, just do not block the progress.

[Laczen]

These design limits have been disturbing other platforms as well (e.g. ST devices with large erase-block-size).

We are able to fix these limits, just do not block the progress.

That is a very strong promise. Please elaborate, if you can show the way to make nvs a efficient storage solution on large write-block-sizes I will be the first to support them. The current block is in no way influencing the progress, it is something that can be changed once the promises have been fulfilled.

[butok]

These design limits have been disturbing other platforms as well (e.g. ST devices with large erase-block-size).

We are able to fix these limits, just do not block the progress.

That is a very strong promise. Please elaborate, if you can show the way to make nvs a efficient storage solution on large write-block-sizes I will be the first to support them. The current block is in no way influencing the progress, it is something that can be changed once the promises have been fulfilled.

• We are able to fix the size limits, to make it functional for platforms >32B write size.
• We are aware about posible inefficiency for platforms >32B write size.
• Using a different FS cannot be an alternative for all cases because it has a different API and footprint. Users will thus be able to choose the best project solution from the full FS list.