Received-SPF: pass (google.com: domain of linux-ext4+bounces-402-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:36:47 +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 1/5] jbd2: Add errseq to detect client fs's bdev
 writeback error
Message-ID: <20231212143647.pygpilneuonrdedq@quack3>
References: <20231103145250.2995746-1-chengzhihao1@huawei.com>
 <20231103145250.2995746-2-chengzhihao1@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231103145250.2995746-2-chengzhihao1@huawei.com>

On Fri 03-11-23 22:52:46, Zhihao Cheng wrote:
> Add errseq in journal, so that JBD2 can detect whether metadata is
> successfully fallen on fs bdev. This patch adds detection in recovery
               ^^^^^^^^^ written to

> process to replace original solution(using local variable wb_err).
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Suggested-by: Jan Kara <jack@suse.cz>

Otherwise the patch looks good. Feel free to add:

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

								Honza

> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 30dec2bd2ecc..a655d9a88f79 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1535,6 +1535,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>  	journal->j_fs_dev = fs_dev;
>  	journal->j_blk_offset = start;
>  	journal->j_total_len = len;
> +	jbd2_init_fs_dev_write_error(journal);
>  
>  	err = journal_load_superblock(journal);
>  	if (err)
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 01f744cb97a4..1f7664984d6e 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -289,8 +289,6 @@ int jbd2_journal_recover(journal_t *journal)
>  	journal_superblock_t *	sb;
>  
>  	struct recovery_info	info;
> -	errseq_t		wb_err;
> -	struct address_space	*mapping;
>  
>  	memset(&info, 0, sizeof(info));
>  	sb = journal->j_superblock;
> @@ -308,9 +306,6 @@ int jbd2_journal_recover(journal_t *journal)
>  		return 0;
>  	}
>  
> -	wb_err = 0;
> -	mapping = journal->j_fs_dev->bd_inode->i_mapping;
> -	errseq_check_and_advance(&mapping->wb_err, &wb_err);
>  	err = do_one_pass(journal, &info, PASS_SCAN);
>  	if (!err)
>  		err = do_one_pass(journal, &info, PASS_REVOKE);
> @@ -334,7 +329,7 @@ int jbd2_journal_recover(journal_t *journal)
>  	err2 = sync_blockdev(journal->j_fs_dev);
>  	if (!err)
>  		err = err2;
> -	err2 = errseq_check_and_advance(&mapping->wb_err, &wb_err);
> +	err2 = jbd2_check_fs_dev_write_error(journal);
>  	if (!err)
>  		err = err2;
>  	/* Make sure all replayed data is on permanent storage */
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 52772c826c86..15798f88ade4 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -998,6 +998,13 @@ struct journal_s
>  	 */
>  	struct block_device	*j_fs_dev;
>  
> +	/**
> +	 * @j_fs_dev_wb_err:
> +	 *
> +	 * Records the errseq of the client fs's backing block device.
> +	 */
> +	errseq_t		j_fs_dev_wb_err;
> +
>  	/**
>  	 * @j_total_len: Total maximum capacity of the journal region on disk.
>  	 */
> @@ -1695,6 +1702,25 @@ static inline void jbd2_journal_abort_handle(handle_t *handle)
>  	handle->h_aborted = 1;
>  }
>  
> +static inline void jbd2_init_fs_dev_write_error(journal_t *journal)
> +{
> +	struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
> +
> +	/*
> +	 * Save the original wb_err value of client fs's bdev mapping which
> +	 * could be used to detect the client fs's metadata async write error.
> +	 */
> +	errseq_check_and_advance(&mapping->wb_err, &journal->j_fs_dev_wb_err);
> +}
> +
> +static inline int jbd2_check_fs_dev_write_error(journal_t *journal)
> +{
> +	struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
> +
> +	return errseq_check(&mapping->wb_err,
> +			    READ_ONCE(journal->j_fs_dev_wb_err));
> +}
> +
>  #endif /* __KERNEL__   */
>  
>  /* Comparison functions for transaction IDs: perform comparisons using
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR