linux-ext4 - Re: [PATCH 3/5] jbd2: refactor JBD2_COMMIT_BLOCK process in do_one

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240926130126.okrdm37lef33xg2a@quack3>
Date: Thu, 26 Sep 2024 15:01:26 +0200
From: Jan Kara <jack@...e.cz>
To: Ye Bin <yebin@...weicloud.com>
Cc: tytso@....edu, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
	jack@...e.cz, zhangxiaoxu5@...wei.com
Subject: Re: [PATCH 3/5] jbd2: refactor JBD2_COMMIT_BLOCK process in
 do_one_pass()

On Wed 18-09-24 19:36:02, Ye Bin wrote:
> From: Ye Bin <yebin10@...wei.com>
> 
> To make JBD2_COMMIT_BLOCK process more clean, no functional change.
> 
> Signed-off-by: Ye Bin <yebin10@...wei.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/jbd2/recovery.c | 55 ++++++++++++++++++++++++----------------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 0adf0cb31a03..0d697979d83e 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -728,6 +728,11 @@ static int do_one_pass(journal_t *journal,
>  			continue;
>  
>  		case JBD2_COMMIT_BLOCK:
> +			if (pass != PASS_SCAN) {
> +				next_commit_ID++;
> +				continue;
> +			}
> +
>  			/*     How to differentiate between interrupted commit
>  			 *               and journal corruption ?
>  			 *
> @@ -790,8 +795,7 @@ static int do_one_pass(journal_t *journal,
>  			 * much to do other than move on to the next sequence
>  			 * number.
>  			 */
> -			if (pass == PASS_SCAN &&
> -			    jbd2_has_feature_checksum(journal)) {
> +			if (jbd2_has_feature_checksum(journal)) {
>  				struct commit_header *cbh =
>  					(struct commit_header *)bh->b_data;
>  				unsigned found_chksum =
> @@ -815,34 +819,33 @@ static int do_one_pass(journal_t *journal,
>  					goto chksum_error;
>  
>  				crc32_sum = ~0;
> +				goto chksum_ok;
>  			}
> -			if (pass == PASS_SCAN &&
> -			    !jbd2_commit_block_csum_verify(journal,
> -							   bh->b_data)) {
> -				if (jbd2_commit_block_csum_verify_partial(
> -								  journal,
> -								  bh->b_data)) {
> -					pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
> -						  next_commit_ID, next_log_block);
> -					goto chksum_ok;
> -				}
> -			chksum_error:
> -				if (commit_time < last_trans_commit_time)
> -					goto ignore_crc_mismatch;
> -				info->end_transaction = next_commit_ID;
> -				info->head_block = head_block;
>  
> -				if (!jbd2_has_feature_async_commit(journal)) {
> -					journal->j_failed_commit =
> -						next_commit_ID;
> -					break;
> -				}
> +			if (jbd2_commit_block_csum_verify(journal, bh->b_data))
> +				goto chksum_ok;
> +
> +			if (jbd2_commit_block_csum_verify_partial(journal,
> +								  bh->b_data)) {
> +				pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
> +					  next_commit_ID, next_log_block);
> +				goto chksum_ok;
>  			}
> -			if (pass == PASS_SCAN) {
> -			chksum_ok:
> -				last_trans_commit_time = commit_time;
> -				head_block = next_log_block;
> +
> +chksum_error:
> +			if (commit_time < last_trans_commit_time)
> +				goto ignore_crc_mismatch;
> +			info->end_transaction = next_commit_ID;
> +			info->head_block = head_block;
> +
> +			if (!jbd2_has_feature_async_commit(journal)) {
> +				journal->j_failed_commit = next_commit_ID;
> +				break;
>  			}
> +
> +chksum_ok:
> +			last_trans_commit_time = commit_time;
> +			head_block = next_log_block;
>  			next_commit_ID++;
>  			continue;
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR