Skip to content

fuse: raise i_size over bytes a DLM read reply returned - #267

Merged
hbirth merged 1 commit into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1from
yongzech:red-51747-read-isize-6.17
Oct 16, 2026
Merged

hbirth merged 1 commit into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1from
yongzech:red-51747-read-isize-6.17

Conversation

@yongzech

Copy link
Copy Markdown
Collaborator

Summary

Under DLM with the writeback cache, a node's i_size covers only its own writes. When two nodes share a page at a block boundary, the lower writer reads the page before its partial write. The read returns the upper writer's bytes, but those bytes lie past the lower writer's local i_size. At writeback, iomap_writeback_handle_eof() zeroes them in the cached folio as if they were past EOF, and the folio stays uptodate. If i_size grows before the send is cropped, the zeroes overwrite the other node's data on the server (RED-51747).

A READ reply proves the file is at least pos + res bytes long, because the server clips READ replies at its file size. This change raises i_size to cover the returned bytes, so handle_eof never zeroes them.

Changes

  • fuse_dlm_read_extend_size() takes fi->lock and raises i_size to pos + res only when all of these hold:

    • fc->dlm && fc->writeback_cache;
    • the new size is larger than i_size;
    • FUSE_I_SIZE_UNSTABLE is clear;
    • fi->size_gen is unchanged since the READ was sent.

    When it raises i_size it also bumps attr_version.

  • It is called from fuse_do_readfolio() (read_folio, and iomap's read before a partial write) and from fuse_readpages_end() (readahead). O_DIRECT is untouched.

  • New fi->size_gen guards against stale replies. attr_version cannot do this job: every revoke bumps it, including the revoke of the range being read, which happened in the traced failure. size_gen is bumped only in two places:

    • fuse_do_setattr() for a truncate, when FUSE_I_SIZE_UNSTABLE is set and at both places it is cleared;
    • fuse_reverse_inval_inode() for a whole-file invalidate (offset <= 0 && len <= 0). This is the form a remote truncate takes on the client, because the server takes the sattr lock EX and that revokes the sattr locks of other clients.

Relation to #264

This PR does not depend on #264 and applies to the 6.17 branch on its own. The raise is skipped in some cases: a short read, a whole-file invalidate racing the read, or a page that did not come from a READ reply. In those cases the folio tail is still zeroed, and only #264 (clamp the writeback send to end_pos) keeps it from reaching the server. Merge both.

redfs-ubuntu-resolute-7.0.0-14.14 needs the same change.

Testing

brlock_mh.py ran on tyan-h4-u7..u12: three writer hosts and three hosts that only verify, with kernel 6.17.0-40 and a DKMS build of red-client-fs 2.5.1~alpha.1. The stock module corrupted data at iteration 2596 in the untraced run and at iteration 86 in the traced run.

Build Run Iterations Corrupt #264 clamp hits
#264 only plain, 1 h 17,990 0 34
#264 + this PR plain, 1 h 17,782 0 16
#264 + this PR --midsync, 1 h 12,036 0 2

--midsync calls sync_file_range after each pwrite while the grant is held. Each writer host then re-reads its own page cache before fsync, to catch a zeroed folio that stayed uptodate. That early check found 0 failures.

🤖 Generated with Claude Code

Under DLM with the writeback cache, a node's i_size only covers its own
writes. When two nodes share a page at a block boundary, the lower writer's
read before its partial write returns the upper writer's bytes from the
server, but they lie past the local i_size. iomap_writeback_handle_eof()
then zeroes them in the cached folio as if they were past EOF, the folio
stays uptodate, and if i_size grows before the send is cropped the zeroes
overwrite the other node's data on the server (RED-51747).

A read reply proves the file extends at least to pos + res, since the server
clips reads at its size. Raise i_size to cover those bytes in
fuse_do_readfolio() and fuse_readpages_end(), so the folio tail is never
treated as past EOF.

The raise must not resurrect bytes a truncate removed while the read was in
flight. attr_version cannot guard this, because every revoke bumps it,
including the revoke of the range being read. Add fi->size_gen instead,
bumped only by a local truncate and by a whole-file invalidate, which is how
a remote truncate reaches the client, and drop the raise when it changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hbirth
hbirth merged commit 92eee3c into DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1 Oct 16, 2026
2 checks passed
@cryptosven76

Copy link
Copy Markdown

Post-merge check on tyan-h3-u24: the deployed redfs.ko (DKMS red-client-fs-dkms 20261016.092513, kernel 6.17.0-42) contains this change with #264 and #268. The deterministic cached-zeros test from red/red #29256 (cached_zeros_repro.py, based on Haizhong Zhou's script) reported 5 of 5 runs intact, the case this PR fixes. The RED-51747 reproducer with 4 mounts and 3 writers had 0 corruptions in 6,110 iterations so far; a 3-hour run is in progress, with results on RED-51747.

🤖 Generated with Claude Code

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants