From ed7650301d9e423832d1045c94d584e8f0b5a0ae Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 12 Jan 2026 17:09:53 -0800 Subject: [PATCH 1/4] bad free real estate detector --- build/xtask/src/sizes.rs | 106 +++++++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 8d0bf6446a..ddcdb4fc51 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -70,26 +70,16 @@ pub fn run( print!("\n\n"); print_task_table(&toml, &map)?; print!("\n\n"); - print_task_stacks(&toml)?; } + // Always do stack size estimation, but only print the stacks if we are in + // verbose mode. + let stacks = estimate_task_stacks(&toml, !only_suggest)?; // Because tasks are autosized, the only place where we can improve - // memory allocation is in the kernel. + // memory allocation is in the kernel... let mut printed_header = false; let mut printed_name = false; - for (mem, &used) in sizes.sizes["kernel"].iter() { - if used == 0 { - continue; - } - let size = toml.kernel.requires[&mem.to_string()]; - - let suggestion = toml.suggest_memory_region_size("kernel", used, 1); - assert_eq!(suggestion.len(), 1, "kernel should not be > 1 region"); - let suggestion = suggestion[0]; - - if suggestion >= size as u64 { - continue; - } + let mut maybe_print_header = |out: &mut dyn Write| -> std::io::Result<()> { if !printed_header { printed_header = true; if only_suggest { @@ -104,6 +94,23 @@ pub fn run( )?; } } + Ok(()) + }; + + for (mem, &used) in sizes.sizes["kernel"].iter() { + if used == 0 { + continue; + } + let size = toml.kernel.requires[&mem.to_string()]; + + let suggestion = toml.suggest_memory_region_size("kernel", used, 1); + assert_eq!(suggestion.len(), 1, "kernel should not be > 1 region"); + let suggestion = suggestion[0]; + + if suggestion >= size as u64 { + continue; + } + maybe_print_header(&mut out)?; if !printed_name { printed_name = true; writeln!(out, "kernel:")?; @@ -117,6 +124,45 @@ pub fn run( )?; } + // ... and also stack sizes that are over margin. + let mut total_free_real_estate = 0; + for (task_name, stack) in stacks { + let total_ram = sizes.sizes[task_name]["ram"]; + let oldlim = stack.limit; + let nonstack_ram = total_ram.saturating_sub(oldlim); + let newlim = stack.max_estimate + 8; + let newram = nonstack_ram + newlim; + let free_real_estate = total_ram - newram; + + if free_real_estate > 0 { + maybe_print_header(&mut out)?; + writeln!(out, "{task_name}:")?; + writeln!( + out, + " {:<6} {newlim: >5} {}", + "stack", + format!(" (currently {oldlim})").dimmed() + )?; + writeln!( + out, + " {:<6} {newram: >5} {}{}", + "ram", + format!(" (currently {total_ram})").dimmed(), + " <- requires stack size change".dimmed() + )?; + total_free_real_estate += free_real_estate; + } + } + + if total_free_real_estate > 0 { + writeln!( + out, + "\nthere may be up to {total_free_real_estate} bytes of free \ + real estate in this image,\nif you're brave enough to mess with \ + stack sizes!\n" + )?; + } + Ok(()) } @@ -427,23 +473,41 @@ fn print_memory_map( Ok(()) } -fn print_task_stacks(toml: &Config) -> Result<()> { +struct Stacks { + max_estimate: u64, + limit: u64, +} + +fn estimate_task_stacks<'a>( + toml: &'a Config, + print: bool, +) -> Result> { + let mut tasks = IndexMap::with_capacity(toml.tasks.len()); for (i, (task_name, task)) in toml.tasks.iter().enumerate() { let task_stack_size = task.stacksize.unwrap_or_else(|| toml.stacksize.unwrap()); let max_stack = get_max_stack(toml, task_name, false)?; let total: u64 = max_stack.iter().map(|(n, _)| *n).sum(); - println!("{task_name}: {total} bytes (limit is {task_stack_size})"); - for (frame_size, name) in max_stack { - let s = format!("[+{frame_size}]"); - println!(" {s:>7} {name}"); + if print { + println!("{task_name}: {total} bytes (limit is {task_stack_size})"); + for (frame_size, name) in max_stack { + let s = format!("[+{frame_size}]"); + println!(" {s:>7} {name}"); + } } if i + 1 < toml.tasks.len() { println!(); } + tasks.insert( + task_name.as_ref(), + Stacks { + max_estimate: total, + limit: task_stack_size as u64, + }, + ); } - Ok(()) + Ok(tasks) } /// Loads the size of the given task (or kernel) From 7b15cca0339a7260bcd5459fd720b85da12e6b0c Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 12 Jan 2026 17:18:14 -0800 Subject: [PATCH 2/4] only make good suggestions --- build/xtask/src/sizes.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index ddcdb4fc51..23f4f10282 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -132,9 +132,13 @@ pub fn run( let nonstack_ram = total_ram.saturating_sub(oldlim); let newlim = stack.max_estimate + 8; let newram = nonstack_ram + newlim; - let free_real_estate = total_ram - newram; - if free_real_estate > 0 { + // Only claim that there's free real estate if we could shrink the power + // of two memory region by resizing the task's RAM request. + let old_pow2 = total_ram.next_power_of_two(); + let new_pow2 = newram.next_power_of_two(); + if new_pow2 < old_pow2 { + let free_real_estate = old_pow2 - new_pow2; maybe_print_header(&mut out)?; writeln!(out, "{task_name}:")?; writeln!( @@ -148,7 +152,8 @@ pub fn run( " {:<6} {newram: >5} {}{}", "ram", format!(" (currently {total_ram})").dimmed(), - " <- requires stack size change".dimmed() + format!(" !!! {free_real_estate}B of free real estate !!!") + .dimmed() )?; total_free_real_estate += free_real_estate; } From 55337567a753972cb635f0880a6496a4f99a3dd2 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 13 Jan 2026 10:43:06 -0800 Subject: [PATCH 3/4] use the actual region suggester to suggest region sizes --- build/xtask/src/sizes.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 23f4f10282..49682cab2d 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -127,20 +127,35 @@ pub fn run( // ... and also stack sizes that are over margin. let mut total_free_real_estate = 0; for (task_name, stack) in stacks { - let total_ram = sizes.sizes[task_name]["ram"]; + let oldram = sizes.sizes[task_name]["ram"]; let oldlim = stack.limit; - let nonstack_ram = total_ram.saturating_sub(oldlim); + let nonstack_ram = oldram.saturating_sub(oldlim); + // TODO(eliza): consider also allowing a "bonus stack margin" to be + // requested for paranoia purposes? let newlim = stack.max_estimate + 8; let newram = nonstack_ram + newlim; // Only claim that there's free real estate if we could shrink the power - // of two memory region by resizing the task's RAM request. - let old_pow2 = total_ram.next_power_of_two(); - let new_pow2 = newram.next_power_of_two(); - if new_pow2 < old_pow2 { - let free_real_estate = old_pow2 - new_pow2; + // of two memory region by resizing the task's RAM request. If changing + // the RAM request does *not* result in a region shrink, then changing + // it doesn't actually give us back any memory. + let old_regions = toml.suggest_memory_region_size(task_name, oldram, 1); + let new_regions = toml.suggest_memory_region_size(task_name, newram, 1); + assert_eq!(old_regions.len(), 1); + assert_eq!(new_regions.len(), 1); + let old_region_size = old_regions[0]; + let new_region_size = new_regions[0]; + + if new_region_size < old_region_size { + let free_real_estate = old_region_size - new_region_size; maybe_print_header(&mut out)?; - writeln!(out, "{task_name}:")?; + writeln!( + out, + "{task_name}: {:>width$}", + format!(" !!! {free_real_estate}B of free real estate !!!") + .dimmed(), + width = 76 - task_name.len(), + )?; writeln!( out, " {:<6} {newlim: >5} {}", @@ -149,11 +164,9 @@ pub fn run( )?; writeln!( out, - " {:<6} {newram: >5} {}{}", + " {:<6} {newram: >5} {}", "ram", - format!(" (currently {total_ram})").dimmed(), - format!(" !!! {free_real_estate}B of free real estate !!!") - .dimmed() + format!(" (currently {oldram})").dimmed(), )?; total_free_real_estate += free_real_estate; } From 38cb9518d878305a7d22d4ac3f98cb730ef6ed10 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 16 Jan 2026 09:45:53 -0800 Subject: [PATCH 4/4] comment on where 8 comes from --- build/xtask/src/sizes.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 49682cab2d..1bcbc52353 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -130,6 +130,12 @@ pub fn run( let oldram = sizes.sizes[task_name]["ram"]; let oldlim = stack.limit; let nonstack_ram = oldram.saturating_sub(oldlim); + + // The stack limit in the TOML must be *greater* than the maximum + // estimated stack depth. Stack sizes must be a multiple of 8 bytes. + // Thus, we will increase the stack limit to the maximum estimated depth + // plus 8 bytes. + // // TODO(eliza): consider also allowing a "bonus stack margin" to be // requested for paranoia purposes? let newlim = stack.max_estimate + 8;