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

On 8/5/2026 5:00 PM, Jan Kara wrote:
> On Wed 05-08-26 12:52:11, Zhang Yi wrote:
>> 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 <[email protected]>
>>
>> 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.
> 
> Deninitely :) There are several hacks in ext4 (and other filesystems as
> well) that could be removed after this patch set. But let's settle on the
> generic changes first.
> 
> 								Honza

Yeah, that would be great!

Best Regards,
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);
>>