From d04c1392c0b5bdd517a7032b158a689b5ab3d79e Mon Sep 17 00:00:00 2001
From: Christopher Haster <chaster@utexas.edu>
Date: Sun, 22 Nov 2020 00:40:58 -0600
Subject: [PATCH 1/2] Fixed allocation-eviction issue when erase state is
 multiple of block_cycles+1

This rather interesting corner-case arises in lfs_dir_alloc anytime the
uninitialized revision count happens to be a multiple of block_cycles+1.

For example, the source of the bug found by tim-nordell-nimbelink:

rev = 2742492087
block_cycles = 100

2742492087 % (100+1) = 0

The reason for this weird block_cycles+1 case is due to a fix for a
previous bug in fe957de. To avoid aliasing, which would cause metadata
pairs to wear unevenly, block_cycles incremented to the next odd number.

Normally, littlefs tweaks the revision count of blocks during
lfs_dir_alloc in order to make sure evictions can't happen on the first
compact. Otherwise, higher-level logic such as lfs_format would break.

However, this wasn't updated with the aliasing fix in fe957de, so
lfs_dir_alloc was only rounding the revision count to the nearest even
number.

The current fix is to change the logic in lfs_dir_alloc to explicitly
check for the eviction condition and increment if eviction would occur.

Found by tim-nordell-nimbelink
---
 lfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lfs.c b/lfs.c
index eb832fa0..3899c281 100644
--- a/lfs.c
+++ b/lfs.c
@@ -1375,8 +1375,12 @@ static int lfs_dir_alloc(lfs_t *lfs, lfs_mdir_t *dir) {
         return err;
     }
 
-    // make sure we don't immediately evict
-    dir->rev += dir->rev & 1;
+    // make sure we don't immediately evict, see lfs_dir_compact for why
+    // this check is so complicated
+    if (lfs->cfg->block_cycles > 0 &&
+            (dir->rev + 1) % ((lfs->cfg->block_cycles+1)|1) == 0) {
+        dir->rev += 1;
+    }
 
     // set defaults
     dir->off = sizeof(dir->rev);

From b8dcf10974e893270e8dd7e5ed5b18536e36f84b Mon Sep 17 00:00:00 2001
From: Christopher Haster <chaster@utexas.edu>
Date: Sat, 28 Nov 2020 22:46:11 -0600
Subject: [PATCH 2/2] Changed lfs_dir_alloc to maximize block cycles for new
 metadata pairs

Previously we only bumped the revision count if an eviction would occur
immediately (and possibly corrupt littlefs). This works, but does risk
an unoptimal superblock size if an almost-exhausted superblock was
allocated during lfs_format.

As pointed out by tim-nordell-nimbelink, we can align the revision count
to maximize the number of block cycles without breaking the existing
requirements of increasing revision counts.

As an added benefit, littlefs's wear-leveling should behave more
consistently after this change.
---
 lfs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lfs.c b/lfs.c
index 3899c281..ee47fb43 100644
--- a/lfs.c
+++ b/lfs.c
@@ -1375,11 +1375,11 @@ static int lfs_dir_alloc(lfs_t *lfs, lfs_mdir_t *dir) {
         return err;
     }
 
-    // make sure we don't immediately evict, see lfs_dir_compact for why
-    // this check is so complicated
-    if (lfs->cfg->block_cycles > 0 &&
-            (dir->rev + 1) % ((lfs->cfg->block_cycles+1)|1) == 0) {
-        dir->rev += 1;
+    // to make sure we don't immediately evict, align the new revision count
+    // to our block_cycles modulus, see lfs_dir_compact for why our modulus
+    // is tweaked this way
+    if (lfs->cfg->block_cycles > 0) {
+        dir->rev = lfs_alignup(dir->rev, ((lfs->cfg->block_cycles+1)|1));
     }
 
     // set defaults