Re: [PATCH 19/19] buffer: stop clearing BH_Uptodate when a write fails

[Zhang Yi]

On 8/2/2026 6:01 AM, Chao Shi wrote:
> A buffer whose write failed still holds exactly the data the filesystem
> asked to be written.  It is the disk that is out of date, not the buffer.
> Clearing BH_Uptodate says the opposite, and callers act on it:
> 
>  - mark_buffer_dirty() has a WARN_ON_ONCE(!buffer_uptodate(bh)).  A
>    filesystem that dirties the buffer again after a failed write - which is
>    the normal way to retry - trips it.  That is the warning this series
>    started from.
> 
>  - a buffer that is not up to date gets re-read from disk, which replaces
>    the data the filesystem was trying to write with the stale on-disk copy,
>    silently.
> 
>  - the window between the write completing and the buffer being marked not
>    up to date is visible to anyone holding the folio lock, so the state is
>    not even self consistent while it lasts.
> 
> BH_Write_EIO already records the failure, and by now every place in the
> tree that needs to know about it tests that flag instead: the two core
> helpers in this file, adfs, exfat, ext2, ext4, fat, gfs2, jbd2, ocfs2 and
> omfs, converted one filesystem at a time in the preceding patches.  The
> private completion handlers in jbd2 and ext4 fast commit were converted
> along with their waiters.  Nothing is left that reads BH_Uptodate to find
> out whether a write failed, so the clears can go.
> 
> Found by FuzzNvme.
> 
> Signed-off-by: Chao Shi <coshi036@xxxxxxxxx>

It looks like after this series, we may need to revisit the necessity of
ext4_buffer_uptodate() in the ext4 filesystem. IIRC, it was originally
introduced as a compromise to address this issue. Now that we no longer
clear the buffer uptodate flag after a failed IO write, this flag will
still remain valid when getting/reading bh. Therefore, the necessity of
ext4_buffer_uptodate() is worth reconsidering.

Thanks,
Yi.

> ---
>  fs/buffer.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index ac978d9090c2..0002f0736398 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -207,7 +207,6 @@ void bh_end_write(struct bio *bio)
>  	} else {
>  		buffer_io_error(bh, ", lost sync page write");
>  		mark_buffer_write_io_error(bh);
> -		clear_buffer_uptodate(bh);
>  	}
>  	unlock_buffer(bh);
>  }
> @@ -441,7 +440,6 @@ void bh_end_async_write(struct bio *bio)
>  	} else {
>  		buffer_io_error(bh, ", lost async page write");
>  		mark_buffer_write_io_error(bh);
> -		clear_buffer_uptodate(bh);
>  	}
>  
>  	first = folio_buffers(folio);