Github/littlefs
1
Fork 0
Code Actions Activity

Rough implementation of ckreads

Browse Source 
With the adoption of the odd-parity-zero rbyd perturb scheme, it's now
possible to validate individual tag's parity with neighboring valid
bits. This sparked an idea that I previously thought was intractable.

If we:

1. Validate all metadata reads by checking their on-disk parity bits.

2. Validate all data reads by checking their in-metadata checksums.

We end up with a closed system where all reads are checked by at least
a parity bit.

Being able to check all reads is a very valuable filesystem feature, but
difficult for littlefs:

- We need to keep relevant data in RAM while validating checksums.

  We can't just validate checksums and then perform a second read as
  that creates a hole where new bit-errors may be introduced.

- This is solved in other filesystems by loading and checking whole
  blocks in RAM. We just can't do that here.

- Without parity, we would need to check the rbyd's checksum on every
  tag read. This would lead to a crazy O(n^2 log n) rbyd compaction
  runtime.

  Which is why I original thought ckreads was just intractable.

Now, this isn't all sunshine and rainbows. ckreads, as implemented here,
has some deeply concerning flaws:

- A parity bit is, mathematically, the minimum possible error-detection
  possible. Is validating reads with only a parity bit sufficient for
  real world applications?

- Validating data checksums on every read may have severe performance
  implications. We need to read up to the entire block, which can lead
  to O(n^2) behavior when performing a lot of small reads in a file.

- In order to validate checksums/parity-bits, we need to know where the
  checksums/parity-bits actually are for each piece of data.

  Our lfsr_data_t struct provides a surprisingly nice abstraction for
  this, but oof is it expensive.

For the added code/stack cost alone, we probably want to eventually make
this an opt-in compile-time feature.

---

Implementation notes:

- This found an actual compiler bug! Turns out increasing lfsr_data_t
  from 3-words to 5-words confuses GCC:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101854

- Mid-commit, we may have not actually written the last tag's parity
  yet, which is a bit of a problem because we may read the last tag when
  building the next trunk!

  Fixing this required a whole separate tailck mechanism, which just
  tracks in-progress commit's parity bits.

  This doesn't help the code/stack cost situation...

- lfsr_bd_read/cmp/cpy all need to be extended to support calculating a
  checksum on the side, which is a bit of a mess.

- bptr's cksize/cksum is redundant now, which is going to make
  conditional compilation a mess.

- The extra parity byte we need to read makes hint calculation a pain.

Code cost wise... yeah, it's significant. Turns out almost doubling
lfsr_data_t has a significant impact on stack usage. Add in all the
extra code to track checksums/parity-bits and validate checksums/
parity-bits and you got yourself a pretty heavy feature:

           code          stack
  before: 36352           2672
  after:  38100 (+4.8%)   3032 (+13.5%)
This commit is contained in:
Christopher Haster
2024-08-07 15:05:22 -05:00
parent 1044c9d2b7
commit ccc073faed
4 changed files with 700 additions and 139 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Makefile
+2
View File
@@ -75,6 +75,8 @@ CFLAGS += -fcallgraph-info=su
CFLAGS += -g3
CFLAGS += -I.
CFLAGS += -std=c99 -Wall -Wextra -pedantic
# compiler bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101854
CFLAGS += -Wno-stringop-overflow
CFLAGS += -ftrack-macro-expansion=0
ifdef DEBUG
CFLAGS += -O0
lfs.c
+648 -107
View File
File diff suppressed because it is too large Load Diff
lfs.h
+21 -3
View File
@@ -534,17 +534,29 @@ typedef struct lfsr_omdir {
// lfs_block_t tail[2];
//} lfs_mdir_t;
// context for validating data
typedef struct lfsr_ck {
// sign(cksize)=0 => cksum check
// sign(cksize)=1 => parity check
lfs_size_t cksize;
union {
lfs_size_t ckoff;
uint32_t cksum;
} u;
} lfsr_ck_t;
// either an on-disk or in-device data pointer
typedef struct lfsr_data {
// sign(size)=0 => in-RAM buffer
// sign(size)=1 => on-disk reference
lfs_size_t size;
union {
const uint8_t *buffer;
struct {
lfs_block_t block;
lfs_size_t off;
lfsr_ck_t ck;
} disk;
const uint8_t *buffer;
} u;
} lfsr_data_t;
@@ -574,8 +586,6 @@ typedef lfsr_data_t lfsr_sprout_t;
typedef struct lfsr_bptr {
lfsr_data_t data;
lfs_size_t cksize;
uint32_t cksum;
} lfsr_bptr_t;
// the lfsr_bshrub_t struct represents the on-disk component of a file
@@ -714,6 +724,12 @@ typedef struct lfsr_grm {
lfsr_smid_t mids[2];
} lfsr_grm_t;
typedef struct lfsr_tailck {
lfs_block_t ckblock;
// sign(ckoff) => tail parity
lfs_size_t ckoff;
} lfsr_tailck_t;
// The littlefs filesystem type
typedef struct lfs {
const struct lfs_config *cfg;
@@ -746,6 +762,8 @@ typedef struct lfs {
uint8_t *buffer;
} pcache;
lfsr_tailck_t tailck;
struct lfs_lookahead {
lfs_block_t window;
lfs_block_t off;
tests/test_rbyd.toml
+29 -29
View File
@@ -2815,7 +2815,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -3140,7 +3140,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -3290,7 +3290,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -3905,7 +3905,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -6054,7 +6054,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -6221,7 +6221,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -6401,7 +6401,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -10615,7 +10615,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try deleting each rid
@@ -10784,7 +10784,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try deleting each rid
@@ -10977,7 +10977,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try deleting each rid
@@ -11114,7 +11114,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try deleting each rid
@@ -11547,7 +11547,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -11703,7 +11703,7 @@ code = '''
// copy block so we can reset after each delete
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// test all permutations of a given size
@@ -13808,7 +13808,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try growing each rid
@@ -13940,7 +13940,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try growing each rid
@@ -14084,7 +14084,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try growing each rid
@@ -14241,7 +14241,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try shrinking each rid
@@ -14373,7 +14373,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try shrinking each rid
@@ -14517,7 +14517,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try shrinking each rid
@@ -14673,7 +14673,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try deleting each rid
@@ -14837,7 +14837,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try appending an attr to each rid, this should not affect
@@ -15471,7 +15471,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -15615,7 +15615,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try replacing each tag
@@ -15857,7 +15857,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -16011,7 +16011,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try replacing each tag
@@ -16248,7 +16248,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -16392,7 +16392,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try replacing each tag
@@ -16622,7 +16622,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try removing each tag
@@ -16766,7 +16766,7 @@ code = '''
// copy block so we can reset after each remove
lfsr_rbyd_t backup_rbyd = rbyd;
uint8_t *backup_block = malloc(lfsr_rbyd_eoff(&rbyd));
lfsr_bd_read(&lfs, rbyd.blocks[0], 0, lfsr_rbyd_eoff(&rbyd),
CFG->read(CFG, rbyd.blocks[0], 0,
backup_block, lfsr_rbyd_eoff(&rbyd)) => 0;
// try replacing each tag