Re: [PATCH 17/19] jbd2: assert on a failed write, not on a buffer that is not up to date

On Sat 01-08-26 18:01:01, Chao Shi wrote:
> jbd2_freeze_jh_data() asserts that the buffer it is about to copy out is up
> to date.  Once this series stops clearing BH_Uptodate on write error, that
> assertion stops firing for the case it was written for, because a buffer
> whose write failed stays up to date - which is the point: the in-memory
> copy being frozen is still the data the filesystem wants written.
> 
> Assert on the condition that still means something went wrong.  The message
> loses its "Possible", because BH_Write_EIO is not a guess.
> 
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Chao Shi <[email protected]>
...
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 5cc7d097b2ac..fdabe34a7e22 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -920,7 +920,7 @@ static void jbd2_freeze_jh_data(struct journal_head *jh)
>  	char *source;
>  	struct buffer_head *bh = jh2bh(jh);
>  
> -	J_EXPECT_JH(jh, buffer_uptodate(bh), "Possible IO failure.\n");
> +	J_EXPECT_JH(jh, !buffer_write_io_error(bh), "IO failure.\n");

After some more thought I think getting here a buffer_write_io_error()
buffer is actually OK and we'll rewrite it. Instead this assert should just
have a different message:

	J_EXPECT_JH(jh, buffer_uptodate(bh), "Buffer not uptodate!\n");

to verify the buffer has valid data.

								Honza


>  	source = kmap_local_folio(bh->b_folio, bh_offset(bh));
>  	/* Fire data frozen trigger just before we copy the data */
>  	jbd2_buffer_frozen_trigger(jh, source, jh->b_triggers);
> -- 
> 2.43.0
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR