Skip to content

Redfs ubuntu resolute 7.0.0 14.14 - #273

Open
hbirth wants to merge 6 commits into
DDNStorage:redfs-ubuntu-resolute-7.0.0-14.14from
hbirth:redfs-ubuntu-resolute-7.0.0-14.14
Open

hbirth wants to merge 6 commits into
DDNStorage:redfs-ubuntu-resolute-7.0.0-14.14from
hbirth:redfs-ubuntu-resolute-7.0.0-14.14

Conversation

@hbirth

@hbirth hbirth commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

hbirth and others added 6 commits October 16, 2026 13:28
fuse_reverse_inval_inode() ignored the result of
invalidate_inode_pages2_range(), so folios left in the page cache after a
notify went unnoticed. Log a rate-limited warning when the invalidate
fails, and when the write in fuse_launder_folio() fails.
iomap_writeback_handle_eof() zeroes the folio straddling i_size past
that size before ->writeback_range() runs, and passes the size on as
end_pos. fuse_iomap_writeback_range() ignored end_pos and queued the
whole block, relying on the crop in fuse_send_writepage() to drop the
bytes past EOF. That crop reads i_size again when the request is sent.

Under DLM with the writeback cache, i_size is only this node's view:
each writer claims its own extension under fi->lock while the inode
lock is held shared. A partial EOF folio can hold another node's bytes,
read in by the read-modify-write read under the write grant. If a
concurrent local writer extends i_size between the zeroing and the
crop, the crop no longer trims the folio and the zeros are written over
the other node's acknowledged data.

RED-51747 hits this with three nodes writing interleaved 47008-byte
blocks of one file: the higher block's head reads back as zeros up to
the page boundary. Traces show the lower writer's flush leaving with
"own block + zeros" while the page it read from the server, and the
i_size it zeroed at, were correct; a higher-offset write had extended
i_size 40 us before the request was sent.

Send only up to end_pos and still report the whole range as handled,
so the bytes iomap zeroed never reach the server. The 6.12 tree
avoided this because its writeback crops at i_size without zeroing.

Signed-off-by: Sven Oehme <soehme@ddn.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
…eplies

Under DLM with the writeback cache, i_size is only this node's view of
the file, and whether writeback treats bytes as past EOF depends on
what i_size was at that exact moment. When RED-51747 lost another
node's bytes to zeroes, the local i_size at writeback had to be
reconstructed from the order of DLM grants, because nothing records it.

Add three tracepoints:

  fuse_isize_change      every i_size change and why: the cached-write
                         extension claim, its retraction, or a size
                         taken from an attribute reply
  fuse_writeback_range   each writeback chunk with pos, len, bytes sent,
                         iomap's end_pos and the current i_size
  fuse_readfolio_reply   each fuse_do_readfolio() READ with the length
                         asked, the bytes returned and i_size

With these, the RED-51747 race shows up directly as a writeback chunk
whose end_pos is below i_size while its whole block is sent, preceded
by a READ reply that returned bytes past the local EOF.

Signed-off-by: Sven Oehme <soehme@ddn.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under DLM the server's attr_valid can keep the attributes of a locked
inode valid indefinitely. Once a DLM lock was taken on an inode, cap
its i_time to dlm_attr_timeout seconds so a GETATTR goes out at most
that long after the lock. The first attribute reply from the server
after the lock ends the bound; a reply to a request sent before it is
discarded by bumping attr_version. Default 30s, module parameter, 0
keeps the server's timeout.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
…nd open

Split file, address space, inode and vm operations into wt and wb
variants chosen in fuse_init_file_inode()/fuse_init_common() from
fc->writeback_cache. Drop fc->dlm, now unread; -ENOSYS to a DLM
request aborts the connection.

DAX is fixed per inode, passthrough per open. Install DAX file
operations from fuse_dax_inode_init() and switch a passthrough open to
passthrough file operations in fuse_finish_open(), so the cached
read/write/mmap/splice paths no longer test for either. Each keeps the
open, flush and release of its cache mode.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
@hbirth
hbirth force-pushed the redfs-ubuntu-resolute-7.0.0-14.14 branch from 185adff to 719a629 Compare October 21, 2026 11:19
@hbirth

hbirth commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

This does well on xfstests:
Failures: generic/029 generic/030 generic/062 generic/184 generic/193 generic/263 generic/294 generic/355 generic/434 generic/452 generic/531 generic/617 generic/631 generic/633 generic/634 generic/683
Failed 16 of 783 tests

and two of the fails are due to a wrong version of installed software (need that version for systing)

