From 130281ac053df11210564cc5c6b3714e7685c79f Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 16 Mar 2024 17:00:30 -0500 Subject: [PATCH] Reworked compat flags a bit Now with a bit more granularity for possibly-future-optional on-disk data structures: LFSR_RCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved) LFSR_RCOMPAT_MLEAF 0x0002 ---- ---- ---- --1- LFSR_RCOMPAT_MSHRUB 0x0004 ---- ---- ---- -1-- (reserved) LFSR_RCOMPAT_MTREE 0x0008 ---- ---- ---- 1--- LFSR_RCOMPAT_BSPROUT 0x0010 ---- ---- ---1 ---- LFSR_RCOMPAT_BLEAF 0x0020 ---- ---- --1- ---- LFSR_RCOMPAT_BSHRUB 0x0040 ---- ---- -1-- ---- LFSR_RCOMPAT_BTREE 0x0080 ---- ---- 1--- ---- LFSR_RCOMPAT_GRM 0x0100 ---- ---1 ---- ---- LFSR_WCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved) LFSR_OCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved) This adds a couple reserved flags: - LFSR_*COMPAT_NONSTANDARD - This flag will never be set by a standard version of littlefs. The idea is to allow implementations with non-standard extensions a way to signal potential compatibility issues without worrying about future compat flag conflicts. This is limited to a single bit, but hey, it's not like it's possible to predict all future extensions. If a non-standard extension needs more granularity, reservations of standard compat flags can always be requested, even if they don't end up implemented in standard littlefs. (Though such reservations will need a strong motivation, it's not like these flags are free). - LFSR_RCOMPAT_MSHRUB - In theory littlefs supports a shrubbed mtree, where the root is inlined into the mroot. But in practice this turned out to be more complicated than it was worth. Still, a future implementation may find an mshrub useful, so preserving a compat flag for such a case makes sense. That being said, I have no plans to add support for mshrubs even in the dbg scripts. I would like the expected feature-set for debug tools to be well-defined, but also conservative. This gets a bit tricky with theoretical features like the mshrubs, but until mshrubs are actually implemented in littlefs, I would like to consider them non-standard. The implication of this is that, while LFSR_RCOMPAT_MSHRUB is currently "reserved", it may be repurposed for some other meaning in the future. These changes also rename *COMPATFLAGS -> *COMPAT, and reorder the tags by decreasing importance. This ordering seems more valuable than the original intention of making rcompat/wcompat a single bit flip. Implementation-wise, it's interesting to note the internal-only LFSR_*COMPAT_OVERFLOW flag. This gets set when out-of-range bits are set on-disk, and allows us to detect unrepresentable compat flags without too much extra complexity. The extra encoding/decoding overhead does add a bit of cost though: code stack before: 33944 2880 after: 34124 (+0.5%) 2880 (+0.0%) --- lfs.c | 174 +++++++++++++++++++++++++++++++++----------- scripts/dbgbmap.py | 6 +- scripts/dbgbtree.py | 12 +-- scripts/dbglfs.py | 48 ++++++------ scripts/dbgmtree.py | 12 +-- scripts/dbgrbyd.py | 12 +-- 6 files changed, 175 insertions(+), 89 deletions(-) diff --git a/lfs.c b/lfs.c index 07c1eb1e..1dd031e1 100644 --- a/lfs.c +++ b/lfs.c @@ -718,9 +718,9 @@ enum lfsr_tag { LFSR_TAG_CONFIG = 0x0000, LFSR_TAG_MAGIC = 0x0003, LFSR_TAG_VERSION = 0x0004, - LFSR_TAG_OCOMPATFLAGS = 0x0005, - LFSR_TAG_RCOMPATFLAGS = 0x0006, - LFSR_TAG_WCOMPATFLAGS = 0x0007, + LFSR_TAG_RCOMPAT = 0x0005, + LFSR_TAG_WCOMPAT = 0x0006, + LFSR_TAG_OCOMPAT = 0x0007, LFSR_TAG_BLOCKSIZE = 0x0008, LFSR_TAG_BLOCKCOUNT = 0x0009, LFSR_TAG_NAMELIMIT = 0x000a, @@ -7776,24 +7776,111 @@ static int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t, // compatibility flags // -// - WCOMPAT => Must understand to write to the filesystem // - RCOMPAT => Must understand to read the filesystem +// - WCOMPAT => Must understand to write to the filesystem +// - OCOMPAT => Don't need to understand, we don't really use these // // note, "understanding" does not necessarily mean support // enum lfsr_rcompat { - LFSR_RCOMPAT_GRM = 0x01, + LFSR_RCOMPAT_NONSTANDARD = 0x0001, + LFSR_RCOMPAT_MLEAF = 0x0002, + LFSR_RCOMPAT_MTREE = 0x0008, + LFSR_RCOMPAT_BSPROUT = 0x0010, + LFSR_RCOMPAT_BLEAF = 0x0020, + LFSR_RCOMPAT_BSHRUB = 0x0040, + LFSR_RCOMPAT_BTREE = 0x0080, + LFSR_RCOMPAT_GRM = 0x0100, + // internal + LFSR_RCOMPAT_OVERFLOW = 0x8000, }; -typedef uint8_t lfsr_rcompat_t; -typedef uint8_t lfsr_wcompat_t; +#define LFSR_RCOMPAT_COMPAT \ + (LFSR_RCOMPAT_MLEAF \ + | LFSR_RCOMPAT_MTREE \ + | LFSR_RCOMPAT_BSPROUT \ + | LFSR_RCOMPAT_BLEAF \ + | LFSR_RCOMPAT_BSHRUB \ + | LFSR_RCOMPAT_BTREE \ + | LFSR_RCOMPAT_GRM) -static inline bool lfsr_rcompat_hasgrm(lfsr_rcompat_t rcompat) { - return rcompat & LFSR_RCOMPAT_GRM; +enum lfsr_wcompat { + LFSR_WCOMPAT_NONSTANDARD = 0x0001, + // internal + LFSR_WCOMPAT_OVERFLOW = 0x8000, +}; + +#define LFSR_WCOMPAT_COMPAT 0 + +enum lfsr_ocompat { + LFSR_OCOMPAT_NONSTANDARD = 0x0001, + // internal + LFSR_OCOMPAT_OVERFLOW = 0x8000, +}; + +#define LFSR_OCOMPAT_COMPAT 0 + +typedef uint16_t lfsr_rcompat_t; +typedef uint16_t lfsr_wcompat_t; +typedef uint16_t lfsr_ocompat_t; + +static inline bool lfsr_rcompat_isincompat(lfsr_rcompat_t rcompat) { + return rcompat != LFSR_RCOMPAT_COMPAT; } -static inline bool lfsr_rcompat_hasunknown(lfsr_rcompat_t rcompat) { - return rcompat & ~LFSR_RCOMPAT_GRM; +static inline bool lfsr_wcompat_isincompat(lfsr_wcompat_t wcompat) { + return wcompat != LFSR_WCOMPAT_COMPAT; +} + +static inline bool lfsr_ocompat_isincompat(lfsr_ocompat_t ocompat) { + return ocompat != LFSR_OCOMPAT_COMPAT; +} + +// compat flags on-disk encoding +// +// little-endian, truncated bits must be assumed zero + +#define LFSR_DATA_FROMRCOMPAT(_rcompat) \ + LFSR_DATA_IMM(((uint8_t[]){ \ + (((_rcompat) >> 0) & 0xff), \ + (((_rcompat) >> 8) & 0xff)}), 2) + +static int lfsr_data_readrcompat(lfs_t *lfs, lfsr_data_t *data, + lfsr_rcompat_t *rcompat) { + // allow truncated rcompat flags + uint8_t buf[2] = {0}; + lfs_ssize_t d = lfsr_data_read(lfs, data, buf, 2); + if (d < 0) { + return d; + } + *rcompat = lfs_fromle16_(buf); + + // if any out-of-range flags are set, set the internal overflow bit, + // this is a compromise in correctness and and compat-flag complexity + // + // we don't really care about performance here + while (lfsr_data_size(*data) > 0) { + lfs_scmp_t cmp = lfsr_data_cmp(lfs, *data, (uint8_t[]){0}, 1); + if (cmp < 0) { + return cmp; + } + + if (cmp != LFS_CMP_EQ) { + *rcompat |= LFSR_RCOMPAT_OVERFLOW; + } + + *data = lfsr_data_slice(*data, d, -1); + } + + return 0; +} + +// all the compat parsing is basically the same, so try to reuse code +#define LFSR_DATA_FROMWCOMPAT(_wcompat) LFSR_DATA_NULL() + +static int lfsr_data_readwcompat(lfs_t *lfs, lfsr_data_t *data, + lfsr_wcompat_t *wcompat) { + return lfsr_data_readrcompat(lfs, data, wcompat); } @@ -7868,57 +7955,56 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) { // check for any rcompatflags, we must understand these to read // the filesystem - err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_RCOMPATFLAGS, + lfsr_rcompat_t rcompat = 0; + err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_RCOMPAT, &data); if (err && err != LFS_ERR_NOENT) { return err; } - if (err == LFS_ERR_NOENT) { - data = LFSR_DATA_NULL(); + if (err != LFS_ERR_NOENT) { + err = lfsr_data_readrcompat(lfs, &data, &rcompat); + if (err) { + return err; + } } - lfsr_rcompat_t rcompat; - lfs_ssize_t size = lfsr_data_read(lfs, &data, &rcompat, 1); - if (size < 0) { - return size; - } - if (size < 1) { - rcompat = 0; - } - - // unknown rcompat flags? flags must be tightly sized - if (lfsr_rcompat_hasunknown(rcompat) || lfsr_data_size(data) > 0) { - LFS_ERROR("Incompatible rcompat flags 0x%s%"PRIx8, - (lfsr_data_size(data) > 0) ? "??" : "", - rcompat); - return LFS_ERR_INVAL; - } - - // grm supported? - if (!lfsr_rcompat_hasgrm(rcompat)) { - LFS_ERROR("Incompatible rcompat flags, no grm"); - // TODO switch to read-only? upgrade? + // incompatible rcompat flags? + if (lfsr_rcompat_isincompat(rcompat)) { + LFS_ERROR("Incompatible rcompat flags 0x%0"PRIx16 + " (!= 0x%0"PRIx16")", + rcompat, + LFSR_RCOMPAT_COMPAT); return LFS_ERR_INVAL; } // check for any wcompatflags, we must understand these to write // the filesystem - err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_WCOMPATFLAGS, + lfsr_wcompat_t wcompat = 0; + err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_WCOMPAT, &data); if (err && err != LFS_ERR_NOENT) { return err; } - if (err == LFS_ERR_NOENT) { - data = LFSR_DATA_NULL(); + if (err != LFS_ERR_NOENT) { + err = lfsr_data_readwcompat(lfs, &data, &wcompat); + if (err) { + return err; + } } - // unknown wcompat flags? flags must be tightly sized - if (lfsr_data_size(data) > 0) { - LFS_ERROR("Incompatible wcompat flags 0x??"); - // TODO switch to read-only? + // incompatible wcompat flags? + // TODO switch to readonly? + if (lfsr_wcompat_isincompat(wcompat)) { + LFS_ERROR("Incompatible wcompat flags 0x%0"PRIx16 + " (!= 0x%0"PRIx16")", + wcompat, + LFSR_WCOMPAT_COMPAT); return LFS_ERR_INVAL; } + // we don't bother to check for any ocompatflags, we would just + // ignore these anyways + // check block size err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_BLOCKSIZE, &data); @@ -8203,8 +8289,8 @@ static int lfsr_formatinited(lfs_t *lfs) { LFS_DISK_VERSION_MAJOR, LFS_DISK_VERSION_MINOR}), 2)), LFSR_ATTR( - LFSR_TAG_RCOMPATFLAGS, 0, - LFSR_DATA_IMM(((uint8_t[1]){LFSR_RCOMPAT_GRM}), 1)), + LFSR_TAG_RCOMPAT, 0, + LFSR_DATA_FROMRCOMPAT(LFSR_RCOMPAT_COMPAT)), LFSR_ATTR( LFSR_TAG_BLOCKSIZE, 0, LFSR_DATA_LEB128(lfs->cfg->block_size-1)), diff --git a/scripts/dbgbmap.py b/scripts/dbgbmap.py index feefc38a..2dcb0ffe 100755 --- a/scripts/dbgbmap.py +++ b/scripts/dbgbmap.py @@ -14,9 +14,9 @@ TAG_NULL = 0x0000 TAG_CONFIG = 0x0000 TAG_MAGIC = 0x0003 TAG_VERSION = 0x0004 -TAG_OCOMPATFLAGS = 0x0005 -TAG_RCOMPATFLAGS = 0x0006 -TAG_WCOMPATFLAGS = 0x0007 +TAG_RCOMPAT = 0x0005 +TAG_WCOMPAT = 0x0006 +TAG_OCOMPAT = 0x0007 TAG_BLOCKSIZE = 0x0008 TAG_BLOCKCOUNT = 0x0009 TAG_NAMELIMIT = 0x000a diff --git a/scripts/dbgbtree.py b/scripts/dbgbtree.py index 7e7a398b..276109d6 100755 --- a/scripts/dbgbtree.py +++ b/scripts/dbgbtree.py @@ -12,9 +12,9 @@ TAG_NULL = 0x0000 TAG_CONFIG = 0x0000 TAG_MAGIC = 0x0003 TAG_VERSION = 0x0004 -TAG_OCOMPATFLAGS = 0x0005 -TAG_RCOMPATFLAGS = 0x0006 -TAG_WCOMPATFLAGS = 0x0007 +TAG_RCOMPAT = 0x0005 +TAG_WCOMPAT = 0x0006 +TAG_OCOMPAT = 0x0007 TAG_BLOCKSIZE = 0x0008 TAG_BLOCKCOUNT = 0x0009 TAG_NAMELIMIT = 0x000a @@ -165,9 +165,9 @@ def tagrepr(tag, w, size, off=None): 'shrub' if tag & TAG_SHRUB else '', 'magic' if (tag & 0xfff) == TAG_MAGIC else 'version' if (tag & 0xfff) == TAG_VERSION - else 'ocompatflags' if (tag & 0xfff) == TAG_OCOMPATFLAGS - else 'rcompatflags' if (tag & 0xfff) == TAG_RCOMPATFLAGS - else 'wcompatflags' if (tag & 0xfff) == TAG_WCOMPATFLAGS + else 'rcompat' if (tag & 0xfff) == TAG_RCOMPAT + else 'wcompat' if (tag & 0xfff) == TAG_WCOMPAT + else 'ocompat' if (tag & 0xfff) == TAG_OCOMPAT else 'blocksize' if (tag & 0xfff) == TAG_BLOCKSIZE else 'blockcount' if (tag & 0xfff) == TAG_BLOCKCOUNT else 'sizelimit' if (tag & 0xfff) == TAG_SIZELIMIT diff --git a/scripts/dbglfs.py b/scripts/dbglfs.py index 7e644732..d2dba779 100755 --- a/scripts/dbglfs.py +++ b/scripts/dbglfs.py @@ -13,9 +13,9 @@ TAG_NULL = 0x0000 TAG_CONFIG = 0x0000 TAG_MAGIC = 0x0003 TAG_VERSION = 0x0004 -TAG_OCOMPATFLAGS = 0x0005 -TAG_RCOMPATFLAGS = 0x0006 -TAG_WCOMPATFLAGS = 0x0007 +TAG_RCOMPAT = 0x0005 +TAG_WCOMPAT = 0x0006 +TAG_OCOMPAT = 0x0007 TAG_BLOCKSIZE = 0x0008 TAG_BLOCKCOUNT = 0x0009 TAG_NAMELIMIT = 0x000a @@ -196,9 +196,9 @@ def tagrepr(tag, w, size, off=None): 'shrub' if tag & TAG_SHRUB else '', 'magic' if (tag & 0xfff) == TAG_MAGIC else 'version' if (tag & 0xfff) == TAG_VERSION - else 'ocompatflags' if (tag & 0xfff) == TAG_OCOMPATFLAGS - else 'rcompatflags' if (tag & 0xfff) == TAG_RCOMPATFLAGS - else 'wcompatflags' if (tag & 0xfff) == TAG_WCOMPATFLAGS + else 'rcompat' if (tag & 0xfff) == TAG_RCOMPAT + else 'wcompat' if (tag & 0xfff) == TAG_WCOMPAT + else 'ocompat' if (tag & 0xfff) == TAG_OCOMPAT else 'blocksize' if (tag & 0xfff) == TAG_BLOCKSIZE else 'blockcount' if (tag & 0xfff) == TAG_BLOCKCOUNT else 'sizelimit' if (tag & 0xfff) == TAG_SIZELIMIT @@ -1031,25 +1031,25 @@ class Config: return (None, None) @ft.cached_property - def ocompatflags(self): - if TAG_OCOMPATFLAGS in self.config: - _, data = self.config[TAG_OCOMPATFLAGS] + def rcompat(self): + if TAG_RCOMPAT in self.config: + _, data = self.config[TAG_RCOMPAT] return data else: return None @ft.cached_property - def rcompatflags(self): - if TAG_RCOMPATFLAGS in self.config: - _, data = self.config[TAG_RCOMPATFLAGS] + def wcompat(self): + if TAG_WCOMPAT in self.config: + _, data = self.config[TAG_WCOMPAT] return data else: return None @ft.cached_property - def wcompatflags(self): - if TAG_WCOMPATFLAGS in self.config: - _, data = self.config[TAG_WCOMPATFLAGS] + def ocompat(self): + if TAG_OCOMPAT in self.config: + _, data = self.config[TAG_OCOMPAT] return data else: return None @@ -1098,15 +1098,15 @@ class Config: for b in map(chr, self.magic)) elif tag == TAG_VERSION: return 'version v%d.%d' % self.version - elif tag == TAG_OCOMPATFLAGS: - return 'ocompatflags 0x%s' % ''.join( - '%02x' % f for f in reversed(self.ocompatflags)) - elif tag == TAG_RCOMPATFLAGS: - return 'rcompatflags 0x%s' % ''.join( - '%02x' % f for f in reversed(self.rcompatflags)) - elif tag == TAG_WCOMPATFLAGS: - return 'wcompatflags 0x%s' % ''.join( - '%02x' % f for f in reversed(self.wcompatflags)) + elif tag == TAG_RCOMPAT: + return 'rcompat 0x%s' % ''.join( + '%x' % f for f in reversed(self.rcompat)) + elif tag == TAG_WCOMPAT: + return 'wcompat 0x%s' % ''.join( + '%x' % f for f in reversed(self.wcompat)) + elif tag == TAG_OCOMPAT: + return 'ocompat 0x%s' % ''.join( + '%x' % f for f in reversed(self.ocompat)) elif tag == TAG_BLOCKSIZE: return 'blocksize %d' % self.block_size elif tag == TAG_BLOCKCOUNT: diff --git a/scripts/dbgmtree.py b/scripts/dbgmtree.py index 378f0754..c4d03245 100755 --- a/scripts/dbgmtree.py +++ b/scripts/dbgmtree.py @@ -12,9 +12,9 @@ TAG_NULL = 0x0000 TAG_CONFIG = 0x0000 TAG_MAGIC = 0x0003 TAG_VERSION = 0x0004 -TAG_OCOMPATFLAGS = 0x0005 -TAG_RCOMPATFLAGS = 0x0006 -TAG_WCOMPATFLAGS = 0x0007 +TAG_RCOMPAT = 0x0005 +TAG_WCOMPAT = 0x0006 +TAG_OCOMPAT = 0x0007 TAG_BLOCKSIZE = 0x0008 TAG_BLOCKCOUNT = 0x0009 TAG_NAMELIMIT = 0x000a @@ -180,9 +180,9 @@ def tagrepr(tag, w, size, off=None): 'shrub' if tag & TAG_SHRUB else '', 'magic' if (tag & 0xfff) == TAG_MAGIC else 'version' if (tag & 0xfff) == TAG_VERSION - else 'ocompatflags' if (tag & 0xfff) == TAG_OCOMPATFLAGS - else 'rcompatflags' if (tag & 0xfff) == TAG_RCOMPATFLAGS - else 'wcompatflags' if (tag & 0xfff) == TAG_WCOMPATFLAGS + else 'rcompat' if (tag & 0xfff) == TAG_RCOMPAT + else 'wcompat' if (tag & 0xfff) == TAG_WCOMPAT + else 'ocompat' if (tag & 0xfff) == TAG_OCOMPAT else 'blocksize' if (tag & 0xfff) == TAG_BLOCKSIZE else 'blockcount' if (tag & 0xfff) == TAG_BLOCKCOUNT else 'sizelimit' if (tag & 0xfff) == TAG_SIZELIMIT diff --git a/scripts/dbgrbyd.py b/scripts/dbgrbyd.py index b1041c2a..ae0e5c0e 100755 --- a/scripts/dbgrbyd.py +++ b/scripts/dbgrbyd.py @@ -21,9 +21,9 @@ TAG_NULL = 0x0000 TAG_CONFIG = 0x0000 TAG_MAGIC = 0x0003 TAG_VERSION = 0x0004 -TAG_OCOMPATFLAGS = 0x0005 -TAG_RCOMPATFLAGS = 0x0006 -TAG_WCOMPATFLAGS = 0x0007 +TAG_RCOMPAT = 0x0005 +TAG_WCOMPAT = 0x0006 +TAG_OCOMPAT = 0x0007 TAG_BLOCKSIZE = 0x0008 TAG_BLOCKCOUNT = 0x0009 TAG_NAMELIMIT = 0x000a @@ -167,9 +167,9 @@ def tagrepr(tag, w, size, off=None): 'shrub' if tag & TAG_SHRUB else '', 'magic' if (tag & 0xfff) == TAG_MAGIC else 'version' if (tag & 0xfff) == TAG_VERSION - else 'ocompatflags' if (tag & 0xfff) == TAG_OCOMPATFLAGS - else 'rcompatflags' if (tag & 0xfff) == TAG_RCOMPATFLAGS - else 'wcompatflags' if (tag & 0xfff) == TAG_WCOMPATFLAGS + else 'rcompat' if (tag & 0xfff) == TAG_RCOMPAT + else 'wcompat' if (tag & 0xfff) == TAG_WCOMPAT + else 'ocompat' if (tag & 0xfff) == TAG_OCOMPAT else 'blocksize' if (tag & 0xfff) == TAG_BLOCKSIZE else 'blockcount' if (tag & 0xfff) == TAG_BLOCKCOUNT else 'sizelimit' if (tag & 0xfff) == TAG_SIZELIMIT