Request for comments: Expand sbrk to allocate memory from two regions

[marcuschangarm]

Description

Some MCUs, like the STM32L475, have multiple discrete RAM regions. Accessing memory across these boundaries will cause a hard fault. The current sbrk implementation for GCC only allocates heap from one of these regions, leaving unused memory in other regions unavailable.

The change here allows sbrk to allocate from the second region once the first is full and assumes that malloc is able to handle the discontinuity in assigned heap.

Question: The sbrk documentation is not clear on whether this discontinuity is allowed behavior or dependent on the malloc implementation.

From https://linux.die.net/man/2/sbrk:

On success, sbrk() returns the previous program break. (If the break was increased, then this value is a pointer to the start of the newly allocated memory). On error, (void *) -1 is returned, and errno is set to ENOMEM.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

[ciarmcom]

@marcuschangarm, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

[jeromecoutant]

Hi

• DISCO_L475VG_IOT01A: tests-mbedmicro-rtos-mbed-heap_and_stack becomes FAIL

• other targets supporting TWO_RAM_REGIONS: build is failing

[marcuschangarm]

Hi @jeromecoutant!
This isn't ready as a PR yet. What I'm looking for is an opinion on the approach: is this a valid interpretation of how sbrk is supposed to work? Thanks!

[kjbracey]

If a library asks you to provide sbrk, the natural assumption is that you're allowed to provide discontiguous responses. That's the reason for the form of the API.

I've looked at the newlib-nano malloc code, I can see no limitations here. Discontiguous heap is no big deal, particularly for a simple heap allocator - malloc just needs to add the new sbrk region to its free block list - it doesn't matter whether if the total free list could ever be combined into one single contiguous area.

My only real query is what is that existing "l4_retarget.c" code doing there if it's not already providing two ram regions? Looking in the history, it seems the target-specific sbrk support hook was added to Mbed OS, and activated for the L4, but no-one ever actually made it do anything until now?

I'm concerned there may be some background here we may be missing - some reason why it's not already active.

[marcuschangarm]

@kjbracey-arm Thank you for the input!

Perhaps we should have a standard way to handle multiple RAM regions in Mbed OS since this is not specific to the STM32L4 and I'm not sure how to reconcile multiple RAM regions in tests-mbedmicro-rtos-mbed-heap_and_stack?

[kjbracey]

Well, that "TWO_RAM_REGIONS" define is a general hook - if defined, the target gets to provide their own totally-custom sbrk (for however many regions, despite the name). So it's similar to "MBED_MPU_CUSTOM". Would better be called "MBED_SBRK_CUSTOM" or "MBED_HEAP_CUSTOM".

But like for the MPU, providing a complete replacement is a fiddle, so yes, it might be nice to have built-in handling for 2 regions based purely on fixed area names.

If you did have such built-in handling, then those tests could handle it. But with the custom generic handling in use I think those tests just have to be deactivated.

[marcuschangarm]

As it sounds like the approach is valid but would require tighter integration with the rest of Mbed OS and other targets, I'm moving this over to issues. Thank you for all the input.

[juhoeskeli]

@marcuschangarm @kjbracey-arm we really need mechanism like this to utilise whole target RAM. Please keep me posted as well if something that I can use is inbound. I am actually hoping to use this particular target, but am being blocked by lack of memory due to two ram regions.