Comment thread fs/fuse/inode.c
fc->blocked = 0;
fc->initialized = 0;
fc->connected = 1;
fc->dlm = 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch removes the old fc->dlm flag which functioned as a sort of dlm discovery mechanic.
Now we just abort the connection, which means we might end up with a package skew if we have
kernel new module and an old redfsd (unless there's something that pins them?)

the aborts happen in two places:

  1. fuse_dlm_cache.c:823-828 in fuse_get_dlm_lock() when a writeback-cache server without DLM is
    aborted on its first cached read/write via fuse_wb_cache_{read,write}_iter() which calls
    fuse_get_dlm_lock()

  2. file.c:3815-3820 in fuse_get_page_mkwrite_lock() when a writethrough server is aborted on its
    first MAP_SHARED|PROT_WRITE fault

A cheap fix would be to add a no_dlm flag in fuse_conn, and set it in when we get ENOSYS. Then
the function can just return early if it finds the flag already set. This is similar to the the
old wt behavior.

Then we can either do the same in fuse_get_dlm_lock() or maybe add a or add an INIT flag so redfsd
declares DLM present once.

@hbirth hbirth Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot protect our pagecache without DLM against other nodes ... and even worse, we hav changed the behavior so badly, that I don't see any other possibility than terminating the connection if the fuse server (in whatever state that might be) using the redfs device (we actually use another /dev/redfs than fuse) cannot manage to perform DLM_LOCK

Comment thread fs/fuse/file.c
* -EAGAIN: Legacy error code, maintained for backward compatibility
*/
if ((res == -EDEADLK || res == -EAGAIN) && fm->fc->dlm)
if (res == -EDEADLK || res == -EAGAIN)

@achhenderson achhenderson Oct 22, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is reasonable here since the dlm flag is gone, but a wt server that returns -EAGAIN on a READ what ever reason now gets the folio retried indefinitely. If that wasnt intentional then we could gate it on fuse_read_size_fn

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@achhenderson

Copy link
Copy Markdown

Ok, there's a lot of changes going on here, but overall i think the structural direction
is sound and probably worth the divergence. If upstream FUSE-X / iomap-writeback
work lands later, the new wb tables is where most of the delta is and would rebase
well. Until then though, I think the biggest trip hazard would be porting upstream
fixes on to a dissimilar base assuming there's something that we really want to port.

Pros:

  • Each path is straight-line code; the wb fault/mkwrite path is far easier
    to audit without the if (fc->writeback_cache && fc->dlm) nesting.
  • No runtime mode flips, so the "fc->dlm cleared mid-write, re-decide the
    lock" hazards (comment block deleted around old line 2343) are gone by
    construction.
  • Op-table dispatch is cheaper than per-call flag tests on the hot
    read/write path.

Cons:

  • file.c grew by ~600 lines and has 8 file_operations tables. Every
    upstream change to fuse_file_operations, fuse_open(), fuse_flush(),
    fuse_file_read_iter() etc. will conflict and need applying two to four
    times. The 6.17 -> 7.0 forward-port already needed hand-merging here.
  • Hard-codes "writeback => DLM". Right for redfs; it makes the in-tree
    fs/fuse unusable for other wb servers, which only matters on hosts that
    boot a redfs-ubuntu-* kernel directly (finding 1).
  • fuse_do_setattr() keeps a mode-recomputing wrapper for the direct-IO
    rollback caller, so the "decided once" invariant already has an exception.

@hbirth

hbirth commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

Cons:

  • file.c grew by ~600 lines and has 8 file_operations tables. Every
    upstream change to fuse_file_operations, fuse_open(), fuse_flush(),
    fuse_file_read_iter() etc. will conflict and need applying two to four
    times. The 6.17 -> 7.0 forward-port already needed hand-merging here.

we have done this with 6.8, 6.12 and 6.17 for a while now
(changing patches and only using the semantics not really git to take the 'patch')

  • Hard-codes "writeback => DLM". Right for redfs; it makes the in-tree
    fs/fuse unusable for other wb servers, which only matters on hosts that
    boot a redfs-ubuntu-* kernel directly (finding 1).

there is no other system using this, since we create our dkms and src.rpms and deliver with every installation of infinia.
We actually renamed the device, so that it works fine with another present /dev/fuse user. (only in DDNStorage/redfs)
That might not be the best of ideas in this context, but it prevents bad things from happening.

  • fuse_do_setattr() keeps a mode-recomputing wrapper for the direct-IO
    rollback caller, so the "decided once" invariant already has an exception.

I have to take a look at this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants