From 3fbf12487c6c01c488210bc56b0f342f07098df9 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Wed, 29 Nov 2023 11:22:39 +0200 Subject: [PATCH] kernel: Introduce a way to specify minimum system heap size 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. Signed-off-by: Johan Hedberg --- arch/xtensa/core/ptables.c | 2 +- doc/kernel/memory_management/heap.rst | 12 +++++++ include/zephyr/kernel.h | 4 +-- kernel/CMakeLists.txt | 32 ++++++++++++++++++- kernel/Kconfig | 20 ++++++++++-- kernel/include/kswap.h | 2 +- kernel/mempool.c | 4 +-- .../mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c | 4 +-- subsys/net/buf.c | 4 +-- subsys/net/lib/lwm2m/lwm2m_shell.c | 2 +- subsys/portability/cmsis_rtos_v2/mempool.c | 2 +- subsys/portability/cmsis_rtos_v2/msgq.c | 4 +-- subsys/shell/modules/kernel_service.c | 6 ++-- subsys/zbus/zbus.c | 2 +- 14 files changed, 79 insertions(+), 21 deletions(-) diff --git a/arch/xtensa/core/ptables.c b/arch/xtensa/core/ptables.c index 4788584d64d..b6d81e21db1 100644 --- a/arch/xtensa/core/ptables.c +++ b/arch/xtensa/core/ptables.c @@ -133,7 +133,7 @@ static const struct xtensa_mmu_range mmu_zephyr_ranges[] = { #endif .name = "data", }, -#if CONFIG_HEAP_MEM_POOL_SIZE > 0 +#if K_HEAP_MEM_POOL_SIZE > 0 /* System heap memory */ { .start = (uint32_t)_heap_start, diff --git a/doc/kernel/memory_management/heap.rst b/doc/kernel/memory_management/heap.rst index 70d719c7396..7dd332464d2 100644 --- a/doc/kernel/memory_management/heap.rst +++ b/doc/kernel/memory_management/heap.rst @@ -165,6 +165,18 @@ the kernel not to define the heap memory pool object. The maximum size is limite by the amount of available memory in the system. The project build will fail in the link stage if the size specified can not be supported. +In addition, each subsystem (board, driver, library, etc) can set a custom +requirement by defining a Kconfig option with the prefix +``HEAP_MEM_POOL_ADD_SIZE_`` (this value is in bytes). If multiple subsystems +specify custom values, the sum of these will be used as the minimum requirement. +If the application tries to set a value that's less than the minimum value, this +will be ignored and the minimum value will be used instead. + +To force a smaller than minimum value to be used, the application may enable the +:kconfig:option:`CONFIG_HEAP_MEM_POOL_IGNORE_MIN` option. This can be useful +when optimizing the heap size and the minimum requirement can be more accurately +determined for a speficic application. + Allocating Memory ================= diff --git a/include/zephyr/kernel.h b/include/zephyr/kernel.h index ee47caed199..adf121f72ee 100644 --- a/include/zephyr/kernel.h +++ b/include/zephyr/kernel.h @@ -420,7 +420,7 @@ __syscall int k_thread_stack_space_get(const struct k_thread *thread, size_t *unused_ptr); #endif -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) /** * @brief Assign the system heap as a thread's resource pool * @@ -434,7 +434,7 @@ __syscall int k_thread_stack_space_get(const struct k_thread *thread, * */ void k_thread_system_pool_assign(struct k_thread *thread); -#endif /* (CONFIG_HEAP_MEM_POOL_SIZE > 0) */ +#endif /* (K_HEAP_MEM_POOL_SIZE > 0) */ /** * @brief Sleep until a thread exits diff --git a/kernel/CMakeLists.txt b/kernel/CMakeLists.txt index 5e55b05be61..cd9f39c9558 100644 --- a/kernel/CMakeLists.txt +++ b/kernel/CMakeLists.txt @@ -117,8 +117,38 @@ target_sources_ifdef(CONFIG_OBJ_CORE kernel PRIVATE obj_core.c) if(${CONFIG_KERNEL_MEM_POOL}) target_sources(kernel PRIVATE mempool.c) -endif() + if(CONFIG_HEAP_MEM_POOL_IGNORE_MIN) + set(final_heap_size ${CONFIG_HEAP_MEM_POOL_SIZE}) + else() + # Import all custom HEAP_MEM_POOL size requirements + import_kconfig(CONFIG_HEAP_MEM_POOL_ADD_SIZE_ ${DOTCONFIG} add_size_keys) + + # Calculate the sum of all "ADD_SIZE" requirements + set(add_size_sum 0) + foreach(add_size ${add_size_keys}) + math(EXPR add_size_sum "${add_size_sum} + ${${add_size}}") + endforeach() + + if(CONFIG_HEAP_MEM_POOL_SIZE LESS "${add_size_sum}") + # Only warn if default value 0 has been modified + if(NOT CONFIG_HEAP_MEM_POOL_SIZE EQUAL 0) + message(WARNING " + CONFIG_HEAP_MEM_POOL_SIZE is less than requested minimum: + ${CONFIG_HEAP_MEM_POOL_SIZE} < ${add_size_sum} + Setting the system heap size to ${add_size_sum}") + endif() + + set(final_heap_size ${add_size_sum}) + else() + # CONFIG_HEAP_MEM_POOL_SIZE was greater than the sum of the requirements + set(final_heap_size ${CONFIG_HEAP_MEM_POOL_SIZE}) + endif() + + endif() + + zephyr_compile_definitions(K_HEAP_MEM_POOL_SIZE=${final_heap_size}) +endif() # The last 2 files inside the target_sources_ifdef should be # userspace_handler.c and userspace.c. If not the linker would complain. diff --git a/kernel/Kconfig b/kernel/Kconfig index bd157d8bcc7..3b916aba062 100644 --- a/kernel/Kconfig +++ b/kernel/Kconfig @@ -840,8 +840,24 @@ config HEAP_MEM_POOL_SIZE help This option specifies the size of the heap memory pool used when dynamically allocating memory using k_malloc(). The maximum size of - the memory pool is only limited to available memory. A size of zero - means that no heap memory pool is defined. + the memory pool is only limited to available memory. If subsystems + specify HEAP_MEM_POOL_ADD_SIZE_* options, these will be added together + and the sum will be compared to the HEAP_MEM_POOL_SIZE value. + If the sum is greater than the HEAP_MEM_POOL_SIZE option (even if this + has the default 0 value), then the actual heap size will be rounded up + to the sum of the individual requirements (unless the + HEAP_MEM_POOL_IGNORE_MIN option is enabled). If the final value, after + considering both this option as well as sum of the custom + requirements, ends up being zero, then no system heap will be + available. + +config HEAP_MEM_POOL_IGNORE_MIN + bool "Ignore the minimum heap memory pool requirement" + help + This option can be set to force setting a smaller heap memory pool + size than what's specified by enabled subsystems. This can be useful + when optimizing memory usage and a more precise minimum heap size + is known for a given application. endif # KERNEL_MEM_POOL diff --git a/kernel/include/kswap.h b/kernel/include/kswap.h index fa1e538b8e8..01a72744b00 100644 --- a/kernel/include/kswap.h +++ b/kernel/include/kswap.h @@ -250,7 +250,7 @@ static inline void z_dummy_thread_init(struct k_thread *dummy_thread) #ifdef CONFIG_USERSPACE dummy_thread->mem_domain_info.mem_domain = &k_mem_domain_default; #endif -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) k_thread_system_pool_assign(dummy_thread); #else dummy_thread->resource_pool = NULL; diff --git a/kernel/mempool.c b/kernel/mempool.c index 3cbfa201222..b3943b52728 100644 --- a/kernel/mempool.c +++ b/kernel/mempool.c @@ -56,9 +56,9 @@ void k_free(void *ptr) } } -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) -K_HEAP_DEFINE(_system_heap, CONFIG_HEAP_MEM_POOL_SIZE); +K_HEAP_DEFINE(_system_heap, K_HEAP_MEM_POOL_SIZE); #define _SYSTEM_HEAP (&_system_heap) void *k_aligned_alloc(size_t align, size_t size) diff --git a/subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c b/subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c index 0af1f19ba88..303f112709a 100644 --- a/subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c @@ -371,12 +371,12 @@ int img_mgmt_read(int slot, unsigned int offset, void *dst, unsigned int num_byt int img_mgmt_write_image_data(unsigned int offset, const void *data, unsigned int num_bytes, bool last) { - /* Even if CONFIG_HEAP_MEM_POOL_SIZE will be able to match size of the structure, + /* Even if K_HEAP_MEM_POOL_SIZE will be able to match size of the structure, * keep in mind that when application will put the heap under pressure, obtaining * of a flash image context may not be possible, so plan bigger heap size or * make sure to limit application pressure on heap when DFU is expected. */ - BUILD_ASSERT(CONFIG_HEAP_MEM_POOL_SIZE >= (sizeof(struct flash_img_context)), + BUILD_ASSERT(K_HEAP_MEM_POOL_SIZE >= (sizeof(struct flash_img_context)), "Not enough heap mem for flash_img_context."); int rc = IMG_MGMT_ERR_OK; diff --git a/subsys/net/buf.c b/subsys/net/buf.c index 09f937da59d..4862a65f226 100644 --- a/subsys/net/buf.c +++ b/subsys/net/buf.c @@ -166,7 +166,7 @@ const struct net_buf_data_cb net_buf_fixed_cb = { .unref = fixed_data_unref, }; -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) static uint8_t *heap_data_alloc(struct net_buf *buf, size_t *size, k_timeout_t timeout) @@ -205,7 +205,7 @@ const struct net_buf_data_alloc net_buf_heap_alloc = { .cb = &net_buf_heap_cb, }; -#endif /* CONFIG_HEAP_MEM_POOL_SIZE > 0 */ +#endif /* K_HEAP_MEM_POOL_SIZE > 0 */ static uint8_t *data_alloc(struct net_buf *buf, size_t *size, k_timeout_t timeout) { diff --git a/subsys/net/lib/lwm2m/lwm2m_shell.c b/subsys/net/lib/lwm2m/lwm2m_shell.c index a4cedf8cd62..5dc891178ff 100644 --- a/subsys/net/lib/lwm2m/lwm2m_shell.c +++ b/subsys/net/lib/lwm2m/lwm2m_shell.c @@ -565,7 +565,7 @@ static int cmd_unlock(const struct shell *sh, size_t argc, char **argv) static int cmd_cache(const struct shell *sh, size_t argc, char **argv) { -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) int rc; int elems; struct lwm2m_time_series_elem *cache; diff --git a/subsys/portability/cmsis_rtos_v2/mempool.c b/subsys/portability/cmsis_rtos_v2/mempool.c index 183d3ac789a..a5a08919753 100644 --- a/subsys/portability/cmsis_rtos_v2/mempool.c +++ b/subsys/portability/cmsis_rtos_v2/mempool.c @@ -30,7 +30,7 @@ osMemoryPoolId_t osMemoryPoolNew(uint32_t block_count, uint32_t block_size, { struct cv2_mslab *mslab; - BUILD_ASSERT(CONFIG_HEAP_MEM_POOL_SIZE >= + BUILD_ASSERT(K_HEAP_MEM_POOL_SIZE >= CONFIG_CMSIS_V2_MEM_SLAB_MAX_DYNAMIC_SIZE, "heap must be configured to be at least the max dynamic size"); diff --git a/subsys/portability/cmsis_rtos_v2/msgq.c b/subsys/portability/cmsis_rtos_v2/msgq.c index d23a5569031..af8c910c189 100644 --- a/subsys/portability/cmsis_rtos_v2/msgq.c +++ b/subsys/portability/cmsis_rtos_v2/msgq.c @@ -28,7 +28,7 @@ osMessageQueueId_t osMessageQueueNew(uint32_t msg_count, uint32_t msg_size, { struct cv2_msgq *msgq; - BUILD_ASSERT(CONFIG_HEAP_MEM_POOL_SIZE >= + BUILD_ASSERT(K_HEAP_MEM_POOL_SIZE >= CONFIG_CMSIS_V2_MSGQ_MAX_DYNAMIC_SIZE, "heap must be configured to be at least the max dynamic size"); @@ -55,7 +55,7 @@ osMessageQueueId_t osMessageQueueNew(uint32_t msg_count, uint32_t msg_size, CONFIG_CMSIS_V2_MSGQ_MAX_DYNAMIC_SIZE, "message queue size exceeds dynamic maximum"); -#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if (K_HEAP_MEM_POOL_SIZE > 0) msgq->pool = k_calloc(msg_count, msg_size); if (msgq->pool == NULL) { k_mem_slab_free(&cv2_msgq_slab, (void *)msgq); diff --git a/subsys/shell/modules/kernel_service.c b/subsys/shell/modules/kernel_service.c index 58a3a454dc5..8c82322729a 100644 --- a/subsys/shell/modules/kernel_service.c +++ b/subsys/shell/modules/kernel_service.c @@ -16,7 +16,7 @@ #include #include #include -#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (K_HEAP_MEM_POOL_SIZE > 0) #include #endif #if defined(CONFIG_LOG_RUNTIME_FILTERING) @@ -276,7 +276,7 @@ static int cmd_kernel_stacks(const struct shell *sh, } #endif -#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (K_HEAP_MEM_POOL_SIZE > 0) extern struct sys_heap _system_heap; static int cmd_kernel_heap(const struct shell *sh, @@ -400,7 +400,7 @@ SHELL_STATIC_SUBCMD_SET_CREATE(sub_kernel, SHELL_CMD(stacks, NULL, "List threads stack usage.", cmd_kernel_stacks), SHELL_CMD(threads, NULL, "List kernel threads.", cmd_kernel_threads), #endif -#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (CONFIG_HEAP_MEM_POOL_SIZE > 0) +#if defined(CONFIG_SYS_HEAP_RUNTIME_STATS) && (K_HEAP_MEM_POOL_SIZE > 0) SHELL_CMD(heap, NULL, "System heap usage statistics.", cmd_kernel_heap), #endif SHELL_CMD_ARG(uptime, NULL, "Kernel uptime. Can be called with the -p or --pretty options", diff --git a/subsys/zbus/zbus.c b/subsys/zbus/zbus.c index 440d4c09315..959ef59aa8e 100644 --- a/subsys/zbus/zbus.c +++ b/subsys/zbus/zbus.c @@ -18,7 +18,7 @@ LOG_MODULE_REGISTER(zbus, CONFIG_ZBUS_LOG_LEVEL); NET_BUF_POOL_HEAP_DEFINE(_zbus_msg_subscribers_pool, CONFIG_ZBUS_MSG_SUBSCRIBER_NET_BUF_POOL_SIZE, sizeof(struct zbus_channel *), NULL); -BUILD_ASSERT(CONFIG_HEAP_MEM_POOL_SIZE > 0, "MSG_SUBSCRIBER feature requires heap memory pool."); +BUILD_ASSERT(K_HEAP_MEM_POOL_SIZE > 0, "MSG_SUBSCRIBER feature requires heap memory pool."); static inline struct net_buf *_zbus_create_net_buf(struct net_buf_pool *pool, size_t size, k_timeout_t timeout)