kern: stack watermark support

[cbiffle]

Each task has grown a new field, low_stack_pointer, that tracks the lowest stack pointer value seen so far (because stacks grow down). We update this value on any kernel entry (interrupt, timer, fault, syscall), so while we will miss very brief excursions of stack between syscalls or interrupts, we should still get a useful value.

In addition, the field past_low_stack_pointer tracks the lowest stack pointer value across all previous incarnations of the task. This way if the task is periodically restarting, we can still get useful stack stats, including at stack overflow.

All of this stuff is conditional on the stack-watermark feature, since it increases the kernel RAM requirement, which is explicit in Oxide's Hubris build system.

[hawkw]

Overall, very straightforward! Looks good to me once Clippy is made happy again.

[hawkw]

i wonder if this whole block should be conditional on the feature flag? I see that update_stack_watermark is a nop if the feature isn't enabled, and the compiler is probably smart enough to eliminate the CURRENT_TASK_PTR access when the function that's called on it doesn't do anything, but... I just kinda don't trust it...

[cbiffle]

I was trying to make as little code conditional as possible, so that it always gets compiled; this is why it's only the body of update_watermark that's cfg'd. The compiler does appear to optimize this load out, fwiw.

[mkeeter]

Vibes – I'd vote to put this into a separate object with helper functions, e.g.

#[cfg(feature = "stack-watermark")]
struct StackWatermark {
    /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on
    /// any kernel entry for this instance of this task.
    ///
    /// Initialized to `u32::MAX` if the task has not yet run.
    #[cfg(feature = "stack-watermark")]
    stack_pointer_low: u32,

    /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on
    /// any kernel entry across *any* instance of this task.
    ///
    /// Initialized to `u32::MAX` if the task has not yet run.
    #[cfg(feature = "stack-watermark")]
    past_stack_pointer_low: u32,
}

#[cfg(feature = "stack-watermark")]
impl Default for StackWatermark {
    fn default() -> Self {
        StackWatermark {
            stack_pointer_low: u32::MAX,
            past_stack_pointer_low: u32::MAX,
        }
    }
}

#[cfg(feature = "stack-watermark")]
impl StackWatermark {
    fn reinitialize() {
        self.past_stack_pointer_low =
            u32::min(self.past_stack_pointer_low, self.stack_pointer_low);
        self.stack_pointer_low = u32::MAX;
    }

    fn update(&mut self, stack_pointer: u32) {
        self.stack_pointer_low =
            u32::min(self.stack_pointer_low, self.save().stack_pointer());
    }
}

[cbiffle]

Sold.

[mkeeter]

Nit: why is this chunk of code different from the diff above, with the uassert! and no safety comment?

[cbiffle]

Hmm. Probably some historical reason and/or author-in-a-hurry. I'll take a pass over it.

[cbiffle]

Alright, I have rearranged the code and added more explanatory comments. Will post the new commits shortly.

[cbiffle]

Alright, pushed fixes for your comments, PTAL!