No logic change, just avoid goto statement.
Signed-off-by: Chao Yu <[email protected]>
---
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
nr_free may be less than len, we should update age extent cache
w/ range [fofs, len] rather than [fofs, nr_free].
Fixes: 71644dff4811 ("f2fs: add block_age-based extent cache")
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f4de96a3744b..746ffcd09b6c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -617,7 +617,7 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
fofs = f2fs_start_bidx_of_node(ofs_of_node(dn->node_page),
dn->inode) + ofs;
f2fs_update_read_extent_cache_range(dn, fofs, 0, len);
- f2fs_update_age_extent_cache_range(dn, fofs, nr_free);
+ f2fs_update_age_extent_cache_range(dn, fofs, len);
dec_valid_block_count(sbi, dn->inode, nr_free);
}
dn->ofs_in_node = ofs;
--
2.36.1
We should update age extent in f2fs_do_zero_range() like we
did in f2fs_truncate_data_blocks_range().
Fixes: 71644dff4811 ("f2fs: add block_age-based extent cache")
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 746ffcd09b6c..60488749c35e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1494,6 +1494,7 @@ static int f2fs_do_zero_range(struct dnode_of_data *dn, pgoff_t start,
}
f2fs_update_read_extent_cache_range(dn, start, 0, index - start);
+ f2fs_update_age_extent_cache_range(dn, start, index - start);
return ret;
}
--
2.36.1
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 <[email protected]>
> ---
> 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
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:
On Tue, 31 Jan 2023 22:46:59 +0800 you wrote:
> No logic change, just avoid goto statement.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/extent_cache.c | 66 ++++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 35 deletions(-)
Here is the summary with links:
- [f2fs-dev,1/3] f2fs: clean up __update_extent_tree_range()
(no matching commit)
- [f2fs-dev,2/3] f2fs: fix to update age extent correctly during truncation
https://git.kernel.org/jaegeuk/f2fs/c/334ce4a79c9e
- [f2fs-dev,3/3] f2fs: fix to update age extent in f2fs_do_zero_range()
https://git.kernel.org/jaegeuk/f2fs/c/de6b3a5e09b2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On 2023/2/1 2:56, Jaegeuk Kim wrote:
> I wanted to avoid a deep if/else statement.
So how about this:
From 418b408420367ac5491c97a7c4d26e3d0e68ea57 Mon Sep 17 00:00:00 2001
From: Chao Yu <[email protected]>
Date: Tue, 31 Jan 2023 22:46:59 +0800
Subject: [PATCH v2] f2fs: clean up __update_extent_tree_range()
Introduce __update_read_extent_cache() and __update_age_extent_cache()
to clean up __update_extent_tree_range(), no logic change.
Signed-off-by: Chao Yu <[email protected]>
---
v2
- introduce __update_read_extent_cache() and __update_age_extent_cache()
to avoid a deep if/else statement in __update_extent_tree_range().
fs/f2fs/extent_cache.c | 116 +++++++++++++++++++++++++++--------------
1 file changed, 77 insertions(+), 39 deletions(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index d70ad6a44cbf..887b0b2898b9 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -666,6 +666,75 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
return en;
}
+static inline bool __update_read_extent_cache(struct inode *inode,
+ struct extent_info *tei,
+ struct extent_info *ei,
+ struct extent_info *dei,
+ struct extent_info *prev,
+ unsigned int fofs, unsigned int len,
+ struct extent_node *prev_en,
+ struct extent_node *next_en,
+ struct rb_node **insert_p,
+ struct rb_node *insert_parent,
+ bool leftmost)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct extent_tree *et = F2FS_I(inode)->extent_tree[EX_READ];
+
+ if (!tei->blk)
+ return false;
+
+ __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 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;
+ return true;
+ }
+
+ return false;
+}
+
+static inline void __update_age_extent_cache(struct inode *inode,
+ struct extent_info *tei,
+ struct extent_info *ei,
+ unsigned int fofs, unsigned int len,
+ struct extent_node *prev_en,
+ struct extent_node *next_en,
+ struct rb_node **insert_p,
+ struct rb_node *insert_parent,
+ bool leftmost)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct extent_tree *et = F2FS_I(inode)->extent_tree[EX_BLOCK_AGE];
+
+ if (!tei->last_blocks)
+ return;
+
+ __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);
+}
+
static void __update_extent_tree_range(struct inode *inode,
struct extent_info *tei, enum extent_type type)
{
@@ -782,47 +851,16 @@ 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)
+ updated = __update_read_extent_cache(inode, tei, &ei, &dei,
+ &prev, fofs, len, prev_en, next_en,
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);
- }
- }
-
- 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;
-
- __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,
+ else if (type == EX_BLOCK_AGE)
+ __update_age_extent_cache(inode, tei, &ei,
+ fofs, len, prev_en, next_en,
insert_p, insert_parent, leftmost);
-out_read_extent_cache:
+
write_unlock(&et->lock);
if (updated)
--
2.25.1
Thanks,
On 02/01, Chao Yu wrote:
> On 2023/2/1 2:56, Jaegeuk Kim wrote:
> > I wanted to avoid a deep if/else statement.
>
> So how about this:
Nothing impressive.
>
> From 418b408420367ac5491c97a7c4d26e3d0e68ea57 Mon Sep 17 00:00:00 2001
> From: Chao Yu <[email protected]>
> Date: Tue, 31 Jan 2023 22:46:59 +0800
> Subject: [PATCH v2] f2fs: clean up __update_extent_tree_range()
>
> Introduce __update_read_extent_cache() and __update_age_extent_cache()
> to clean up __update_extent_tree_range(), no logic change.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> v2
> - introduce __update_read_extent_cache() and __update_age_extent_cache()
> to avoid a deep if/else statement in __update_extent_tree_range().
> fs/f2fs/extent_cache.c | 116 +++++++++++++++++++++++++++--------------
> 1 file changed, 77 insertions(+), 39 deletions(-)
>
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index d70ad6a44cbf..887b0b2898b9 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -666,6 +666,75 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
> return en;
> }
>
> +static inline bool __update_read_extent_cache(struct inode *inode,
> + struct extent_info *tei,
> + struct extent_info *ei,
> + struct extent_info *dei,
> + struct extent_info *prev,
> + unsigned int fofs, unsigned int len,
> + struct extent_node *prev_en,
> + struct extent_node *next_en,
> + struct rb_node **insert_p,
> + struct rb_node *insert_parent,
> + bool leftmost)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct extent_tree *et = F2FS_I(inode)->extent_tree[EX_READ];
> +
> + if (!tei->blk)
> + return false;
> +
> + __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 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;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static inline void __update_age_extent_cache(struct inode *inode,
> + struct extent_info *tei,
> + struct extent_info *ei,
> + unsigned int fofs, unsigned int len,
> + struct extent_node *prev_en,
> + struct extent_node *next_en,
> + struct rb_node **insert_p,
> + struct rb_node *insert_parent,
> + bool leftmost)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct extent_tree *et = F2FS_I(inode)->extent_tree[EX_BLOCK_AGE];
> +
> + if (!tei->last_blocks)
> + return;
> +
> + __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);
> +}
> +
> static void __update_extent_tree_range(struct inode *inode,
> struct extent_info *tei, enum extent_type type)
> {
> @@ -782,47 +851,16 @@ 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)
> + updated = __update_read_extent_cache(inode, tei, &ei, &dei,
> + &prev, fofs, len, prev_en, next_en,
> 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);
> - }
> - }
> -
> - 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;
> -
> - __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,
> + else if (type == EX_BLOCK_AGE)
> + __update_age_extent_cache(inode, tei, &ei,
> + fofs, len, prev_en, next_en,
> insert_p, insert_parent, leftmost);
> -out_read_extent_cache:
> +
> write_unlock(&et->lock);
>
> if (updated)
> --
> 2.25.1
>
> Thanks,
On 2023/2/6 11:41, Jaegeuk Kim wrote:
> On 02/01, Chao Yu wrote:
>> On 2023/2/1 2:56, Jaegeuk Kim wrote:
>>> I wanted to avoid a deep if/else statement.
>>
>> So how about this:
>
> Nothing impressive.
Oops...
Thanks,