Skip to content

Switch from MallocAllocator to Zephyr k_malloc allocator#327

Open
yudataguy wants to merge 15 commits intomainfrom
update-malloc
Open

Switch from MallocAllocator to Zephyr k_malloc allocator#327
yudataguy wants to merge 15 commits intomainfrom
update-malloc

Conversation

@yudataguy
Copy link
Copy Markdown
Collaborator

@yudataguy yudataguy commented Feb 5, 2026

Description

Replace Fw::MallocAllocator (libc malloc) with a custom ZephyrKmallocAllocator backed by Zephyr's k_malloc/k_free. This ensures heap usage is tracked by the kernel memory pool, giving an accurate picture of runtime RAM consumption. Previously, the build reported ~75% RAM usage but libc malloc allocations were invisible to Zephyr's memory accounting.

Also unifies the separate ComCcsdsSband and ComCcsdsLora allocation namespaces into a single ComCcsds::Allocation namespace, and configures HEAP_MEM_POOL_SIZE=65536 in prj.conf for the Zephyr kernel heap.

Related Issues/Tickets

Closes #313

How Has This Been Tested?

  • Unit tests
  • Integration tests
  • Z Tests
  • Manual testing (describe steps)

Screenshots / Recordings (if applicable)

N/A

Checklist

  • Written detailed sdd with requirements, channels, ports, commands, telemetry defined and correctly formatted and spelled
  • Have written relevant integration tests and have documented them in the sdd
  • Have done a code review with
  • Have tested this PR on every supported board with correct board definitions

Further Notes / Considerations

Replace Fw::MallocAllocator with a ZephyrKmallocAllocator backed by
Zephyr's k_malloc/k_free so that heap usage is tracked by the kernel
memory pool, giving an accurate picture of runtime RAM consumption.

- Implement ZephyrKmallocAllocator with alignment support via
  k_aligned_alloc
- Unify ComCcsdsSband and ComCcsdsLora allocation namespaces into a
  single ComCcsds::Allocation namespace
- Update ReferenceDeploymentTopology to use the shared allocator
- Configure HEAP_MEM_POOL_SIZE=65536 in prj.conf for the kernel heap

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

Another thing, it this should be configured as the default memory allocator using the allocator.

@yudataguy yudataguy marked this pull request as draft February 9, 2026 20:02
@Mikefly123 Mikefly123 moved this to In review in V1.X.X Feb 20, 2026
…tion

Update fprime-zephyr submodule to pick up the upstreamed
Fw::ZephyrKmallocAllocator and remove the local copy from
ComCcsdsSubtopologyConfig, keeping the same ComCcsds::Allocation
interface.
@yudataguy yudataguy marked this pull request as ready for review February 26, 2026 00:16
The header only needs the base Fw::MemAllocator type for the extern
declaration. The concrete ZephyrKmallocAllocator type from
MemoryAllocation.hpp is only needed in the .cpp for instantiation.
fprime config headers are resolved via the config/ include path prefix,
matching how the framework itself includes them (e.g. in MemAllocator.cpp).
@yudataguy yudataguy marked this pull request as draft February 27, 2026 03:20
Picks up the fix that rounds allocation size up to a multiple of
alignment, satisfying the C11 aligned_alloc requirement and enabling
ZephyrKmallocAllocator as the project-wide default allocator.
Restore ComCcsdsSubtopologyConfig to use MallocAllocator with separate
ComCcsdsSband/ComCcsdsLora namespaces (matching main), and update
fprime-zephyr submodule to reverted main.
Update fprime-zephyr submodule to include ZephyrKmallocAllocator as a
targeted allocator (not project-wide override). Update ComCcsds config
to use unified namespace and ZephyrKmallocAllocator for S-Band and
LoRa subtopologies only.
@ineskhou ineskhou moved this from In review to In progress in V1.X.X Mar 2, 2026
yudataguy and others added 5 commits March 2, 2026 14:49
Points to aligned-alloc-size-rounding branch (f228013) which rounds
allocation sizes to a multiple of alignment before calling
k_aligned_alloc, preventing undefined behavior on Zephyr targets.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Points submodule to a5e01c3, the merge commit on fprime-zephyr main
that includes the aligned_alloc size-rounding fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Points to 171c8cc which adds alignof(max_align_t) threshold to avoid
k_aligned_alloc on ARM when not needed, on top of the size-rounding fix.
@yudataguy
Copy link
Copy Markdown
Collaborator Author

