Re: [PATCH 1/3] f2fs: clean up __update_extent_tree_range()

From: Jaegeuk Kim
Date: Tue Jan 31 2023 - 13:57:14 EST


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@xxxxxxxxxx>
> ---
> 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