Skip to content

Commit

Permalink
Moved revision count noise behind ifdef LFS_NOISY
Browse files Browse the repository at this point in the history
littlefs is intentionally designed to not rely on noise, even with cksum
collisions (hello, perturb bit!). So it makes sense for this to be an
optional feature, even if it's a small one.

Disabling revision count noise by default also helps with testing. The
whole point of revision count noise is to make cksum collisions less
likely, which is a bit counterproductive when that's something we want
to test!

This doesn't really change the revision count encoding:

  vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
  '-.''----.----''---------.--------'
    '------|---------------|---------- 4-bit relocation revision
           '---------------|---------- recycle-bits recycle counter
                           '---------- pseudorandom noise (optional)

I considered moving the recycle-bits down when we're not adding noise,
but the extra logic just isn't worth making the revision count a bit
more human-readable.

---

This saves a small bit of code in the default build, at the cost of some
code for the runtime checks in the LFS_NOISY build. Though I'm hoping
future config work will let users opt-out of these runtime checks:

                    code          stack          ctx
  before:          38548           2624          640
  default after:   38508 (-0.1%)   2624 (+0.0%)  640 (+0.0%)
  LFS_NOISY after: 38568 (+0.1%)   2624 (+0.0%)  640 (+0.0%)

Honestly the thing I'm more worried about is using one of our precious
mount flags for this... There's not that many bits left!
  • Loading branch information
geky committed Jan 29, 2025
1 parent 6fb9211 commit 1f3c47b
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 138 deletions.
36 changes: 26 additions & 10 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -6888,6 +6888,12 @@ static inline bool lfsr_m_isrdonly(uint32_t flags) {
return flags & LFS_M_RDONLY;
}

#ifdef LFS_NOISY
static inline bool lfsr_m_isnoisy(uint32_t flags) {
return flags & LFS_M_NOISY;
}
#endif

