Skip to content

Redfs ubuntu hwe writeback switch to dio#187

Merged
hbirth merged 7 commits into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1from
hbirth:redfs-ubuntu-hwe-writeback-switch-to-dio
Jul 17, 2026
Merged

Redfs ubuntu hwe writeback switch to dio#187
hbirth merged 7 commits into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1from
hbirth:redfs-ubuntu-hwe-writeback-switch-to-dio

Conversation

@hbirth

@hbirth hbirth commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
@hbirth
hbirth requested review from bsbernd, hazhou-ddn and yongzech July 8, 2026 12:08
@hbirth
hbirth force-pushed the redfs-ubuntu-hwe-writeback-switch-to-dio branch 2 times, most recently from 3b7eec3 to ec8fc13 Compare July 8, 2026 14:22
@hbirth
hbirth requested a review from achhenderson July 9, 2026 07:10
hbirth and others added 3 commits July 9, 2026 15:39
A FUSE_NOTIFY_INVAL_INODE data invalidation means another (remote) entity
is modifying the file.

Rather than react to a single notify, keep a per-inode moving average of
how fast data invalidations arrive for the whole file: an EWMA of the
inter-arrival interval, updated under fi->lock on every notify
(fuse_notify_inval_hot()). The inode is latched only once the average
spacing drops below an internal threshold (FUSE_NOTIFY_DIO_INTERVAL) while
a local writer is open; a lone or occasional notify keeps the average high
and does not trip the switch.  The heuristic has no external knob -- its
parameters (EWMA weight, threshold, seed) are source-level constants.

Introduce the forced-direct-IO latch (FUSE_I_FORCE_DIO):

  - fuse_reverse_inval_inode() folds each data invalidation into the moving
    average and sets the latch when it trips with a local writer present;
  - fuse_file_{read,write}_iter() and fuse_cache_write_iter() route to the
    direct path while latched; fuse_dio_{wr_exclusive_lock,lock,unlock}()
    use the shared parallel-dio path and bypass the cached/uncached
    accounting;
  - fuse_file_io_open() opens new files uncached so they do not re-enter
    caching mode;
  - fuse_prepare_release() clears the latch once the last writer is gone
    and fuse_file_release() drops any clean folios a racing read
    repopulated; fuse_file_mmap() reverts to caching mode (a mapping needs
    the page cache).

Latching to direct IO is only coherent if no buffered write can deposit
dirty folios into the page cache after it has been dropped. Add a
per-inode rw_semaphore, wb_inval_rwsem, to serialise the buffered-write
page-cache dirtying against the latch transition. The writeback path
holds it for read around the dirtying and re-checks the latch under it;
fuse_reverse_inval_inode() holds it for write around its invalidate +
latch set. The notification may be delivered by the same server thread
that still owes a reply to an in-flight write holding the inode lock, so
it takes the rwsem with a trylock and never blocks: if the writer has gone
it skips the latch and only invalidates the notified range. The writer's
read-side section stays free of server round-trips because under fc->dlm
the partial-write RMW read is skipped.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
commit 0c58a97 ("fuse: remove tmp folio for writebacks and internal
rb tree") removed temp folios for dirty page writeback. Consequently,
fuse can now use the default writeback accounting.

With switching fuse to use default writeback accounting, there are some
added benefits. This updates wb->writeback_inodes tracking as well now
and updates writeback throughput estimates after writeback completion.

This commit also removes inc_wb_stat() and dec_wb_stat(). These have no
callers anymore now that fuse does not call them.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Bernd Schubert <bschubert@ddn.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
(cherry picked from commit 494d2f5)
fuse_readahead() batches whole folios into a single request, capped at
min(fc->max_pages, fc->max_read/PAGE_SIZE) pages, but fuse_init_file_inode()
let the page cache build folios up to MAX_PAGECACHE_ORDER. A large
sequential read could thus produce a folio bigger than one request can
carry: the first loop iteration took the folio_pages > cur_pages path,
fired WARN_ON(!pages), and broke with ap->num_folios == 0.
fuse_send_readpages() was still called and dereferenced a NULL
ap->folios[0] via folio_pos(), oopsing at CR2=0x20 (folio->index).

Cap the folio order to the per-request page limit so the page cache can
never build an unserviceable folio.

There is a patch for upstream that does exactly the same.
In later versions this is not needed but for 6.17 we need this.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
@hbirth
hbirth force-pushed the redfs-ubuntu-hwe-writeback-switch-to-dio branch from ec8fc13 to 8f359ed Compare July 9, 2026 13:40
Comment thread fs/fuse/file.c
Comment thread fs/fuse/file.c Outdated
Comment thread fs/fuse/file.c Outdated
Comment thread fs/fuse/file.c
Comment thread fs/fuse/fuse_i.h
* Only regular files initialise it -- it shares storage
* with the readdir-cache union arm.
*/
struct rw_semaphore wb_inval_rwsem;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Horst, this lock can also be used for the serialization between the cached read/write I/Os and invalidation, that is the AOP_TRUNCATED_PAGE we discussed before, so this lock perhaps is not only used for this DIO switch purpose.

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.

If we use this patch ... I don't think that that case will happen again exactly for the reason you mention.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good, however, I did see the synchronization between the invalidation and write, but didn't see where the synchronization between the read and invalidation is? That is it seems the cached read path doesn't hold this lock to synchronize with invalidation?

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.

That is true. Maybe I have to think about that ...

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.

I think we should do that in another PR ... this is already too large as it is

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So just to clarify that: the serialization missing part between the invalidation and read will be done in another PR? Also for the invalidation and write serialization, I think still there's a window: once the write gets permission to run, one invalidation could be just done and cleared the DLM lock, so the write path should check whether the DLM cache is still there before moving forward.

@hbirth hbirth Jul 17, 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.

to your first question ... yes. But due to a technical problem. If we make such heavy use of this synchronization for READ and WRITE I changed the data type (percpu_rw_semaphore) for one that is supposedly be faster in the light path. That leads to a way bigger patch.

bsbernd and others added 3 commits July 16, 2026 13:26
Extending FOPEN_PARALLEL_DIRECT_WRITES writes were forced onto the
exclusive inode lock, re-serializing the parallel phase. The exclusive
lock only bundled "write + advance i_size + undo-on-failure" into one
unit. But i_size is committed by fuse_write_update_attr() under
fi->lock, only on a successful growing write and independent of the
inode rwsem -- so shared-lock writers commit size correctly and have
nothing to undo. Drop the past-EOF exclusive triggers and gate the
whole-file fuse_do_truncate() rollback on holding the exclusive lock.

Lock mode is passed to __fuse_direct_IO(); i_size is committed at the
same point in every path, only the failure rollback differs:

  non-exclusive (relaxed, parallel):
    fuse_direct_write_iter
      fuse_dio_lock -> inode_lock_shared       (exclusive=false)
      __fuse_direct_IO(.., false)
        fuse_direct_io()          write to server
        fuse_write_update_attr()  commit i_size (on success)
        no rollback

  exclusive (append / caching / !parallel):
    fuse_direct_write_iter
      fuse_dio_lock -> inode_lock              (exclusive=true)
      __fuse_direct_IO(.., true)
        fuse_direct_io()          write to server
        fuse_write_update_attr()  commit i_size (on success)
        ret<0 & extend -> fuse_do_truncate()  rollback

  exclusive (caching-mode O_DIRECT):
    fuse_cache_write_iter -> inode_lock (exclusive)
      generic_file_direct_write -> fuse_direct_IO
        __fuse_direct_IO(.., true)
          fuse_direct_io()          write to server
          fuse_write_update_attr()  commit i_size (on success)
          ret<0 & extend -> fuse_do_truncate()  rollback

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
fuse_dio_lock() takes an uncached_io reference (via
fuse_inode_uncached_io_start()) only when FUSE_I_FORCE_DIO is clear,
while fuse_dio_unlock() decided whether to drop it by re-reading
FUSE_I_FORCE_DIO. On this tree the latch is toggled asynchronously by
the inode-invalidation notify-storm path, so the bit can differ between
the lock and the unlock of a single direct write:

 - clear at lock (reference taken), set before unlock: the reference is
   never dropped, leaving fi->iocachectr permanently negative and
   hanging the next caching-mode open;

 - set at lock (no reference), cleared before unlock:
   fuse_inode_uncached_io_end() is called without a matching start,
   tripping WARN_ON(fi->iocachectr >= 0) and corrupting the counter.

Record in fuse_dio_lock() whether a reference was actually taken and
have fuse_dio_unlock() drop it based on that captured decision instead
of re-testing the racy bit, so the accounting stays balanced regardless
of any FORCE_DIO transition mid-write.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
@hbirth
hbirth force-pushed the redfs-ubuntu-hwe-writeback-switch-to-dio branch from c02ef5c to 1c52882 Compare July 16, 2026 12:11
@hbirth

hbirth commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

@hazhou-ddn @achhenderson so do you guys think we can merge this?

@hazhou-ddn hazhou-ddn left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@hbirth
hbirth merged commit 9d6feaa into DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1 Jul 17, 2026
2 checks passed
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.

5 participants