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

Add lfs_fs_stat for access to filesystem status/configuration #838

Merged
merged 7 commits into from
Jun 30, 2023
Merged

Conversation

geky
Copy link
Member

@geky geky commented Jun 6, 2023

This is more-or-less cherry-picked from #753, which is paused for now.

lfs_fs_stat introduces a way to access some of the littlefs-specific status/configuration that hasn't been available yet. The intention is to be analogous the POSIX statvfs:

// Find on-disk info about the filesystem
//
// Fills out the fsinfo structure based on the filesystem found on-disk.
// Returns a negative error code on failure.
int lfs_fs_stat(lfs_t *lfs, struct lfs_fsinfo *fsinfo);

The immediate benefit of this function is a way to get the minor version on disk, which @TjoBitDk noted was oddly missing in the recent release. This should resolve #826.

Currently lfs_fs_stat includes:

  • disk_version - on-disk version
  • name_max - configurable name limit
  • file_max - configurable file limit
  • attr_max - configurable attr limit

These are currently the only configuration operations that need to be written to disk. Other configuration is either needed to mount, such as block_size, or does not change the on-disk representation, such as read/prog_size.

This also includes the current block usage, which is common in other filesystems, though a more expensive to find in littlefs. I figure it's not unreasonable to make lfs_fs_stat no worse than block allocation, hopefully this isn't a mistake. It may be worth caching the current usage after the most recent lookahead scan. EDIT: Removed due to concerns about runtime cost. May add back to fsinfo in the future.

More configuration may be added to this struct in the future.

geky added 3 commits June 6, 2023 13:02
Currently this includes:

- minor_version - on-disk minor version
- block_usage - estimated number of in-use blocks
- name_max - configurable name limit
- file_max - configurable file limit
- attr_max - configurable attr limit

These are currently the only configuration operations that need to be
written to disk. Other configuration is either needed to mount, such as
block_size, or does not change the on-disk representation, such as
read/prog_size.

This also includes the current block usage, which is common in other
filesystems, though a more expensive to find in littlefs. I figure it's
not unreasonable to make lfs_fs_stat no worse than block allocation,
hopefully this isn't a mistake. It may be worth caching the current
usage after the most recent lookahead scan.

More configuration may be added to this struct in the future.
LFS_VERSION -> LFS_DISK_VERSION

These tests shouldn't depend on LFS_VERSION. It's a bit subtle, but
LFS_VERSION versions the API, and LFS_DISK_VERSION versions the
on-disk format, which is what test_compat should be testing.
Needed for minor version reporting in lfs_fs_stat to work correctly.
geky added 3 commits June 6, 2023 22:00
This function naturally doesn't exist in the previous version. We should
eventually add these calls when we can expect the previous version to
support this function, though it's a bit unclear when that should happen.

Or maybe not! Maybe this is testing more of the previous version than we
really care about.
Valgrind output is very verbose but useful, with a default limit of 5
lines the output usually doesn't contain much useful info.
Version are now returned with major/minor packed into 32-bits,
so 0x00020001 is the current disk version, for example.

1. This needed to change to use a disk_* prefix for consistency with the
   defines that already exist for LFS_VERSION/LFS_DISK_VERSION.

2. Encoding the version this way has the nice side effect of making 0 an
   invalid value. This is useful for adding a similar config option
   that needs to have reasonable default behavior for backwards
   compatibility.

In theory this uses more space, but in practice most other config/status
is 32-bits in littlefs. We would be wasting this space for alignment
anyways.
@geky-bot
Copy link
Collaborator

geky-bot commented Jun 7, 2023

Tests passed ✓, Code: 16682 B (+0.8%), Stack: 1432 B (+0.0%), Structs: 792 B (+2.6%)
Code Stack Structs Coverage
Default 16682 B (+0.8%) 1432 B (+0.0%) 792 B (+2.6%) Lines 2315/2496 lines (-0.0%)
Readonly 6138 B (+2.5%) 536 B (+19.6%) 792 B (+2.6%) Branches 1182/1504 branches (+0.1%)
Threadsafe 17506 B (+0.8%) 1432 B (+0.0%) 800 B (+2.6%) Benchmarks
Migrate 18366 B (+0.7%) 1736 B (+0.0%) 796 B (+2.6%) Readed 29369693876 B (+0.0%)
Error-asserts 17318 B (+0.7%) 1424 B (+0.0%) 792 B (+2.6%) Proged 1482874766 B (+0.0%)
Erased 1568888832 B (+0.0%)

@geky geky added the needs minor version new functionality only allowed in minor versions label Jun 14, 2023
@geky geky added this to the v2.7 milestone Jun 26, 2023
@geky geky added next minor and removed needs minor version new functionality only allowed in minor versions labels Jun 26, 2023
In terms of ease-of-use, a user familiar with other filesystems expects
block_usage in fsinfo. But in terms of practicality, block_usage can be
expensive to find in littlefs, so if it's not needed in the resulting
fsinfo, that operation is wasteful.

It's not clear to me what the best course of action is, but since
block_usage can always be added to fsinfo later, but not removed without
breaking backwards compatibility, I'm leaving this out for now.

Block usage can still be found by explicitly calling lfs_fs_size.
@geky geky changed the base branch from master to devel June 29, 2023 17:35
@geky-bot
Copy link
Collaborator

Tests passed ✓, Code: 16674 B (+0.7%), Stack: 1432 B (+0.0%), Structs: 788 B (+2.1%)
Code Stack Structs Coverage
Default 16674 B (+0.7%) 1432 B (+0.0%) 788 B (+2.1%) Lines 2312/2493 lines (-0.0%)
Readonly 6122 B (+2.2%) 448 B (+0.0%) 788 B (+2.1%) Branches 1181/1502 branches (+0.1%)
Threadsafe 17502 B (+0.8%) 1432 B (+0.0%) 796 B (+2.1%) Benchmarks
Migrate 18358 B (+0.6%) 1736 B (+0.0%) 792 B (+2.1%) Readed 29369693876 B (+0.0%)
Error-asserts 17310 B (+0.7%) 1424 B (+0.0%) 788 B (+2.1%) Proged 1482874766 B (+0.0%)
Erased 1568888832 B (+0.0%)

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

Successfully merging this pull request may close these issues.

Getting disk version..
2 participants