From 6e2af5bf804924cdd854fd8e05e919d3a12ccfe2 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 11 Aug 2024 18:18:28 -0500 Subject: [PATCH] Carved out ckreads, disabled at compile-time by default This moves all ckread-related logic behind the new opt-in compile-time LFS_CKREADS flag. So in order to use ckreads you need to 1. define LFS_CKREADS at compile time, and 2. pass LFS_M_CKREADS during lfsr_mount. This was always the plan since, even if ckreads worked perfectly, it adds a significant amount of baggage (stack mostly) to track the ck context of all reads. --- This is the first non-trivial opt-in define in littlefs, so more test framework features! test.py and build.py now support the optional ifdef attribute, which makes it easy to indicate a test suite/case should not be compiled when a feature is missing. Also interesting to note is the addition of LFS_IFDEF_CKREADS, which solves several issues (and general ugliness) related to #ifdefs in expression. For example: // does not compile :( (can't embed ifdefs in macros) LFS_ASSERT(flags == ( LFS_M_CKPROGS #ifdef LFS_CKREADS | LFS_M_CKREADS #endif )) // does compile :) LFS_ASSERT(flags == ( LFS_M_CKPROGS | LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0))); --- This brings us way back down to our pre-ckread levels of code/stack: code stack before-ckreads: 36352 2672 ckreads: 38060 (+4.7%) 3056 (+14.4%) after-ckreads: 36428 (+0.2%) 2680 (+0.3%) Unfortunately, we do end up with a bit more code cost than where we started. Mainly due to code moving around to support the ckread infrastructure: code stack lfsr_bd_readtag: +52 (+23.2%) +8 (+10.0%) lfsr_rbyd_fetch: +36 (+5.0%) +8 (+6.2%, cold) lfs_toleb128: -12 (-25.0%) -4 (-20.0%, cold) total: +76 (+0.2%) +8 (+0.3%) But oh well. Note that some of these changes are good even without ckreads, such as only parsing the last ecksum tag. --- lfs.c | 256 +++++++++++++++++++++++++++++++++-------- lfs.h | 16 +++ lfs_util.h | 8 ++ runners/bench_runner.c | 4 +- runners/test_runner.c | 4 +- scripts/bench.py | 55 ++++++++- scripts/test.py | 53 +++++++++ tests/test_ck.toml | 4 + tests/test_mount.toml | 5 +- 9 files changed, 352 insertions(+), 53 deletions(-) diff --git a/lfs.c b/lfs.c index 494be5d8..8a70ad71 100644 --- a/lfs.c +++ b/lfs.c @@ -695,31 +695,44 @@ static int lfsr_bd_set(lfs_t *lfs, lfs_block_t block, lfs_size_t off, /// lfsr_ck_t stuff /// +#ifdef LFS_CKREADS #define LFSR_CK_ISPARITY 0x80000000 +#endif +#ifdef LFS_CKREADS #define LFSR_CK_CKSUM(_cksize, _cksum) \ ((lfsr_ck_t){ \ .cksize=_cksize, \ .u.cksum=_cksum}) +#endif +#ifdef LFS_CKREADS #define LFSR_CK_PARITY(_ckoff, _cksize) \ ((lfsr_ck_t){ \ .cksize=LFSR_CK_ISPARITY | (_cksize), \ .u.ckoff=_ckoff}) +#endif // ck helpers +#ifdef LFS_CKREADS static inline bool lfsr_ck_isparity(lfsr_ck_t ck) { return ck.cksize & LFSR_CK_ISPARITY; } +#endif +#ifdef LFS_CKREADS static inline bool lfsr_ck_iscksum(lfsr_ck_t ck) { return !(ck.cksize & LFSR_CK_ISPARITY); } +#endif +#ifdef LFS_CKREADS static inline lfs_size_t lfsr_ck_cksize(lfsr_ck_t ck) { return ck.cksize & ~LFSR_CK_ISPARITY; } +#endif +#ifdef LFS_CKREADS static inline lfs_size_t lfsr_ck_ckoff(lfsr_ck_t ck) { if (lfsr_ck_isparity(ck)) { return ck.u.ckoff; @@ -727,22 +740,30 @@ static inline lfs_size_t lfsr_ck_ckoff(lfsr_ck_t ck) { return 0; } } +#endif // lfsr_tailck_t stuff // // tailck tracks the most recent trunk's parity so we can parity-check // if it hasn't been written to disk yet +#ifdef LFS_CKREADS #define LFSR_TAILCK_PARITY 0x80000000 +#endif +#ifdef LFS_CKREADS static inline bool lfsr_tailck_parity(const lfsr_tailck_t *tailck) { return tailck->ckoff & LFSR_TAILCK_PARITY; } +#endif +#ifdef LFS_CKREADS static inline lfs_size_t lfsr_tailck_ckoff(const lfsr_tailck_t *tailck) { return tailck->ckoff & ~LFSR_TAILCK_PARITY; } +#endif +#ifdef LFS_CKREADS // read the pesky parity byte from disk static lfs_sbool_t lfsr_bd_readparity(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint) { @@ -774,7 +795,9 @@ static lfs_sbool_t lfsr_bd_readparity(lfs_t *lfs, return p >> 7; } } +#endif +#ifdef LFS_CKREADS static lfs_ssize_t lfsr_bd_ckprefix(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint, lfsr_ck_t ck, @@ -804,7 +827,9 @@ static lfs_ssize_t lfsr_bd_ckprefix(lfs_t *lfs, // earlier, otherwise we'd have real problems with hint=-1! return hint_ - (off-lfsr_ck_ckoff(ck)); } +#endif +#ifdef LFS_CKREADS static int lfsr_bd_cksuffix(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint, lfsr_ck_t ck, @@ -861,7 +886,9 @@ static int lfsr_bd_cksuffix(lfs_t *lfs, return 0; } +#endif +#ifdef LFS_CKREADS // caching read with parity/checksum checks // // the main downside of ckreads is we need to read all data that @@ -906,7 +933,9 @@ static int lfsr_bd_readck_(lfs_t *lfs, return 0; } +#endif +#ifdef LFS_CKREADS // needed in lfsr_bd_readck static inline bool lfsr_m_isckreads(uint32_t flags); @@ -926,6 +955,16 @@ static int lfsr_bd_readck(lfs_t *lfs, buffer, size); } } +#endif + +// redirect to lfsr_bd_read if not checking reads +#ifdef LFS_CKREADS +#define LFSR_BD_READCK(_lfs, _block, _off, _hint, _buffer, _size, _ck) \ + lfsr_bd_readck(_lfs, _block, _off, _hint, _buffer, _size, _ck) +#else +#define LFSR_BD_READCK(_lfs, _block, _off, _hint, _buffer, _size, _ck) \ + lfsr_bd_read(_lfs, _block, _off, _hint, _buffer, _size) +#endif // these could probably be a bit better deduplicated with their // unchecked counterparts, but we don't generally use both at the same @@ -933,6 +972,7 @@ static int lfsr_bd_readck(lfs_t *lfs, // // we'd also need to worry about early termination in lfsr_bd_cmp/cmpck +#ifdef LFS_CKREADS static lfs_scmp_t lfsr_bd_cmpck_(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint, const void *buffer, lfs_size_t size, @@ -991,7 +1031,9 @@ static lfs_scmp_t lfsr_bd_cmpck_(lfs_t *lfs, return cmp; } +#endif +#ifdef LFS_CKREADS static lfs_scmp_t lfsr_bd_cmpck(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint, const void *buffer, lfs_size_t size, @@ -1008,7 +1050,18 @@ static lfs_scmp_t lfsr_bd_cmpck(lfs_t *lfs, buffer, size); } } +#endif +// redirect to lfsr_bd_cmp if not checking reads +#ifdef LFS_CKREADS +#define LFSR_BD_CMPCK(_lfs, _block, _off, _hint, _buffer, _size, _ck) \ + lfsr_bd_cmpck(_lfs, _block, _off, _hint, _buffer, _size, _ck) +#else +#define LFSR_BD_CMPCK(_lfs, _block, _off, _hint, _buffer, _size, _ck) \ + lfsr_bd_cmp(_lfs, _block, _off, _hint, _buffer, _size) +#endif + +#ifdef LFS_CKREADS static int lfsr_bd_cpyck_(lfs_t *lfs, lfs_block_t dst_block, lfs_size_t dst_off, lfs_block_t src_block, lfs_size_t src_off, lfs_size_t hint, @@ -1080,7 +1133,9 @@ static int lfsr_bd_cpyck_(lfs_t *lfs, return 0; } +#endif +#ifdef LFS_CKREADS static int lfsr_bd_cpyck(lfs_t *lfs, lfs_block_t dst_block, lfs_size_t dst_off, lfs_block_t src_block, lfs_size_t src_off, lfs_size_t hint, @@ -1103,6 +1158,35 @@ static int lfsr_bd_cpyck(lfs_t *lfs, cksum, align); } } +#endif + +// redirect to lfsr_bd_cpy if not checking reads +#ifdef LFS_CKREADS +#define LFSR_BD_CPYCK(_lfs, \ + _dst_block, _dst_off, \ + _src_block, _src_off, _hint, \ + _size, \ + _ck, \ + _cksum, _align) \ + lfsr_bd_cpyck(_lfs, \ + _dst_block, _dst_off, \ + _src_block, _src_off, _hint, \ + _size, \ + _ck, \ + _cksum, _align) +#else +#define LFSR_BD_CPYCK(_lfs, \ + _dst_block, _dst_off, \ + _src_block, _src_off, _hint, \ + _size, \ + _ck, \ + _cksum, _align) \ + lfsr_bd_cpy(_lfs, \ + _dst_block, _dst_off, \ + _src_block, _src_off, _hint, \ + _size, \ + _cksum, _align) +#endif @@ -1625,6 +1709,7 @@ static lfs_ssize_t lfsr_bd_readtag(lfs_t *lfs, *cksum = lfs_crc32c(*cksum, tag_buf, d); } + #ifdef LFS_CKREADS // check the parity if we're checking reads and not already // calculating a checksum // @@ -1649,6 +1734,7 @@ static lfs_ssize_t lfsr_bd_readtag(lfs_t *lfs, return err; } } + #endif // save what we found, clearing the valid bit, we don't need it // anymore @@ -1720,28 +1806,35 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs, .size=_size, \ .u.buffer=(const void*)(_buffer)}) -// TODO -#if 0 -#define LFSR_DATA_DISK(_block, _off, _size) \ - ((lfsr_data_t){ \ - .size=LFSR_DATA_ONDISK | (_size), \ - .u.disk.block=_block, \ - .u.disk.off=_off}) -#endif - +#ifdef LFS_CKREADS #define LFSR_DATA_DISKCKSUM(_block, _off, _size, _cksize, _cksum) \ ((lfsr_data_t){ \ .size=LFSR_DATA_ONDISK | (_size), \ .u.disk.block=_block, \ .u.disk.off=_off, \ .u.disk.ck=LFSR_CK_CKSUM(_cksize, _cksum)}) +#else +#define LFSR_DATA_DISKCKSUM(_block, _off, _size, _cksize, _cksum) \ + ((lfsr_data_t){ \ + .size=LFSR_DATA_ONDISK | (_size), \ + .u.disk.block=_block, \ + .u.disk.off=_off}) +#endif +#ifdef LFS_CKREADS #define LFSR_DATA_DISKPARITY(_block, _off, _size, _ckoff, _cksize) \ ((lfsr_data_t){ \ .size=LFSR_DATA_ONDISK | (_size), \ .u.disk.block=_block, \ .u.disk.off=_off, \ .u.disk.ck=LFSR_CK_PARITY(_ckoff, _cksize)}) +#else +#define LFSR_DATA_DISKPARITY(_block, _off, _size, _ckoff, _cksize) \ + ((lfsr_data_t){ \ + .size=LFSR_DATA_ONDISK | (_size), \ + .u.disk.block=_block, \ + .u.disk.off=_off}) +#endif // data helpers static inline bool lfsr_data_ondisk(lfsr_data_t data) { @@ -1806,7 +1899,7 @@ static lfs_ssize_t lfsr_data_read(lfs_t *lfs, lfsr_data_t *data, // on-disk? if (lfsr_data_ondisk(*data)) { - int err = lfsr_bd_readck(lfs, + int err = LFSR_BD_READCK(lfs, data->u.disk.block, data->u.disk.off, // note our hint includes the full data range lfsr_data_size(*data), @@ -1895,7 +1988,7 @@ static lfs_scmp_t lfsr_data_cmp(lfs_t *lfs, lfsr_data_t data, // on-disk? if (lfsr_data_ondisk(data)) { - int cmp = lfsr_bd_cmpck(lfs, + int cmp = LFSR_BD_CMPCK(lfs, // note the 0 hint, we don't usually use any following data data.u.disk.block, data.u.disk.off, 0, buffer, d, @@ -1948,7 +2041,7 @@ static int lfsr_bd_progdata(lfs_t *lfs, uint32_t *cksum, bool align) { // on-disk? if (lfsr_data_ondisk(data)) { - int err = lfsr_bd_cpyck(lfs, block, off, + int err = LFSR_BD_CPYCK(lfs, block, off, data.u.disk.block, data.u.disk.off, lfsr_data_size(data), lfsr_data_size(data), data.u.disk.ck, @@ -2386,6 +2479,25 @@ static int lfsr_data_readecksum(lfs_t *lfs, lfsr_data_t *data, #define LFSR_DATA_BPTR(_bptr) \ LFSR_DATA_BPTR_(_bptr, (uint8_t[LFSR_BPTR_DSIZE]){0}) +// checked reads introduces ck info into lfsr_data_t that we don't want +// to unnecessarily duplicate, so accessing ck info gets annoyingly +// messy... +static inline lfs_size_t lfsr_bptr_cksize(const lfsr_bptr_t *bptr) { + #ifdef LFS_CKREADS + return bptr->data.u.disk.ck.cksize; + #else + return bptr->cksize; + #endif +} + +static inline uint32_t lfsr_bptr_cksum(const lfsr_bptr_t *bptr) { + #ifdef LFS_CKREADS + return bptr->data.u.disk.ck.u.cksum; + #else + return bptr->cksum; + #endif +} + static lfsr_data_t lfsr_data_frombptr(const lfsr_bptr_t *bptr, uint8_t buffer[static LFSR_BPTR_DSIZE]) { // size should not exceed 28-bits @@ -2395,7 +2507,7 @@ static lfsr_data_t lfsr_data_frombptr(const lfsr_bptr_t *bptr, // off should not exceed 28-bits LFS_ASSERT(bptr->data.u.disk.off <= 0x0fffffff); // cksize should not exceed 28-bits - LFS_ASSERT(bptr->data.u.disk.ck.cksize <= 0x0fffffff); + LFS_ASSERT(lfsr_bptr_cksize(bptr) <= 0x0fffffff); lfs_ssize_t d = 0; // write the block, offset, size @@ -2418,13 +2530,13 @@ static lfsr_data_t lfsr_data_frombptr(const lfsr_bptr_t *bptr, d += d_; // write the cksize, cksum - d_ = lfs_toleb128(bptr->data.u.disk.ck.cksize, &buffer[d], 4); + d_ = lfs_toleb128(lfsr_bptr_cksize(bptr), &buffer[d], 4); if (d_ < 0) { LFS_UNREACHABLE(); } d += d_; - lfs_tole32_(bptr->data.u.disk.ck.u.cksum, &buffer[d]); + lfs_tole32_(lfsr_bptr_cksum(bptr), &buffer[d]); d += 4; return LFSR_DATA_BUF(buffer, d); @@ -2449,12 +2561,18 @@ static int lfsr_data_readbptr(lfs_t *lfs, lfsr_data_t *data, } // read the cksize, cksum - err = lfsr_data_readlleb128(lfs, data, &bptr->data.u.disk.ck.cksize); + err = lfsr_data_readlleb128(lfs, data, + LFS_IFDEF_CKREADS( + &bptr->data.u.disk.ck.cksize, + &bptr->cksize)); if (err) { return err; } - err = lfsr_data_readle32(lfs, data, &bptr->data.u.disk.ck.u.cksum); + err = lfsr_data_readle32(lfs, data, + LFS_IFDEF_CKREADS( + &bptr->data.u.disk.ck.u.cksum, + &bptr->cksum)); if (err) { return err; } @@ -2470,20 +2588,20 @@ static int lfsr_bptr_ck(lfs_t *lfs, const lfsr_bptr_t *bptr) { uint32_t cksum = 0; int err = lfsr_bd_cksum(lfs, bptr->data.u.disk.block, 0, 0, - bptr->data.u.disk.ck.cksize, + lfsr_bptr_cksize(bptr), &cksum); if (err) { return err; } // test that our cksum matches what's expected - if (cksum != bptr->data.u.disk.ck.u.cksum) { + if (cksum != lfsr_bptr_cksum(bptr)) { LFS_ERROR("Found bptr cksum mismatch " "0x%"PRIx32".%"PRIx32" %"PRId32", " "cksum %08"PRIx32" (!= %08"PRIx32")", bptr->data.u.disk.block, 0, - bptr->data.u.disk.ck.cksize, - cksum, bptr->data.u.disk.ck.u.cksum); + lfsr_bptr_cksize(bptr), + cksum, lfsr_bptr_cksum(bptr)); return LFS_ERR_CORRUPT; } @@ -3125,6 +3243,7 @@ static int lfsr_rbyd_appendtag(lfs_t *lfs, lfsr_rbyd_t *rbyd, rbyd->eoff += d; + #ifdef LFS_CKREADS // keep track of most recent parity lfs->tailck.ckblock = rbyd->blocks[0]; lfs->tailck.ckoff @@ -3132,6 +3251,7 @@ static int lfsr_rbyd_appendtag(lfs_t *lfs, lfsr_rbyd_t *rbyd, lfs_parity(rbyd->cksum) ^ lfsr_rbyd_isperturb(rbyd) ) << (8*sizeof(lfs_size_t)-1)) | lfsr_rbyd_eoff(rbyd); + #endif return 0; } @@ -3154,6 +3274,7 @@ static int lfsr_rbyd_appendcat(lfs_t *lfs, lfsr_rbyd_t *rbyd, rbyd->eoff += lfsr_cat_size(cat, count); + #ifdef LFS_CKREADS // keep track of most recent parity lfs->tailck.ckblock = rbyd->blocks[0]; lfs->tailck.ckoff @@ -3161,6 +3282,7 @@ static int lfsr_rbyd_appendcat(lfs_t *lfs, lfsr_rbyd_t *rbyd, lfs_parity(rbyd->cksum) ^ lfsr_rbyd_isperturb(rbyd) ) << (8*sizeof(lfs_size_t)-1)) | lfsr_rbyd_eoff(rbyd); + #endif return 0; } @@ -6565,9 +6687,11 @@ static inline bool lfsr_m_isckprogs(uint32_t flags) { return flags & LFS_M_CKPROGS; } +#ifdef LFS_CKREADS static inline bool lfsr_m_isckreads(uint32_t flags) { return flags & LFS_M_CKREADS; } +#endif static inline bool lfsr_m_isflush(uint32_t flags) { return flags & LFS_M_FLUSH; @@ -7260,7 +7384,7 @@ static int lfsr_mdir_swap__(lfs_t *lfs, lfsr_mdir_t *mdir_, // first thing we need to do is read our current revision count uint32_t rev; - int err = lfsr_bd_readck(lfs, mdir->rbyd.blocks[0], 0, 0, + int err = LFSR_BD_READCK(lfs, mdir->rbyd.blocks[0], 0, 0, &rev, sizeof(uint32_t), LFSR_CK_PARITY(0, sizeof(uint32_t))); if (err && err != LFS_ERR_CORRUPT) { @@ -11051,7 +11175,13 @@ static int lfsr_file_carve(lfs_t *lfs, lfsr_file_t *file, LFSR_TAG_GROW | LFSR_TAG_SUB | LFSR_TAG_BLOCK, -(bid+1 - pos), LFSR_DATA_BPTR_( - (&(lfsr_bptr_t){.data=left_slice_}), + LFS_IFDEF_CKREADS( + (&(lfsr_bptr_t){ + .data=left_slice_}), + (&(lfsr_bptr_t){ + .data=left_slice_, + .cksize=bptr_.cksize, + .cksum=bptr_.cksum})), left.buf)); } else { @@ -11103,7 +11233,13 @@ static int lfsr_file_carve(lfs_t *lfs, lfsr_file_t *file, tag_, bid+1 - (pos+weight), LFSR_DATA_BPTR_( - (&(lfsr_bptr_t){.data=right_slice_}), + LFS_IFDEF_CKREADS( + (&(lfsr_bptr_t){ + .data=right_slice_}), + (&(lfsr_bptr_t){ + .data=right_slice_, + .cksize=bptr_.cksize, + .cksum=bptr_.cksum})), right.buf)); } else { @@ -11343,6 +11479,12 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } bptr.data = LFSR_DATA_DISKCKSUM(block, 0, 0, 0, 0); + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.cksize, + bptr.cksize) = 0; + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.u.cksum, + bptr.cksum) = 0; compact:; // compact data into our block @@ -11371,9 +11513,11 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, d, size - (pos_ - pos)); int err = lfsr_bd_prog(lfs, bptr.data.u.disk.block, - bptr.data.u.disk.ck.cksize, + lfsr_bptr_cksize(&bptr), &buffer[pos_ - pos], d_, - &bptr.data.u.disk.ck.u.cksum, true); + LFS_IFDEF_CKREADS( + &bptr.data.u.disk.ck.u.cksum, + &bptr.cksum), true); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); // bad prog? try another block @@ -11384,7 +11528,9 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } pos_ += d_; - bptr.data.u.disk.ck.cksize += d_; + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.cksize, + bptr.cksize) += d_; d -= d_; } @@ -11430,11 +11576,13 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, lfsr_data_size(bptr_.data) - (pos_ - (bid_-(weight_-1)))); err = lfsr_bd_progdata(lfs, bptr.data.u.disk.block, - bptr.data.u.disk.ck.cksize, + lfsr_bptr_cksize(&bptr), lfsr_data_slice(bptr_.data, pos_ - (bid_-(weight_-1)), d_), - &bptr.data.u.disk.ck.u.cksum, true); + LFS_IFDEF_CKREADS( + &bptr.data.u.disk.ck.u.cksum, + &bptr.cksum), true); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); // bad prog? try another block @@ -11445,7 +11593,9 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } pos_ += d_; - bptr.data.u.disk.ck.cksize += d_; + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.cksize, + bptr.cksize) += d_; d -= d_; } @@ -11455,9 +11605,11 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, // found a hole? fill with zeros int err = lfsr_bd_set(lfs, - bptr.data.u.disk.block, bptr.data.u.disk.ck.cksize, + bptr.data.u.disk.block, lfsr_bptr_cksize(&bptr), 0, d, - &bptr.data.u.disk.ck.u.cksum, true); + LFS_IFDEF_CKREADS( + &bptr.data.u.disk.ck.u.cksum, + &bptr.cksum), true); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); // bad prog? try another block @@ -11468,19 +11620,25 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } pos_ += d; - bptr.data.u.disk.ck.cksize += d; + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.cksize, + bptr.cksize) += d; } // A bit of a hack here, we need to truncate our block to prog_size // alignment to avoid padding issues. Doing this retroactively to // the pcache greatly simplifies the above loop, though we may end // up reading more than is strictly necessary. - lfs_ssize_t d = bptr.data.u.disk.ck.cksize % lfs->cfg->prog_size; + lfs_ssize_t d = lfsr_bptr_cksize(&bptr) % lfs->cfg->prog_size; lfs->pcache.size -= d; - bptr.data.u.disk.ck.cksize -= d; + LFS_IFDEF_CKREADS( + bptr.data.u.disk.ck.cksize, + bptr.cksize) -= d; // finalize our write - int err = lfsr_bd_flush(lfs, &bptr.data.u.disk.ck.u.cksum, true); + int err = lfsr_bd_flush(lfs, LFS_IFDEF_CKREADS( + &bptr.data.u.disk.ck.u.cksum, + &bptr.cksum), true); if (err) { // bad prog? try another block if (err == LFS_ERR_CORRUPT) { @@ -11490,14 +11648,14 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } // prepare our block pointer - LFS_ASSERT(bptr.data.u.disk.ck.cksize > 0); - LFS_ASSERT(bptr.data.u.disk.ck.cksize <= lfs->cfg->block_size); + LFS_ASSERT(lfsr_bptr_cksize(&bptr) > 0); + LFS_ASSERT(lfsr_bptr_cksize(&bptr) <= lfs->cfg->block_size); bptr.data = LFSR_DATA_DISKCKSUM( bptr.data.u.disk.block, bptr.data.u.disk.off, - bptr.data.u.disk.ck.cksize - bptr.data.u.disk.off, - bptr.data.u.disk.ck.cksize, - bptr.data.u.disk.ck.u.cksum); + lfsr_bptr_cksize(&bptr) - bptr.data.u.disk.off, + lfsr_bptr_cksize(&bptr), + lfsr_bptr_cksum(&bptr)); lfs_off_t block_end = block_start + lfsr_data_size(bptr.data); // and write it into our tree @@ -11512,9 +11670,9 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file, } // keep track of any remaining erased-state - if (bptr.data.u.disk.ck.cksize < lfs->cfg->block_size) { + if (lfsr_bptr_cksize(&bptr) < lfs->cfg->block_size) { file->eblock = bptr.data.u.disk.block; - file->eoff = bptr.data.u.disk.ck.cksize; + file->eoff = lfsr_bptr_cksize(&bptr); } // note compacting fragments -> blocks may not actually make any @@ -12476,9 +12634,11 @@ static int lfs_init(lfs_t *lfs, uint32_t flags, } } + #ifdef LFS_CKREADS // setup tailck, nothing should actually check off=0 lfs->tailck.ckblock = 0; lfs->tailck.ckoff = 0; + #endif // setup lookahead buffer, note mount finishes initializing this after // we establish a decent pseudo-random seed @@ -13184,7 +13344,7 @@ int lfsr_mount(lfs_t *lfs, uint32_t flags, LFS_M_RDWR | LFS_M_RDONLY | LFS_M_CKPROGS - | LFS_M_CKREADS + | LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0) | LFS_M_FLUSH | LFS_M_SYNC | LFS_M_MTREEONLY @@ -13342,7 +13502,11 @@ static int lfsr_formatinited(lfs_t *lfs) { int lfsr_format(lfs_t *lfs, const struct lfs_config *cfg) { // TODO hmmm, should lfsr_format take flags? - int err = lfs_init(lfs, LFS_M_RDWR | LFS_M_CKPROGS | LFS_M_CKREADS, cfg); + int err = lfs_init(lfs, + LFS_M_RDWR + | LFS_M_CKPROGS + | LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0), + cfg); if (err) { return err; } @@ -13373,7 +13537,7 @@ int lfsr_fs_stat(lfs_t *lfs, struct lfs_fsinfo *fsinfo) { fsinfo->flags = lfs->flags & ( LFS_I_RDONLY | LFS_I_CKPROGS - | LFS_I_CKREADS + | LFS_IFDEF_CKREADS(LFS_I_CKREADS, 0) | LFS_I_FLUSH | LFS_I_SYNC | LFS_I_UNCOMPACTED); diff --git a/lfs.h b/lfs.h index dfd299c9..43679a24 100644 --- a/lfs.h +++ b/lfs.h @@ -155,7 +155,9 @@ enum lfs_type { #define LFS_M_RDWR 0 // Mount the filesystem as read and write #define LFS_M_RDONLY 1 // Mount the filesystem as read only #define LFS_M_CKPROGS 0x00000010 // Check progs by reading back progged data +#ifdef LFS_CKREADS #define LFS_M_CKREADS 0x00000020 // Check reads via parity bits/checksums +#endif #define LFS_M_FLUSH 0x00000040 // Open all files with LFS_O_FLUSH #define LFS_M_SYNC 0x00000080 // Open all files with LFS_O_SYNC @@ -177,7 +179,9 @@ enum lfs_type { // Filesystem info flags #define LFS_I_RDONLY 0x00000001 // Filesystem mounted read only #define LFS_I_CKPROGS 0x00000010 // Filesystem mounted with LFS_M_CKPROGS +#ifdef LFS_CKREADS #define LFS_I_CKREADS 0x00000020 // Filesystem mounted with LFS_M_CKREADS +#endif #define LFS_I_FLUSH 0x00000040 // Filesystem mounted with LFS_M_FLUSH #define LFS_I_SYNC 0x00000080 // Filesystem mounted with LFS_M_SYNC @@ -536,6 +540,7 @@ typedef struct lfsr_omdir { // lfs_block_t tail[2]; //} lfs_mdir_t; +#ifdef LFS_CKREADS // context for validating data typedef struct lfsr_ck { // sign(cksize)=0 => cksum check @@ -546,6 +551,7 @@ typedef struct lfsr_ck { uint32_t cksum; } u; } lfsr_ck_t; +#endif // either an on-disk or in-device data pointer typedef struct lfsr_data { @@ -557,7 +563,9 @@ typedef struct lfsr_data { struct { lfs_block_t block; lfs_size_t off; + #ifdef LFS_CKREADS lfsr_ck_t ck; + #endif } disk; } u; } lfsr_data_t; @@ -588,6 +596,10 @@ typedef lfsr_data_t lfsr_sprout_t; typedef struct lfsr_bptr { lfsr_data_t data; + #ifndef LFS_CKREADS + lfs_size_t cksize; + uint32_t cksum; + #endif } lfsr_bptr_t; // the lfsr_bshrub_t struct represents the on-disk component of a file @@ -726,11 +738,13 @@ typedef struct lfsr_grm { lfsr_smid_t mids[2]; } lfsr_grm_t; +#ifdef LFS_CKREADS typedef struct lfsr_tailck { lfs_block_t ckblock; // sign(ckoff) => tail parity lfs_size_t ckoff; } lfsr_tailck_t; +#endif // The littlefs filesystem type typedef struct lfs { @@ -764,7 +778,9 @@ typedef struct lfs { uint8_t *buffer; } pcache; + #ifdef LFS_CKREADS lfsr_tailck_t tailck; + #endif struct lfs_lookahead { lfs_block_t window; diff --git a/lfs_util.h b/lfs_util.h index 3d7307d9..e5ec8104 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -137,6 +137,14 @@ extern "C" #endif +// Some ifdef conveniences +#ifdef LFS_CKREADS +#define LFS_IFDEF_CKREADS(a, b) a +#else +#define LFS_IFDEF_CKREADS(a, b) b +#endif + + // Builtin functions, these may be replaced by more efficient // toolchain-specific implementations. LFS_NO_BUILTINS falls back to a more // expensive basic C implementation for debugging purposes diff --git a/runners/bench_runner.c b/runners/bench_runner.c index be8b55c4..daeb4f73 100644 --- a/runners/bench_runner.c +++ b/runners/bench_runner.c @@ -849,7 +849,7 @@ void perm_count( state->total += 1; - if (!bench_all && case_->if_ && !case_->if_()) { + if (!case_->run || !(bench_all || !case_->if_ || case_->if_())) { return; } @@ -1334,7 +1334,7 @@ void perm_run( bench_step += 1; // filter? - if (!bench_all && case_->if_ && !case_->if_()) { + if (!case_->run || !(bench_all || !case_->if_ || case_->if_())) { printf("skipped "); perm_printid(suite, case_); printf("\n"); diff --git a/runners/test_runner.c b/runners/test_runner.c index 33094427..968914bd 100644 --- a/runners/test_runner.c +++ b/runners/test_runner.c @@ -812,7 +812,7 @@ void perm_count( // set pls to 1 if running under powerloss so it useful for if predicates TEST_PLS = (powerloss->run != run_powerloss_none); - if (!test_all && case_->if_ && !case_->if_()) { + if (!case_->run || !(test_all || !case_->if_ || case_->if_())) { return; } @@ -1808,7 +1808,7 @@ void perm_run( // set pls to 1 if running under powerloss so it useful for if predicates TEST_PLS = (powerloss->run != run_powerloss_none); // filter? - if (!test_all && case_->if_ && !case_->if_()) { + if (!case_->run || !(test_all || !case_->if_ || case_->if_())) { printf("skipped "); perm_printid(suite, case_, NULL, 0); printf("\n"); diff --git a/scripts/bench.py b/scripts/bench.py index a7ff6259..0d379633 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -55,6 +55,9 @@ class BenchCase: self.if_ = config.pop('if', []) if not isinstance(self.if_, list): self.if_ = [self.if_] + self.ifdef = config.pop('ifdef', []) + if not isinstance(self.ifdef, list): + self.ifdef = [self.ifdef] self.code = config.pop('code') self.code_lineno = config.pop('code_lineno', None) self.in_ = config.pop('in', @@ -199,6 +202,10 @@ class BenchSuite: if not isinstance(self.if_, list): self.if_ = [self.if_] + self.ifdef = config.pop('ifdef', []) + if not isinstance(self.ifdef, list): + self.ifdef = [self.ifdef] + self.code = config.pop('code', None) self.code_lineno = min( (l for l in code_linenos @@ -357,6 +364,12 @@ def compile(bench_paths, **args): # note it's up to the specific generated file to declare # the bench defines def write_case_functions(f, suite, case): + # write any ifdef prologues + if case.ifdef: + for ifdef in case.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + # create case define functions for i, permutation in enumerate(case.permutations): for k, vs in sorted(permutation.items()): @@ -413,7 +426,19 @@ def compile(bench_paths, **args): f.writeln('}') f.writeln() + # write any ifdef epilogues + if case.ifdef: + for ifdef in case.ifdef: + f.writeln('#endif') + f.writeln() + if not args.get('source'): + # write any ifdef prologues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + # write any suite defines if suite.defines: for define in sorted(suite.defines): @@ -451,6 +476,12 @@ def compile(bench_paths, **args): % (case.name)) f.writeln() + # write any ifdef epilogues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#endif') + f.writeln() + # create suite struct f.writeln('const struct bench_suite __bench__%s__suite = {' % suite.name) @@ -460,6 +491,8 @@ def compile(bench_paths, **args): % (' | '.join(filter(None, [ 'BENCH_INTERNAL' if suite.internal else None])) or 0)) + for ifdef in suite.ifdef: + f.writeln(4*' '+'#ifdef %s' % ifdef) # create suite defines if suite.defines: f.writeln(4*' '+'.defines = (const bench_define_t[]){') @@ -468,6 +501,8 @@ def compile(bench_paths, **args): % (k, k)) f.writeln(4*' '+'},') f.writeln(4*' '+'.define_count = %d,' % len(suite.defines)) + for ifdef in suite.ifdef: + f.writeln(4*' '+'#endif') if suite.cases: f.writeln(4*' '+'.cases = (const struct bench_case[]){') for case in suite.cases: @@ -479,6 +514,8 @@ def compile(bench_paths, **args): % (' | '.join(filter(None, [ 'BENCH_INTERNAL' if suite.internal else None])) or 0)) + for ifdef in it.chain(suite.ifdef, case.ifdef): + f.writeln(12*' '+'#ifdef %s' % ifdef) # create case defines if case.defines: f.writeln(12*' '+'.defines' @@ -506,6 +543,8 @@ def compile(bench_paths, **args): % (case.name)) f.writeln(12*' '+'.run = __bench__%s__run,' % (case.name)) + for ifdef in it.chain(suite.ifdef, case.ifdef): + f.writeln(12*' '+'#endif') f.writeln(8*' '+'},') f.writeln(4*' '+'},') f.writeln(4*' '+'.case_count = %d,' % len(suite.cases)) @@ -538,6 +577,13 @@ def compile(bench_paths, **args): # write any internal benches for suite in suites: + # any ifdef prologues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + + # any suite code if suite.isin(args['source']): if suite.code_lineno is not None: f.writeln('#line %d "%s"' @@ -548,10 +594,17 @@ def compile(bench_paths, **args): % (f.lineno+1, args['output'])) f.writeln() + # any case functions for case in suite.cases: if case.isin(args['source']): write_case_functions(f, suite, case) + # any ifdef epilogues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#endif') + f.writeln() + # declare our bench suites # # by declaring these as weak we can write these to every @@ -1486,7 +1539,7 @@ if __name__ == "__main__": bench_parser.add_argument( '-a', '--all', action='store_true', - help="Ignore test filters.") + help="Ignore bench filters.") bench_parser.add_argument( '-d', '--disk', help="Direct block device operations to this file.") diff --git a/scripts/test.py b/scripts/test.py index 8596a6d1..896826fd 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -55,6 +55,9 @@ class TestCase: self.if_ = config.pop('if', []) if not isinstance(self.if_, list): self.if_ = [self.if_] + self.ifdef = config.pop('ifdef', []) + if not isinstance(self.ifdef, list): + self.ifdef = [self.ifdef] self.code = config.pop('code') self.code_lineno = config.pop('code_lineno', None) self.in_ = config.pop('in', @@ -204,6 +207,10 @@ class TestSuite: if not isinstance(self.if_, list): self.if_ = [self.if_] + self.ifdef = config.pop('ifdef', []) + if not isinstance(self.ifdef, list): + self.ifdef = [self.ifdef] + self.code = config.pop('code', None) self.code_lineno = min( (l for l in code_linenos @@ -368,6 +375,12 @@ def compile(test_paths, **args): # note it's up to the specific generated file to declare # the test defines def write_case_functions(f, suite, case): + # write any ifdef prologues + if case.ifdef: + for ifdef in case.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + # create case define functions for i, permutation in enumerate(case.permutations): for k, vs in sorted(permutation.items()): @@ -424,7 +437,19 @@ def compile(test_paths, **args): f.writeln('}') f.writeln() + # write any ifdef epilogues + if case.ifdef: + for ifdef in case.ifdef: + f.writeln('#endif') + f.writeln() + if not args.get('source'): + # write any ifdef prologues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + # write any suite defines if suite.defines: for define in sorted(suite.defines): @@ -462,6 +487,12 @@ def compile(test_paths, **args): % (case.name)) f.writeln() + # write any ifdef epilogues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#endif') + f.writeln() + # create suite struct f.writeln('const struct test_suite __test__%s__suite = {' % suite.name) @@ -473,6 +504,8 @@ def compile(test_paths, **args): 'TEST_REENTRANT' if suite.reentrant else None, 'TEST_FUZZ' if suite.fuzz else None])) or 0)) + for ifdef in suite.ifdef: + f.writeln(4*' '+'#ifdef %s' % ifdef) # create suite defines if suite.defines: f.writeln(4*' '+'.defines = (const test_define_t[]){') @@ -481,6 +514,8 @@ def compile(test_paths, **args): % (k, k)) f.writeln(4*' '+'},') f.writeln(4*' '+'.define_count = %d,' % len(suite.defines)) + for ifdef in suite.ifdef: + f.writeln(4*' '+'#endif') if suite.cases: f.writeln(4*' '+'.cases = (const struct test_case[]){') for case in suite.cases: @@ -494,6 +529,8 @@ def compile(test_paths, **args): 'TEST_REENTRANT' if case.reentrant else None, 'TEST_FUZZ' if case.fuzz else None])) or 0)) + for ifdef in it.chain(suite.ifdef, case.ifdef): + f.writeln(12*' '+'#ifdef %s' % ifdef) # create case defines if case.defines: f.writeln(12*' '+'.defines' @@ -521,6 +558,8 @@ def compile(test_paths, **args): % (case.name)) f.writeln(12*' '+'.run = __test__%s__run,' % (case.name)) + for ifdef in it.chain(suite.ifdef, case.ifdef): + f.writeln(12*' '+'#endif') f.writeln(8*' '+'},') f.writeln(4*' '+'},') f.writeln(4*' '+'.case_count = %d,' % len(suite.cases)) @@ -553,6 +592,13 @@ def compile(test_paths, **args): # write any internal tests for suite in suites: + # any ifdef prologues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#ifdef %s' % ifdef) + f.writeln() + + # any suite code if suite.isin(args['source']): if suite.code_lineno is not None: f.writeln('#line %d "%s"' @@ -563,10 +609,17 @@ def compile(test_paths, **args): % (f.lineno+1, args['output'])) f.writeln() + # any case functions for case in suite.cases: if case.isin(args['source']): write_case_functions(f, suite, case) + # any ifdef epilogues + if suite.ifdef: + for ifdef in suite.ifdef: + f.writeln('#endif') + f.writeln() + # declare our test suites # # by declaring these as weak we can write these to every diff --git a/tests/test_ck.toml b/tests/test_ck.toml index eeffb50c..10d1e64f 100644 --- a/tests/test_ck.toml +++ b/tests/test_ck.toml @@ -2,6 +2,7 @@ after = ['test_traversal', 'test_gc', 'test_mount'] + # Test filesystem-level checksum things # test we can detect at least fully clobbered blocks @@ -804,6 +805,7 @@ defines.BADBLOCK_BEHAVIOR = [ ] # this should stay inlined defines.SIZE = 'BLOCK_SIZE/16' +ifdef = 'LFS_CKREADS' code = ''' // test all bad bits in the mroot for (lfs_size_t i = 0; @@ -915,6 +917,7 @@ defines.BADBLOCK_BEHAVIOR = [ ] # this should create a single block file defines.SIZE = 'BLOCK_SIZE' +ifdef = 'LFS_CKREADS' code = ''' // first we need to figure out where the data block will actually // end up, fortunately our block randomization is intentionally @@ -1048,6 +1051,7 @@ defines.INLINE_SIZE = 0 defines.CRYSTAL_THRESH = -1 defines.FRAGMENT_SIZE = 'BLOCK_SIZE/8' defines.SIZE = '2*FRAGMENT_SIZE' +ifdef = 'LFS_CKREADS' code = ''' // first we need to figure out where the btree block will actually // end up, fortunately our block randomization is intentionally diff --git a/tests/test_mount.toml b/tests/test_mount.toml index 1c9c45e7..2ddc2808 100644 --- a/tests/test_mount.toml +++ b/tests/test_mount.toml @@ -18,13 +18,14 @@ defines.CKPROGS = [false, true] defines.CKREADS = [false, true] defines.FLUSH = [false, true] defines.SYNC = [false, true] +if = 'LFS_IFDEF_CKREADS(true, !CKREADS)' code = ''' lfs_t lfs; lfsr_format(&lfs, CFG) => 0; lfsr_mount(&lfs, ((RDONLY) ? LFS_M_RDONLY : LFS_M_RDWR) | ((CKPROGS) ? LFS_M_CKPROGS : 0) - | ((CKREADS) ? LFS_M_CKREADS : 0) + | ((CKREADS) ? LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0) : 0) | ((FLUSH) ? LFS_M_FLUSH : 0) | ((SYNC) ? LFS_M_SYNC : 0), CFG) => 0; @@ -34,7 +35,7 @@ code = ''' assert(fsinfo.flags == ( ((RDONLY) ? LFS_I_RDONLY : 0) | ((CKPROGS) ? LFS_I_CKPROGS : 0) - | ((CKREADS) ? LFS_I_CKREADS : 0) + | ((CKREADS) ? LFS_IFDEF_CKREADS(LFS_I_CKREADS, 0) : 0) | ((FLUSH) ? LFS_I_FLUSH : 0) | ((SYNC) ? LFS_I_SYNC : 0) | LFS_I_CANLOOKAHEAD