Skip to content

Commit

Permalink
counters ipc: handle single-task dumps correctly
Browse files Browse the repository at this point in the history
Currently, `humility counters ipc` will die if run against a dump that
only contains a single task. This is because because it attempts to load
the task table from the kernel in order to read task generations/restart
counts, and returns an error if the task table can't be loaded.

This branch changes `humility counters ipc` to handle single-task dumps
more gracefully. Now, if we can't load the task table, we just print a
warning and continue on with an empty task table. We won't print
generations or restart counts in this case, since there's no way to
determine that from a dump of a single task, but we will still show the
counters, which seems strictly better than the current behavior.

For example:

```console
$ cargo run -- -d tests/cmd/cores/hubris.core.u16-ringbuf counters ipc
   Compiling humility-cmd-counters v0.1.0 (/home/eliza/Code/oxide/humility/cmd/counters)
   Compiling humility v0.11.4 (/home/eliza/Code/oxide/humility)
    Finished dev [unoptimized + debuginfo] target(s) in 6.46s
     Running `target/debug/humility -d tests/cmd/cores/hubris.core.u16-ringbuf counters ipc`
humility: attached to dump
humility: WARNING: failed to load task table: read of 4704 bytes from invalid address: 0x24000490
humility: WARNING: no generations/restart counts will be displayed.
humility: note: this may be a single-task dump.
drv_spi_api::__SPI_CLIENT_COUNTERS
 fn Spi::write() .................................................... 530 calls
    clients:
    task gimlet_seq .................................... = 0 ......... = 530 ok

 fn Spi::exchange() ................................................... 5 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 5 ok

 fn Spi::lock() ....................................................... 4 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 4 ok

 fn Spi::release() .................................................... 1 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 1 ok

drv_stm32xx_sys_api::__SYS_CLIENT_COUNTERS
 fn Sys::gpio_configure_raw() ........................................ 12 calls
    clients:
    task gimlet_seq .................................... = 0 .......... = 12 ok

 fn Sys::gpio_set_reset() ............................................ 12 calls
    clients:
    task gimlet_seq .................................... = 0 .......... = 12 ok

 fn Sys::gpio_read_input() ............................................ 5 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 5 ok

task_jefe_api::__JEFE_CLIENT_COUNTERS
 fn Jefe::set_state() ................................................. 1 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 1 ok

task_packrat_api::__PACKRAT_CLIENT_COUNTERS
 fn Packrat::set_spd_eeprom() ........................................ 32 calls
    clients:
    task gimlet_seq .................................... = 0 .......... = 32 ok

 fn Packrat::set_mac_address_block() .................................. 1 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 1 ok

 fn Packrat::set_identity() ........................................... 1 calls
    clients:
    task gimlet_seq .................................... = 0 ........... = 1 ok

```

This branch depends on @mkeeter's PR #476, because that branch added a
failing dump to the test suite, and I wanted to update the test output
from that dump.
  • Loading branch information
hawkw committed Apr 16, 2024
1 parent 20e6160 commit 97b5986
Show file tree
Hide file tree
Showing 30 changed files with 966 additions and 71 deletions.
51 changes: 34 additions & 17 deletions cmd/counters/src/ipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,29 @@ impl Args {
let Self { clients, opts } = self;
// In order to display task generations accurately, we must find and load
// the task table:
let task_table = {
let (base, task_count) = hubris.task_table(core)?;
let task_t = hubris.lookup_struct_byname("Task")?.clone();
humility_doppel::Task::load_tcbs(
hubris,
core,
base,
task_count as usize,
&task_t,
)?
};
let task_table = hubris
.task_table(core)
.and_then(|(base, task_count)| {
let task_t = hubris.lookup_struct_byname("Task")?.clone();
humility_doppel::Task::load_tcbs(
hubris,
core,
base,
task_count as usize,
&task_t,
)
})
// This may be a single-task dump, in which case, we cannot load the
// task table. In that case, rather than bailing, we'll just not
// display restart counts.
.unwrap_or_else(|err| {
humility::warn!("failed to load task table: {err}");
humility::warn!(
"no generations/restart counts will be displayed."
);
humility::msg!("note: this may be a single-task dump.");
Vec::new()
});

let mut ipcs = BTreeMap::new();

Expand All @@ -60,7 +72,9 @@ impl Args {
continue;
}

let gen = task_table[task.task() as usize].generation;
let gen = task_table
.get(task.task() as usize)
.map(|task| task.generation);
// Only select counters matching the provided filter, if there is
// one.
if let Some(ref name) = opts.name {
Expand Down Expand Up @@ -126,9 +140,9 @@ struct IpcIface<'a> {
}

#[derive(Default)]
struct Ipc<'taskname>(
IndexMap<(&'taskname str, GenOrRestartCount), CounterVariant>,
);
struct Ipc<'taskname>(IndexMap<ClientTask<'taskname>, CounterVariant>);

type ClientTask<'taskname> = (&'taskname str, Option<GenOrRestartCount>);

impl IpcIface<'_> {
fn total(&self) -> usize {
Expand Down Expand Up @@ -231,12 +245,15 @@ impl fmt::Display for IpcIface<'_> {
let sign = if formatted_tasks > 1 { "+" } else { "=" };
let ok_str = fmt_oks(ok, format!("{sign} {ok}",));
let restarts = match gen {
GenOrRestartCount::Gen(gen) => {
Some(GenOrRestartCount::Gen(gen)) => {
format!(" (gen {gen:?})")
}
GenOrRestartCount::RestartCount(restarts) => {
Some(GenOrRestartCount::RestartCount(restarts)) => {
format!(" ({restarts} restarts)")
}
// This is a single-task dump, so we don't have generation
// numbers or restart counts.
None => String::new(),
};
write!(f, "{INDENT}task {}{restarts} ", task.italic())?;

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions tests/cmd/counters-ipc-full/counters-ipc-full.ouray.55.stderr

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions tests/cmd/counters-ipc-full/counters-ipc-full.ouray.61.stderr

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions tests/cmd/counters-ipc-full/counters-ipc-full.ouray.62.stderr

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 97b5986

Please sign in to comment.