Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel: Introduce a way to specify minimum system heap size #65908

Merged
merged 14 commits into from
Dec 20, 2023

Conversation

jhedberg
Copy link
Member

@jhedberg jhedberg commented Nov 29, 2023

There are several subsystems and boards which require a relatively large system heap (used by k_malloc()) to function properly. This became even more notable with the recent introduction of the ACPICA library, which causes ACPI-using boards to require a system heap of up to several megabytes in size.

Until now, subsystems and boards have tried to solve this by having Kconfig overlays which modify the default value of HEAP_MEM_POOL_SIZE. This works ok, except when applications start explicitly setting values in their prj.conf files:

$ git grep CONFIG_HEAP_MEM_POOL_SIZE= tests samples|wc -l
     157

The vast majority of values set by current sample or test applications is much too small for subsystems like ACPI, which results in the application not being able to run on such boards.

To solve this situation, we introduce support for subsystems to specify their own custom system heap size requirement. Subsystems do this by defining Kconfig options with the prefix HEAP_MEM_POOL_ADD_SIZE_. The final value of the system heap is the sum of the custom minimum requirements, or the value existing HEAP_MEM_POOL_SIZE option, whichever is greater.

We also introduce a new HEAP_MEM_POOL_IGNORE_MIN Kconfig option which applications can use to force a lower value than what subsystems have specficied, however this behavior is disabled by default.

Whenever the minimum is greater than the requested value a CMake warning will be issued in the build output.

This patch ends up modifying several places outside of kernel code, since the presence of the system heap is no longer detected using a non-zero CONFIG_HEAP_MEM_POOL_SIZE value, rather it's now detected using a new K_HEAP_MEM_POOL_SIZE value that's evaluated at build time.

Fixes #65722

Johan Hedberg added 9 commits December 18, 2023 21:19
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Kconfig options with a HEAP_MEM_POOL_ADD_SIZE_ prefix should be used to
set the minimum required system heap size. This helps prevent
applications from creating a non-working image by trying to set a too
small value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Use the new HEAP_MEM_POOL_ADD_SIZE_ prefix to construct a minimum
requirement for posix message queue usage. This way we can remove the
"special case" default values from the HEAP_MEM_POOL_SIZE Kconfig
definition.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
This is needed so that module uses K_HEAP_MEM_POOL_SIZE for determining
the availablility of the k_malloc family of functions.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
@jhedberg
Copy link
Member Author

Rebased, and updated the libmetal reference to point at zephyrproject-rtos/libmetal#25 instead of zephyrproject-rtos/libmetal#23

Johan Hedberg added 2 commits December 18, 2023 22:28
This is needed so that module uses K_HEAP_MEM_POOL_SIZE for determining
the availablility of the k_malloc family of functions.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Add a note about the new K_MEM_POOL_HEAP_SIZE define and the mechanism
for specifying custom system heap size requirements.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Dec 18, 2023
@jhedberg
Copy link
Member Author

..and now rebased again with the final libmetal SHA after zephyrproject-rtos/libmetal#25 was merged

Now all CI checks should (hopefully) be green.

@nordicjm thanks for the thorough analysis of places that most likely still require updating. I think those updates can be done gradually after this PR has been merged. The idea with having similar template support as we have for logging kconfig symbols sounds like a nice follow-up improvement as well.

@jhedberg jhedberg requested a review from carlescufi December 18, 2023 20:35
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Minor nit observed. Kconfig.defconfigs should generally not define settings, only adjust defaults of settings defined elsewhere.
But as this PR is changing HEAP_MEM_POOL_SIZE -> HEAP_MEM_POOL_ADD_SIZE, then moving this to the board's Kconfig can done as followup.

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now, thank you!

@nordic-krch
Copy link
Contributor

I know that it's merged already but it would be better if <option>_ADD_<subsys> pattern were followed (HEAP_MEM_POOL_SIZE_ADD_ instead of HEAP_MEM_POOL_ADD_SIZE_). I guess that this would allow to wrap cmake operation into a function. As you can see in #67854 same approach is taken for channel count where same cmake code need to be copied and modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There should be a way to declare a minimum value for CONFIG_HEAP_MEM_POOL_SIZE