Received-SPF: pass (google.com: domain of linux-ext4+bounces-405-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 12 Dec 2023 15:38:05 +0100
From: Jan Kara <jack@suse.cz>
To: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: tytso@mit.edu, jack@suse.com, linux-ext4@vger.kernel.org,
	yi.zhang@huawei.com
Subject: Re: [PATCH 4/5] jbd2: Abort journal when detecting metadata
 writeback error of fs dev
Message-ID: <20231212143805.annty5auy6q2uf3d@quack3>
References: <20231103145250.2995746-1-chengzhihao1@huawei.com>
 <20231103145250.2995746-5-chengzhihao1@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231103145250.2995746-5-chengzhihao1@huawei.com>

On Fri 03-11-23 22:52:49, Zhihao Cheng wrote:
> This is a replacement solution of commit bc71726c725767 ("ext4: abort
> the filesystem if failed to async write metadata buffer"), JBD2 can
> detects metadata writeback error of fs dev by 'j_fs_dev_wb_err'.
  ^^^ detect

> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>

Otherwise looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/transaction.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 5f08b5fd105a..cb0b8d6fc0c6 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1231,11 +1231,25 @@ static bool jbd2_write_access_granted(handle_t *handle, struct buffer_head *bh,
>  int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
>  {
>  	struct journal_head *jh;
> +	journal_t *journal;
>  	int rc;
>  
>  	if (is_handle_aborted(handle))
>  		return -EROFS;
>  
> +	journal = handle->h_transaction->t_journal;
> +	if (jbd2_check_fs_dev_write_error(journal)) {
> +		/*
> +		 * If the fs dev has writeback errors, it may have failed
> +		 * to async write out metadata buffers in the background.
> +		 * In this case, we could read old data from disk and write
> +		 * it out again, which may lead to on-disk filesystem
> +		 * inconsistency. Aborting journal can avoid it happen.
> +		 */
> +		jbd2_journal_abort(journal, -EIO);
> +		return -EIO;
> +	}
> +
>  	if (jbd2_write_access_granted(handle, bh, false))
>  		return 0;
>  
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR