linux-ext4 - Re: [PATCH 5/5] jbd2: remove useless 'block

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: <1c52c8c2-43a0-43e1-8fca-18ac21581391@huaweicloud.com>
Date: Wed, 18 Sep 2024 21:19:51 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Ye Bin <yebin@...weicloud.com>, tytso@....edu, adilger.kernel@...ger.ca,
 linux-ext4@...r.kernel.org
Cc: jack@...e.cz, zhangxiaoxu5@...wei.com
Subject: Re: [PATCH 5/5] jbd2: remove useless 'block_error' variable

On 2024/9/18 19:36, Ye Bin wrote:
> From: Ye Bin <yebin10@...wei.com>
> 
> The judgement 'if (block_error && success == 0)' is never valid. Just
> remove useless 'block_error' variable.
> 
> Signed-off-by: Ye Bin <yebin10@...wei.com>
> ---
>  fs/jbd2/recovery.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 05ea449b95c4..0bcbb58d634b 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -490,7 +490,7 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  					  struct buffer_head *bh,
>  					  unsigned long *next_log_block,
>  					  unsigned int next_commit_ID,
> -					  int *success, int *block_error)
> +					  int *success)

I wonder an unrelated question, why do we need this 'success' variable? Now we
keep on replaying later log blocks even if some bad things happened(read I/O
error or checksum error) instead of just quit, is it to replay as many log
blocks as possible, and then to reduce the data lose and also reduce e2fsck's
workload?

Thanks,
Yi.

>  {
>  	char *tagp;
>  	int flags;
> @@ -542,7 +542,6 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  				*success = -EFSBADCRC;
>  				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
>  				      blocknr, io_block);
> -				*block_error = 1;
>  				goto skip_write;
>  			}
>  
> @@ -596,7 +595,6 @@ static int do_one_pass(journal_t *journal,
>  	unsigned int		sequence;
>  	int			blocktype;
>  	__u32			crc32_sum = ~0; /* Transactional Checksums */
> -	int			block_error = 0;
>  	bool			need_check_commit_time = false;
>  	__u64			last_trans_commit_time = 0, commit_time;
>  
> @@ -721,8 +719,7 @@ static int do_one_pass(journal_t *journal,
>  			 * done here!
>  			 */
>  			err = jbd2_do_replay(journal, info, bh, &next_log_block,
> -					     next_commit_ID, &success,
> -					     &block_error);
> +					     next_commit_ID, &success);
>  			if (err)
>  				goto failed;
>  
> @@ -913,8 +910,6 @@ static int do_one_pass(journal_t *journal,
>  			success = err;
>  	}
>  
> -	if (block_error && success == 0)
> -		success = -EIO;
>  	return success;
>  
>   failed: