Skip to content

fix(fs): align write size to 4096 to support all NVMEs#11335

Merged
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/fix_dio_block_size
Mar 20, 2026
Merged

fix(fs): align write size to 4096 to support all NVMEs#11335
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/fix_dio_block_size

Conversation

@kskalski
Copy link
Copy Markdown

@kskalski kskalski commented Mar 16, 2026

Problem

#9856 ended up with 512 alignment for direct I/O writes, but it fails for some NVMEs. As reported in FD tests:

Archive error: failed to unpack "accounts/316628658.54075": IO error: Invalid argument (os error 22)

The used hardware spec:

Node                  Generic               SN                   Model                                    Namespace  Usage                      Format           FW Rev
--------------------- --------------------- -------------------- ---------------------------------------- ---------- -------------------------- ---------------- --------
..
/dev/nvme1n1          /dev/ng1n1            KND7N5364I1308G1S    DELL NVME ISE PE8110 RI U.2 960GB        0x1        960.20  GB / 960.20  GB      4 KiB +  0 B   1.1.0

indicates the NVME sector size of 4KiB, which ends up as required alignment for O_DIRECT

independently there were failures noticed on some CI workers where disks are

$ lsblk -o NAME,SIZE,LOG-SEC,PHY-SEC,MOUNTPOINT
NAME          SIZE LOG-SEC PHY-SEC MOUNTPOINT
nvme2n1       3.5T    4096    4096
└─vg01-lv01   7.4T    4096    4096 /var/lib/buildkite-agent
nvme0n1     447.1G     512    4096
├─nvme0n1p1     1M     512    4096
└─nvme0n1p2 447.1G     512    4096 /
nvme3n1       3.5T    4096    4096
└─vg01-lv01   7.4T    4096    4096 /var/lib/buildkite-agent
nvme1n1     447.1G    4096    4096
└─vg01-lv01   7.4T    4096    4096 /var/lib/buildkite-agent

Again: LOG-SEC column showing 4096 makes it the most likely culprit.

Summary of Changes

Bump constant used for alignment of writes to 4096 and update comment.

@kskalski kskalski marked this pull request as ready for review March 17, 2026 00:07
@kskalski kskalski requested a review from alessandrod March 17, 2026 00:07
@kskalski
Copy link
Copy Markdown
Author

@alpeng-jump feel free to add any additional context for the hardware / kernel / filesystem that you used

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (08b606c) to head (5219d3a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11335   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         841      841           
  Lines      318275   318275           
=======================================
+ Hits       265019   265022    +3     
+ Misses      53256    53253    -3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@steviez
Copy link
Copy Markdown

steviez commented Mar 17, 2026

I had not previously heard reports of this issue, but am I correct in assuming we'd need/want this for v4.0 given that we merged the direct IO v4.0 BP (#11153) ?

@kskalski
Copy link
Copy Markdown
Author

Yes, we will need to backport it to 4.0, in order to keep direct-io functionality added there working properly.

@kskalski kskalski added this pull request to the merge queue Mar 20, 2026
Merged via the queue into anza-xyz:master with commit 504d2ca Mar 20, 2026
51 checks passed
@kskalski kskalski deleted the ks/fix_dio_block_size branch March 20, 2026 04:04
@kskalski kskalski added the v4.0 Backport to v4.0 branch label Mar 20, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 20, 2026

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify Bot pushed a commit that referenced this pull request Mar 20, 2026
kskalski added a commit that referenced this pull request Mar 20, 2026
…t of #11335) (#11424)

fix(fs): align write size to 4096 to support all NVMEs (#11335)

(cherry picked from commit 504d2ca)

Co-authored-by: Kamil Skalski <kamil.skalski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v4.0 Backport to v4.0 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants