[PATCH v2 0/4] iomap: fix error handling regressions

Andrea Parri posted 4 patches 2 hours ago
fs/iomap/direct-io.c         | 19 +++++++------------
fs/iomap/fiemap.c            |  8 ++++----
fs/iomap/ioend.c             | 10 +++++++++-
fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
fs/xfs/xfs_reflink.c         |  5 +++++
5 files changed, 29 insertions(+), 19 deletions(-)
[PATCH v2 0/4] iomap: fix error handling regressions
Posted by Andrea Parri 2 hours ago
Adding Carlos Maiolino to the To: list since patch 2 (new in v2) touches
fs/xfs/ outside the iomap directory.

Fix three independent error handling regressions in iomap:

 - clear the writeback context after a failed ->writeback_submit() call,
   preventing an already completed ioend from being submitted again;
 - preserve an iomap iteration error when iomap_fiemap() has a pending
   extent to emit; and
 - preserve a direct I/O data path error when sub-block tail zeroing
   succeeds.

The three fixes have no ordering dependencies and can be applied or
backported independently.  Each issue was reproduced before and after
its respective fix.

Patch 2 is not a fix and carries no stable tag: it adds an XFS error
tag (requested by Christoph) that lets patch 1's ->writeback_submit()
failure be injected without the boot-param hack used for the original
reproduction.  I used it to write an xfstest reproducing the bug (list
corruption caught via CONFIG_DEBUG_LIST on an unfixed kernel, clean
writeback failure on a fixed one); I'll send that separately to
fstests@vger.kernel.org once this series is out for review.

Changes since v1:
 - Patch 1: reworded the added comment to fit 80 columns (Christoph);
   answered Christoph's request for a reproducer in the commit message
   (fault-injected ->writeback_submit() failure on a reflinked XFS file,
   reliably hits list_add double add on the unfixed kernel); confirmed
   with Christoph that the final ->writeback_submit() call in
   iomap_writepages() needs no equivalent fix, since wpc is never reused
   after it; picked up the version-scoped stable tag Darrick suggested;
   picked up Brian's Reviewed-by.
 - Patch 2: new, added per Christoph's request.
 - Patch 3 (was 2/3): no code change; picked up the version-scoped
   stable tag Darrick suggested; picked up Reviewed-by tags from Brian,
   Darrick, and Christoph.
 - Patch 4 (was 3/3): reworked per Christoph's suggestion to drop the
   zerror/ret handling entirely instead of just fixing its type, since
   iomap_dio_zero() can only fail via a can't-happen WARN_ON_ONCE()
   assert; this also resolves Darrick's question about the local
   variable's type, since there's no longer a local variable to type.

Andrea Parri (4):
  iomap: don't resubmit an ioend after ->writeback_submit() failed
  xfs: add an error tag to inject a ->writeback_submit() failure
  iomap: don't lose a fiemap iteration error when emitting the last
    extent
  iomap: don't lose a failed direct I/O bio's error when zeroing the
    tail

 fs/iomap/direct-io.c         | 19 +++++++------------
 fs/iomap/fiemap.c            |  8 ++++----
 fs/iomap/ioend.c             | 10 +++++++++-
 fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
 fs/xfs/xfs_reflink.c         |  5 +++++
 5 files changed, 29 insertions(+), 19 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.53.0