fsext: do not confuse f_bsize with f_frsize on Linux - #14192
Open
Socialpranker wants to merge 1 commit into
Open
fsext: do not confuse f_bsize with f_frsize on Linux#14192Socialpranker wants to merge 1 commit into
Socialpranker wants to merge 1 commit into
Conversation
On Linux `f_bsize` is the preferred transfer size and `f_frsize` the block size the f_blocks/f_bfree/f_bavail counts are expressed in; the two were swapped. They are equal on most filesystems, so this only shows up where they differ - virtiofs reports a 1 MiB f_bsize next to a 4 KiB f_frsize, and df then reported a 461 GiB mount as 116 TiB. `stat -f` had %s and %S the other way round for the same reason.
Merging this PR will degrade performance by 10.17%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | split_lines |
9.2 ms | 10.6 ms | -13.16% |
| ❌ | Simulation | split_numeric_suffix |
9.7 ms | 11.1 ms | -12.55% |
| ❌ | Simulation | split_bytes |
644.5 µs | 675.3 µs | -4.57% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Socialpranker:fsext-frsize (2aea7a2) with main (0d8310c)
Footnotes
-
50 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
GNU testsuite comparison: |
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.
On Linux
struct statfscarries two sizes:f_bsize, the preferred transfersize, and
f_frsize, the block size thatf_blocks,f_bfreeandf_bavailare counted in.
FsMetahad them the wrong way round —block_size()returnedf_bsizeandio_size()returnedf_frsize— andFsUsage::newscaled thecounts by
f_bsize(with a// or statvfs.f_frsize ?comment next to it).Almost every filesystem reports the two as equal, so nothing shows. virtiofs —
the filesystem Docker Desktop uses for bind mounts — does not: on a 461 GiB
host disk it reports
f_bsize1 MiB andf_frsize4 KiB. In adebian:stable-slimcontainer with a bind-mounted directory:Every size is 256x too large — exactly
f_bsize / f_frsize. The raw countsback this up:
f_blocksis 120699413, and 120699413 x 4096 = 494384795648bytes = 461 GiB, the actual size of the disk; multiplying by
f_bsizegives118 TiB, which no disk on the machine has.
The same swap made
stat -fprint%sand%Sthe other way round:which contradicts this crate's own help text —
%sis documented as "blocksize (for faster transfers)" and
%Sas "fundamental block size (for blockcounts)".
This PR gives Linux and Android their own
block_size()(the fragment size,falling back to
f_bsizewhenf_frsizeis zero, as it is on pre-2.6 kernels)and
io_size()(the transfer size), and hasFsUsage::newscale byblock_size()instead of reading the field directly. Other targets areuntouched: Apple, FreeBSD and OpenBSD have no
f_frsizeinstruct statfsandtheir
f_bsizealready is the block size the counts use, so the existingcfgarms are kept verbatim minus the Android entries that moved.Testing: three unit tests in
fsext.rs. Two build astatfsholdingvirtiofs's numbers and check that
block_size()andio_size()come out theright way round and that
FsUsagescales 120699413 blocks to 494384795648bytes; the third covers the
f_frsize == 0fallback. The first two fail on currentmainand pass here; the fallback testpasses either way.
cargo test -p uucore --features fsextis green (9 passed),as are the
df(60 passed) andstat(38 passed) suites,cargo clippy -p uucore --features fsext --all-targetsandcargo fmt --all --check.cargo check -p uucore --features fsextalso passesfor
aarch64-linux-androidand on macOS, so both newcfgarms compile.Against the system
dfindebian:stable-slim,df -B1 --output=sizenowmatches byte for byte on
/,/dev,/dev/shmand the virtiofs mount, wherebefore the virtiofs row was off by 256x;
stat -f -c '%s %S'matches on all ofthem too.
Behaviour was established by observing GNU's output and the raw
statfsvalueson a Debian system — no GNU source was consulted.