From 185f209dbf5dc2adbbee1dc79ec31215f265421b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 10 Aug 2024 23:29:54 -0500 Subject: [PATCH] Moved ckreads behind the LFS_M_CKREADS flag Added some code, though we don't _really_ care: code stack before: 37872 3048 after: 38060 (+0.5%) 3056 (+0.3%) Also interesting to note the difference in testing time, this highlights _some_ of the performance cost of ckreads: with ckreads: 1135.92s without ckreads: 821.24s --- lfs.c | 84 ++++++++++++++++++++++++++++++++++++++----- lfs.h | 2 ++ tests/test_ck.toml | 16 ++++----- tests/test_mount.toml | 3 ++ 4 files changed, 89 insertions(+), 16 deletions(-) diff --git a/lfs.c b/lfs.c index 4aff7e50..494be5d8 100644 --- a/lfs.c +++ b/lfs.c @@ -868,7 +868,7 @@ static int lfsr_bd_cksuffix(lfs_t *lfs, // contributes to the relevant parity/checksum, this may be // significantly more than the data we actually end up using // -static int lfsr_bd_readck(lfs_t *lfs, +static int lfsr_bd_readck_(lfs_t *lfs, lfs_block_t block, lfs_size_t off, lfs_size_t hint, void *buffer, lfs_size_t size, lfsr_ck_t ck) { @@ -907,13 +907,33 @@ static int lfsr_bd_readck(lfs_t *lfs, return 0; } +// needed in lfsr_bd_readck +static inline bool lfsr_m_isckreads(uint32_t flags); + +static int lfsr_bd_readck(lfs_t *lfs, + lfs_block_t block, lfs_size_t off, lfs_size_t hint, + void *buffer, lfs_size_t size, + lfsr_ck_t ck) { + // are we actually checking reads? + if (lfsr_m_isckreads(lfs->flags)) { + return lfsr_bd_readck_(lfs, + block, off, hint, + buffer, size, + ck); + } else { + return lfsr_bd_read(lfs, + block, off, hint, + buffer, size); + } +} + // these could probably be a bit better deduplicated with their // unchecked counterparts, but we don't generally use both at the same // time // // we'd also need to worry about early termination in lfsr_bd_cmp/cmpck -static lfs_scmp_t lfsr_bd_cmpck(lfs_t *lfs, +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, lfsr_ck_t ck) { @@ -972,7 +992,24 @@ static lfs_scmp_t lfsr_bd_cmpck(lfs_t *lfs, return cmp; } -static int lfsr_bd_cpyck(lfs_t *lfs, +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, + lfsr_ck_t ck) { + // are we actually checking reads? + if (lfsr_m_isckreads(lfs->flags)) { + return lfsr_bd_cmpck_(lfs, + block, off, hint, + buffer, size, + ck); + } else { + return lfsr_bd_cmp(lfs, + block, off, hint, + buffer, size); + } +} + +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, lfs_size_t size, @@ -1044,6 +1081,29 @@ static int lfsr_bd_cpyck(lfs_t *lfs, return 0; } +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, + lfs_size_t size, + lfsr_ck_t ck, + uint32_t *cksum, bool align) { + // are we actually checking reads? + if (lfsr_m_isckreads(lfs->flags)) { + return lfsr_bd_cpyck_(lfs, + dst_block, dst_off, + src_block, src_off, hint, + size, + ck, + cksum, align); + } else { + return lfsr_bd_cpy(lfs, + dst_block, dst_off, + src_block, src_off, hint, + size, + cksum, align); + } +} + @@ -1563,12 +1623,14 @@ static lfs_ssize_t lfsr_bd_readtag(lfs_t *lfs, *cksum ^= tag_buf[0] & 0x00000080; // calculate checksum *cksum = lfs_crc32c(*cksum, tag_buf, d); + } - // check the parity if we're not already calculating a checksum + // check the parity if we're checking reads and not already + // calculating a checksum // - // this requires reading the data too, but with any luck the data - // will stick around in the cache - } else { + // this requires reading all of the data as well, but with any luck + // the data will stick around in the cache + if (lfsr_m_isckreads(lfs->flags) && !cksum) { // pesky parity byte lfs_size_t size__ = (!lfsr_tag_isalt(tag)) ? size : 0; if (off+d+size__ >= lfs->cfg->block_size) { @@ -6503,6 +6565,10 @@ static inline bool lfsr_m_isckprogs(uint32_t flags) { return flags & LFS_M_CKPROGS; } +static inline bool lfsr_m_isckreads(uint32_t flags) { + return flags & LFS_M_CKREADS; +} + static inline bool lfsr_m_isflush(uint32_t flags) { return flags & LFS_M_FLUSH; } @@ -13118,6 +13184,7 @@ int lfsr_mount(lfs_t *lfs, uint32_t flags, LFS_M_RDWR | LFS_M_RDONLY | LFS_M_CKPROGS + | LFS_M_CKREADS | LFS_M_FLUSH | LFS_M_SYNC | LFS_M_MTREEONLY @@ -13275,7 +13342,7 @@ 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, cfg); + int err = lfs_init(lfs, LFS_M_RDWR | LFS_M_CKPROGS | LFS_M_CKREADS, cfg); if (err) { return err; } @@ -13306,6 +13373,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_I_FLUSH | LFS_I_SYNC | LFS_I_UNCOMPACTED); diff --git a/lfs.h b/lfs.h index 96b34886..dfd299c9 100644 --- a/lfs.h +++ b/lfs.h @@ -155,6 +155,7 @@ 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 +#define LFS_M_CKREADS 0x00000020 // Check reads via parity bits/checksums #define LFS_M_FLUSH 0x00000040 // Open all files with LFS_O_FLUSH #define LFS_M_SYNC 0x00000080 // Open all files with LFS_O_SYNC @@ -176,6 +177,7 @@ 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 +#define LFS_I_CKREADS 0x00000020 // Filesystem mounted with LFS_M_CKREADS #define LFS_I_FLUSH 0x00000040 // Filesystem mounted with LFS_M_FLUSH #define LFS_I_SYNC 0x00000080 // Filesystem mounted with LFS_M_SYNC diff --git a/tests/test_ck.toml b/tests/test_ck.toml index f5c44d64..eeffb50c 100644 --- a/tests/test_ck.toml +++ b/tests/test_ck.toml @@ -832,7 +832,7 @@ code = ''' if (err == LFS_ERR_CORRUPT) { goto corrupt; } - err = lfsr_mount(&lfs, LFS_M_RDWR, CFG); + err = lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG); assert(!err || err == LFS_ERR_CORRUPT); if (err == LFS_ERR_CORRUPT) { goto corrupt; @@ -868,7 +868,7 @@ code = ''' // remount? if (remount) { lfsr_unmount(&lfs) => 0; - err = lfsr_mount(&lfs, LFS_M_RDWR, CFG); + err = lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG); if (err == LFS_ERR_CORRUPT) { goto corrupt; } @@ -923,7 +923,7 @@ code = ''' // format lfs_t lfs; lfsr_format(&lfs, CFG) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; // create a file lfsr_file_t file; @@ -974,7 +974,7 @@ code = ''' // format lfs_t lfs; lfsr_format(&lfs, CFG) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; { // create a file @@ -1002,7 +1002,7 @@ code = ''' // remount? if (remount) { lfsr_unmount(&lfs) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; } // yes reads can fail here @@ -1056,7 +1056,7 @@ code = ''' // format lfs_t lfs; lfsr_format(&lfs, CFG) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; // create a file lfsr_file_t file; @@ -1109,7 +1109,7 @@ code = ''' // format lfs_t lfs; lfsr_format(&lfs, CFG) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; { // create a file @@ -1137,7 +1137,7 @@ code = ''' // remount? if (remount) { lfsr_unmount(&lfs) => 0; - lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR | LFS_M_CKREADS, CFG) => 0; } // yes reads can fail here diff --git a/tests/test_mount.toml b/tests/test_mount.toml index 96cb4539..1c9c45e7 100644 --- a/tests/test_mount.toml +++ b/tests/test_mount.toml @@ -15,6 +15,7 @@ code = ''' [cases.test_mount_flags] defines.RDONLY = [false, true] defines.CKPROGS = [false, true] +defines.CKREADS = [false, true] defines.FLUSH = [false, true] defines.SYNC = [false, true] code = ''' @@ -23,6 +24,7 @@ code = ''' lfsr_mount(&lfs, ((RDONLY) ? LFS_M_RDONLY : LFS_M_RDWR) | ((CKPROGS) ? LFS_M_CKPROGS : 0) + | ((CKREADS) ? LFS_M_CKREADS : 0) | ((FLUSH) ? LFS_M_FLUSH : 0) | ((SYNC) ? LFS_M_SYNC : 0), CFG) => 0; @@ -32,6 +34,7 @@ code = ''' assert(fsinfo.flags == ( ((RDONLY) ? LFS_I_RDONLY : 0) | ((CKPROGS) ? LFS_I_CKPROGS : 0) + | ((CKREADS) ? LFS_I_CKREADS : 0) | ((FLUSH) ? LFS_I_FLUSH : 0) | ((SYNC) ? LFS_I_SYNC : 0) | LFS_I_CANLOOKAHEAD