Added check_progs for immediate prog validation

This configuration option enables the previous behavior of reading back
every prog to check that the data was written correctly.

Unfortunately, this brings a bit of baggage, thanks to our cache
interactions being more complicated now:

- We really want to reuse the rcache for prog validation, despite the
  cache performance implications. Unfortunately, we simply can't, thanks
  to the new bd utility functions tying up the rcache. lfsr_bd_cpy, for
  example, does not expect rcache to be invalidated between a read and
  prog, and if it is, things break (I may or may not have found this by
  experience).

  These bd utilities are valuable, so we really need some other way to
  validate our progs.

- Since we can't rely on the rcache, this leaves checksumming as the
  only option for validating progs. Checksumming isn't perfect, as there
  is a decent chance of false negatives, but to be honest it's probably
  good enough for anything that's not malicious.

- This also adds the new constraint that we need to be able to read back
  any prog into the pcache, which implies read_size <= prog_size. This
  constraint didn't exist when we could clobber our rcache, but this is
  not worth throwing away the new bd utilities. Not to mention
  clobbering our rcache could hurt cache performance.

  Why not make read_size <= prog_size conditional on check_progs?

  The main reason is convenience. One very compelling use case for
  check_progs is to help debug unknown filesystem/integration failures,
  buf if you can't enable check_progs without changing the filesystem
  configuration, you can't really rely on check_progs for debugging.

  This helps future proof what we expect from block devices, in case
  future error detection/correction mechanisms can benefit from our
  prog_size always being readable.

Code changes were not that significant, however there was a surprising
stack cost. This seems to be because lfsr_bd_read__ can now be called
from multiple places, causing it to no longer be inlined in
lfsr_bd_read_, costing a bit of stack for the additional function call:

  before: 33566           2624
  after:  33682 (+0.3%)   2640 (+0.6%)
This commit is contained in:
Christopher Haster
2024-05-29 01:59:16 -05:00
parent 31eebc1328
commit c648f96dc5
6 changed files with 171 additions and 76 deletions
+6 -1
View File
@@ -195,7 +195,7 @@ struct lfs_config {
lfs_size_t read_size;
// Minimum size of a program in bytes. All program operations will be a
// multiple of this value.
// multiple of this value. Must be a multiple of the read size.
lfs_size_t prog_size;
// Size of an erasable block in bytes. This does not impact ram consumption
@@ -301,6 +301,11 @@ struct lfs_config {
// 0 only writes blocks, minimizing disk usage, while -1 or any value >=
// block_size only writes fragments, minimizing random-write cost.
lfs_size_t crystal_thresh;
// TODO should lfs_mount accept flags?
// Check progs by immediately reading back any progged data
bool check_progs;
};
// File info structure