xtask: bad free real estate detector #2353
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hawkw
commented
Jan 13, 2026
hawkw
changed the title
labbott
reviewed
Jan 16, 2026
Collaborator
labbott
left a comment
labbott
left a comment
There was a problem hiding this comment.
(bad free real estate) detector
bad (free real state detector)
Comment on lines
+133
to
+135
| // TODO(eliza): consider also allowing a "bonus stack margin" to be | ||
| // requested for paranoia purposes? | ||
| let newlim = stack.max_estimate + 8; |
Collaborator
There was a problem hiding this comment.
I'm not 100% what this is doing , I think this is checking for an 8 byte margin. Could we make that a constant and document that some more?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allocation of memory regions (in flash and in RAM) to Hubris tasks is mostly automatic, and the code in
cargo xtask sizesthat used to tell you a bunch of warnings about suboptimal memory allocations now only does so for the kernel.However, there is one allocation that is still not done automatically, which is stack sizes. Stack sizes are a bit different from the RAM and flash sizes, as they don't directly impact how the image is laid out in flash or the allocation of memory regions to tasks when the image runs. However, they do sort of impact RAM region sizes, because the stack size limit determines how much of the RAM allocation is used for the image's stack. If the stack limit for a task decreases, it is possible to decrease the size of the RAM allocated to that task, since it needs less RAM for its stack.
It turns out we have a lot of tasks for which the stack limit is substantially larger than the estimated max stack depth produced by
cargo xtask sizes. If this means that stacks can actually be made smaller for those tasks, well...It isn't quite that simple, though. For a stack size decrease to actually yield free real estate that can be used by other tasks, it must shrink the RAM allocation for that task by a whole power of two, since memory regions must be power of two sized. Otherwise, we will have just decreased the stack limit without actually freeing up space to be used by other tasks.
Therefore, this branch adds a thingy to
cargo xtask sizesthat detects potential FREE REAL ESTATE in a hubris image by looking at all the tasks, comparing their currently configured stack limits with the maximum stack depth guessed byget_max_stack, and then checking whether decreasing the stack limit would allow a region allocation shrink. If it would, we yell about it, and track the total amount of potential FREE REAL ESTATE in the image.For example,
app/cosmo/rev-b.tomlappears to have in excess of 88 KiB of FREE REAL ESTATE in it, which is a huge amount of RAM:1However, there is one big and extremely sketchy thing about just merging the free real estate detector and blindly applying the changes it suggests, which is that I HAVE INVENTED AN EVIL MACHINE THAT TELLS YOU TO ADD STACK OVERFLOWS TO YOUR MISSION CRITICAL FIRMWARE. To wit, the max stack depth estimates are an estimate. As the comment points out:
hubris/build/xtask/src/dist.rs
Lines 1089 to 1092 in aa7951c
It also may not consider stacks involving panics, if I recall some past trauma correctly. So blindly following the advice from the free real estate detector is probably not a good idea.
I'd like to figure out a good strategy to make it safer, such as allowing the task's TOML to explicitly request some amount of stack over-provision, or to have some "paranoia margin" of, say, 25-50% of the stack size, which we add to the prospective stack shrink before determining whether it would let us do a RAM shrink into a smaller region allocation. Any ideas?
Footnotes
Provided that you have a severe case of embedded brain damage... ↩