#ifdef LFS_CKPROGS
static inline bool lfsr_m_isckprogs(uint32_t flags) {
return flags & LFS_M_CKPROGS;
Expand Down Expand Up @@ -7198,15 +7204,20 @@ static int lfsr_fs_consumegdelta(lfs_t *lfs, const lfsr_mdir_t *mdir) {
// '-.''----.----''---------.--------'
// '------|---------------|---------- 4-bit relocation revision
// '---------------|---------- recycle-bits recycle counter
// '---------- pseudorandom nonce
// '---------- pseudorandom noise (optional)

static inline uint32_t lfsr_rev_init(lfs_t *lfs, uint32_t rev) {
(void)lfs;
// we really only care about the top revision bits here
rev &= ~((1 << 28)-1);
// increment revision
rev += 1 << 28;
// xor in a pseudorandom nonce
rev ^= ((1 << (28-lfs_smax(lfs->recycle_bits, 0)))-1) & lfs->gcksum;
// xor in pseudorandom noise
#ifdef LFS_NOISY
if (lfsr_m_isnoisy(lfs->flags)) {
rev ^= ((1 << (28-lfs_smax(lfs->recycle_bits, 0)))-1) & lfs->gcksum;
}
#endif
return rev;
}

Expand All @@ -7223,8 +7234,12 @@ static inline bool lfsr_rev_needsrelocation(lfs_t *lfs, uint32_t rev) {
static inline uint32_t lfsr_rev_inc(lfs_t *lfs, uint32_t rev) {
// increment recycle counter/revision
rev += 1 << (28-lfs_smax(lfs->recycle_bits, 0));
// xor in a pseudorandom nonce
rev ^= ((1 << (28-lfs_smax(lfs->recycle_bits, 0)))-1) & lfs->gcksum;
// xor in pseudorandom noise
#ifdef LFS_NOISY
if (lfsr_m_isnoisy(lfs->flags)) {
rev ^= ((1 << (28-lfs_smax(lfs->recycle_bits, 0)))-1) & lfs->gcksum;
}
#endif
return rev;
}

Expand Down Expand Up @@ -13421,6 +13436,7 @@ static int lfs_init(lfs_t *lfs, uint32_t flags,
| LFS_M_RDONLY
| LFS_M_FLUSH
| LFS_M_SYNC
| LFS_IFDEF_NOISY(LFS_M_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_M_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_M_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_M_CKPARITY, 0)
Expand Down Expand Up @@ -14249,6 +14265,7 @@ int lfsr_mount(lfs_t *lfs, uint32_t flags,
| LFS_M_RDONLY
| LFS_M_FLUSH
| LFS_M_SYNC
| LFS_IFDEF_NOISY(LFS_M_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_M_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_M_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_M_CKPARITY, 0)
Expand All @@ -14262,16 +14279,14 @@ int lfsr_mount(lfs_t *lfs, uint32_t flags,
LFS_ASSERT(!lfsr_m_isrdonly(flags) || !lfsr_t_ismkconsistent(flags));
LFS_ASSERT(!lfsr_m_isrdonly(flags) || !lfsr_t_islookahead(flags));
LFS_ASSERT(!lfsr_m_isrdonly(flags) || !lfsr_t_iscompact(flags));
// some flags don't make sense when only traversing the mtree
LFS_ASSERT(!lfsr_t_ismtreeonly(flags) || !lfsr_t_islookahead(flags));
LFS_ASSERT(!lfsr_t_ismtreeonly(flags) || !lfsr_t_isckdata(flags));

int err = lfs_init(lfs,
flags & (
LFS_M_RDWR
| LFS_M_RDONLY
| LFS_M_FLUSH
| LFS_M_SYNC
| LFS_IFDEF_NOISY(LFS_M_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_M_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_M_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_M_CKPARITY, 0)
Expand Down Expand Up @@ -14441,18 +14456,18 @@ int lfsr_format(lfs_t *lfs, uint32_t flags,
// unknown flags?
LFS_ASSERT((flags & ~(
LFS_F_RDWR
| LFS_IFDEF_NOISY(LFS_F_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_F_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_F_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_F_CKPARITY, 0)
| LFS_IFDEF_CKDATACKSUMS(LFS_F_CKDATACKSUMS, 0)
| LFS_F_CKMETA
| LFS_F_CKDATA)) == 0);
// some flags don't make sense when only traversing the mtree
LFS_ASSERT(!lfsr_t_ismtreeonly(flags) || !lfsr_t_isckdata(flags));

int err = lfs_init(lfs,
flags & (
LFS_F_RDWR
| LFS_IFDEF_NOISY(LFS_F_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_F_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_F_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_F_CKPARITY, 0)
Expand Down Expand Up @@ -14512,6 +14527,7 @@ int lfsr_fs_stat(lfs_t *lfs, struct lfs_fsinfo *fsinfo) {
LFS_I_RDONLY
| LFS_I_FLUSH
| LFS_I_SYNC
| LFS_IFDEF_NOISY(LFS_I_NOISY, 0)
| LFS_IFDEF_CKPROGS(LFS_I_CKPROGS, 0)
| LFS_IFDEF_CKFETCHES(LFS_I_CKFETCHES, 0)
| LFS_IFDEF_CKPARITY(LFS_I_CKPARITY, 0)
Expand Down
9 changes: 9 additions & 0 deletions lfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ enum lfs_type {
// Filesystem format flags
#define LFS_F_MODE 1 // Format's access mode
#define LFS_F_RDWR 0 // Format the filesystem as read and write
#ifdef LFS_NOISY
#define LFS_F_NOISY 0x00000010 // Add noise to revision counts
#endif
#ifdef LFS_CKPROGS
#define LFS_F_CKPROGS 0x00000800 // Check progs by reading back progged data
#endif
Expand All @@ -185,6 +188,9 @@ enum lfs_type {
#define LFS_M_RDONLY 1 // Mount the filesystem as read only
#define LFS_M_FLUSH 0x00000040 // Open all files with LFS_O_FLUSH
#define LFS_M_SYNC 0x00000080 // Open all files with LFS_O_SYNC
#ifdef LFS_NOISY
#define LFS_M_NOISY 0x00000010 // Add noise to revision counts
#endif
#ifdef LFS_CKPROGS
#define LFS_M_CKPROGS 0x00000800 // Check progs by reading back progged data
#endif
Expand All @@ -210,6 +216,9 @@ enum lfs_type {
#define LFS_I_RDONLY 0x00000001 // Mounted read only
#define LFS_I_FLUSH 0x00000040 // Mounted with LFS_M_FLUSH
#define LFS_I_SYNC 0x00000080 // Mounted with LFS_M_SYNC
#ifdef LFS_NOISY
#define LFS_I_NOISY 0x00000010 // Mounted with LFS_M_NOISY
#endif
#ifdef LFS_CKPROGS
#define LFS_I_CKPROGS 0x00000800 // Mounted with LFS_M_CKPROGS
#endif
Expand Down
9 changes: 9 additions & 0 deletions lfs_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@

// LFS_BIGGEST enables all opt-in features
#ifdef LFS_BIGGEST
#ifndef LFS_NOISY
#define LFS_NOISY
#endif
#ifndef LFS_CKPROGS
#define LFS_CKPROGS
#endif
Expand Down Expand Up @@ -189,6 +192,12 @@ extern "C"


// Some ifdef conveniences
#ifdef LFS_NOISY
#define LFS_IFDEF_NOISY(a, b) (a)
#else
#define LFS_IFDEF_NOISY(a, b) (b)
#endif

#ifdef LFS_CKPROGS
#define LFS_IFDEF_CKPROGS(a, b) (a)
#else
Expand Down
3 changes: 3 additions & 0 deletions scripts/dbgflags.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
# Filesystem format flags
('F', 'MODE', 1, "Format's access mode" ),
('^', 'RDWR', 0, "Format the filesystem as read and write" ),
('F', 'NOISY', 0x00000010, "Add noise to revision counts" ),
('F', 'CKPROGS', 0x00000800, "Check progs by reading back progged data" ),
('F', 'CKFETCHES', 0x00001000, "Check block checksums before first use" ),
('F', 'CKPARITY', 0x00002000, "Check metadata tag parity bits" ),
Expand All @@ -72,6 +73,7 @@
('^', 'RDONLY', 1, "Mount the filesystem as read only" ),
('M', 'FLUSH', 0x00000040, "Open all files with LFS_O_FLUSH" ),
('M', 'SYNC', 0x00000080, "Open all files with LFS_O_SYNC" ),
('M', 'NOISY', 0x00000010, "Add noise to revision counts" ),
('M', 'CKPROGS', 0x00000800, "Check progs by reading back progged data" ),
('M', 'CKFETCHES', 0x00001000, "Check block checksums before first use" ),
('M', 'CKPARITY', 0x00002000, "Check metadata tag parity bits" ),
Expand All @@ -97,6 +99,7 @@
('I', 'RDONLY', 0x00000001, "Mounted read only" ),
('I', 'FLUSH', 0x00000040, "Mounted with LFS_M_FLUSH" ),
('I', 'SYNC', 0x00000080, "Mounted with LFS_M_SYNC" ),
('I', 'NOISY', 0x00000010, "Mounted with LFS_M_NOISY" ),
('I', 'CKPROGS', 0x00000800, "Mounted with LFS_M_CKPROGS" ),
('I', 'CKFETCHES', 0x00001000, "Mounted with LFS_M_CKFETCHES" ),
('I', 'CKPARITY', 0x00002000, "Mounted with LFS_M_CKPARITY" ),
Expand Down
7 changes: 7 additions & 0 deletions tests/test_mount.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ code = '''
defines.RDONLY = [false, true]
defines.FLUSH = [false, true]
defines.SYNC = [false, true]
defines.NOISY = [false, true]
defines.CKPROGS = [false, true]
defines.CKFETCHES = [false, true]
defines.CKPARITY = [false, true]
Expand All @@ -27,6 +28,7 @@ defines.COMPACT = [false, true]
defines.CKMETA = [false, true]
defines.CKDATA = [false, true]
if = [
'LFS_IFDEF_NOISY(true, !NOISY)',
'LFS_IFDEF_CKPROGS(true, !CKPROGS)',
'LFS_IFDEF_CKFETCHES(true, !CKFETCHES)',
'LFS_IFDEF_CKPARITY(true, !CKPARITY)',
Expand All @@ -42,6 +44,7 @@ code = '''
((RDONLY) ? LFS_M_RDONLY : LFS_M_RDWR)
| ((FLUSH) ? LFS_M_FLUSH : 0)
| ((SYNC) ? LFS_M_SYNC : 0)
| ((NOISY) ? LFS_IFDEF_NOISY(LFS_M_NOISY, -1) : 0)
| ((CKPROGS) ? LFS_IFDEF_CKPROGS(LFS_M_CKPROGS, -1) : 0)
| ((CKFETCHES) ? LFS_IFDEF_CKFETCHES(LFS_M_CKFETCHES, -1) : 0)
| ((CKPARITY) ? LFS_IFDEF_CKPARITY(LFS_M_CKPARITY, -1) : 0)
Expand All @@ -62,6 +65,7 @@ code = '''
((RDONLY) ? LFS_I_RDONLY : 0)
| ((FLUSH) ? LFS_I_FLUSH : 0)
| ((SYNC) ? LFS_I_SYNC : 0)
| ((NOISY) ? LFS_IFDEF_NOISY(LFS_M_NOISY, -1) : 0)
| ((CKPROGS) ? LFS_IFDEF_CKPROGS(LFS_I_CKPROGS, -1) : 0)
| ((CKFETCHES) ? LFS_IFDEF_CKFETCHES(LFS_I_CKFETCHES, -1) : 0)
| ((CKPARITY) ? LFS_IFDEF_CKPARITY(LFS_I_CKPARITY, -1) : 0)
Expand All @@ -82,13 +86,15 @@ code = '''
#
# these end up passed to mount internally
[cases.test_mount_format_flags]
defines.NOISY = [false, true]
defines.CKPROGS = [false, true]
defines.CKFETCHES = [false, true]
defines.CKPARITY = [false, true]
defines.CKDATACKSUMS = [false, true]
defines.CKMETA = [false, true]
defines.CKDATA = [false, true]
if = [
'LFS_IFDEF_NOISY(true, !NOISY)',
'LFS_IFDEF_CKPROGS(true, !CKPROGS)',
'LFS_IFDEF_CKFETCHES(true, !CKFETCHES)',
'LFS_IFDEF_CKPARITY(true, !CKPARITY)',
Expand All @@ -98,6 +104,7 @@ code = '''
lfs_t lfs;
lfsr_format(&lfs,
LFS_F_RDWR
| ((NOISY) ? LFS_IFDEF_NOISY(LFS_F_NOISY, -1) : 0)
| ((CKPROGS) ? LFS_IFDEF_CKPROGS(LFS_F_CKPROGS, -1) : 0)
| ((CKFETCHES) ? LFS_IFDEF_CKFETCHES(LFS_F_CKFETCHES, -1) : 0)
| ((CKPARITY) ? LFS_IFDEF_CKPARITY(LFS_F_CKPARITY, -1) : 0)
Expand Down
Loading

0 comments on commit 1f3c47b

Please sign in to comment.