From 7d8315a598f5fd6b16efc6f50ba40fe174bf9ff1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 9 Jan 2024 12:06:18 -0600 Subject: [PATCH] Dropped becksums from direct block pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct block pointers are turning out to be a bit of an awkward file representation for littlefs. Thanks to shrubs, direct block pointers really don't offer that much in terms of disk savings. Direct bptrs save ~40 B: direct bptr: 1 attr + 1 bptr 40 B + 24 B = 64 B indirect bshrub: 2 attr + 1 trunk + 1 bptr 2*40 B + 10 B + 24 B = 114 B δ = +40 B (+78.1%) Which is nice, but not really significant on disk. Their original motivation was to avoid the cost of a btree root node for one block files. But this can now be avoided with bshrubs, which also generalizes to other few-block files. I can see the argument for carving out a special case for entirely inlined files. +~40B may be a significant cost there. But I'm just not seeing the value for bptrs. But direct bptrs exist as a natural extension of littlefs's design. Files can have: 1. nothing, null data, 2. a data entry (bptr/bsprout) 3. a bshrub/btree of data entries (bptr/bsprout) Prohibiting direct bptrs, would be a bit strange, and a future version of littlefs may find direct bptrs useful. Say, for example, a version that doesn't support bshrubs, suddenly bptrs become more valuable. So this is a compromise: 1. Support reading of bptrs, this is not that much extra work on top of supporting bsprouts. Though we do need to be aware of them in the block allocator. 2. Convert bptrs to bshrubs/btrees on first write. 3. Ignore any extra bptr metadata, becksums, cids, etc. These add an additional attr which complicates things. Downside: We may lose out on potential erased-state when writing to files created on a different device that uses bptrs. Upside: Simpler code and a bit of code savings. code stack before: 33260 3024 after: 33136 (-0.4%) 3000 (-0.8%) Ok, maybe not that much code savings... --- lfs.c | 79 ++++++++++++++++++++--------------------------------------- lfs.h | 10 ++------ 2 files changed, 29 insertions(+), 60 deletions(-) diff --git a/lfs.c b/lfs.c index 810ca252..b53cd355 100644 --- a/lfs.c +++ b/lfs.c @@ -1847,13 +1847,13 @@ static int lfsr_data_readgrm(lfs_t *lfs, lfsr_data_t *data, static inline bool lfsr_ftree_isbnull(const lfsr_ftree_t *ftree); static inline bool lfsr_ftree_isbsprout( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree); -static inline bool lfsr_ftree_isbleaf( +static inline bool lfsr_ftree_isbptr( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree); static inline bool lfsr_ftree_isbshrub( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree); static inline bool lfsr_ftree_isbtree( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree); -static inline bool lfsr_ftree_isbnullorbsproutorbleaf( +static inline bool lfsr_ftree_isbnullorbsproutorbptr( const lfsr_ftree_t *ftree); static inline bool lfsr_ftree_isbshruborbtree( const lfsr_ftree_t *ftree); @@ -7292,7 +7292,7 @@ static int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *traversal, // found a direct block? if (err != LFS_ERR_NOENT && tag == LFSR_TAG_BLOCK) { err = lfsr_data_readbptr(lfs, &data, - &traversal->ftree.u.bleaf.bptr); + &traversal->ftree.u.bptr); if (err) { return err; } @@ -8872,55 +8872,55 @@ int lfsr_dir_rewind(lfs_t *lfs, lfsr_dir_t *dir) { /// File operations /// -#define LFSR_FTREE_ISBNULLORBSPROUTORBLEAF 0x80000000 +#define LFSR_FTREE_ISBNULLORBSPROUTORBPTR 0x80000000 #define LFSR_FTREE_BNULL() \ - ((lfsr_ftree_t){.u.size=(LFSR_FTREE_ISBNULLORBSPROUTORBLEAF | 0)}) + ((lfsr_ftree_t){.u.size=(LFSR_FTREE_ISBNULLORBSPROUTORBPTR | 0)}) static inline bool lfsr_ftree_isbnull(const lfsr_ftree_t *ftree) { return (lfs_size_t)ftree->u.size - == (LFSR_FTREE_ISBNULLORBSPROUTORBLEAF | 0); + == (LFSR_FTREE_ISBNULLORBSPROUTORBPTR | 0); } static inline bool lfsr_ftree_isbsprout( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree) { return (lfs_size_t)ftree->u.size - > (LFSR_FTREE_ISBNULLORBSPROUTORBLEAF | 0) + > (LFSR_FTREE_ISBNULLORBSPROUTORBPTR | 0) && ftree->u.bsprout.u.disk.block == mdir->rbyd.blocks[0]; } -static inline bool lfsr_ftree_isbleaf( +static inline bool lfsr_ftree_isbptr( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree) { return (lfs_size_t)ftree->u.size - > (LFSR_FTREE_ISBNULLORBSPROUTORBLEAF | 0) + > (LFSR_FTREE_ISBNULLORBSPROUTORBPTR | 0) && ftree->u.bsprout.u.disk.block != mdir->rbyd.blocks[0]; } static inline bool lfsr_ftree_isbshrub( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree) { - return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBLEAF) + return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBPTR) && ftree->u.bshrub.blocks[0] == mdir->rbyd.blocks[0]; } static inline bool lfsr_ftree_isbtree( const lfsr_mdir_t *mdir, const lfsr_ftree_t *ftree) { - return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBLEAF) + return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBPTR) && ftree->u.bshrub.blocks[0] != mdir->rbyd.blocks[0]; } -static inline bool lfsr_ftree_isbnullorbsproutorbleaf( +static inline bool lfsr_ftree_isbnullorbsproutorbptr( const lfsr_ftree_t *ftree) { - return ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBLEAF; + return ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBPTR; } static inline bool lfsr_ftree_isbshruborbtree( const lfsr_ftree_t *ftree) { - return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBLEAF); + return !(ftree->u.size & LFSR_FTREE_ISBNULLORBSPROUTORBPTR); } // the on-disk size/weight lines up to the same word across all unions static inline lfs_off_t lfsr_ftree_size(const lfsr_ftree_t *ftree) { - return ftree->u.size & ~LFSR_FTREE_ISBNULLORBSPROUTORBLEAF; + return ftree->u.size & ~LFSR_FTREE_ISBNULLORBSPROUTORBPTR; } // flag things @@ -9072,29 +9072,11 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, // or a direct block } else if (err != LFS_ERR_NOENT && tag == LFSR_TAG_BLOCK) { err = lfsr_data_readbptr(lfs, &data, - &file->ftree.u.bleaf.bptr); + &file->ftree.u.bptr); if (err) { return err; } - // also fetch the becksum here if we are writable - file->ftree.u.bleaf.becksum.size = -1; - if (lfsr_o_iswriteable(flags)) { - lfsr_data_t data; - err = lfsr_mdir_lookupnext(lfs, &file->mdir, - file->mdir.mid, LFSR_TAG_BECKSUM, - NULL, &data); - if (err && err != LFS_ERR_NOENT) { - return err; - } - - err = lfsr_data_readecksum(lfs, &data, - &file->ftree.u.bleaf.becksum); - if (err) { - return err; - } - } - // or a bshrub (inlined btree) } else if (err != LFS_ERR_NOENT && tag == LFSR_TAG_BSHRUB) { err = lfsr_data_readshrub(lfs, &data, &file->mdir, @@ -9291,21 +9273,21 @@ static int lfsr_ftree_lookupnext(lfs_t *lfs, return 0; // block pointer? - } else if (lfsr_ftree_isbleaf(mdir, ftree)) { + } else if (lfsr_ftree_isbptr(mdir, ftree)) { if (bid_) { - *bid_ = lfsr_data_size(&ftree->u.bleaf.bptr.data)-1; + *bid_ = lfsr_data_size(&ftree->u.bptr.data)-1; } if (tag_) { *tag_ = LFSR_TAG_BLOCK; } if (weight_) { - *weight_ = lfsr_data_size(&ftree->u.bleaf.bptr.data); + *weight_ = lfsr_data_size(&ftree->u.bptr.data); } if (bptr_) { - *bptr_ = ftree->u.bleaf.bptr; + *bptr_ = ftree->u.bptr; } if (becksum_) { - *becksum_ = ftree->u.bleaf.becksum; + becksum_->size = -1; } return 0; @@ -9382,17 +9364,17 @@ static int lfsr_ftree_traverse(lfs_t *lfs, } // block pointer? - if (lfsr_ftree_isbleaf(mdir, ftree)) { + if (lfsr_ftree_isbptr(mdir, ftree)) { if (btraversal->bid > 0) { return LFS_ERR_NOENT; } if (bid_) { - *bid_ = lfsr_data_size(&ftree->u.bleaf.bptr.data)-1; + *bid_ = lfsr_data_size(&ftree->u.bptr.data)-1; } if (tinfo_) { tinfo_->tag = LFSR_TAG_BLOCK; - tinfo_->u.bptr = ftree->u.bleaf.bptr; + tinfo_->u.bptr = ftree->u.bptr; } return 0; @@ -9688,18 +9670,11 @@ static int lfsr_ftree_carve(lfs_t *lfs, attrs_[attr_count_++] = LFSR_ATTR(0, DATA, +lfsr_ftree_size(ftree), DATA(ftree->u.bsprout)); - } else if (lfsr_ftree_isbleaf(mdir, ftree)) { + } else if (lfsr_ftree_isbptr(mdir, ftree)) { attrs_[attr_count_++] = LFSR_ATTR(0, BLOCK, +lfsr_ftree_size(ftree), - FROMBPTR(&ftree->u.bleaf.bptr, &buf[buf_size])); + FROMBPTR(&ftree->u.bptr, &buf[buf_size])); buf_size += LFSR_BPTR_DSIZE; - - if (ftree->u.bleaf.becksum.size != -1) { - attrs_[attr_count_++] = LFSR_ATTR(lfsr_ftree_size(ftree)-1, - BECKSUM, 0, - FROMECKSUM(&ftree->u.bleaf.becksum, &buf[buf_size])); - buf_size += LFSR_ECKSUM_DSIZE; - } } ftree->u.bshrub.blocks[0] = mdir->rbyd.blocks[0]; @@ -10780,7 +10755,7 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) { // // this is convenient because bptrs are a bit annoying to commit LFS_ASSERT(!lfsr_ftree_isbsprout(&file->mdir, &file->ftree)); - LFS_ASSERT(!lfsr_ftree_isbleaf(&file->mdir, &file->ftree)); + LFS_ASSERT(!lfsr_ftree_isbptr(&file->mdir, &file->ftree)); // small files should start as zero, const prop should optimize this out LFS_ASSERT(!lfsr_f_isunflushed(file->flags) || file->buffer_pos == 0); diff --git a/lfs.h b/lfs.h index 2ab2249f..78b8212b 100644 --- a/lfs.h +++ b/lfs.h @@ -511,12 +511,6 @@ typedef struct lfsr_bptr { uint32_t cksum; } lfsr_bptr_t; -// a bleaf is just a bptr with all optional attrs -typedef struct lfsr_bleaf { - lfsr_bptr_t bptr; - lfsr_ecksum_t becksum; -} lfsr_bleaf_t; - // a shrub is a secondary trunk in an mdir, we really only need // trunk/weight/block, so we sneak our estimate into some // overlapping fields @@ -540,14 +534,14 @@ typedef struct lfsr_ftree { // // sign(size)=1, data.size==0 => bnull // sign(size)=1, data.block==mdir.block => bsprout - // sign(size)=1, data.block!=mdir.block => bleaf + // sign(size)=1, data.block!=mdir.block => bptr // sign(size)=0, data.block==mdir.block => bshrub // sign(size)=0, data.block!=mdir.block => btree // union { lfs_soff_t size; lfsr_data_t bsprout; - lfsr_bleaf_t bleaf; + lfsr_bptr_t bptr; lfsr_shrub_t bshrub; lfsr_btree_t btree; } u, u_;