mbox series

[0/2] btrfs: error handling fixes for extent_writepage()

Message ID cover.1732596971.git.wqu@suse.com (mailing list archive)
Headers show
Series btrfs: error handling fixes for extent_writepage() | expand

Message

Qu Wenruo Nov. 26, 2024, 6:29 a.m. UTC
Now with the new compressed write support and the incoming partial
uptodate folio and inline write support, it's easier and easier to hit
crashes for test cases likes generic/750, when the sector size is
smaller than page size.

The main symptom is ordered extent double accounting, causing various
problems mostly kernel warning and crashes (for debug builds).

The direct cause the failure from writepage_delalloc() with -ENOSPC,
which is another rabbit hole, but here we need to focus on the error
handling.

All the call traces points to the btrfs_mark_ordered_io_finished()
inside extent_writepage() for error handling.

It turns out that that unconditional, full folio cleanup is no doubt the
root cause, and there are two error paths leading to the situation:

- btrfs_run_delalloc_range() failure
  Which can lead some delalloc ranges are submitted asynchronously
  (compression mostly), and we try to clean up those which we shouldn't.

  This is the most common one, as I'm hitting quite some -ENOSPC during
  my fstests runs, and all the hang/crashes are following such errors.

- submit_one_sector() failure
  This is much rare, as I haven't really hit one.
  But the idea is pretty much the same, so we should not touch ranges we
  shouldn't.

Both fixes are similar, by moving the btrfs_mark_ordered_io_finished()
calls for error handling into each function, so that we can avoid
touching async extents.

Although these fixes are mostly for backports, the proper fix in the end
would be reworking how we handle dirty folio writeback.

The current way is map-map-map, then submit-submit-submit (run delalloc
for every dirty sector of the folio, then submit all dirty sectors).

The planned new fix would be more like iomap to go
map-submit-map-submit-map-submit (run one delalloc, then immeidately submit
it).

Qu Wenruo (2):
  btrfs: handle btrfs_run_delalloc_range() errors correctly
  btrfs: handle submit_one_sector() error inside extent_writepage_io()

 fs/btrfs/extent_io.c | 71 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 18 deletions(-)