Hi KR, WOW! Thank you for going deeper in the rabbit hole to find the root cause of this issue!
Yes, 1400 bytes is not much for a 128KB MCU, but could be a show stopper for a 32KB MCU (or smaller). And you are right, Xiaomi is not working with the mainline directly, they used to work in sync with our mainline, but it changed some time ago because the cadence to inclusion of their patches wasn't ideal. This change helped them to move faster, but it resulted in other issues, since small differences could end-up introducing regressions, so this is why Matteo (linguini) insisted to them to show a real test log using the mainline to double check everything was correct. The rest you know, and that is very sad for NuttX and I think for them as well. I took a look at your commit and that modification seems good to me. I'm copying it here so that more people can take a look. Hi anjiahao, could you please take a look? BR, Alan >From 191fee9f98c54238bd1f33293cbd053ca126702e Mon Sep 17 00:00:00 2001 From: Kerogit <[email protected]> Date: Sun, 22 Feb 2026 11:14:07 +0100 Subject: [PATCH] mm/mm_heap: add option to remove optional parameter in mm_initialize_heap This patch optionally removes parameter heap from struct mm_heap_config_s, based on setting of associated configuration option. Presence of this parameter causes increase of compiled size while there is only a single in-tree user of this functionality. The patch makes this parameter optional and disabled for architectures/boards with DEFAULT_SMALL set. How much program memory is saved differs between architectures and is also based on the overall configuration - biggest part of the savings comes from linker's ability to eliminate some functions. The patch was tested by building and running NSH on AVR128DA28 (breadxavr:nsh) and also on Qemu Risc-V (rv-virt:nsh). Ostest on the latter completed successfully. Signed-off-by: Kerogit <[email protected]> --- drivers/rptun/Kconfig | 1 + include/nuttx/mm/mm.h | 2 ++ mm/Kconfig | 12 ++++++++++++ mm/mm_heap/mm_initialize.c | 9 +++++++++ 4 files changed, 24 insertions(+) diff --git a/drivers/rptun/Kconfig b/drivers/rptun/Kconfig index e10dc044b3..3a36e5513e 100644 --- a/drivers/rptun/Kconfig +++ b/drivers/rptun/Kconfig @@ -6,6 +6,7 @@ menuconfig RPTUN bool "Remote Proc Tunnel Driver Support" default n + depends on !MM_DISABLE_INIT_HEAP_POINTER select RPMSG_VIRTIO ---help--- RPTUN driver is used for multi-cores' communication. diff --git a/include/nuttx/mm/mm.h b/include/nuttx/mm/mm.h index 4f8c928b8e..7b58942114 100644 --- a/include/nuttx/mm/mm.h +++ b/include/nuttx/mm/mm.h @@ -191,7 +191,9 @@ struct mm_heap_config_s * If heap != NULL, means malloc struct mm_heap_s from this heap. */ +#ifndef CONFIG_MM_DISABLE_INIT_HEAP_POINTER FAR struct mm_heap_s *heap; +#endif FAR const char *name; FAR void *start; size_t size; diff --git a/mm/Kconfig b/mm/Kconfig index a5904dd19b..34ccd729df 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -337,6 +337,18 @@ config FS_PROCFS_EXCLUDE_MEMPOOL default DEFAULT_SMALL depends on FS_PROCFS && MM_HEAP_MEMPOOL_THRESHOLD > 0 +config MM_DISABLE_INIT_HEAP_POINTER + bool "Disable heap parameter in mm_initialize_heap" + depends on MM_DEFAULT_MANAGER + default DEFAULT_SMALL + ---help--- + This parameter is used to specify caller's heap struct + to avoid reserving the heap struct from heap range. + + It is currently needed only by the driver for Remote + Proc Tunnel and causes code size increase even if this + driver is not in use. + source "mm/kasan/Kconfig" config MM_UBSAN diff --git a/mm/mm_heap/mm_initialize.c b/mm/mm_heap/mm_initialize.c index cf111e8d3c..c0d47a17be 100644 --- a/mm/mm_heap/mm_initialize.c +++ b/mm/mm_heap/mm_initialize.c @@ -234,14 +234,21 @@ void mm_addregion(FAR struct mm_heap_s *heap, FAR void *heapstart, FAR struct mm_heap_s * mm_initialize_heap(FAR const struct mm_heap_config_s *config) { +#ifndef CONFIG_MM_DISABLE_INIT_HEAP_POINTER FAR struct mm_heap_s *heap = config->heap; +#else + FAR struct mm_heap_s *heap; +#endif + FAR const char *name = config->name; FAR void *heapstart = config->start; size_t heapsize = config->size; int i; minfo("Heap: name=%s, start=%p size=%zu\n", name, heapstart, heapsize); +#ifndef CONFIG_MM_DISABLE_INIT_HEAP_POINTER if (heap == NULL) +#endif { /* First ensure the memory to be used is aligned */ @@ -257,6 +264,7 @@ mm_initialize_heap(FAR const struct mm_heap_config_s *config) DEBUGASSERT(MM_MIN_CHUNK >= MM_SIZEOF_ALLOCNODE); } +#ifndef CONFIG_MM_DISABLE_INIT_HEAP_POINTER else { heap = mm_memalign(heap, MM_ALIGN, sizeof(struct mm_heap_s)); @@ -265,6 +273,7 @@ mm_initialize_heap(FAR const struct mm_heap_config_s *config) return NULL; } } +#endif /* Set up global variables */ -- 2.43.0 On Mon, Feb 23, 2026 at 9:22 AM <[email protected]> wrote: > Hello, > > as I mentioned in my previous message (the request for pull request ;-) > ), I noticed flash (program memory) usage increase that happened over > last few months. > > I bisected the source code and it pointed at commit > 6ed4ea63d8168be30227415ea85ae936ca41bb6f . It adds new parameter to > mm_initialize_heap (renamed from mm_initialize.) Further investigation > showed that the program size increase is caused by the new branch (heap > != NULL). I verified this by forcing the variable to be NULL, the branch > was optimized away and linker eliminated mm_memalign and mm_shrinkchunk > from the resulting binary - yielding 1400 bytes of reduced size. This > matched the increase I spotted. > > I work with 8bit microcontroller with 128kB program flash, so 1400 bytes > is not an insignificant amount. > > I then went to analyze how is this function called, I found these > occurences: > > 1. drivers/rptun/rptun.c - called with heap parameter set > 2. inline function mm_initialize - heap set to NULL > > Other than that, mm_initialize_heap is not called directly. Indirectly, > it is either called from mm_initialize_pool, or it replaces this > function via #define (mm_initialize_pool translated to > mm_initialize_heap) > > Considering this, I also analyzed how mm_initialize_pool is called, I > found: > > 1. multiple occurrences in various functions in mm/ directory. These > either call mm_initialize_heap with heap set to NULL, or pass whatever > was given to them through > 2. fs/fs_heap.c - sets heap parameter to NULL > 3. drivers/pci/pci_epc.c - is not using config parameter at all, instead > it passes four parameters so it was probably not converted when the > interface changed? > > (As a side note - this is the second time this week where I found a > change authored or co-authored by Xiaomi that alters some internal > interface and then converts users of that interface... except some. > Seemingly missing are the ones that are fairly recent additions - the > other one is AVR DA/DB subarchitecture which currently fails to build > with default configuration; proposed fix is in my previous message. > > Are they/are you working on some outdated fork of NuttX upstream? Even > more curious is that the broken - at least if I understand that > correctly - file pci_epc.c is also authored by Xiaomi. Anyway, back to > the topic at hand.) > > If my research above is correct, then in-tree code has only a single use > case for the change added by the commit but incurs program memory size > penalty for all users (with varying impact.) In such case, I think this > should be made optional. (This may be up for discussion though - the > memory reduction I achieved is 1400 bytes on AVR but only 400 bytes on > rv-virt. The question is if the code clutter - albeit a small one - is > worth it for one less significant architecture.) > > I prepared a patch that eliminates this program memory size increase if > the heap parameter is not actually used by anything. It wraps member > value FAR struct mm_heap_s *heap in struct mm_heap_config_s with #ifndef > CONFIG_MM_DISABLE_INIT_HEAP_POINTER , a new configuration option which > defaults to DEFAULT_SMALL. The RPTUN driver is made to depends on > !MM_DISABLE_INIT_HEAP_POINTER . Other than that, mm_initialize_heap adds > multiple #ifndef directives to eliminate code related to heap parameter. > > This approach has two benefits: > > 1. if more users of the heap parameter are added but not made to depend > on !MM_DISABLE_INIT_HEAP_POINTER, build with DEFAULT_SMALL will fail - > no hidden errors > 2. other users of struct mm_heap_config_s use memset to clear it before > use. The heap parameter is not set so no code change is needed for those > calls > > The patch is available in a git repository nuttx.git at git.kerogit.eu > accessible through HTTP/S. (Trying to prevent bot traffic by not posting > the URL in machine-readable form.) The relevant branch is called > mm_init_optional_heap. If someone has the time to look at it and provide > feedback and/or create pull request on GitHub (I don't have an account), > it would be greatly appreciated. I think the change is quite trivial to > be reasonably sure that it does not break anything but this is > definitively an area where I only have a vague understanding of the code > at best. > > As of note - I did some tests of the change but there was one thing I > was not able to test - the RPTUN driver itself. Don't have any hardware > to try with and I wasn't even able to compile it for AVR (well, to be > more precise - when I tried, it started downloading things from the web > so I stopped the build.) > > Following is for the GitHub PR (if it comes to it) > > ================ > > Summary > > Make heap parameter of function mm_initialize_heap optional in > mm/mm_heap/mm_initialize.c > > Problem: > > Parameter heap added in commit 6ed4ea63d8168be30227415ea85ae936ca41bb6f > necessitates call of function mm_memalign which can otherwise be > eliminated from compiled binary on small architectures, namely 8-bit AVR > DA/DB family. This increases program memory consumption by 1400 bytes on > that architecture. This parameter is currently only used by RPTUN > driver. > > Solution: > > Add MM_DISABLE_INIT_HEAP_POINTER configuration option which removes this > parameter and the associated code, if set > > Impact > > Users: will be able to use more program memory for their application > code > > Build: > > - new Kconfig option MM_DISABLE_INIT_HEAP_POINTER, defaults to > DEFAULT_SMALL > - rptun driver depends on !MM_DISABLE_INIT_HEAP_POINTER > > Compatibility: > > - no functional change for architectures/boards that do not set > DEFAULT_SMALL > - architectures that set DEFAULT_SMALL will be unable to build rptun > driver unless the new option is unset manually > > Testing > > Test 1 > > Hardware: AVR128DA28 (NSH build for breadxavr board) > > Tests performed: > > - build test with the new option unset to verify that code does not > change > - build test with the new option set to verify that the code is > functional > > Before the patch: > > $ make > Register: nsh > Register: sh > LD: nuttx > Memory region Used Size Region Size %age Used > flash: 51489 B 128 KB 39.28% > sram: 636 B 16 KB 3.88% > eeprom: 0 B 512 B 0.00% > rodata: 592 B 4 KB 14.45% > CP: nuttx.hex > CP: nuttx.asm > $ sha256sum nuttx > 818bad0a984a3b75ccd0f138e4bfcfa00264807b9a9d6d16683ea42eac01413d nuttx > > After the patch (new option not set) > > $ make > LD: nuttx > Memory region Used Size Region Size %age Used > flash: 51489 B 128 KB 39.28% > sram: 636 B 16 KB 3.88% > eeprom: 0 B 512 B 0.00% > rodata: 592 B 4 KB 14.45% > CP: nuttx.hex > CP: nuttx.asm > $ sha256sum nuttx > 818bad0a984a3b75ccd0f138e4bfcfa00264807b9a9d6d16683ea42eac01413d nuttx > > Result - the patch does not change the compiled binary when the new > configuration option is not enabled > > After the patch (new option set) > > $ make > LD: nuttx > Memory region Used Size Region Size %age Used > flash: 50079 B 128 KB 38.21% > sram: 636 B 16 KB 3.88% > eeprom: 0 B 512 B 0.00% > rodata: 592 B 4 KB 14.45% > CP: nuttx.hex > CP: nuttx.asm > > $ minicom > Welcome to minicom 2.10 > > OPTIONS: I18n > Port /dev/serial/by-id/usb-FTDI_FT230X_Basic_UART_DU0DQ17J-if00-port0, > 16:37:48 [U] > > Press CTRL-A Z for help on special keys > > > NuttShell (NSH) NuttX-12.12.0 > nsh> help > help usage: help [-v] [<cmd>] > > ? help > nsh> > > Result - NSH is functional and flash usage is reduced > > Test 2 > > Hardware: building for Risc-V Qemu - rv-virt:nsh > > Tests performed: > - build test with the new option unset to verify that code does not > change, run in Qemu, run ostest > - build test with the new option set to verify that code runs correctly, > run in Qemu, run ostest > > Before the patch > > $ sha256sum nuttx nuttx.asm > 9c68c09fae0effec154957ed0a3b45b9e45879d708c3cd55f15f7be3c07c443d nuttx > 146c24fad25f74f923b7398fbce5310adfdb88fc3875ed5127f74c058ebf7c60 > nuttx.asm > > ostest: > dump_assert_info: Current Version: NuttX 12.12.0 b4242042e1 Feb 22 2026 > 14:44:13 risc-v > ... > ostest_main: Exiting with status 0 > > After the patch (new option in default state, ie. unset) > > $ sha256sum nuttx nuttx.asm > 79005fc3be7f240896caa2b03b231254f903705b38bfc1eefeb263da880f7150 nuttx > 146c24fad25f74f923b7398fbce5310adfdb88fc3875ed5127f74c058ebf7c60 > nuttx.asm > > dump_assert_info: Current Version: NuttX 12.12.0 ee396982d7 Feb 22 2026 > 15:46:21 risc-v > ... > ostest_main: Exiting with status 0 > > Result - the resulting binary does differ but the disassembly does not. > Ostest completes successfully. > > After the patch (new option set) > > Program size reduced by cca 400 bytes (compared by last instruction's > address in disassembly.) > > dump_assert_info: Current Version: NuttX 12.12.0 ee396982d7 Feb 22 2026 > 16:24:11 risc-v > ... > ostest_main: Exiting with status 0 > > NOTE - RPTUN driver, the only user of the (optionally removed) heap > parameter, was not tested - no access to relevant hardware. > > ================ > > Thanks for your time >
