The documentation does not match the implementation here. The intended behavior of superblock expansion was to duplicate the current superblock entry into the new superblock:
The main benefit is that we can rely on the magic string "littlefs" always residing in blocks 0x{0,1}, even if the superblock chain has multiple superblocks.
The downside is that earlier superblocks in the superblock chain may contain out-of-date configuration. This is a bit annoying, and risks hard-to-reach bugs, but in theory shouldn't break anything as long as the filesystem is aware of this.
Unfortunately this was lost at some point during refactoring in the early v2-alpha work. A lot of code was moving around in this stage, so it's a bit hard to track down the change and if it was intentional. The result is superblock expansion creates a valid linked-list of superblocks, but only the last superblock contains a valid superblock entry:
What's interesting is this isn't invalid as far as lfs_mount is concerned. lfs_mount is happy as long as a superblock entry exists anywhere in the superblock chain. This is good for compat flexibility, but is the main reason this has gone unnoticed for so long.
With the benefit of more time to think about the problem, it may have been more preferable to copy only the "littlefs" magic string and NOT the superblock entry:
This would allow for simple "littlefs" magic string checks without the risks associated with out-of-date superblock entries.
Unfortunately the current implementation errors if it finds a "littlefs" magic string without an associated superblock entry, so such a change would not be compatible with old drivers.
This PR tweaks superblock expansion to duplicate the superblock entry instead of simply moving it to the new superblock. And adds tests over the magic string "littlefs" both before and after superblock expansion.
It also cleans up some documentation around what can be found at specific offsets in the superblock in SPEC.md, which had fallen out-of-date.
geky-bot
commented
3 months ago
The documentation does not match the implementation here. The intended behavior of superblock expansion was to duplicate the current superblock entry into the new superblock:
The main benefit is that we can rely on the magic string "littlefs" always residing in blocks 0x{0,1}, even if the superblock chain has multiple superblocks.
The downside is that earlier superblocks in the superblock chain may contain out-of-date configuration. This is a bit annoying, and risks hard-to-reach bugs, but in theory shouldn't break anything as long as the filesystem is aware of this.
Unfortunately this was lost at some point during refactoring in the early v2-alpha work. A lot of code was moving around in this stage, so it's a bit hard to track down the change and if it was intentional. The result is superblock expansion creates a valid linked-list of superblocks, but only the last superblock contains a valid superblock entry:
What's interesting is this isn't invalid as far as
lfs_mountis concerned.lfs_mountis happy as long as a superblock entry exists anywhere in the superblock chain. This is good for compat flexibility, but is the main reason this has gone unnoticed for so long.With the benefit of more time to think about the problem, it may have been more preferable to copy only the "littlefs" magic string and NOT the superblock entry:
This would allow for simple "littlefs" magic string checks without the risks associated with out-of-date superblock entries.
Unfortunately the current implementation errors if it finds a "littlefs" magic string without an associated superblock entry, so such a change would not be compatible with old drivers.
This PR tweaks superblock expansion to duplicate the superblock entry instead of simply moving it to the new superblock. And adds tests over the magic string "littlefs" both before and after superblock expansion.
It also cleans up some documentation around what can be found at specific offsets in the superblock in SPEC.md, which had fallen out-of-date.
Found by @rojer and Nikola Kosturski Related https://github.com/littlefs-project/littlefs/issues/953