Skip to content

Commit 6a2ecba

Browse files
gekygeky
committed
Replaced bool with lfs3->pcksum for prog-aligned cksums
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.
1 parent fb0d8e5 commit 6a2ecba

File tree

3 files changed

+99
-97
lines changed

3 files changed

+99
-97
lines changed

‎lfs3.c‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ static lfs3_scmp_t lfs3_bd_cmp(lfs3_t *lfs3,
312312
#ifndef LFS3_RDONLY
313313
static int lfs3_bd_prog_(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
314314
const void *buffer, lfs3_size_t size,
315-
uint32_t *cksum, bool align) {
315+
uint32_t *cksum) {
316316
// must be in-bounds
317317
LFS3_ASSERT(block < lfs3->block_count);
318318
LFS3_ASSERT(off+size <= lfs3->cfg->block_size);
@@ -358,9 +358,9 @@ static int lfs3_bd_prog_(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
358358
lfs3->rcache.size - (off-lfs3->rcache.off));
359359
}
360360

361-
// optional aligned checksum
362-
if (cksum && align) {
363-
*cksum = lfs3_crc32c(*cksum, buffer, size);
361+
// optional prog-aligned checksum
362+
if (cksum && cksum == &lfs3->pcksum) {
363+
lfs3->pcksum = lfs3_crc32c(lfs3->pcksum, buffer, size);
364364
}
365365

366366
return 0;
@@ -369,7 +369,7 @@ static int lfs3_bd_prog_(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
369369

370370
// flush the pcache
371371
#ifndef LFS3_RDONLY
372-
static int lfs3_bd_flush(lfs3_t *lfs3, uint32_t *cksum, bool align) {
372+
static int lfs3_bd_flush(lfs3_t *lfs3, uint32_t *cksum) {
373373
if (lfs3->pcache.size != 0) {
374374
// must be in-bounds
375375
LFS3_ASSERT(lfs3->pcache.block < lfs3->block_count);
@@ -386,7 +386,7 @@ static int lfs3_bd_flush(lfs3_t *lfs3, uint32_t *cksum, bool align) {
386386
// flush
387387
int err = lfs3_bd_prog_(lfs3, lfs3->pcache.block,
388388
lfs3->pcache.off, lfs3->pcache.buffer, size,
389-
cksum, align);
389+
cksum);
390390
if (err) {
391391
return err;
392392
}
@@ -403,7 +403,7 @@ static int lfs3_bd_flush(lfs3_t *lfs3, uint32_t *cksum, bool align) {
403403
static int lfs3_bd_prognext(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
404404
lfs3_size_t size,
405405
uint8_t **buffer_, lfs3_size_t *size_,
406-
uint32_t *cksum, bool align) {
406+
uint32_t *cksum) {
407407
// must be in-bounds
408408
LFS3_ASSERT(block < lfs3->block_count);
409409
LFS3_ASSERT(off+size <= lfs3->cfg->block_size);
@@ -430,7 +430,7 @@ static int lfs3_bd_prognext(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
430430
}
431431

432432
// flush pcache?
433-
int err = lfs3_bd_flush(lfs3, cksum, align);
433+
int err = lfs3_bd_flush(lfs3, cksum);
434434
if (err) {
435435
return err;
436436
}
@@ -455,7 +455,7 @@ static int lfs3_bd_prognext(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
455455
#ifndef LFS3_RDONLY
456456
static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
457457
const void *buffer, lfs3_size_t size,
458-
uint32_t *cksum, bool align) {
458+
uint32_t *cksum) {
459459
// must be in-bounds
460460
LFS3_ASSERT(block < lfs3->block_count);
461461
LFS3_ASSERT(off+size <= lfs3->cfg->block_size);
@@ -494,7 +494,7 @@ static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
494494
//
495495
// flush even if we're bypassing pcache, some devices don't
496496
// support out-of-order progs in a block
497-
int err = lfs3_bd_flush(lfs3, cksum, align);
497+
int err = lfs3_bd_flush(lfs3, cksum);
498498
if (err) {
499499
return err;
500500
}
@@ -505,7 +505,7 @@ static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
505505
&& size_ >= lfs3->cfg->pcache_size) {
506506
lfs3_size_t d = lfs3_aligndown(size_, lfs3->cfg->prog_size);
507507
int err = lfs3_bd_prog_(lfs3, block, off_, buffer_, d,
508-
cksum, align);
508+
cksum);
509509
if (err) {
510510
return err;
511511
}
@@ -528,7 +528,7 @@ static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
528528
}
529529

530530
// optional checksum
531-
if (cksum && !align) {
531+
if (cksum && cksum != &lfs3->pcksum) {
532532
*cksum = lfs3_crc32c(*cksum, buffer, size);
533533
}
534534

@@ -539,7 +539,7 @@ static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
539539
#ifndef LFS3_RDONLY
540540
static int lfs3_bd_sync(lfs3_t *lfs3) {
541541
// make sure we flush any caches
542-
int err = lfs3_bd_flush(lfs3, NULL, false);
542+
int err = lfs3_bd_flush(lfs3, NULL);
543543
if (err) {
544544
return err;
545545
}
@@ -637,7 +637,7 @@ static int lfs3_bd_cpy(lfs3_t *lfs3,
637637
lfs3_block_t dst_block, lfs3_size_t dst_off,
638638
lfs3_block_t src_block, lfs3_size_t src_off, lfs3_size_t hint,
639639
lfs3_size_t size,
640-
uint32_t *cksum, bool align) {
640+
uint32_t *cksum) {
641641
// must be in-bounds
642642
LFS3_ASSERT(dst_block < lfs3->block_count);
643643
LFS3_ASSERT(dst_off+size <= lfs3->cfg->block_size);
@@ -656,7 +656,7 @@ static int lfs3_bd_cpy(lfs3_t *lfs3,
656656
lfs3_size_t size__;
657657
int err = lfs3_bd_prognext(lfs3, dst_block, dst_off_, size_,
658658
&buffer__, &size__,
659-
cksum, align);
659+
cksum);
660660
if (err) {
661661
return err;
662662
}
@@ -668,7 +668,7 @@ static int lfs3_bd_cpy(lfs3_t *lfs3,
668668
}
669669

670670
// optional checksum
671-
if (cksum && !align) {
671+
if (cksum && cksum != &lfs3->pcksum) {
672672
*cksum = lfs3_crc32c(*cksum, buffer__, size__);
673673
}
674674

@@ -685,7 +685,7 @@ static int lfs3_bd_cpy(lfs3_t *lfs3,
685685
#ifndef LFS3_RDONLY
686686
static int lfs3_bd_set(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
687687
uint8_t c, lfs3_size_t size,
688-
uint32_t *cksum, bool align) {
688+
uint32_t *cksum) {
689689
// must be in-bounds
690690
LFS3_ASSERT(block < lfs3->block_count);
691691
LFS3_ASSERT(off+size <= lfs3->cfg->block_size);
@@ -697,15 +697,15 @@ static int lfs3_bd_set(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
697697
lfs3_size_t size__;
698698
int err = lfs3_bd_prognext(lfs3, block, off_, size_,
699699
&buffer__, &size__,
700-
cksum, align);
700+
cksum);
701701
if (err) {
702702
return err;
703703
}
704704

705705
lfs3_memset(buffer__, c, size__);
706706

707707
// optional checksum
708-
if (cksum && !align) {
708+
if (cksum && cksum != &lfs3->pcksum) {
709709
*cksum = lfs3_crc32c(*cksum, buffer__, size__);
710710
}
711711

@@ -938,7 +938,7 @@ static int lfs3_bd_cpyck(lfs3_t *lfs3,
938938
lfs3_block_t src_block, lfs3_size_t src_off, lfs3_size_t hint,
939939
lfs3_size_t size,
940940
lfs3_size_t src_cksize, uint32_t src_cksum,
941-
uint32_t *cksum, bool align) {
941+
uint32_t *cksum) {
942942
// must be in-bounds
943943
LFS3_ASSERT(dst_block < lfs3->block_count);
944944
LFS3_ASSERT(dst_off+size <= lfs3->cfg->block_size);
@@ -971,7 +971,7 @@ static int lfs3_bd_cpyck(lfs3_t *lfs3,
971971
lfs3_size_t size__;
972972
err = lfs3_bd_prognext(lfs3, dst_block, dst_off_, size_,
973973
&buffer__, &size__,
974-
cksum, align);
974+
cksum);
975975
if (err) {
976976
return err;
977977
}
@@ -986,7 +986,7 @@ static int lfs3_bd_cpyck(lfs3_t *lfs3,
986986
cksum__ = lfs3_crc32c(cksum__, buffer__, size__);
987987

988988
// optional prog checksum
989-
if (cksum && !align) {
989+
if (cksum && cksum != &lfs3->pcksum) {
990990
*cksum = lfs3_crc32c(*cksum, buffer__, size__);
991991
}
992992

@@ -1540,7 +1540,7 @@ static lfs3_ssize_t lfs3_bd_readtag(lfs3_t *lfs3,
15401540
static lfs3_ssize_t lfs3_bd_progtag(lfs3_t *lfs3,
15411541
lfs3_block_t block, lfs3_size_t off, bool perturb,
15421542
lfs3_tag_t tag, lfs3_rid_t weight, lfs3_size_t size,
1543-
uint32_t *cksum, bool align) {
1543+
uint32_t *cksum) {
15441544
// we set the valid bit here
15451545
LFS3_ASSERT(!(tag & 0x8000));
15461546
// bit 7 is reserved for future subtype extensions
@@ -1576,7 +1576,7 @@ static lfs3_ssize_t lfs3_bd_progtag(lfs3_t *lfs3,
15761576
d += d_;
15771577

15781578
int err = lfs3_bd_prog(lfs3, block, off, tag_buf, d,
1579-
cksum, align);
1579+
cksum);
15801580
if (err) {
15811581
return err;
15821582
}
@@ -1867,7 +1867,7 @@ static lfs3_scmp_t lfs3_data_namecmp(lfs3_t *lfs3, lfs3_data_t data,
18671867
#ifndef LFS3_RDONLY
18681868
static int lfs3_bd_progdata(lfs3_t *lfs3,
18691869
lfs3_block_t block, lfs3_size_t off, lfs3_data_t data,
1870-
uint32_t *cksum, bool align) {
1870+
uint32_t *cksum) {
18711871
// on-disk?
18721872
if (lfs3_data_ondisk(data)) {
18731873
// validating data cksums?
@@ -1880,7 +1880,7 @@ static int lfs3_bd_progdata(lfs3_t *lfs3,
18801880
data.u.disk.block, data.u.disk.off, lfs3_data_size(data),
18811881
lfs3_data_size(data),
18821882
lfs3_data_cksize(data), lfs3_data_cksum(data),
1883-
cksum, align);
1883+
cksum);
18841884
if (err) {
18851885
return err;
18861886
}
@@ -1890,7 +1890,7 @@ static int lfs3_bd_progdata(lfs3_t *lfs3,
18901890
int err = lfs3_bd_cpy(lfs3, block, off,
18911891
data.u.disk.block, data.u.disk.off, lfs3_data_size(data),
18921892
lfs3_data_size(data),
1893-
cksum, align);
1893+
cksum);
18941894
if (err) {
18951895
return err;
18961896
}
@@ -1900,7 +1900,7 @@ static int lfs3_bd_progdata(lfs3_t *lfs3,
19001900
} else {
19011901
int err = lfs3_bd_prog(lfs3, block, off,
19021902
data.u.buffer, data.size,
1903-
cksum, align);
1903+
cksum);
19041904
if (err) {
19051905
return err;
19061906
}
@@ -3318,7 +3318,7 @@ static int lfs3_rbyd_appendrev(lfs3_t *lfs3, lfs3_rbyd_t *rbyd, uint32_t rev) {
33183318
int err = lfs3_bd_prog(lfs3,
33193319
rbyd->blocks[0], lfs3_rbyd_eoff(rbyd),
33203320
&rev_buf, sizeof(uint32_t),
3321-
&rbyd->cksum, false);
3321+
&rbyd->cksum);
33223322
if (err) {
33233323
return err;
33243324
}
@@ -3346,7 +3346,7 @@ static int lfs3_rbyd_appendtag(lfs3_t *lfs3, lfs3_rbyd_t *rbyd,
33463346
lfs3_ssize_t d = lfs3_bd_progtag(lfs3,
33473347
rbyd->blocks[0], lfs3_rbyd_eoff(rbyd), lfs3_rbyd_isperturb(rbyd),
33483348
tag, weight, size,
3349-
&rbyd->cksum, false);
3349+
&rbyd->cksum);
33503350
if (d < 0) {
33513351
return d;
33523352
}
@@ -3568,7 +3568,7 @@ static int lfs3_rbyd_appendrattr_(lfs3_t *lfs3, lfs3_rbyd_t *rbyd,
35683568
for (lfs3_size_t i = 0; i < data_count; i++) {
35693569
err = lfs3_bd_progdata(lfs3,
35703570
rbyd->blocks[0], lfs3_rbyd_eoff(rbyd), datas[i],
3571-
&rbyd->cksum, false);
3571+
&rbyd->cksum);
35723572
if (err) {
35733573
return err;
35743574
}
@@ -4472,13 +4472,13 @@ static int lfs3_rbyd_appendcksum_(lfs3_t *lfs3, lfs3_rbyd_t *rbyd,
44724472
// prog, when this lands on disk commit is committed
44734473
int err = lfs3_bd_prog(lfs3, rbyd->blocks[0], lfs3_rbyd_eoff(rbyd),
44744474
cksum_buf, 2+1+4+4,
4475-
NULL, false);
4475+
NULL);
44764476
if (err) {
44774477
return err;
44784478
}
44794479

44804480
// flush any pending progs
4481-
err = lfs3_bd_flush(lfs3, NULL, false);
4481+
err = lfs3_bd_flush(lfs3, NULL);
44824482
if (err) {
44834483
return err;
44844484
}
@@ -12530,7 +12530,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1253012530
lfs3_off_t pos_ = block_pos
1253112531
+ lfs3_bptr_off(&file->leaf.bptr)
1253212532
+ lfs3_bptr_size(&file->leaf.bptr);
12533-
uint32_t cksum_ = lfs3_bptr_cksum(&file->leaf.bptr);
12533+
lfs3->pcksum = lfs3_bptr_cksum(&file->leaf.bptr);
1253412534
while (pos_ < crystal_limit) {
1253512535
// keep track of the next highest priority data offset
1253612536
lfs3_ssize_t d = crystal_limit - pos_;
@@ -12545,7 +12545,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1254512545
lfs3_bptr_block(&file->leaf.bptr),
1254612546
pos_ - block_pos,
1254712547
&buffer[pos_ - pos], d_,
12548-
&cksum_, true);
12548+
&lfs3->pcksum);
1254912549
if (err) {
1255012550
LFS3_ASSERT(err != LFS3_ERR_RANGE);
1255112551
// bad prog? try another block
@@ -12612,7 +12612,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1261212612
lfs3_data_slice(bptr__.d,
1261312613
pos_ - (bid__-(weight__-1)),
1261412614
d_),
12615-
&cksum_, true);
12615+
&lfs3->pcksum);
1261612616
if (err) {
1261712617
LFS3_ASSERT(err != LFS3_ERR_RANGE);
1261812618
// bad prog? try another block
@@ -12635,7 +12635,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1263512635
lfs3_bptr_block(&file->leaf.bptr),
1263612636
pos_ - block_pos,
1263712637
0, d,
12638-
&cksum_, true);
12638+
&lfs3->pcksum);
1263912639
if (err) {
1264012640
LFS3_ASSERT(err != LFS3_ERR_RANGE);
1264112641
// bad prog? try another block
@@ -12674,7 +12674,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1267412674

1267512675
// finalize our write
1267612676
int err = lfs3_bd_flush(lfs3,
12677-
&cksum_, true);
12677+
&lfs3->pcksum);
1267812678
if (err) {
1267912679
// bad prog? try another block
1268012680
if (err == LFS3_ERR_CORRUPT) {
@@ -12695,7 +12695,7 @@ static int lfs3_file_crystallize__(lfs3_t *lfs3, lfs3_file_t *file,
1269512695
pos_ - file->leaf.pos),
1269612696
// mark as erased
1269712697
(pos_ - block_pos) | LFS3_BPTR_ISERASED,
12698-
cksum_);
12698+
lfs3->pcksum);
1269912699
return 0;
1270012700

1270112701
relocate:;

‎lfs3.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,8 @@ typedef struct lfs3 {
838838
lfs3_size_t size;
839839
uint8_t *buffer;
840840
} pcache;
841+
// optional prog-aligned cksum
842+
uint32_t pcksum;
841843
#endif
842844

843845
#if !defined(LFS3_RDONLY) && defined(LFS3_CKMETAPARITY)

0 commit comments

Comments
 (0)