Re: [PATCH 06/19] buffer: detect metadata write errors with buffer_write_io_error()

Jan Kara <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,dev.linux.lists.gfs2,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <dyhdveo63nn34lbburq5zpy5e3w2g5e7zecmbew6w6bh4vvlrc@kvwwrxpmmvwn>
On Sat 01-08-26 18:00:50, Chao Shi wrote:
> Both places in this file that report a metadata write error to a caller do
> it by testing !buffer_uptodate() after waiting for the write.  That works
> only because the write completion handlers clear BH_Uptodate when the write
> fails, which is what this series is removing: a buffer whose write failed
> still holds the correct data, and saying otherwise makes callers rewrite,
> re-read or WARN over a buffer that was never wrong.
> 
> BH_Write_EIO is the flag that actually means "the last write of this buffer
> failed", and both handlers already set it via mark_buffer_write_io_error().
> Test that instead.
> 
> No behaviour change: today a failed write through bh_end_write() or
> bh_end_async_write() sets BH_Write_EIO and clears BH_Uptodate together, so
> the two tests agree.  They stop agreeing at the end of the series, and this
> one stays right.
> 
> In __sync_dirty_buffer() the flag also refers unambiguously to the write we
> just issued, because __bh_submit() clears it when it resubmits a buffer for
> write.
> 
> Signed-off-by: Chao Shi <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  fs/buffer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 50a63d964815..ac978d9090c2 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -618,7 +618,7 @@ int mmb_sync(struct mapping_metadata_bhs *mmb)
>  		}
>  		spin_unlock(&mmb->lock);
>  		wait_on_buffer(bh);
> -		if (!buffer_uptodate(bh))
> +		if (buffer_write_io_error(bh))
>  			err = -EIO;
>  		brelse(bh);
>  		spin_lock(&mmb->lock);
> @@ -2743,7 +2743,7 @@ int __sync_dirty_buffer(struct buffer_head *bh, blk_opf_t op_flags)
>  
>  		bh_submit(bh, REQ_OP_WRITE | op_flags, bh_end_write);
>  		wait_on_buffer(bh);
> -		if (!buffer_uptodate(bh))
> +		if (buffer_write_io_error(bh))
>  			return -EIO;
>  	} else {
>  		unlock_buffer(bh);
> -- 
> 2.43.0
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.