Re: [PATCH v2 18/21] jbd2: say what jbd2_freeze_jh_data()'s assertion is actually checking

[Jan Kara]

On Thu 06-08-26 12:58:41, Chao Shi wrote:
> The assertion that the buffer about to be copied out is up to date is
> correct and stays, but its message - "Possible IO failure" - describes what
> a buffer that is not up to date used to mean rather than what is being
> checked.
> 
> Once this series stops clearing BH_Uptodate on write error, that reading is
> wrong twice over.  A failed write no longer makes a buffer not up to date,
> and a buffer that does carry BH_Write_EIO is fine here: it still holds
> valid data and the journal will write it again.  What the assertion is
> really guarding is that there is something valid to copy at all.
> 
> Say that instead.
> 
> Suggested-by: Jan Kara <jack@xxxxxxx>
> 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/jbd2/transaction.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 5cc7d097b2ac..85d84d909f78 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_uptodate(bh), "Buffer not uptodate!\n");
>  	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 <jack@xxxxxxxx>
SUSE Labs, CR