Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic volume recognition #349

Open
davidefer opened this issue Dec 11, 2019 · 22 comments
Open

Automatic volume recognition #349

davidefer opened this issue Dec 11, 2019 · 22 comments

Comments

@davidefer
Copy link

davidefer commented Dec 11, 2019

Hi everybody
I was wondering whether lfs could automatically detect the volume setup when mounting, without specifying the volume configuration in the lfs_config structure.
Would it be possible to store somewhere in the volume this information while formatting? Or at least all the information needed to detect the volume topology and let it be mounted.
The only information to be hardcoded in the fw would be the start address of lfs in the flash.
Thanks in advance.

@geky
Copy link
Member

geky commented Dec 17, 2019

Hi @davidefer, this has been on my mind for a while.

littlefs does have a "superblock entry" that contains static info about the filesystem (can you can add your own with lfs_setattr("/"), though I realize this doesn't help before you mount the filesystem).

Sidenote: Most of the configuration isn't needed for portability.

  • cache_size - Does nothing with disk.
  • read_size/prog_size - As long as these are compatible with the underlying disk, they are only used for cache-alignment and can change between systems.
  • block_cycles - Only used to determine when to evict blocks, can change.

littlefs only needs to know two things: The block_size and block_count.

The block_count is easy, we can read that from the superblock entry.

The block_size, however, present a chicken-and-egg problem I haven't found a good answer for yet.

The root/superblock in littlefs is a small two-block log in blocks 0 and 1. This means that at any given time one of the two blocks may be in the middle of an erase operation. For most flash, this means temporarily setting the flash to 1s, losing any data on the block. Handling is a hard requirement for power-resilience.

So we have a superblock in one of the two blocks at the start of storage. If we can get to one of the two blocks, we can fetch the configuration. But this is where we run into a problem. How do we load the second block if we don't know our block size? We only need the offset (not the size) to fetch a block, but without knowing the size, we don't know where the second block could be.

I'm very interested in solving this problem. But have no idea how to.


Some possible solutions:

  1. Provide configuration-detection as a "best-effort" operation, allowing for a possible failure if the device was caught erasing the superblock. I really don't like this "solution" as it could create rare problems for unaware-users and can't be relied on for any sort of real product

  2. Reserve one permanently-static block to store the superblock. This is a problem for small devices, such as internal flash, that only have two blocks in total. It could be an optional feature, though that doesn't mesh well with a consistent user-interface for storage. It's also costly.

  3. Iterate though possible block sizes (multiples of physical block size) until we find the superblock. This is the most promising, though it has the downside that invalid mounts could take an awfully long time to fail (O(n)). It would also be inconsistent depending on which of the two blocks the superblock resides in. We could reduce the runtime cost to O(log n) by only searching for powers-of-two of the physical block size, but this currently isn't a requirement.

@davidefer
Copy link
Author

Hi @geky
"3.Iterate though possible block size"
I think this is the easiest solution. If I remember correctly, the block size we can configure is dictated by the flash erasable areas , e.g. 4KB and 64KB blocks or sectors, so there would be only two options in this case to choose from. Am I missing something?

@e107steved
Copy link

e107steved commented Dec 18, 2019

Afraid so. Not everyone is using flash. For example, I'm using FRAM with a block size of 128, and small EEPROMs would be similar. Other people here appear to be using SD cards with 1M block sizes and so on. In any case, it could cause future problems if assumptions were made about the size of erasable blocks.

@e107steved
Copy link

Further thought. Option 3 looks to be the best (least bad?) choice for auto-determination. Firstly, I suggest this should be a compile-time option, since not all users will need it. Secondly, with auto-determination enabled, how about treating the value in the blocksize field as a hint? If its zero, try different block sizes according to some algorithm; otherwise, start from the hint value and try bigger and smaller values from there.

It may be possible to speed up the process by defining a search order for block sizes; perhaps 128, 256, 512, 4K, 1M early in the list, then fill in the gaps?

@davidefer
Copy link
Author

davidefer commented Dec 18, 2019

I was meaning that for every chip you don't have so many possibilities for block sizes. It was just an example, lfs doesn't have to hardcode those numbers. They should be proposed to lfs while mounting or similar.
So basically we are saying the same thing, lfs should get like an ordered array of possible block sizes to choose from while mounting.
In addition, lfs should be able to report which valid block size has found while mounting.

@geky
Copy link
Member

geky commented Dec 18, 2019

I was meaning that for every chip you don't have so many possibilities for block sizes.

If there is a known set of block sizes, you could iterate and try mounting each. This is one way to the filesystem which block sizes you support.

lfs_size_t block_sizes[] = {4096, 65536}; 
for (int i = 0; i < sizeof(block_sizes)/sizeof(block_sizes[0]); i++) {
    cfg.block_size = block_sizes[i];
    int err = lfs_mount(&lfs, &cfg);
    if (!err) {
        break;
    }
}

Of course right now littlefs is still missing the option of loading the block_count from disk. #279 and #238 are similar issues around this.

Keep in mind the block_size config option is a "virtual block size". It can be any multiple of the physical block, which is useful if you're working around performance issues.

It may be possible to speed up the process by defining a search order for block sizes; perhaps 128, 256, 512, 4K, 1M early in the list, then fill in the gaps?

That's a good idea. It wouldn't be unreasonable to search all powers-of-two (O(log n)) before searching other multiple (O(n)). It wouldn't change the amount of time needed to reject an unformated disk though.

I'm currently waiting on this to build up better benchmarks, which is needed for a number of other scalability issues. My concern is that the time taken to scan makes this option unreasonable. Disks can be surprisingly large.

@geky
Copy link
Member

geky commented Dec 18, 2019

Firstly, I suggest this should be a compile-time option, since not all users will need it.

It sounds like we should have a special value for lfs->cfg->block_size that is "LFS_BLOCK_SIZE_UNKNOWN" or similar.

There's been some ideas around improving how the lfs_config struct works to be possible to optimize out at compile time: #158 (comment)

@e107steved
Copy link

Is there actually any merit in integrating this within littleFs? That basic loop to try opening with different block sizes is so simple, and easily added by those that want it. I was concerned that code size would be increased with a feature that some (many? most?) don't want. Just needs a way of reading the block_count from disc. (And that possibly has to be configurable, since the config structure could be in ROM.)

@davidefer
Copy link
Author

If lfs could deal with something like cfg .block_count = LFS_BLOCK_COUNT_UNKNOWN (0xFFFFFFFFUL) and the search by itself the real block count while mounting (and providing it back as info), then the loop with the block sizes would be acceptable.

@dpgeorge
Copy link
Contributor

We wanted this feature (automatic block size and block count detection) in MicroPython and implemented it using option 1 (best effort, just look at the first part of the first block). But indeed it turns out to be unreliable, there have been cases where the first block was partially erased and did not contain a valid superblock (but the second block did).

Option 2 seems interesting if there's another place the data can be stored so it doesn't waste too much flash (external to the filesystem data).

Another idea is to store a copy of the info at the very end of the block device, so it's either at the start or end and there are only 2 places to search for it (only one of these would be erased at any one time, at the most).

But in the short term I think option 3 is the best way to solve it. Instead of making lfs_mount() automatic, there could be a new (optional) function like:

// Will fill in block_size and block_count in the given config struct if a valid FS is found.
// Returns 0 on success, or non-zero if no filesystem found.
int lfs_detect(lfs2_t *lfs2, const struct lfs2_config *config);

@geky
Copy link
Member

geky commented Feb 3, 2021

Another idea is to store a copy of the info at the very end of the block device, so it's either at the start or end and there are only 2 places to search for it (only one of these would be erased at any one time, at the most).

This is a clever idea, and almost works, except right now the metadata blocks can only be fetched from the beginning. So ironically we would need to know the block size in order to read a metadata block at the end of disk.

But maybe you could search for the magic string to try to guess where the block starts? Unfortunately that ends up again needing to search half of the entire block device in case the block size = 1/2 the disk size.

Hmm, but I suppose nothing says we couldn't just always invert the order we write one of the blocks in our metadata pair...

@geky
Copy link
Member

geky commented Feb 3, 2021

Ah, though storing the superblock at both the front+end means you need to know the block size. A sort of Heisenberg superblock.

Though maybe that's an acceptable tradeoff? The main use case you would lose is being able to store extra data after the filesystem without needing a partition table.

@jimparis
Copy link

jimparis commented Apr 2, 2021

I'd actually prefer the opposite of autodetection: for lfs_mount to return an error if the configured block_count doesn't match the superblock's block_count. Otherwise, littlefs will happily mount a filesystem even after I've resized its partition in my flash. Just another use case to consider. Locally, I've added the check for superblock.block_count == lfs->cfg->block_count and return a mount error on mismatch.

@geky
Copy link
Member

geky commented Nov 9, 2022

Circling back around to this issue, had a chance to explore it a bit more and found some interesting observations:

  1. It's a bit unintuitive, but if we know the disk contains a littlefs image, a naive scan for second superblock takes no worse than the current mount. This is because we will find the second superblock in the second logical block, searching at most $O(\mathrm{block\_size})$ bytes. So the only concern is empty/unformatted/corrupted disks.

  2. If we know the block_count, we can at least filter out block_sizes that don't divide the block count. I didn't think this would gain much but it turns out the divisor function, $d(n)$, is in fact sublinear.

    Word of warning, at the time of writing that wikipedia article is confusing and conflates the generalized divisor function? The talk page even has an "Utterly confusing article" section. Fortunately I found a much more useful blog post by Terence Tao here which shows that the divisor function is bounded by the ridiculous $O(e^{O(\frac{\log{n}}{\log{\log{n}}})})$. This is apparently somewhere between $O(\sqrt{n})$ and $O(\log{n})$, and is conveniently $O(\log{n})$ for powers of 2 and $O(\log{n})$ on average.

    Here's a plot of 1000 random numbers less than 2^32:

    divs

    What this means is that filtering our search by valid block_count divisors may lead to an acceptable runtime in the general case.

    This doesn't help if we don't know the block_size either, unfortunately the best you can do then is likely $O(n)$ which may take unacceptably long on large disks. Maybe in this case it's acceptable to print a warning so the user can distinguish between this case and littlefs getting stuck in an infinite loop.


I did explore other options for superblock locations, but unfortunately each come with their own set of problems:

  1. Superblocks at {0,1}

    .--------.--------.--------.--------.--------.--------.
    | s      | s      |        |        |        |        |
    | b ->   | b ->   |        |        |        |        |
    | 0      | 1      |        |        |        |        |
    '--------'--------'--------'--------'--------'--------'
    

    This is the current scheme, I just wanted to include the diagram.

  2. Superblocks at {0,n/2}

    .--------.--------.--------.--------.--------.--------.
    | s      |        |        | s      |        |        |
    | b ->   |        |        | b ->   |        |        |
    | 0      |        |        | 1      |        |        |
    '--------'--------'--------'--------'--------'--------'
    

    This scheme would work if we know the block_count. Well, unless our block_count isn't a multiple of 2, so we would also need to add that requirement.

    The problem is that this makes lfs_fs_grow (question: is it possible to "grow" the filesystem? #279) a much more difficult problem. If files exist at the new superblock location we'd have to move those files and that creates a big mess.

  3. Superblocks at {0,n-1}

    .--------.--------.--------.--------.--------.--------.
    | s      |        |        |        |        |      s |
    | b ->   |        |        |        |        |   <- b |
    | 0      |        |        |        |        |      1 |
    '--------'--------'--------'--------'--------'--------'
    

    If we store the second superblock at n-1 as is, this wouldn't quite work because we'd still need to know the block_size to know the offset in the disk where superblock 1 starts.

    However, if we store superblock 1 in reverse order at the byte-level, it becomes trivial to find superblock 1 as long as we know the block_count. Storing superblock 1 in reverse order adds some complexity, but it's not intractable.

    I really wanted this scheme to work. It has some nice properties such as easy detection of filesystem truncation and not getting in the way of lfs_fs_grow.

    Unfortunately, the problem child as always, idiosynchrasies of NAND devices prevent this scheme from working. It turns out some NAND devices require program operations to only only in a very specific order.

    From the mt29f's datasheet:

    Within a block, the pages must be programmed consecutively from the least significant bit (LSB) page of the block to most significant bit (MSB) pages of the block. Random page address programming is prohibited.

    From digging around in the details of how NAND flash works, it seems this is intended to limit the affect of program-disturb errors.

    This might mean that programming in reverse-order is ok, since program-disturbs would occur to the same number of neighbors as in-order. But it could also mean that there is extra circuitry somehow directing program-disturbs in one direction to limit disturbs to rows that will be programmed next. Either way it's probably a good idea to not intentionally violate datasheets.

    It's interesting to note this scheme of superblocks at {0,n-1} is used by GPT and ZFS, though both have a very fixed-size superblock.

@geky
Copy link
Member

geky commented Nov 13, 2022

Is there actually any merit in integrating this within littleFs? That basic loop to try opening with different block sizes is so simple, and easily added by those that want it. I was concerned that code size would be increased with a feature that some (many? most?) don't want.

@e107steved, this is a valid concern, deciding when to include features vs code size is hard. Unfortunately additional configuration options bring their own problems with fragmenting the code base and making testing more difficult.

There are a couple of benefits putting the block size search in littlefs, 1. we can handle tricky corner cases a bit better, such as a larger superblock being picked up by a search for a smaller superblock, 2. we can avoid the expensive search when we find a valid, but incompatible superblock, 3. there may be opportunities to optimize the search better, such as not fetching superblock 0 every search, reusing cachees, memory allocations, etc.

I'm currently seeing a ~818 byte (~4.5%) increase in code size and ~40 byte (~2.9%) increase in stack usage (branch). Though most of this comes from moving block_size/block_count into lfs_t which is necessary to make littlefs not strictly bound to the block device dimensions.

Code: +818 (+4.5%)
function (3 added, 1 removed)      osize   nsize   dsize
lfs_bd_flush                         184     380    +196 (+106.5%)
lfs_bd_erase                           -     104    +104 (+100.0%)
lfs_bd_rawread                         -     258    +258 (+100.0%)
lfs_fs_stat                            -      42     +42 (+100.0%)
lfs_format                           224     324    +100 (+44.6%)
lfs_mount                            576     816    +240 (+41.7%)
lfs_setattr                           24      26      +2 (+8.3%)
lfs_init                             606     642     +36 (+5.9%)
lfs_fs_rawsize                        56      58      +2 (+3.6%)
lfs_getattr                          148     152      +4 (+2.7%)
lfs_alloc                            232     236      +4 (+1.7%)
lfs_dir_traverse_filter              116     118      +2 (+1.7%)
lfs_dir_compact                      656     660      +4 (+0.6%)
lfs_dir_find                         392     394      +2 (+0.5%)
lfs_dir_commitcrc                    506     508      +2 (+0.4%)
lfs_dir_fetchmatch                  1100    1096      -4 (-0.4%)
lfs_file_flushedwrite                700     696      -4 (-0.6%)
lfs_file_relocate                    318     316      -2 (-0.6%)
lfs_dir_relocatingcommit            1220    1212      -8 (-0.7%)
lfs_file_flush                       300     298      -2 (-0.7%)
lfs_fs_rawtraverse                   364     360      -4 (-1.1%)
lfs_mkdir                            356     352      -4 (-1.1%)
lfs_file_flushedread                 248     244      -4 (-1.6%)
lfs_fs_pred                           80      78      -2 (-2.5%)
lfs_alloc_lookahead                   54      52      -2 (-3.7%)
lfs_removeattr                        20      18      -2 (-10.0%)
lfs_bd_read                          440     362     -78 (-17.7%)
lfs_bd_erase.isra.0                   64       -     -64 (-100.0%)
TOTAL                              18120   18938    +818 (+4.5%)
Stack: +40 (+2.9%)
function (3 added, 1 removed)     oframe  olimit  nframe  nlimit  dframe  dlimit
lfs_bd_erase                           -       -      16      16     +16     +16 (+100.0%, +100.0%)
lfs_bd_rawread                         -       -      40      40     +40     +40 (+100.0%, +100.0%)
lfs_fs_stat                            -       -      16     376     +16    +376 (+100.0%, +100.0%)
lfs_init                              24      32      32      40      +8      +8 (+33.3%, +25.0%)
lfs_bd_flush                          56     184      72     240     +16     +56 (+28.6%, +30.4%)
lfs_mount                            112     304     128     360     +16     +56 (+14.3%, +18.4%)
lfs_bd_read                           56      56      56      96      +0     +40 (+71.4%)
lfs_fs_parent_match                   48     104      48     144      +0     +40 (+38.5%)
lfs_bd_cmp.constprop                  72     128      72     168      +0     +40 (+31.2%)
lfs_ctz_find.constprop                72     128      72     168      +0     +40 (+31.2%)
lfs_ctz_traverse                      72     128      72     168      +0     +40 (+31.2%)
lfs_dir_getslice                      72     128      72     168      +0     +40 (+31.2%)
lfs_dir_get                           24     152      24     192      +0     +40 (+26.3%)
lfs_bd_prog                           32     216      32     272      +0     +56 (+25.9%)
lfs_dir_find_match                    32     160      32     200      +0     +40 (+25.0%)
lfs_dir_commitprog                    40     256      40     312      +0     +56 (+21.9%)
lfs_dir_fetchmatch                   128     184     128     224      +0     +40 (+21.7%)
lfs_dir_getgstate                     40     192      40     232      +0     +40 (+20.8%)
lfs_dir_getread.constprop             64     192      64     232      +0     +40 (+20.8%)
lfs_dir_getinfo                       48     200      48     240      +0     +40 (+20.0%)
lfs_dir_fetch                         24     208      24     248      +0     +40 (+19.2%)
lfs_dir_commitcrc                     80     296      80     352      +0     +56 (+18.9%)
lfs_dir_rewind                         8     216       8     256      +0     +40 (+18.5%)
lfs_dir_read                          24     232      24     272      +0     +40 (+17.2%)
lfs_dir_commitattr                    72     328      72     384      +0     +56 (+17.1%)
lfs_dir_commit_commit                  8     336       8     392      +0     +56 (+16.7%)
lfs_fs_pred                           32     240      32     280      +0     +40 (+16.7%)
lfs_dir_seek                          40     248      40     288      +0     +40 (+16.1%)
lfs_file_flushedread                  56     248      56     288      +0     +40 (+16.1%)
lfs_fs_parent                         64     248      64     288      +0     +40 (+16.1%)
lfs_dir_find                          96     280      96     320      +0     +40 (+14.3%)
lfs_dir_traverse.constprop           224     280     224     320      +0     +40 (+14.3%)
lfs_fs_rawtraverse                    96     304      96     344      +0     +40 (+13.2%)
lfs_fs_traverse                        8     312       8     352      +0     +40 (+12.8%)
lfs_fs_rawsize                        16     320      16     360      +0     +40 (+12.5%)
lfs_dir_open                          48     328      48     368      +0     +40 (+12.2%)
lfs_fs_size                            8     328       8     368      +0     +40 (+12.2%)
lfs_stat                              56     336      56     376      +0     +40 (+11.9%)
lfs_alloc                             40     344      40     384      +0     +40 (+11.6%)
lfs_getattr                           64     344      64     384      +0     +40 (+11.6%)
lfs_dir_alloc                         32     376      32     416      +0     +40 (+10.6%)
lfs_file_relocate                     64     408      64     448      +0     +40 (+9.8%)
lfs_dir_compact                      136     480     136     520      +0     +40 (+8.3%)
lfs_file_flushedwrite                 96     504      96     544      +0     +40 (+7.9%)
lfs_dir_split                        104     584     104     624      +0     +40 (+6.8%)
lfs_file_flush                       128     632     128     672      +0     +40 (+6.3%)
lfs_file_read                         24     656      24     696      +0     +40 (+6.1%)
lfs_file_rawwrite                     40     672      40     712      +0     +40 (+6.0%)
lfs_file_rawseek                      48     680      48     720      +0     +40 (+5.9%)
lfs_file_rewind                        8     688       8     728      +0     +40 (+5.8%)
lfs_file_write                        24     696      24     736      +0     +40 (+5.7%)
lfs_file_seek                         24     704      24     744      +0     +40 (+5.7%)
lfs_dir_relocatingcommit             136     720     136     760      +0     +40 (+5.6%)
lfs_file_truncate                     48     728      48     768      +0     +40 (+5.5%)
lfs_dir_orphaningcommit              160     880     160     920      +0     +40 (+4.5%)
lfs_fs_deorphan                      184    1064     184    1104      +0     +40 (+3.8%)
lfs_dir_commit                         8    1072       8    1112      +0     +40 (+3.7%)
lfs_dir_drop                          32    1104      32    1144      +0     +40 (+3.6%)
lfs_file_rawsync                      56    1128      56    1168      +0     +40 (+3.5%)
lfs_commitattr                        72    1144      72    1184      +0     +40 (+3.5%)
lfs_file_rawclose                     16    1144      16    1184      +0     +40 (+3.5%)
lfs_file_sync                         16    1144      16    1184      +0     +40 (+3.5%)
lfs_fs_forceconsistency               72    1144      72    1184      +0     +40 (+3.5%)
lfs_file_close                        16    1160      16    1200      +0     +40 (+3.4%)
lfs_removeattr                        16    1160      16    1200      +0     +40 (+3.4%)
lfs_setattr                           24    1168      24    1208      +0     +40 (+3.4%)
lfs_format                           112    1184     112    1224      +0     +40 (+3.4%)
lfs_file_rawopencfg                   72    1216      72    1256      +0     +40 (+3.3%)
lfs_file_open                         32    1248      32    1288      +0     +40 (+3.2%)
lfs_file_opencfg                      32    1248      32    1288      +0     +40 (+3.2%)
lfs_remove                           128    1272     128    1312      +0     +40 (+3.1%)
lfs_mkdir                            192    1336     192    1376      +0     +40 (+3.0%)
lfs_rename                           216    1360     216    1400      +0     +40 (+2.9%)
lfs_bd_erase.isra                      8       8       -       -      -8      -8 (-100.0%, -100.0%)
TOTAL                               4576    1360    4680    1400    +104     +40 (+2.3%, +2.9%)
Struct: +40 (+5.2%)
struct (1 added, 0 removed)    osize   nsize   dsize
lfs_fsinfo                         -      24     +24 (+100.0%)
lfs                              120     132     +12 (+10.0%)
lfs_config                        76      80      +4 (+5.3%)
TOTAL                            772     812     +40 (+5.2%)

@geky
Copy link
Member

geky commented Nov 14, 2022

Added a benchmark, shows the expected sub-linear runtime when block_count is known, but linear runtime both block_size and block_count is unknown. If we know the block_size the lfs_mount reads only ~136 bytes:

bench-mount-linear
bench-mount

Choosing a random device, mt25q, shows a max of ~90MiB/s reads in the datasheet. So for a very rough estimate:

unformatted mount 1GiB @ 90MiB/s
known block_count ~125us
known block_size ~1.44us
known bs/bc ~1.44us
unknown bs/bc ~376ms

I'm thinking this is quite reasonable for an exceptional operation, seeing as this performance hit only happens when the disk is unformatted.

@e107steved
Copy link

From the comments it's clear that there would be mixed responses to adding this feature - for example jimparis definitely doesn't want it (and I don't have any need for it). So IMO it should be optional.
I started thinking about why it could be useful. I came up with a couple of possible use cases (no doubt there are more):
a) Using multiple sizes of storage device. Probably the actual hardware configuration will be known to the application (if nothing else, the flash driver will have to handle it). So there should only be a few cases to be handled outside of littleFS.
b) I believe some popular applications (MicroPython?) don't put the file system in a fixed place, so other code might want to find it.

Having said all that, I can see the logic in storing the block size and count somewhere within the file system (then you have a self-contained binary blob to move around).

The extra code and RAM requirements for this feature are more than I would like to incur. I'd prefer to see it as a wrapper round the littleFS mount function. Maybe with a few more options than would be sensible in an integral implementation (for example, the "guided" list of block sizes to try).

What if the block count and size were simply stored in the superblock, but not otherwise used? Presumably that would require an absolutely minimal code increase, but make some of the things discussed here more feasible.

@geky
Copy link
Member

geky commented Dec 5, 2022

I've gone ahead and opened a PR for tracking purposes: #753.

I started thinking about why it could be useful.

Keep in mind the main benefit is not needing to know the block_size during mount. This is an uncommon requirement from a filesystem.

Two things this would give us that I personally like:

  1. Easier integration into existing OSs, requiring the block_size during mount is uncommon and needs more context than "this is a littlefs disk".
  2. Being able to read any littlefs blob without knowing the geometry. With this change, read_size=1, prog_size=1, erase_size=1, block_size=0, block_count=0 can mount any littlefs image.

What if the block count and size were simply stored in the superblock, but not otherwise used? Presumably that would require an absolutely minimal code increase, but make some of the things discussed here more feasible.

This is the current state of things. Unfortunately you can't read the superblock without knowing the block_size, so this is only useful as a redundant check after finding the block_size. Such a check was added in #584 after some of these comments.

The extra code and RAM requirements for this feature are more than I would like to incur. I'd prefer to see it as a wrapper round the littleFS mount function. Maybe with a few more options than would be sensible in an integral implementation (for example, the "guided" list of block sizes to try).

Most of these costs are from moving the block_size/block_count into RAM, and the translation between block_sizes in the low-level read/prog/erase operations. I don't think it would be possible to avoid these costs by adding an additional mount function. They are currently just being hidden in the block device implementations.

The block_size search itself is at most 240 bytes (+1.3%), as it is all contained in lfs_mount.


It sounds like this will also be helped by #491, which should allow you to avoid the code cost if the block_size is known at compile-time. Though this wouldn't help if you also have multiple littlefs disks or only know the block_size at runtime.

Long-term (probably in tandem with #491), I think we should move all of the lfs_config struct into RAM and get rid of the requirement that lfs_config be allocated for the applications duration. As it is, allowing lfs_config to be read-only prevents tweaking the configuration during mount, requiring duplicate conditions in some places, and, more often than not, lfs_config ends up in RAM anyways due to either abstraction layers or runtime-dependent configuration. I suspect this move will actually save code/RAM in the general case, but we won't know until it's measured.

@e107steved
Copy link

I'm not sure I understand about the "move lfs-config to RAM" suggestion. At present, code which has to operate with a single fixed configuration can be in ROM; code which has to support a number of configurations (e.g. different memory sizes) can be kept in RAM, provided the data is persistent. Seems very simple to me!
Perhaps what you are really suggesting is that littleFS maintains their own copy of all the lfs_config data in some way? If so, I can see some issues with that on small to medium sized single chip micros, since these tend to be relatively short on RAM. They are also the type of devices which will require only a single hardware configuration, so could at present keep the lfs_config in ROM. Unsurprisingly littleFS storing config data in RAM will require an additional 80 bytes (approx) of RAM for littleFS; a significant demand on a small micro which may only have 4K or 8K or RAM to start with.
A slight concern with RAM is its potential to be changed - not only by rare external events, but by program malfunction. In the second case, keeping lfs_config in ROM means that potentially littleFS remains functional (assuming littleFS uses data from the config block whenever it can), and could log data to a file. With the current situation, the user has the option of whether to protect or monitor the RAM in some way.
What makes a read-only lfs_config a problem within littleFS? To me, it all appears to be data which is naturally read-only (having been set up outside littleFS, which is "Someone else's problem".

@geky
Copy link
Member

geky commented Dec 6, 2022

I'm not sure I understand about the "move lfs-config to RAM" suggestion. At present, code which has to operate with a single fixed configuration can be in ROM; code which has to support a number of configurations (e.g. different memory sizes) can be kept in RAM, provided the data is persistent.

My belief is that the first use case, a single fixed configuration, is better implemented by using defines at compile-time. This would strip out runtime checks and allow more compiler optimizations. This isn't currently possible with littlefs, but is the goal of #491.

With #491 and lfs_config in RAM, both the fixed configuration and OS-like use cases should be improved, though there may be a cost for 2+ fixed configurations.

A slight concern with RAM is its potential to be changed - not only by rare external events, but by program malfunction. In the second case, keeping lfs_config in ROM means that potentially littleFS remains functional

This is an interesting consideration, but littlefs already has important state in RAM. I'm not sure it could continue to function without remounting, otherwise it won't know where the root directory starts for example.

What makes a read-only lfs_config a problem within littleFS? To me, it all appears to be data which is naturally read-only (having been set up outside littleFS, which is "Someone else's problem".

This was the original idea, but it turns out configuration is a bit more dynamic than expected during littlefs's lifetime.

name_max for example is already duplicated in both lfs_config and lfs_t since we need to handle the case where the disk configuration doesn't match the driver configuration.

metadata_max has a default behavior when zero, this could be easily implemented by allowing littlefs to modify metadata_max during lfs_mount, but as it is littlefs needs to check for this special case at every use of metadata_max, adding code cost.

Those are just some examples, it will be interesting to see exactly how this changes code size.

@e107steved
Copy link

On the RAM corruption issue, I was thinking about corruption of small areas - maybe 1-16 bytes. IIRC externally-derived corruption is mostly a few bytes. And software bugs such as an incorrect pointer value can lead to corruption of just a few bytes. If lfs_config is in ROM, a write will not affect it (and may on some architectures lead to an exception which should quickly lead to the root cause). If lfs_config is in RAM then sure, all bets are off and you may well have a difficult to find bug.

@geky
Copy link
Member

geky commented Sep 12, 2023

Just an FYI for anyone watching this issue, at the moment I'm considering #753 defunct.

More info in #753 (comment), but at the moment it looks like block-size will always be a required configuration option.

There's just not a tractable way to find the block-size faster than a naive search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants