Re: [PATCH v2 19/21] ext4, jbd2: report fast commit write errors with BH_Write_EIO

[Jan Kara]

On Thu 06-08-26 12:58:42, Chao Shi wrote:
> ext4_end_buffer_io_sync() is the third completion handler in this
> series that reports a failed write by clearing BH_Uptodate, and
> jbd2_fc_wait_bufs() is the only thing that looks at the result.
> Convert both.
> 
> Like the jbd2 handler, this one stops touching BH_Uptodate at all.
> ext4_fc_submit_bh() marks the buffer up to date before submitting, so
> setting it again on completion said nothing.  The local flag and the debug
> messages are reworded to describe the write rather than the buffer's
> contents, which the write does not change.
> 
> They have to move in the same patch.  The handler lives in ext4 and
> the wait in jbd2, but neither is used by anything else: the buffers
> are ext4's fast commit blocks, submitted by ext4_fc_submit_bh() and
> waited for by jbd2_fc_wait_bufs().  Converting one without the other
> silently disables fast commit write error reporting.
> 
> Acked-by: Weidong Zhu <weizhu@xxxxxxx>
> Signed-off-by: Chao Shi <coshi036@xxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ext4/fast_commit.c | 11 +++++------
>  fs/jbd2/journal.c     |  2 +-
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 8e2259799614..a2028fbd4540 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -203,17 +203,16 @@ static inline void ext4_fc_set_snap_err(int *snap_err, int err)
>  static void ext4_end_buffer_io_sync(struct bio *bio)
>  {
>  	struct buffer_head *bh;
> -	bool uptodate = bio_endio_bh(bio, &bh);
> +	bool success = bio_endio_bh(bio, &bh);
>  
>  	BUFFER_TRACE(bh, "");
> -	if (uptodate) {
> -		ext4_debug("%s: Block %lld up-to-date",
> +	if (success) {
> +		ext4_debug("%s: Block %lld written",
>  			   __func__, bh->b_blocknr);
> -		set_buffer_uptodate(bh);
>  	} else {
> -		ext4_debug("%s: Block %lld not up-to-date",
> +		ext4_debug("%s: Block %lld write failed",
>  			   __func__, bh->b_blocknr);
> -		clear_buffer_uptodate(bh);
> +		mark_buffer_write_io_error(bh);
>  	}
>  
>  	unlock_buffer(bh);
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 6e05dc47e20a..72e8ccbf7de4 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -886,7 +886,7 @@ int jbd2_fc_wait_bufs(journal_t *journal, int num_blks)
>  		 * Update j_fc_off so jbd2_fc_release_bufs can release remain
>  		 * buffer head.
>  		 */
> -		if (unlikely(!buffer_uptodate(bh))) {
> +		if (unlikely(buffer_write_io_error(bh))) {
>  			journal->j_fc_off = i + 1;
>  			return -EIO;
>  		}
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR