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

question: is it possible to "grow" the filesystem? #279

Open
mdevan opened this issue Aug 31, 2019 · 11 comments
Open

question: is it possible to "grow" the filesystem? #279

mdevan opened this issue Aug 31, 2019 · 11 comments

Comments

@mdevan
Copy link

mdevan commented Aug 31, 2019

If lfs is unmounted, and then the underlying block device size increases, is it possible to remount such that lfs can make use of the entire space? Like resize2fs or xfs_growfs, basically.

@lsilvaalmeida
Copy link

lsilvaalmeida commented Sep 2, 2019

The issue #238 can help you

@geky
Copy link
Member

geky commented Sep 4, 2019

Short answer is: lfs_fs_resize is a good idea.

littlefs uses a garbage collection scheme, so if you increase the block device everything will work fine. But, as @ksstms pointed out in #238, there is a pesky block_size field in the superblock that becomes out of date.

The field is also unused, so by pure coincidence, resizing the filesystem will work in the current version. But there will be problems if a future version of littlefs starts relying on this field.

I think adding lfs_fs_resize is a good idea, similar to resize2fs or xfs_growfs as you mentioned. This would also make the filesystem forward compatible if we decide to remove the garbage collection (#75).

@mdevan
Copy link
Author

mdevan commented Sep 5, 2019

Something like this, then?

int lfs_fs_resize(lfs_t *lfs, lfs_size_t new_block_count) {
	int err = 0;

	// read existing superblock from disk
	lfs_superblock_t superblock;
	err = lfs_dir_get(lfs, &dir, LFS_MKTAG(0x7ff, 0x3ff, 0),
			LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)),
			&superblock);
	if (err < 0) {
		goto cleanup;
	}
	lfs_superblock_fromle32(&superblock);

	// size can only be increased
	if (new_block_count <= superblock.block_count) {
		LFS_ERROR("Invalid new block count %d, not > %d", new_block_count,
			superblock.block_count);
		err = LFS_ERR_INVAL;
		goto cleanup;
	}

	// update superblock block_count
	superblock.block_count = new_block_count;

	// write back the updated superblock to disk
	lfs_superblock_tole32(&superblock);
	err = lfs_dir_commit(lfs, lfs->root, LFS_MKATTRS(
			{LFS_MKTAG(LFS_TYPE_SUPERBLOCK, 0, 8), "littlefs"},
			{LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)),
				&superblock}));
	if (err) {
		goto cleanup;
	}

	// succeeded, use new block_count from hereon
	lfs->cfg->block_count = new_block_count;

cleanup:
	return err;
}

and in lfs_mount, check if superblock.block_count == lfs->cfg->block_count so that disk and cfg and consistent at all times?

On a related note, lfs_mount should also probably check if superblock.block_size == lfs->cfg->block_size else there is a good chance of corruption if changes are made to an existing filesystem with a different block size?

@geky
Copy link
Member

geky commented Sep 5, 2019

That looks good, would you be interested in creating a PR?

Thoughts:

  1. Thinking about it, lfs_fs_grow may be a better name since it implies you can not shrink the filesystem.

  2. Do you think lfs_fs_grow should be after mount? Or a standalone operation similar to lfs_format, and lfs_migrate?

  3. lfs->cfg is const, so changing the block_count while mounted isn't that simple. If lfs_fs_grow is standalone, this is no problem. But if we want to be able to grow while mounted, we may need to move block_count to RAM.

    Moving block_count to RAM (similar to name_max), may be an overall good thing as it lets us use lfs->cfg->block_count as a config for the block device, and littlefs could mount a filesystem where the superblock block_count < lfs->cfg->block_count.

Also you're right mount should have more checks in place against block_size and block_count.

@colin-foster-in-advantage
Copy link
Contributor

I know this is pretty stale, but I'll add my two cents if this does get implemented...

  1. Thinking about it, lfs_fs_grow may be a better name since it implies you can not shrink the filesystem.

I'd think lfs_fs_resize might be better. In the future maybe "shrink" would be supported, though it would be more complicated than "grow". Maybe return an "LFS_ERR_NOTSUPP" if trying to shrink...

  1. Do you think lfs_fs_grow should be after mount? Or a standalone operation similar to lfs_format, and lfs_migrate?

As a user, I'd want the separate standalone operation. Maybe a flag during mount for "AUTORESIZE" or something similar... but being able to reason whether I want to attempt a resize vs reformat would be nice. My PR from #584 returns ERR_CORRUPT, but maybe ERR_INVAL or a new error code could return a size mismatch.

@preetpalkang
Copy link

My recommendation is that lfs_mount() should automatically handle the case when the block_count has increased when compared to the superblock written on the disk (storage medium).

Summary:

  • Automatically write updated superblock when block_count has increased hence we get the "resize" for free
  • Fail and return LFS_ERR_CORRUPT when block_count has decreased (we have lost disk space at this point, hence potentially lost files)
  • Fail and return LFS_ERR_CORRUPT when block_size has changed

I don't think an explicit lfs_grow() is necessary and it would complicate things. That would be more work for the code integrator as one would have to first check total blocks utilized by the lfs, and then check if the configured block_count differs, and then invoke lfs_grow() API just to write the superblock. One fundamental issue is that lfs does not expose an API to query superblock info, and I believe there is no API to get total blocks written into the superblock.

@colin-foster-in-advantage
Copy link
Contributor

colin-foster-in-advantage commented Aug 17, 2021

My recommendation is that lfs_mount() should automatically handle the case when the block_count has increased when compared to the superblock written on the disk (storage medium).

There should be some indication of it growing. If there was data outside of the filesystem, the only way to have a chance to recover it would be between the lfs_mount failure and the lfs_resize operations.

That's why I'd suggested the AUTORESIZE option. If you don't care about data outside of the filesystem, run mount with that flag and let it grow on its own. If you do, run mount without that flag, then recover the data before formatting the whole filesystem.

EDIT: There could also be an option to resize that gets passed into lfs_format as well. If that flag is set, attempt to format to the size. If that can't be done, do a full reformat...

@kaetemi
Copy link
Contributor

kaetemi commented Jun 13, 2022

For tooling purposes, it'd also be practical to be able to generate a minimum size flash image in advance, and allow them to grow to whatever hardware they've been flashed on once they get mounted. The suggestion to add an AUTORESIZE mount flag seems to be the most convenient.

@geky
Copy link
Member

geky commented Nov 9, 2022

Sorry about taking so long to get back to this. This is great discussions and with @kaetemi's PR (#702) I would like to see this get in to the next minor release.

That being said, I'm against the autoresizing option for a couple reasons:

  1. lfs_mount would no longer be a strictly readonly operation. This risks surprising users who expect mount+read to have no effect on the underlying disk. It also brings in the write logic to lfs_mount, requires different behavior if LFS_READONLY is set, and increases code size for users dependent on link-time garbage collection.

  2. lfs_fs_grow is a bit of a risky operation, and irreversable. If there is sensitive data past the filesystem and a bug or misconfiguration triggers the autoresize that data is lost. And once grown it's not currently possible to shrink the filesystem back so recovery may be limited to reformatting. It's subjective but I think this sort of one-way operation should be an explicit function call.

I don't think an explicit lfs_grow() is necessary and it would complicate things. That would be more work for the code integrator as one would have to first check total blocks utilized by the lfs, and then check if the configured block_count differs, and then invoke lfs_grow() API just to write the superblock.

This is a good point, however instead of making lfs_fs_grow implicit, we can provide a snippet of code that has the same implicit behavior for integrators to copy-paste if desired. That way integrators can opt-in to the autoresizing behavior if they think it's worth the code cost. This has worked well for automatic reformatting, which can also be a dangerous operation in certain circumstances.

Subject to change:

// mount and resize if disk has grown
int err = lfs_mount(lfs, cfg);
if (err) {
    return err;
}

err = lfs_fs_grow(lfs, cfg->block_count);
if (err) {
    return err;
}

Actually while writing this I realized you don't really need to check formatted blocks. We can make lfs_fs_grow a cheap noop if the formatted block_count already matches.

lfs_mount should always error if the block_count decreased. At that point something's gone very wrong.

One fundamental issue is that lfs does not expose an API to query superblock info, and I believe there is no API to get total blocks written into the superblock.

There hasn't been a need for this API as littlefs has expected the block_size/block_count to be static and unchanging. But with this change we should probably add a way to query. Prior art suggests a form of fs-stat:

struct lfs_fsstat fsstat;
int err = lfs_fs_stat(lfs, &fsstat);
if (err) {
    return err;
}

printf("block_size:  %d\n", fsstat.block_size);
printf("block_count: %d\n", fsstat.block_count);
printf("block_usage: %d\n", fsstat.block_usage); // same as lfs_fs_size
printf("name_max:    %d\n", fsstat.name_max);
printf("file_max:    %d\n", fsstat.file_max);
printf("attr_max:    %d\n", fsstat.attr_max);

I'd think lfs_fs_resize might be better. In the future maybe "shrink" would be supported, though it would be more complicated than "grow". Maybe return an "LFS_ERR_NOTSUPP" if trying to shrink...

Shrinking a filesystem is much more difficult problem. A shrink operation would likely come as a separate fsck-like application with expensive runtime (think mutating almost every block in the filesystem) and risk of failure (the data may just not fit). And if we do see shrink as an additional behavior for lfs_fs_grow it won't be the first API that gets overextended, see ftruncate which is how files grow :)

@geky
Copy link
Member

geky commented Dec 5, 2022

I've gone ahead and added both lfs_fs_grow and lfs_fs_stat to this PR: #753, which includes the necessary changes to make block_count changable after lfs_mount.

@geky
Copy link
Member

geky commented Sep 12, 2023

This feature has been long overdue. This is my bad for letting it get rolled up into #753, which became problematic to merge.

At the moment I've great a standalone PR for lfs_fs_grow, #872, and I'm hopeful to bring this in on the next minor release.

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

6 participants