Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix heap get for split heap #416

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Conversation

JojoS62
Copy link

@JojoS62 JojoS62 commented Jan 6, 2025

Summary of changes

This is a fix for the field heap_stats.reserved_size in the mbed heap statistics. With split heap, the heap can contain two heap areas. In this case, mbed_heap_size changes to be the 2nd heap, and mbed_heap_size_0 will become the first heap part.
The heap statistics did took that change into account, and always used mbed_heap_size for the total heap size.

This PR adds a variable mbed_heap_size_total to reflect the total size, regardless if one or two heaps are used.

Impact of changes

Heap functions are declared as weak, so a custom implementation could be used for even more heap areas or heap in external SDRAM. In this case, the variable mbed_heap_size_total must be calculated in software_init_hook() in mbed_boot_gcc_arm.c for a correct output in the heap statistics.

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@multiplemonomials
Copy link
Collaborator

multiplemonomials commented Jan 6, 2025

@JojoS62 Have you run the heap size Greentea test to make sure there are no breakages? Also, maybe we should update the heap stats test to test for this fix.

Other than that, looks good!

@JojoS62
Copy link
Author

JojoS62 commented Jan 6, 2025

I haven’t used the tests for long time, I remember it is difficult with a custom target.
My patch doesn’t change the alloc code, sbrk uses the start/end addresses and not the heap size.

@JojoS62
Copy link
Author

JojoS62 commented Jan 13, 2025

The following tests passed:
        test-mbed-platform-stats-heap
        test-mbed-rtos-heap-and-stack
        test-mbed-storage-blockdevice-heap_block_device

theses tests have passed on a NUCLEO_H743ZI2 with split_heap.
But the damned git rebase command...

@JojoS62 JojoS62 force-pushed the fix-heap-get-for-split_heap branch from b7c7705 to cdb83c2 Compare January 13, 2025 18:53
@JojoS62 JojoS62 force-pushed the fix-heap-get-for-split_heap branch from cdb83c2 to 96d9e0e Compare January 13, 2025 21:24
@multiplemonomials multiplemonomials merged commit 875cc5d into master Jan 14, 2025
52 checks passed
@multiplemonomials multiplemonomials deleted the fix-heap-get-for-split_heap branch January 14, 2025 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants