2022-09-24 02:11:43

by yebin (H)

[permalink] [raw]
Subject: [PATCH -next v2 0/2] ext4: factor out ext4_free_ext_path()

Diff v2 vs v1:
1. Add one patch to remove unnecessary drop path references in
mext_check_coverage().
2. Make 'ext4_ext_drop_refs()' static in fs/ext4/extents.c.
3. Export non-inline 'ext4_free_ext_path()' API.

Ye Bin (2):
ext4: remove unnecessary drop path references in mext_check_coverage()
ext4: factor out ext4_free_ext_path()

fs/ext4/ext4.h | 2 +-
fs/ext4/extents.c | 107 ++++++++++++++++-----------------------
fs/ext4/extents_status.c | 3 +-
fs/ext4/fast_commit.c | 6 +--
fs/ext4/migrate.c | 3 +-
fs/ext4/move_extent.c | 10 ++--
fs/ext4/verity.c | 6 +--
7 files changed, 54 insertions(+), 83 deletions(-)

--
2.31.1


2022-09-24 02:11:44

by yebin (H)

[permalink] [raw]
Subject: [PATCH -next v2 1/2] ext4: remove unnecessary drop path references in mext_check_coverage()

According to Jan Kara's suggestion:
"The use in mext_check_coverage() can be actually removed
- get_ext_path() -> ext4_find_extent() takes care of dropping the references."
So remove unnecessary call ext4_ext_drop_refs() in mext_check_coverage().

Signed-off-by: Ye Bin <[email protected]>
---
fs/ext4/move_extent.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 701f1d6a217f..782bbda2a717 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -103,7 +103,6 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
if (unwritten != ext4_ext_is_unwritten(ext))
goto out;
from += ext4_ext_get_actual_len(ext);
- ext4_ext_drop_refs(path);
}
ret = 1;
out:
--
2.31.1

2022-09-24 02:12:06

by yebin (H)

[permalink] [raw]
Subject: [PATCH -next v2 2/2] ext4: factor out ext4_free_ext_path()

Factor out ext4_free_ext_path() to free extent path. As after previous patch
'ext4_ext_drop_refs()' is only used in 'extents.c', so make it static.

Signed-off-by: Ye Bin <[email protected]>
---
fs/ext4/ext4.h | 2 +-
fs/ext4/extents.c | 107 ++++++++++++++++-----------------------
fs/ext4/extents_status.c | 3 +-
fs/ext4/fast_commit.c | 6 +--
fs/ext4/migrate.c | 3 +-
fs/ext4/move_extent.c | 9 ++--
fs/ext4/verity.c | 6 +--
7 files changed, 54 insertions(+), 82 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e6674504ca2a..648ea9ccf65e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3713,7 +3713,7 @@ extern int ext4_ext_insert_extent(handle_t *, struct inode *,
extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
struct ext4_ext_path **,
int flags);
-extern void ext4_ext_drop_refs(struct ext4_ext_path *);
+extern void ext4_free_ext_path(struct ext4_ext_path *);
extern int ext4_ext_check_inode(struct inode *inode);
extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c148bb97b527..4653acfa1723 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -106,6 +106,25 @@ static int ext4_ext_trunc_restart_fn(struct inode *inode, int *dropped)
return 0;
}

+static void ext4_ext_drop_refs(struct ext4_ext_path *path)
+{
+ int depth, i;
+
+ if (!path)
+ return;
+ depth = path->p_depth;
+ for (i = 0; i <= depth; i++, path++) {
+ brelse(path->p_bh);
+ path->p_bh = NULL;
+ }
+}
+
+void ext4_free_ext_path(struct ext4_ext_path *path)
+{
+ ext4_ext_drop_refs(path);
+ kfree(path);
+}
+
/*
* Make sure 'handle' has at least 'check_cred' credits. If not, restart
* transaction with 'restart_cred' credits. The function drops i_data_sem
@@ -632,8 +651,7 @@ int ext4_ext_precache(struct inode *inode)
ext4_set_inode_state(inode, EXT4_STATE_EXT_PRECACHED);
out:
up_read(&ei->i_data_sem);
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return ret;
}

@@ -720,19 +738,6 @@ static void ext4_ext_show_move(struct inode *inode, struct ext4_ext_path *path,
#define ext4_ext_show_move(inode, path, newblock, level)
#endif

-void ext4_ext_drop_refs(struct ext4_ext_path *path)
-{
- int depth, i;
-
- if (!path)
- return;
- depth = path->p_depth;
- for (i = 0; i <= depth; i++, path++) {
- brelse(path->p_bh);
- path->p_bh = NULL;
- }
-}
-
/*
* ext4_ext_binsearch_idx:
* binary search for the closest index of the given block
@@ -951,8 +956,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
return path;

err:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
if (orig_path)
*orig_path = NULL;
return ERR_PTR(ret);
@@ -2170,8 +2174,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
err = ext4_ext_dirty(handle, inode, path + path->p_depth);

cleanup:
- ext4_ext_drop_refs(npath);
- kfree(npath);
+ ext4_free_ext_path(npath);
return err;
}

@@ -3057,8 +3060,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
}
}
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
path = NULL;
if (err == -EAGAIN)
goto again;
@@ -4371,8 +4373,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
allocated = map->m_len;
ext4_ext_show_leaf(inode, path);
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);

trace_ext4_ext_map_blocks_exit(inode, flags, map,
err ? err : allocated);
@@ -5241,8 +5242,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
break;
}
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return ret;
}

@@ -5534,15 +5534,13 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
EXT4_GET_BLOCKS_METADATA_NOFAIL);
}

- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
if (ret < 0) {
up_write(&EXT4_I(inode)->i_data_sem);
goto out_stop;
}
} else {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
}

ret = ext4_es_remove_extent(inode, offset_lblk,
@@ -5762,10 +5760,8 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
count -= len;

repeat:
- ext4_ext_drop_refs(path1);
- kfree(path1);
- ext4_ext_drop_refs(path2);
- kfree(path2);
+ ext4_free_ext_path(path1);
+ ext4_free_ext_path(path2);
path1 = path2 = NULL;
}
return replaced_count;
@@ -5844,8 +5840,7 @@ int ext4_clu_mapped(struct inode *inode, ext4_lblk_t lclu)
}

out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);

return err ? err : mapped;
}
@@ -5912,8 +5907,7 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
ret = ext4_ext_dirty(NULL, inode, &path[path->p_depth]);
up_write(&EXT4_I(inode)->i_data_sem);
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
ext4_mark_inode_dirty(NULL, inode);
return ret;
}
@@ -5931,8 +5925,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
return;
ex = path[path->p_depth].p_ext;
if (!ex) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
ext4_mark_inode_dirty(NULL, inode);
return;
}
@@ -5945,8 +5938,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
ext4_ext_dirty(NULL, inode, &path[path->p_depth]);
up_write(&EXT4_I(inode)->i_data_sem);
ext4_mark_inode_dirty(NULL, inode);
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
}
}

@@ -5985,13 +5977,11 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
return PTR_ERR(path);
ex = path[path->p_depth].p_ext;
if (!ex) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
goto out;
}
end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex);
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);

/* Count the number of data blocks */
cur = 0;
@@ -6021,30 +6011,26 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
if (IS_ERR(path))
goto out;
numblks += path->p_depth;
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
while (cur < end) {
path = ext4_find_extent(inode, cur, NULL, 0);
if (IS_ERR(path))
break;
ex = path[path->p_depth].p_ext;
if (!ex) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return 0;
}
cur = max(cur + 1, le32_to_cpu(ex->ee_block) +
ext4_ext_get_actual_len(ex));
ret = skip_hole(inode, &cur);
if (ret < 0) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
break;
}
path2 = ext4_find_extent(inode, cur, NULL, 0);
if (IS_ERR(path2)) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
break;
}
for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) {
@@ -6058,10 +6044,8 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
if (cmp1 != cmp2 && cmp2 != 0)
numblks++;
}
- ext4_ext_drop_refs(path);
- ext4_ext_drop_refs(path2);
- kfree(path);
- kfree(path2);
+ ext4_free_ext_path(path);
+ ext4_free_ext_path(path2);
}

out:
@@ -6088,13 +6072,11 @@ int ext4_ext_clear_bb(struct inode *inode)
return PTR_ERR(path);
ex = path[path->p_depth].p_ext;
if (!ex) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return 0;
}
end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex);
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);

cur = 0;
while (cur < end) {
@@ -6113,8 +6095,7 @@ int ext4_ext_clear_bb(struct inode *inode)
ext4_fc_record_regions(inode->i_sb, inode->i_ino,
0, path[j].p_block, 1, 1);
}
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
}
ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, 0);
ext4_fc_record_regions(inode->i_sb, inode->i_ino,
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 23167efda95e..cd0a861853e3 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -667,8 +667,7 @@ static void ext4_es_insert_extent_ext_check(struct inode *inode,
}
}
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
}

static void ext4_es_insert_extent_ind_check(struct inode *inode,
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 067969f342eb..085cca17d72f 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1776,8 +1776,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb,
ret = ext4_ext_insert_extent(
NULL, inode, &path, &newex, 0);
up_write((&EXT4_I(inode)->i_data_sem));
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
if (ret)
goto out;
goto next;
@@ -1932,8 +1931,7 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)
for (j = 0; j < path->p_depth; j++)
ext4_mb_mark_bb(inode->i_sb,
path[j].p_block, 1, 1);
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
}
cur += ret;
ext4_mb_mark_bb(inode->i_sb, map.m_pblk,
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 54e7d3c95fd7..0a220ec9862d 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -56,8 +56,7 @@ static int finish_range(handle_t *handle, struct inode *inode,
retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0);
err_out:
up_write((&EXT4_I(inode)->i_data_sem));
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
lb->first_pblock = 0;
return retval;
}
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 782bbda2a717..1f314eb6917d 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -32,8 +32,7 @@ get_ext_path(struct inode *inode, ext4_lblk_t lblock,
if (IS_ERR(path))
return PTR_ERR(path);
if (path[ext_depth(inode)].p_ext == NULL) {
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
*ppath = NULL;
return -ENODATA;
}
@@ -106,8 +105,7 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
}
ret = 1;
out:
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return ret;
}

@@ -693,8 +691,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
ext4_discard_preallocations(donor_inode, 0);
}

- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
ext4_double_up_write_data_sem(orig_inode, donor_inode);
unlock_two_nondirectories(orig_inode, donor_inode);

diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index b051d19b5c8a..20cadfb740dc 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -298,16 +298,14 @@ static int ext4_get_verity_descriptor_location(struct inode *inode,
last_extent = path[path->p_depth].p_ext;
if (!last_extent) {
EXT4_ERROR_INODE(inode, "verity file has no extents");
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);
return -EFSCORRUPTED;
}

end_lblk = le32_to_cpu(last_extent->ee_block) +
ext4_ext_get_actual_len(last_extent);
desc_size_pos = (u64)end_lblk << inode->i_blkbits;
- ext4_ext_drop_refs(path);
- kfree(path);
+ ext4_free_ext_path(path);

if (desc_size_pos < sizeof(desc_size_disk))
goto bad;
--
2.31.1

2022-09-26 08:28:17

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next v2 1/2] ext4: remove unnecessary drop path references in mext_check_coverage()

On Sat 24-09-22 10:12:10, Ye Bin wrote:
> According to Jan Kara's suggestion:
> "The use in mext_check_coverage() can be actually removed
> - get_ext_path() -> ext4_find_extent() takes care of dropping the references."
> So remove unnecessary call ext4_ext_drop_refs() in mext_check_coverage().
>
> Signed-off-by: Ye Bin <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/move_extent.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 701f1d6a217f..782bbda2a717 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -103,7 +103,6 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
> if (unwritten != ext4_ext_is_unwritten(ext))
> goto out;
> from += ext4_ext_get_actual_len(ext);
> - ext4_ext_drop_refs(path);
> }
> ret = 1;
> out:
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-09-26 08:28:38

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next v2 2/2] ext4: factor out ext4_free_ext_path()

On Sat 24-09-22 10:12:11, Ye Bin wrote:
> Factor out ext4_free_ext_path() to free extent path. As after previous patch
> 'ext4_ext_drop_refs()' is only used in 'extents.c', so make it static.
>
> Signed-off-by: Ye Bin <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/extents.c | 107 ++++++++++++++++-----------------------
> fs/ext4/extents_status.c | 3 +-
> fs/ext4/fast_commit.c | 6 +--
> fs/ext4/migrate.c | 3 +-
> fs/ext4/move_extent.c | 9 ++--
> fs/ext4/verity.c | 6 +--
> 7 files changed, 54 insertions(+), 82 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index e6674504ca2a..648ea9ccf65e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3713,7 +3713,7 @@ extern int ext4_ext_insert_extent(handle_t *, struct inode *,
> extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
> struct ext4_ext_path **,
> int flags);
> -extern void ext4_ext_drop_refs(struct ext4_ext_path *);
> +extern void ext4_free_ext_path(struct ext4_ext_path *);
> extern int ext4_ext_check_inode(struct inode *inode);
> extern ext4_lblk_t ext4_ext_next_allocated_block(struct ext4_ext_path *path);
> extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index c148bb97b527..4653acfa1723 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -106,6 +106,25 @@ static int ext4_ext_trunc_restart_fn(struct inode *inode, int *dropped)
> return 0;
> }
>
> +static void ext4_ext_drop_refs(struct ext4_ext_path *path)
> +{
> + int depth, i;
> +
> + if (!path)
> + return;
> + depth = path->p_depth;
> + for (i = 0; i <= depth; i++, path++) {
> + brelse(path->p_bh);
> + path->p_bh = NULL;
> + }
> +}
> +
> +void ext4_free_ext_path(struct ext4_ext_path *path)
> +{
> + ext4_ext_drop_refs(path);
> + kfree(path);
> +}
> +
> /*
> * Make sure 'handle' has at least 'check_cred' credits. If not, restart
> * transaction with 'restart_cred' credits. The function drops i_data_sem
> @@ -632,8 +651,7 @@ int ext4_ext_precache(struct inode *inode)
> ext4_set_inode_state(inode, EXT4_STATE_EXT_PRECACHED);
> out:
> up_read(&ei->i_data_sem);
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return ret;
> }
>
> @@ -720,19 +738,6 @@ static void ext4_ext_show_move(struct inode *inode, struct ext4_ext_path *path,
> #define ext4_ext_show_move(inode, path, newblock, level)
> #endif
>
> -void ext4_ext_drop_refs(struct ext4_ext_path *path)
> -{
> - int depth, i;
> -
> - if (!path)
> - return;
> - depth = path->p_depth;
> - for (i = 0; i <= depth; i++, path++) {
> - brelse(path->p_bh);
> - path->p_bh = NULL;
> - }
> -}
> -
> /*
> * ext4_ext_binsearch_idx:
> * binary search for the closest index of the given block
> @@ -951,8 +956,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
> return path;
>
> err:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> if (orig_path)
> *orig_path = NULL;
> return ERR_PTR(ret);
> @@ -2170,8 +2174,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
> err = ext4_ext_dirty(handle, inode, path + path->p_depth);
>
> cleanup:
> - ext4_ext_drop_refs(npath);
> - kfree(npath);
> + ext4_free_ext_path(npath);
> return err;
> }
>
> @@ -3057,8 +3060,7 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> }
> }
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> path = NULL;
> if (err == -EAGAIN)
> goto again;
> @@ -4371,8 +4373,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> allocated = map->m_len;
> ext4_ext_show_leaf(inode, path);
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
>
> trace_ext4_ext_map_blocks_exit(inode, flags, map,
> err ? err : allocated);
> @@ -5241,8 +5242,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
> break;
> }
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return ret;
> }
>
> @@ -5534,15 +5534,13 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
> EXT4_GET_BLOCKS_METADATA_NOFAIL);
> }
>
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> if (ret < 0) {
> up_write(&EXT4_I(inode)->i_data_sem);
> goto out_stop;
> }
> } else {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> }
>
> ret = ext4_es_remove_extent(inode, offset_lblk,
> @@ -5762,10 +5760,8 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> count -= len;
>
> repeat:
> - ext4_ext_drop_refs(path1);
> - kfree(path1);
> - ext4_ext_drop_refs(path2);
> - kfree(path2);
> + ext4_free_ext_path(path1);
> + ext4_free_ext_path(path2);
> path1 = path2 = NULL;
> }
> return replaced_count;
> @@ -5844,8 +5840,7 @@ int ext4_clu_mapped(struct inode *inode, ext4_lblk_t lclu)
> }
>
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
>
> return err ? err : mapped;
> }
> @@ -5912,8 +5907,7 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
> ret = ext4_ext_dirty(NULL, inode, &path[path->p_depth]);
> up_write(&EXT4_I(inode)->i_data_sem);
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> ext4_mark_inode_dirty(NULL, inode);
> return ret;
> }
> @@ -5931,8 +5925,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
> return;
> ex = path[path->p_depth].p_ext;
> if (!ex) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> ext4_mark_inode_dirty(NULL, inode);
> return;
> }
> @@ -5945,8 +5938,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
> ext4_ext_dirty(NULL, inode, &path[path->p_depth]);
> up_write(&EXT4_I(inode)->i_data_sem);
> ext4_mark_inode_dirty(NULL, inode);
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> }
> }
>
> @@ -5985,13 +5977,11 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> return PTR_ERR(path);
> ex = path[path->p_depth].p_ext;
> if (!ex) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> goto out;
> }
> end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex);
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
>
> /* Count the number of data blocks */
> cur = 0;
> @@ -6021,30 +6011,26 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> if (IS_ERR(path))
> goto out;
> numblks += path->p_depth;
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> while (cur < end) {
> path = ext4_find_extent(inode, cur, NULL, 0);
> if (IS_ERR(path))
> break;
> ex = path[path->p_depth].p_ext;
> if (!ex) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return 0;
> }
> cur = max(cur + 1, le32_to_cpu(ex->ee_block) +
> ext4_ext_get_actual_len(ex));
> ret = skip_hole(inode, &cur);
> if (ret < 0) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> break;
> }
> path2 = ext4_find_extent(inode, cur, NULL, 0);
> if (IS_ERR(path2)) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> break;
> }
> for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) {
> @@ -6058,10 +6044,8 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> if (cmp1 != cmp2 && cmp2 != 0)
> numblks++;
> }
> - ext4_ext_drop_refs(path);
> - ext4_ext_drop_refs(path2);
> - kfree(path);
> - kfree(path2);
> + ext4_free_ext_path(path);
> + ext4_free_ext_path(path2);
> }
>
> out:
> @@ -6088,13 +6072,11 @@ int ext4_ext_clear_bb(struct inode *inode)
> return PTR_ERR(path);
> ex = path[path->p_depth].p_ext;
> if (!ex) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return 0;
> }
> end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex);
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
>
> cur = 0;
> while (cur < end) {
> @@ -6113,8 +6095,7 @@ int ext4_ext_clear_bb(struct inode *inode)
> ext4_fc_record_regions(inode->i_sb, inode->i_ino,
> 0, path[j].p_block, 1, 1);
> }
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> }
> ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, 0);
> ext4_fc_record_regions(inode->i_sb, inode->i_ino,
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 23167efda95e..cd0a861853e3 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -667,8 +667,7 @@ static void ext4_es_insert_extent_ext_check(struct inode *inode,
> }
> }
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> }
>
> static void ext4_es_insert_extent_ind_check(struct inode *inode,
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 067969f342eb..085cca17d72f 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1776,8 +1776,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb,
> ret = ext4_ext_insert_extent(
> NULL, inode, &path, &newex, 0);
> up_write((&EXT4_I(inode)->i_data_sem));
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> if (ret)
> goto out;
> goto next;
> @@ -1932,8 +1931,7 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)
> for (j = 0; j < path->p_depth; j++)
> ext4_mb_mark_bb(inode->i_sb,
> path[j].p_block, 1, 1);
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> }
> cur += ret;
> ext4_mb_mark_bb(inode->i_sb, map.m_pblk,
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 54e7d3c95fd7..0a220ec9862d 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -56,8 +56,7 @@ static int finish_range(handle_t *handle, struct inode *inode,
> retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0);
> err_out:
> up_write((&EXT4_I(inode)->i_data_sem));
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> lb->first_pblock = 0;
> return retval;
> }
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 782bbda2a717..1f314eb6917d 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -32,8 +32,7 @@ get_ext_path(struct inode *inode, ext4_lblk_t lblock,
> if (IS_ERR(path))
> return PTR_ERR(path);
> if (path[ext_depth(inode)].p_ext == NULL) {
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> *ppath = NULL;
> return -ENODATA;
> }
> @@ -106,8 +105,7 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
> }
> ret = 1;
> out:
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return ret;
> }
>
> @@ -693,8 +691,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
> ext4_discard_preallocations(donor_inode, 0);
> }
>
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> ext4_double_up_write_data_sem(orig_inode, donor_inode);
> unlock_two_nondirectories(orig_inode, donor_inode);
>
> diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
> index b051d19b5c8a..20cadfb740dc 100644
> --- a/fs/ext4/verity.c
> +++ b/fs/ext4/verity.c
> @@ -298,16 +298,14 @@ static int ext4_get_verity_descriptor_location(struct inode *inode,
> last_extent = path[path->p_depth].p_ext;
> if (!last_extent) {
> EXT4_ERROR_INODE(inode, "verity file has no extents");
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
> return -EFSCORRUPTED;
> }
>
> end_lblk = le32_to_cpu(last_extent->ee_block) +
> ext4_ext_get_actual_len(last_extent);
> desc_size_pos = (u64)end_lblk << inode->i_blkbits;
> - ext4_ext_drop_refs(path);
> - kfree(path);
> + ext4_free_ext_path(path);
>
> if (desc_size_pos < sizeof(desc_size_disk))
> goto bad;
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-09-30 03:21:58

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH -next v2 0/2] ext4: factor out ext4_free_ext_path()

On Sat, 24 Sep 2022 10:12:09 +0800, Ye Bin wrote:
> Diff v2 vs v1:
> 1. Add one patch to remove unnecessary drop path references in
> mext_check_coverage().
> 2. Make 'ext4_ext_drop_refs()' static in fs/ext4/extents.c.
> 3. Export non-inline 'ext4_free_ext_path()' API.
>
> Ye Bin (2):
> ext4: remove unnecessary drop path references in mext_check_coverage()
> ext4: factor out ext4_free_ext_path()
>
> [...]

Applied, thanks!

[1/2] ext4: remove unnecessary drop path references in mext_check_coverage()
commit: cc984fd387b5258fa9eb96811796e4bcadc2b272
[2/2] ext4: factor out ext4_free_ext_path()
commit: 6863dc4b094249163172691f09a91c28f36ee824

Best regards,
--
Theodore Ts'o <[email protected]>