linux-kernel - Re: [PATCH 1/3] f2fs: clean up __update_extent_tree

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: <Y9lkeVCMRA9goN2E@google.com>
Date:   Tue, 31 Jan 2023 10:56:57 -0800
From:   Jaegeuk Kim <jaegeuk@...nel.org>
To:     Chao Yu <chao@...nel.org>
Cc:     linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] f2fs: clean up __update_extent_tree_range()

On 01/31, Chao Yu wrote:
> No logic change, just avoid goto statement.

I wanted to avoid a deep if/else statement.

> 
> Signed-off-by: Chao Yu <chao@...nel.org>
> ---
>  fs/f2fs/extent_cache.c | 66 ++++++++++++++++++++----------------------
>  1 file changed, 31 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index d70ad6a44cbf..cf65a188d112 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -782,47 +782,43 @@ static void __update_extent_tree_range(struct inode *inode,
>  		en = next_en;
>  	}
>  
> -	if (type == EX_BLOCK_AGE)
> -		goto update_age_extent_cache;
> -
> -	/* 3. update extent in read extent cache */
> -	BUG_ON(type != EX_READ);
> -
> -	if (tei->blk) {
> -		__set_extent_info(&ei, fofs, len, tei->blk, false,
> -				  0, 0, EX_READ);
> -		if (!__try_merge_extent_node(sbi, et, &ei, prev_en, next_en))
> -			__insert_extent_tree(sbi, et, &ei,
> +	/* 3. update extent in extent cache */
> +	if (type == EX_READ) {
> +		if (tei->blk) {
> +			__set_extent_info(&ei, fofs, len, tei->blk, false,
> +					  0, 0, EX_READ);
> +			if (!__try_merge_extent_node(sbi, et, &ei,
> +							prev_en, next_en))
> +				__insert_extent_tree(sbi, et, &ei,
>  					insert_p, insert_parent, leftmost);
>  
> -		/* give up extent_cache, if split and small updates happen */
> -		if (dei.len >= 1 &&
> -				prev.len < F2FS_MIN_EXTENT_LEN &&
> -				et->largest.len < F2FS_MIN_EXTENT_LEN) {
> -			et->largest.len = 0;
> -			et->largest_updated = true;
> -			set_inode_flag(inode, FI_NO_EXTENT);
> +			/* give up read extent cache, if split and small updates happen */
> +			if (dei.len >= 1 &&
> +					prev.len < F2FS_MIN_EXTENT_LEN &&
> +					et->largest.len < F2FS_MIN_EXTENT_LEN) {
> +				et->largest.len = 0;
> +				et->largest_updated = true;
> +				set_inode_flag(inode, FI_NO_EXTENT);
> +			}
>  		}
> -	}
> -
> -	if (is_inode_flag_set(inode, FI_NO_EXTENT))
> -		__free_extent_tree(sbi, et);
>  
> -	if (et->largest_updated) {
> -		et->largest_updated = false;
> -		updated = true;
> -	}
> -	goto out_read_extent_cache;
> -update_age_extent_cache:
> -	if (!tei->last_blocks)
> -		goto out_read_extent_cache;
> +		if (is_inode_flag_set(inode, FI_NO_EXTENT))
> +			__free_extent_tree(sbi, et);
>  
> -	__set_extent_info(&ei, fofs, len, 0, false,
> -			tei->age, tei->last_blocks, EX_BLOCK_AGE);
> -	if (!__try_merge_extent_node(sbi, et, &ei, prev_en, next_en))
> -		__insert_extent_tree(sbi, et, &ei,
> +		if (et->largest_updated) {
> +			et->largest_updated = false;
> +			updated = true;
> +		}
> +	} else if (type == EX_BLOCK_AGE) {
> +		if (tei->last_blocks) {
> +			__set_extent_info(&ei, fofs, len, 0, false,
> +				tei->age, tei->last_blocks, EX_BLOCK_AGE);
> +			if (!__try_merge_extent_node(sbi, et, &ei,
> +							prev_en, next_en))
> +				__insert_extent_tree(sbi, et, &ei,
>  					insert_p, insert_parent, leftmost);
> -out_read_extent_cache:
> +		}
> +	}
>  	write_unlock(&et->lock);
>  
>  	if (updated)
> -- 
> 2.36.1