littlefs: Fix some issues with lookahead trust #6604
Conversation
One of the big simplifications in littlefs's implementation is the complete lack of tracking free blocks, allowing operations to simply drop blocks that are no longer in use. However, this means the lookahead buffer can easily contain outdated blocks that were previously deleted. This is usually fine, as littlefs will rescan the storage if it can't find a free block in the lookahead buffer, but after changes that caused littlefs to more conservatively respect the alloc acks (e611cf5), any scanned blocks after an ack would be incorrectly trusted. The fix is to eagerly scan ahead in the lookahead when we allocate so that alloc acks are better able to discredit old lookahead blocks. Since usually alloc acks are tightly coupled to allocations of one or two blocks, this allows littlefs to properly rescan every set of allocations. This may still be a concern if there is a long series of worn out blocks, but in the worst case littlefs will conservatively avoid using blocks it's not sure about. Found by davidefer
…inters As pointed out by davidefer, the lookahead pointer modular arithmetic does not work around integer overflow when the pointer size is not a multiple of the block count. To avoid overflow problems, the easy solution is to stop trying to work around integer overflows and keep the lookahead offset inside the block device. To make this work, the ack was modified into a resetable counter that is decremented every block allocation. As a plus, quite a bit of the allocation logic ended up simplified.
| @@ -301,5 +301,128 @@ tests/test.py << TEST | |||
| lfs_unmount(&lfs) => 0; | |||
| TEST | |||
|
|
|||
| echo "--- Outdated lookahead test ---" | |||
There was a problem hiding this comment.
What does this mean in regards to outdated?
There was a problem hiding this comment.
The test is putting littlefs into the state where the lookahead buffer is filled with files that have been deleted. Making the state of the lookahead buffer outdated.
This happens when you delete files after populating the lookahead buffer.
It's up to the filesystem to recognize this situation and correctly refill the lookahead buffer.
Does that make sense?
There was a problem hiding this comment.
Gotchya! Yeah that makes sense.
| lfs_block_t off; | ||
| lfs_block_t size; | ||
| lfs_block_t i; |
There was a problem hiding this comment.
Can we have better name for i? Other variables pretty much explain the purpose, except i.
There was a problem hiding this comment.
Anything but i .
My guess without looking at what it does - just a counter so a user does not need to initialize another variable for (int i = 0...) 😄
There was a problem hiding this comment.
Well, that's exactly what it is... a counter, it's state just lasts outside of a function
There was a problem hiding this comment.
Then name it counter_for_something (replace somthing what it counts, what value it holds)
There was a problem hiding this comment.
Nah, I like counter_for_what_it_counts
There was a problem hiding this comment.
I joke, sure I can change it to index once I'm somewhere I can push to github
geky
force-pushed
the
lfs-fix-lookahead-trust
branch
from
8729f2f to
9e03b24
Compare
|
/morph build |
Build : SUCCESSBuild number : 1752 Triggering tests/morph test |
Test : SUCCESSBuild number : 1557 |
Exporter Build : SUCCESSBuild number : 1389 |
|
So what is pr-head up to? |
CI team is looking at it currently how to fix it in jenkins and also Github support also looking at the breakage |
Description
Two somewhat related issues were found over here: littlefs-project/littlefs#46
After quite a bit of discussion the result is some fixes a bit of simplification of the lookahead management. Most significantly, removing the dependency on integer overflow. This should hopefully remove a lot of the issues we've been seeing around allocation.
Fix issue with lookahead trusting old lookahead blocks
One of the big simplifications in littlefs's implementation is the complete lack of tracking free blocks, allowing operations to simply drop blocks that are no longer in use.
However, this means the lookahead buffer can easily contain outdated blocks that were previously deleted. This is usually fine, as littlefs will rescan the storage if it can't find a free block in the lookahead
buffer, but after changes that caused littlefs to more conservatively respect the alloc acks (e611cf5), any scanned blocks after an ack would be incorrectly trusted.
The fix is to eagerly scan ahead in the lookahead when we allocate so that alloc acks are better able to discredit old lookahead blocks. Since usually alloc acks are tightly coupled to allocations of one or two blocks, this allows littlefs to properly rescan every set of allocations.
This may still be a concern if there is a long series of worn out blocks, but in the worst case littlefs will conservatively avoid using blocks it's not sure about.
Fixed lookahead overflow and removed unbounded lookahead pointers
The lookahead pointer modular arithmetic does not work around integer overflow when the pointer size is not a multiple of the block count.
To avoid overflow problems, the easy solution is to stop trying to work around integer overflows and keep the lookahead offset inside the block device. To make this work, the ack was modified into a resetable counter that is decremented every block allocation.
As a plus, quite a bit of the allocation logic ended up simplified.
Pull request type
[✓] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change