patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 8 Packages Projects Releases Wiki Activity

mgmt/mcumgr/lib: Fix parasitic use of heap by image management

Browse source 
The commit fixes issue where image management would switch to using
heap, whether developer wanted or not, when CONFIG_HEAP_MEM_POOL
gets value greater than zero.
Now when heap is enabled the user can select whether image management
will keep on using static variable, taking static RAM, or will use
heap to allocate the flash image context only when needed.
For this purpose CONFIG_IMG_MGMT_USE_HEAP_FOR_FLASH_IMG_CONTEXT
has been added, which is available when CONFIG_HEAP_MEM_POOL is enabled.

Fixes #44214

Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
This commit is contained in:
Dominik Ermel 2022-05-25 13:36:23 +00:00 committed by Carles Cufí
parent 93e9b5c49d
commit 37dd4fb775
2 changed files with 62 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
subsys/mgmt/mcumgr/lib/cmd/img_mgmt/Kconfig
View file

@ -12,6 +12,23 @@ menuconfig MCUMGR_CMD_IMG_MGMT
Enables mcumgr handlers for image management
if MCUMGR_CMD_IMG_MGMT
if HEAP_MEM_POOL_SIZE > 0
config IMG_MGMT_USE_HEAP_FOR_FLASH_IMG_CONTEXT
bool "Use heap mem pool for flash image DFU context"
help
Use heap to allocate flash image upload context, otherwise a static variable will
be used. The context object is used by IMG_MANAGER to buffer image writes
and has significant size, mainly affected by image write buffer of
the CONFIG_IMG_BLOCK_BUF_SIZE size and additional fields that hold the state information
(struct flash_img_context).
When the option is not enabled it increases static RAM use.
Make sure with testing, when enabling the option, that the heap has enough size
to allocate this context or it will not be possible to perform DFU; it may also not be
possible to allocate such context when heap is under pressure, due to application
operation, an issue that should also be addressed within application.
endif
config IMG_MGMT_UL_CHUNK_SIZE
int "Maximum chunk size for image uploads (DEPRECATED)"
default 512

90
subsys/mgmt/mcumgr/lib/cmd/img_mgmt/src/zephyr_img_mgmt.c
View file

@ -334,60 +334,37 @@ img_mgmt_impl_read(int slot, unsigned int offset, void *dst,
return 0;
}
/*
* The alloc_ctx and free_ctx are specifically provided for
* the img_mgmt_impl_write_image_data to allocate/free single flash_img_context
* type buffer.
* When heap is enabled these functions will operate on heap; when heap is not
* allocated the alloc_ctx just returns pointer to static, global life-time
* variable, and free_ctx does nothing.
* CONFIG_HEAP_MEM_POOL_SIZE is C preprocessor literal.
*/
static inline struct flash_img_context *alloc_ctx(void)
{
struct flash_img_context *ctx = NULL;
if (CONFIG_HEAP_MEM_POOL_SIZE > 0) {
ctx = k_malloc(sizeof(*ctx));
} else {
static struct flash_img_context stcx;
ctx = &stcx;
}
return ctx;
}
static inline void free_ctx(struct flash_img_context *ctx)
{
if (CONFIG_HEAP_MEM_POOL_SIZE > 0) {
k_free(ctx);
}
}
#if defined(CONFIG_IMG_MGMT_USE_HEAP_FOR_FLASH_IMG_CONTEXT)
int
img_mgmt_impl_write_image_data(unsigned int offset, const void *data, unsigned int num_bytes,
bool last)
{
int rc = 0;
/* Even if CONFIG_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)),
"Not enough heap mem for flash_img_context.");
int rc = MGMT_ERR_EOK;
static struct flash_img_context *ctx;
if (CONFIG_HEAP_MEM_POOL_SIZE > 0 && offset != 0 && ctx == NULL) {
if (offset != 0 && ctx == NULL) {
return MGMT_ERR_EUNKNOWN;
}
if (offset == 0) {
if (ctx == NULL) {
ctx = alloc_ctx();
if (ctx != NULL) {
return MGMT_ERR_EUNKNOWN;
}
ctx = k_malloc(sizeof(struct flash_img_context));
if (ctx == NULL) {
rc = MGMT_ERR_ENOMEM;
goto out;
}
if (ctx == NULL) {
return MGMT_ERR_ENOMEM;
}
rc = flash_img_init_id(ctx, g_img_mgmt_state.area_id);
if (rc != 0) {
if (flash_img_init_id(ctx, g_img_mgmt_state.area_id) != 0) {
rc = MGMT_ERR_EUNKNOWN;
goto out;
}
@ -398,15 +375,13 @@ img_mgmt_impl_write_image_data(unsigned int offset, const void *data, unsigned i
goto out;
}
/* Cast away const. */
rc = flash_img_buffered_write(ctx, (void *)data, num_bytes, last);
if (rc != 0) {
if (flash_img_buffered_write(ctx, data, num_bytes, last) != 0) {
rc = MGMT_ERR_EUNKNOWN;
goto out;
}
out:
if (CONFIG_HEAP_MEM_POOL_SIZE > 0 && (last || rc != 0)) {
if (last || rc != MGMT_ERR_EOK) {
k_free(ctx);
ctx = NULL;
}
@ -414,6 +389,31 @@ out:
return rc;
}
#else
int
img_mgmt_impl_write_image_data(unsigned int offset, const void *data, unsigned int num_bytes,
bool last)
{
static struct flash_img_context ctx;
if (offset == 0) {
if (flash_img_init_id(&ctx, g_img_mgmt_state.area_id) != 0) {
return MGMT_ERR_EUNKNOWN;
}
}
if (offset != ctx.stream.bytes_written + ctx.stream.buf_bytes) {
return MGMT_ERR_EUNKNOWN;
}
if (flash_img_buffered_write(&ctx, data, num_bytes, last) != 0) {
return MGMT_ERR_EUNKNOWN;
}
return MGMT_ERR_EOK;
}
#endif
int
img_mgmt_impl_erase_image_data(unsigned int off, unsigned int num_bytes)
{