ext4: scale default block size by filesystem size#407
Draft
eriknordmark wants to merge 1 commit into
Draft
Conversation
recalculateBlocksize hard-coded 1 KiB blocks for every size bracket, so a default-parameter Create on a filesystem of a few GiB or more asked the journal allocator (capped at 128 MiB) for >65535 1-KiB blocks. That exceeds both the per-extent cap and the inode's 4-extent root limit, and Create returned "could not initialize journal: cannot allocate more than 65535 blocks in a single extent". Follow mke2fs's small-vs-default split instead: 1 KiB blocks below 512 MiB, 4 KiB blocks at or above. A 128 MiB journal then fits in a single maxBlocksPerExtent extent regardless of filesystem size. When *we* picked the non-1 KiB default, also disable reservedGDTBlocksForExpansion because resize_inode support is gated on 1 KiB blocks (existing TODO at the gdt sizing block). Fixes diskfs#402. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
recalculateBlocksizewas a placeholder: every bracket of its size-basedswitch set the same
sectorsPerBlock = 2, so a default-parameterCreatealways used 1 KiB blocks regardless of filesystem size. Onfilesystems of a few GiB the journal allocator — capped at 128 MiB =
131072 1-KiB blocks — asks for more blocks than the per-extent cap
(
maxBlocksPerExtent = 32768) or the inode's 4-extent root limit canfit, and
Createfails with(see #402 for the trace).
This change makes
recalculateBlocksizeactually pick a block size byfilesystem size, matching
mke2fs's small-vs-default split: 1 KiBblocks below 512 MiB, 4 KiB blocks at or above. With 4 KiB blocks a
128 MiB journal needs exactly
maxBlocksPerExtent(32768) blocks andfits in a single extent.
Because
resize_inode/ reserved-GDT growth in go-diskfs currentlyonly supports 1 KiB blocks (there's an existing
TODO: Properly support resize_inodeat the GDT-sizing block, andTestCreateWithBlockSizesalready passesWithFeatureReservedGDTBlocksForExpansion(false)for non-1 KiB cases),this also drops the feature when we picked the larger default. A
caller who explicitly sets
SectorsPerBlockis unaffected: they passtheir own block size and the default-only path doesn't run.
Test
Added
TestCreateMultiGB— Create on 1 GiB and 2 GiB images, validatedwith
e2fsck -f -n. Both pass with the fix; the 2 GiB case is thesize that reproduces #402 on master. Full
go test ./...is green.Behavior change to flag
This is a visible default change: existing callers who got 1 KiB
blocks back from
Createon a >512 MiB filesystem will now get 4 KiBblocks. Almost certainly the right default (matches
mke2fs), butworth calling out. Callers who need the old behavior can set
Params.SectorsPerBlock = 2explicitly.Not in this PR
Large-file writes that need >32768 blocks still hit the same inode
4-extent root limit fixed by no other change —
createRootExtentTreeexplicitly returns "cannot create root internal node" past 4 entries.
That's a separate, pre-existing limitation; this PR closes #402 for
the journal-allocation case without addressing it. Worth filing as its
own issue.
Fixes #402.