gfs2 filesystem and dlm development
 help / color / mirror / Atom feed
* [PATCH 00/19] buffer: stop clearing BH_Uptodate when a write fails
@ 2026-08-01 22:00 Chao Shi
  2026-08-01 22:00 ` [PATCH 01/19] buffer_head: Remove b_page Chao Shi
                   ` (18 more replies)
  0 siblings, 19 replies; 20+ messages in thread
From: Chao Shi @ 2026-08-01 22:00 UTC (permalink / raw)
  To: Jan Kara, Christian Brauner, Alexander Viro, Matthew Wilcox,
	linux-fsdevel
  Cc: Theodore Ts'o, Andreas Dilger, Baokun Li, Ojaswin Mujoo,
	Ritesh Harjani, Zhang Yi, Bob Copeland, Namjae Jeon, Sungjong Seo,
	Yuezhang Mo, OGAWA Hirofumi, Mark Fasheh, Joel Becker, Joseph Qi,
	Andreas Gruenbacher, linux-ext4, ocfs2-devel, gfs2,
	linux-karma-devel, linux-kernel, Chao Shi

When a metadata write fails, the buffer_head completion handlers clear
BH_Uptodate.  The buffer still holds exactly the data the filesystem asked
to be written - it is the disk that is stale, not the buffer - and saying
otherwise has consequences:

  - mark_buffer_dirty() has a WARN_ON_ONCE(!buffer_uptodate(bh)), so a
    filesystem that dirties the buffer again to retry the write trips it.
    That is the warning that started this.

  - a buffer that is not up to date gets re-read from disk, which silently
    replaces the data the filesystem was trying to write with the stale
    on-disk copy.

  - the state is not self consistent while it lasts: the window between the
    write completing and BH_Uptodate being cleared is visible to anyone
    holding the folio lock.

BH_Write_EIO already records that the last write failed.  This series moves
every consumer over to it, then removes the clears.

Patches 1-3 are groundwork.  Matthew's patch 1 removes b_page.  Patches 2
and 3 let a buffer_head point at memory outside the page cache and use that
for jbd2's shadow buffers, which today sit on a slab folio - a slab folio
overloads ->mapping, so mark_buffer_write_io_error() cannot be called on
them at all.  That is what blocked the jbd2 conversion.

Patches 4-5 make BH_Write_EIO safe to leave set: clear it in bforget() and
discard it on invalidate, so a freed or reused block does not inherit
somebody else's write error.

Patches 6-18 convert the consumers, one filesystem at a time: the two core
helpers in fs/buffer.c, then adfs, ext2, omfs, exfat, fat, ext4, ocfs2,
gfs2 and jbd2.  Patch 19 removes the clears.

Every patch builds on its own and the tree behaves identically at each step
until patch 19, because a failed write currently sets BH_Write_EIO and
clears BH_Uptodate together.

Four things are worth a second look, and I would rather point at them than
let you find them:

  - gfs2 (patch 15) is not a pure conversion.  gfs2_end_log_write_bh()
    already marks BH_Write_EIO without clearing BH_Uptodate, so the two ail
    checks are blind to log write errors today and start catching them.
    Jan and I agreed that is the desired fix rather than a regression, but
    it is a real behaviour change for gfs2.

  - ocfs2 (patch 14) has a check that turns the filesystem read-only when a
    buffer whose previous write failed is handed back to a transaction.  It
    sits inside an if (!buffer_uptodate(bh)) that goes dead, so it has to be
    hoisted or ocfs2 silently stops making that check.  The code in that
    patch is Jan's.

  - after patch 19, BH_Write_EIO stays set until the buffer is written
    again, forgotten or invalidated.  So a site like ext4's itable sync
    (patch 12) now reports on every subsequent sync rather than only on the
    write that failed.  That is intended, and matches what ocfs2 has always
    done with this flag.

  - the read path changes too.  __bread_gfp() and bh_uptodate_or_lock()
    currently re-read a buffer whose write failed, replacing the
    filesystem's data with the stale on-disk copy.  After patch 19 they
    return the in-memory data.  Better, but different.

On the original report: patch 19 provably closes that WARN for the write
error case, since the state it warns about can no longer be produced that
way.  I could not reproduce the WARN itself with fail_make_request, which
fails synchronously at submit; the original came from a fuzzer injecting
delayed error completions, which is what opens the window.  So there is no
ready reproducer to offer, only the argument.

Based on vfs.git vfs.all, because Jan's "fs: Fix missed inode write during
fsync" series is in it and rewrites __ext4_handle_dirty_metadata(), which
patch 12 touches.

Testing: built and booted with all of the above filesystems enabled;
checkpatch --strict clean on all 19; each patch built individually.
Patches 2 and 3 were validated by crashing with sysrq-b during ext4
data=journal,journal_checksum traffic engineered to force copy-out into
b_frozen_data, then replaying the journal on the next mount - recovery
completed, file contents matched, e2fsck -fn clean, and an instrumented
build confirmed the folio-less path was actually taken.  I have not
exercised the adfs, omfs or exfat paths beyond compiling them.

The conversion was asked for here:
  https://lore.kernel.org/linux-fsdevel/xaqwfkwjnp7h2lg7ir6wy2tnyay26t3im3ftkdl64rhys3rhmu@lc5vfh5tc2f5/
and the shape of it was settled in the two exchanges after it:
  https://lore.kernel.org/linux-fsdevel/CACd_6n17-pJuNoCLsstP7goocedrr5F8T8BwTjRUNx8frKEPgA@mail.gmail.com/
  https://lore.kernel.org/linux-fsdevel/bnchcfog427eo3bxt5h6qautlq7sb2o4ww7cjgad73biycdn5f@np7qkbyqizup/
Per-filesystem maintainers were not on that thread, hence the summary above.

The earlier attempt at this, which Jan correctly rejected, was
  https://lore.kernel.org/linux-fsdevel/20260430053645.1466196-1-coshi036@gmail.com/


Chao Shi (18):
  buffer: allow a buffer_head to point at memory outside the page cache
  jbd2: point the shadow buffer at the frozen data directly
  buffer: clear BH_Write_EIO when a buffer is forgotten
  buffer: discard BH_Write_EIO along with the rest of the buffer state
  buffer: detect metadata write errors with buffer_write_io_error()
  adfs: check for a directory write error with buffer_write_io_error()
  ext2: check for an xattr block write error with
    buffer_write_io_error()
  omfs: check for an inode write error with buffer_write_io_error()
  exfat: check for a directory write error with buffer_write_io_error()
  fat: check for a metadata write error with buffer_write_io_error()
  ext4: check for a metadata write error with buffer_write_io_error()
  ocfs2: check for a metadata write error with buffer_write_io_error()
  ocfs2: check for a stale write error before reusing a metadata buffer
  gfs2: check for a metadata write error with buffer_write_io_error()
  jbd2: report journal write errors with BH_Write_EIO
  jbd2: assert on a failed write, not on a buffer that is not up to date
  ext4, jbd2: report fast commit write errors with BH_Write_EIO
  buffer: stop clearing BH_Uptodate when a write fails

Matthew Wilcox (Oracle) (1):
  buffer_head: Remove b_page

 fs/adfs/dir.c               |  2 +-
 fs/buffer.c                 | 26 +++++++++++++++++---------
 fs/exfat/misc.c             |  2 +-
 fs/ext2/xattr.c             |  2 +-
 fs/ext4/ext4_jbd2.c         |  2 +-
 fs/ext4/fast_commit.c       |  2 +-
 fs/ext4/mmp.c               |  2 +-
 fs/fat/misc.c               |  2 +-
 fs/gfs2/log.c               |  4 ++--
 fs/gfs2/lops.c              |  2 +-
 fs/jbd2/commit.c            | 20 +++++++++++++-------
 fs/jbd2/journal.c           | 21 +++++++++++++++------
 fs/jbd2/transaction.c       |  2 +-
 fs/ocfs2/buffer_head_io.c   | 12 +++++++-----
 fs/ocfs2/journal.c          | 25 +++++++++++++------------
 fs/omfs/inode.c             |  4 ++--
 include/linux/buffer_head.h |  7 ++-----
 17 files changed, 80 insertions(+), 57 deletions(-)


base-commit: 05c09c9c8a79d5539fef30d42e732eba90a15dcf
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-08-01 22:01 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 22:00 [PATCH 00/19] buffer: stop clearing BH_Uptodate when a write fails Chao Shi
2026-08-01 22:00 ` [PATCH 01/19] buffer_head: Remove b_page Chao Shi
2026-08-01 22:00 ` [PATCH 02/19] buffer: allow a buffer_head to point at memory outside the page cache Chao Shi
2026-08-01 22:00 ` [PATCH 03/19] jbd2: point the shadow buffer at the frozen data directly Chao Shi
2026-08-01 22:00 ` [PATCH 04/19] buffer: clear BH_Write_EIO when a buffer is forgotten Chao Shi
2026-08-01 22:00 ` [PATCH 05/19] buffer: discard BH_Write_EIO along with the rest of the buffer state Chao Shi
2026-08-01 22:00 ` [PATCH 06/19] buffer: detect metadata write errors with buffer_write_io_error() Chao Shi
2026-08-01 22:00 ` [PATCH 07/19] adfs: check for a directory write error " Chao Shi
2026-08-01 22:00 ` [PATCH 08/19] ext2: check for an xattr block " Chao Shi
2026-08-01 22:00 ` [PATCH 09/19] omfs: check for an inode " Chao Shi
2026-08-01 22:00 ` [PATCH 10/19] exfat: check for a directory " Chao Shi
2026-08-01 22:00 ` [PATCH 11/19] fat: check for a metadata " Chao Shi
2026-08-01 22:00 ` [PATCH 12/19] ext4: " Chao Shi
2026-08-01 22:00 ` [PATCH 13/19] ocfs2: " Chao Shi
2026-08-01 22:00 ` [PATCH 14/19] ocfs2: check for a stale write error before reusing a metadata buffer Chao Shi
2026-08-01 22:00 ` [PATCH 15/19] gfs2: check for a metadata write error with buffer_write_io_error() Chao Shi
2026-08-01 22:01 ` [PATCH 16/19] jbd2: report journal write errors with BH_Write_EIO Chao Shi
2026-08-01 22:01 ` [PATCH 17/19] jbd2: assert on a failed write, not on a buffer that is not up to date Chao Shi
2026-08-01 22:01 ` [PATCH 18/19] ext4, jbd2: report fast commit write errors with BH_Write_EIO Chao Shi
2026-08-01 22:01 ` [PATCH 19/19] buffer: stop clearing BH_Uptodate when a write fails Chao Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox