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.

Sign up for GitHub

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

Jump to bottom

Fix metadata_max==prog_size commit->end calculation #1031

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open

Fix metadata_max==prog_size commit->end calculation #1031

wants to merge 3 commits into from

Commits on Oct 4, 2024

  1. Added some metadata_max testing 

    - Added METADATA_MAX to test_runner.
- Added METADATA_MAX to bench_runner.
- Added a simple metadata_max test to test_superblocks, for lack of
  better location.

There have been several issues floating around related to metadata_max
and LFS_ERR_NOSPC which makes me think there's a bug in our metadata_max
logic.

metadata_max was a quick patch and is relatively untested, so an
undetected bug isn't too surprising. This commit adds at least some
testing over metadata_max.

Sure enough, the new test_superblocks_metadata_max test reveals a
curious LFS_ERR_NAMETOOLONG error that shouldn't be there.

More investigation needed.
    @geky
    geky committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    1f82c0f View commit details
    Browse the repository at this point in the history

  2. Fixed metadata_max==prog_size commit->end calculation 

    The inconsistency here between the use of block_size vs metadata_max was
suspicious. Turns out there's a bug when metadata_max == prog_size.

We correctly use metadata_max for the block_size/2 check, but we weren't
using it for the block_size-40 check. The second check seems unnecessary
after the first, but it protects against running out of space in a
commit for commit-related metadata (checksums, tail pointers, etc) when
we can't program half-blocks.

Turns out this is also needed when limiting metadata_max to a single
prog, otherwise we risk erroring with LFS_ERR_NOSPC early.

Found by ajheck, dpkristensen, NLLK, and likely others.
    @geky
    geky committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    2d62d2f View commit details
    Browse the repository at this point in the history

  3. Added some checks that metadata_max makes sense 

    Like the read/prog/block_size checks, these are just asserts. If these
invariants are broken the filesystem will break in surprising ways.
    @geky
    geky committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    ea431bd View commit details
    Browse the repository at this point in the history