Replace the previous Zephyr kmalloc allocator behavior with an alignment-safe implementation for RP2350/M33.

This change fixes ZephyrKmallocAllocator so it only uses k_malloc when the requested alignment is within Zephyr’s documented guarantee, and avoids treating k_malloc as safe for stricter default-alignment requests on 32-bit ARM. The previous behavior could return under-aligned memory during boot and plausibly hard-fault on RP2350/M33.

Once approved, do not merged yet, since it needs to update the fprime-zephyr repo first

@yudataguy yudataguy marked this pull request as ready for review March 21, 2026 05:46
@Mikefly123 Mikefly123 requested a review from LeStarch March 27, 2026 01:13
@ineskhou
Copy link
Copy Markdown
Contributor

Great PR! Learned a lot by looking though it, and we can see a 15% increase in proper RAM allocation. Super awesome! And the consolidated Comcsds bloc is much cleaner as well so thank you for cleaning up the subtopologies ensuring when we add coms subtoplotogies they are all good

I have two quick pieces of feedback, more of suggestions than anything. Even if we deem them important, we can always do them later in another PR

  1. I would want to look at our, zephyr submodule we are some commits behind main, including some commits from Starch that have to do with Open-Source-Space-Foundation/fprime-zephyr@main...fprime-community:fprime-zephyr:main including the Add send recv PR

  2. My understanding is we are covering the allocation in the Com Subtopology bc thats most of our code

Some things we could add to cover more ground:

  • We could change the default allocator by overriding this config and add it to project/config
    This I think would only touch the Zephyr allocator. My understanding of this is would help with zephyrs memory allocation at the beginning (see bellow) but I'm not sure. Would be worth trying and seeing if the number changes, because I do think we are probably using more than 75% RAM

Zephyr’s platform config selects Os_Generic_PriorityQueue (lib/fprime-zephyr/cmake/platform/zephyr/Platform/CMakeLists.txt),

In lib/fprime/Os/Generic/PriorityQueue.cpp allocates internal queue storage via

    // Get the memory allocator configured for priority queues
    Fw::MemAllocator& allocator = Fw::MemAllocatorRegistry::getInstance().getAnAllocator(
        Fw::MemoryAllocation::MemoryAllocatorType::OS_GENERIC_PRIORITY_QUEUE);

In lib/fprime/Fw/Types/MemAllocator.cpp, getAnAllocator falls back to the SYSTEM allocator unless registerAllocator is called, which is it not in our code I believe

MemAllocator& MemAllocatorRegistry::getAnAllocator(const MemoryAllocation::MemoryAllocatorType type) {
    // If the allocator is not registered, return the SYSTEM allocator
    if (this->m_allocators[type] == nullptr) {
        return this->getAllocator(MemoryAllocation::MemoryAllocatorType::SYSTEM);
    }
    return *this->m_allocators[type];
}

SYSTEM is constructed from Fw::MemoryAllocation::DefaultMemoryAllocatorType in lib/fprime/default/config/MemoryAllocation.hpp which currently defaults to Fw::MallocAllocator, which is why I think overrride the prj.conf could be worthwhile!

  1. Lastly This is super awesome but I think it would benefit for clearer documentation. If we ever add another non-comms subtopology, for example, if we want to add data products (probably not in the shoert tmer, but long term seems likley) we want whoeever is mainininting it to remember that each subtopology needs to also have X YZ. Would be good to have a section explaining t in our docs somewhere so they are easy to ref

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

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[FEATURE] Swithc from mallocallocator to kmalloc allocated in Zephyr

4 participants