Github/littlefs
1
Fork 0
Code Actions Activity

Replaced bool with lfs3->pcksum for prog-aligned cksums

Browse Source 
This replaces the `bool align` parameter that goes through all the prog
layers with an optional prog-aligned cksum stored in the lfs3_t struct.
Normally ignored, this prog-aligned cksum can be requested by setting
cksum=&lfs3->pcksum in any prog call.

Does this work? Yes. Is it a great solution? Ehhhh...

I've been tinkering with other solutions that avoid the `bool align`
parameter, but with no luck.

- `bool align`, or previously two cksum arguments, work, but create a
  bit of a messy API. I'd like to find an alternative solution.

- Changing the cksum pointer to a richer lfs3_cksum_t struct with flags
  also works, but would be an even messier API.

- Adding an lfs3_t side-channel, lfs3->pcache could include a pointer to
  an optional prog-aligned cksum. But this would be the same/more cost
  as just storing the pcksum in lfs3_t. And then we'd need to worry
  about disentangling the cksum pointer on errors, etc.

- We could set a flag in lfs3->flags for alignment. This avoids the
  extra 4 bytes of ctx, but still suffers from the risk of entangled
  state on errors, etc.

- We could unconditionally calculate lfs3->pcksum. But then we'd be
  calculating a lot of cksums we don't use (every metadata commit), and
  still using the extra 4 bytes of ctx.

Lacking a good solution, using cksum=&lfs3->pcksum to indicate a
prog-aligned cksum is at least an ok solution.

I will happily change this if an alternative comes up in the future.

Another way of viewing this is that `&lfs3->pcksum` acts as a special
magic pointer value to tell the prog layers to calculate lfs3->pcksum.
A different non-NULL constant value could have worked just as well, but
those are a bit trickier to create in C.

---

Actually, there is a "better" cursed solution:

- Rely on pointer alignment to sneak a flag into the cksum pointer's
  lower bits.

But, while clever, this is is outside of C's machine model and would
limit portability.

---

This trades 4 bytes of ctx for 58 bytes of code and simpler (debatable)
internal prog APIs:

           code          stack          ctx
  before: 36860           2384          652
  after:  36832 (-0.1%)   2384 (+0.0%)  656 (+0.6%)

In theory this also saves stack in all the prog APIs, but none of prog
APIs end up on the stack hot-path. In our codebase the read APIs
dominate the stack thanks to block allocator traversals.
This commit is contained in:
Christopher Haster
2025-07-16 17:50:06 -05:00
parent fb0d8e5380
commit 6a2ecbac87
3 changed files with 99 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_rbyd.toml
+58 -58
View File
@@ -2837,9 +2837,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// update each tag in permutation order
for (unsigned j = 0; j < N; j++) {
@@ -3144,9 +3144,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, -1, LFS3_RATTRS(
LFS3_RATTR(
@@ -3293,9 +3293,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, -1, LFS3_RATTRS(
LFS3_RATTR(
@@ -3905,9 +3905,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove each tag in permutation order
for (unsigned j = 0; j < N; j++) {
@@ -6051,9 +6051,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// update each tag in permutation order
for (unsigned j = 0; j < N*M; j++) {
@@ -6203,9 +6203,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j/M, LFS3_RATTRS(
LFS3_RATTR(
@@ -6402,9 +6402,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove each tag in permutation order
for (unsigned j = 0; j < N*M; j++) {
@@ -10601,9 +10601,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_RM, -1))) => 0;
@@ -10773,9 +10773,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_RM, -1))) => 0;
@@ -10969,9 +10969,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_RM, -1))) => 0;
@@ -11106,9 +11106,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_RM, -1))) => 0;
@@ -11555,9 +11555,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// delete each rid in permutation order
for (unsigned j = 0; j < N; j++) {
@@ -11708,9 +11708,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// delete each rid in permutation order
for (unsigned j = 0; j < N; j++) {
@@ -13786,9 +13786,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_GROW, +D))) => 0;
@@ -13918,9 +13918,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR_BUF(
@@ -14065,9 +14065,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR_BUF(
@@ -14226,9 +14226,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_GROW, -D))) => 0;
@@ -14358,9 +14358,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR_BUF(
@@ -14505,9 +14505,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR_BUF(
@@ -14665,9 +14665,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR(LFS3_TAG_RM, -W))) => 0;
@@ -14834,9 +14834,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
lfs3_rbyd_commit(&lfs3, &rbyd, j*W+W-1, LFS3_RATTRS(
LFS3_RATTR_BUF(
@@ -15465,9 +15465,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove with a wide tag
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -15610,9 +15610,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// replace with bitwise inverse
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -15853,9 +15853,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove with a wide tag
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -16008,9 +16008,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// replace with bitwise inverse
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -16246,9 +16246,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove with a wide tag
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -16391,9 +16391,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// replace with bitwise inverse
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -16622,9 +16622,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// remove with a wide tag
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(
@@ -16768,9 +16768,9 @@ code = '''
lfs3_bd_erase(&lfs3, rbyd.blocks[0]) => 0;
lfs3_bd_prog(&lfs3, rbyd.blocks[0], 0,
backup_block, lfs3_rbyd_eoff(&rbyd),
NULL, false) => 0;
NULL) => 0;
lfs3_bd_flush(&lfs3,
NULL, false) => 0;
NULL) => 0;
// replace with bitwise inverse
lfs3_rbyd_commit(&lfs3, &rbyd, j, LFS3_RATTRS(