From: Baokun Li <libaokun1@huawei.com>
The use of path and ppath is now very confusing, so to make the code more
readable, pass path between functions uniformly, and get rid of ppath.
To get rid of the ppath in ext4_force_split_extent_at(), the following is
done here:
* The ext4_find_extent() can update the extent path so it doesn't have to
allocate and free path repeatedly, thus reducing the consumption of
memory allocation and freeing in ext4_swap_extents().
No functional changes.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/extents.c | 117 ++++++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 57 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c86b1bb7720f..0bd068ed324f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -328,27 +328,20 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
return size;
}
-static inline int
+static inline struct ext4_ext_path *
ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
- struct ext4_ext_path **ppath, ext4_lblk_t lblk,
+ struct ext4_ext_path *path, ext4_lblk_t lblk,
int nofail)
{
- struct ext4_ext_path *path = *ppath;
int unwritten = ext4_ext_is_unwritten(path[path->p_depth].p_ext);
int flags = EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_PRE_IO;
if (nofail)
flags |= EXT4_GET_BLOCKS_METADATA_NOFAIL | EXT4_EX_NOFAIL;
- path = ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
+ return ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
EXT4_EXT_MARK_UNWRIT1|EXT4_EXT_MARK_UNWRIT2 : 0,
flags);
- if (IS_ERR(path)) {
- *ppath = NULL;
- return PTR_ERR(path);
- }
- *ppath = path;
- return 0;
}
static int
@@ -2906,11 +2899,12 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
* fail removing space due to ENOSPC so try to use
* reserved block if that happens.
*/
- err = ext4_force_split_extent_at(handle, inode, &path,
- end + 1, 1);
- if (err < 0)
+ path = ext4_force_split_extent_at(handle, inode, path,
+ end + 1, 1);
+ if (IS_ERR(path)) {
+ err = PTR_ERR(path);
goto out;
-
+ }
} else if (sbi->s_cluster_ratio > 1 && end >= ex_end &&
partial.state == initial) {
/*
@@ -5707,25 +5701,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
int e1_len, e2_len, len;
int split = 0;
- path1 = ext4_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE);
+ path1 = ext4_find_extent(inode1, lblk1, path1, EXT4_EX_NOCACHE);
if (IS_ERR(path1)) {
*erp = PTR_ERR(path1);
- path1 = NULL;
- finish:
- count = 0;
- goto repeat;
+ goto errout;
}
- path2 = ext4_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE);
+ path2 = ext4_find_extent(inode2, lblk2, path2, EXT4_EX_NOCACHE);
if (IS_ERR(path2)) {
*erp = PTR_ERR(path2);
- path2 = NULL;
- goto finish;
+ goto errout;
}
ex1 = path1[path1->p_depth].p_ext;
ex2 = path2[path2->p_depth].p_ext;
/* Do we have something to swap ? */
if (unlikely(!ex2 || !ex1))
- goto finish;
+ goto errout;
e1_blk = le32_to_cpu(ex1->ee_block);
e2_blk = le32_to_cpu(ex2->ee_block);
@@ -5747,7 +5737,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
next2 = e2_blk;
/* Do we have something to swap */
if (next1 == EXT_MAX_BLOCKS || next2 == EXT_MAX_BLOCKS)
- goto finish;
+ goto errout;
/* Move to the rightest boundary */
len = next1 - lblk1;
if (len < next2 - lblk2)
@@ -5757,28 +5747,32 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
lblk1 += len;
lblk2 += len;
count -= len;
- goto repeat;
+ continue;
}
/* Prepare left boundary */
if (e1_blk < lblk1) {
split = 1;
- *erp = ext4_force_split_extent_at(handle, inode1,
- &path1, lblk1, 0);
- if (unlikely(*erp))
- goto finish;
+ path1 = ext4_force_split_extent_at(handle, inode1,
+ path1, lblk1, 0);
+ if (IS_ERR(path1)) {
+ *erp = PTR_ERR(path1);
+ goto errout;
+ }
}
if (e2_blk < lblk2) {
split = 1;
- *erp = ext4_force_split_extent_at(handle, inode2,
- &path2, lblk2, 0);
- if (unlikely(*erp))
- goto finish;
+ path2 = ext4_force_split_extent_at(handle, inode2,
+ path2, lblk2, 0);
+ if (IS_ERR(path2)) {
+ *erp = PTR_ERR(path2);
+ goto errout;
+ }
}
/* ext4_split_extent_at() may result in leaf extent split,
* path must to be revalidated. */
if (split)
- goto repeat;
+ continue;
/* Prepare right boundary */
len = count;
@@ -5789,30 +5783,34 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
if (len != e1_len) {
split = 1;
- *erp = ext4_force_split_extent_at(handle, inode1,
- &path1, lblk1 + len, 0);
- if (unlikely(*erp))
- goto finish;
+ path1 = ext4_force_split_extent_at(handle, inode1,
+ path1, lblk1 + len, 0);
+ if (IS_ERR(path1)) {
+ *erp = PTR_ERR(path1);
+ goto errout;
+ }
}
if (len != e2_len) {
split = 1;
- *erp = ext4_force_split_extent_at(handle, inode2,
- &path2, lblk2 + len, 0);
- if (*erp)
- goto finish;
+ path2 = ext4_force_split_extent_at(handle, inode2,
+ path2, lblk2 + len, 0);
+ if (IS_ERR(path2)) {
+ *erp = PTR_ERR(path2);
+ goto errout;
+ }
}
/* ext4_split_extent_at() may result in leaf extent split,
* path must to be revalidated. */
if (split)
- goto repeat;
+ continue;
BUG_ON(e2_len != e1_len);
*erp = ext4_ext_get_access(handle, inode1, path1 + path1->p_depth);
if (unlikely(*erp))
- goto finish;
+ goto errout;
*erp = ext4_ext_get_access(handle, inode2, path2 + path2->p_depth);
if (unlikely(*erp))
- goto finish;
+ goto errout;
/* Both extents are fully inside boundaries. Swap it now */
tmp_ex = *ex1;
@@ -5830,7 +5828,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
*erp = ext4_ext_dirty(handle, inode2, path2 +
path2->p_depth);
if (unlikely(*erp))
- goto finish;
+ goto errout;
*erp = ext4_ext_dirty(handle, inode1, path1 +
path1->p_depth);
/*
@@ -5840,17 +5838,17 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
* aborted anyway.
*/
if (unlikely(*erp))
- goto finish;
+ goto errout;
+
lblk1 += len;
lblk2 += len;
replaced_count += len;
count -= len;
-
- repeat:
- ext4_free_ext_path(path1);
- ext4_free_ext_path(path2);
- path1 = path2 = NULL;
}
+
+errout:
+ ext4_free_ext_path(path1);
+ ext4_free_ext_path(path2);
return replaced_count;
}
@@ -5965,24 +5963,29 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
ext4_ext_get_actual_len(ex) != len) {
/* We need to split this extent to match our extent first */
down_write(&EXT4_I(inode)->i_data_sem);
- ret = ext4_force_split_extent_at(NULL, inode, &path, start, 1);
+ path = ext4_force_split_extent_at(NULL, inode, path, start, 1);
up_write(&EXT4_I(inode)->i_data_sem);
- if (ret)
+ if (IS_ERR(path)) {
+ ret = PTR_ERR(path);
goto out;
+ }
path = ext4_find_extent(inode, start, path, 0);
if (IS_ERR(path))
return PTR_ERR(path);
+
ex = path[path->p_depth].p_ext;
WARN_ON(le32_to_cpu(ex->ee_block) != start);
if (ext4_ext_get_actual_len(ex) != len) {
down_write(&EXT4_I(inode)->i_data_sem);
- ret = ext4_force_split_extent_at(NULL, inode, &path,
- start + len, 1);
+ path = ext4_force_split_extent_at(NULL, inode, path,
+ start + len, 1);
up_write(&EXT4_I(inode)->i_data_sem);
- if (ret)
+ if (IS_ERR(path)) {
+ ret = PTR_ERR(path);
goto out;
+ }
path = ext4_find_extent(inode, start, path, 0);
if (IS_ERR(path))
--
2.39.2On Wed, Jul 10, 2024 at 12:06:47PM +0800, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
>
> The use of path and ppath is now very confusing, so to make the code more
> readable, pass path between functions uniformly, and get rid of ppath.
>
> To get rid of the ppath in ext4_force_split_extent_at(), the following is
> done here:
>
> * The ext4_find_extent() can update the extent path so it doesn't have to
> allocate and free path repeatedly, thus reducing the consumption of
> memory allocation and freeing in ext4_swap_extents().
>
> No functional changes.
Looks good Baokun, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
One small comment below..
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/extents.c | 117 ++++++++++++++++++++++++----------------------
> 1 file changed, 60 insertions(+), 57 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index c86b1bb7720f..0bd068ed324f 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
.. snip ..
> @@ -5707,25 +5701,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> int e1_len, e2_len, len;
> int split = 0;
>
> - path1 = ext4_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE);
> + path1 = ext4_find_extent(inode1, lblk1, path1, EXT4_EX_NOCACHE);
> if (IS_ERR(path1)) {
> *erp = PTR_ERR(path1);
> - path1 = NULL;
> - finish:
> - count = 0;
> - goto repeat;
> + goto errout;
> }
> - path2 = ext4_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE);
> + path2 = ext4_find_extent(inode2, lblk2, path2, EXT4_EX_NOCACHE);
> if (IS_ERR(path2)) {
> *erp = PTR_ERR(path2);
> - path2 = NULL;
> - goto finish;
> + goto errout;
> }
> ex1 = path1[path1->p_depth].p_ext;
> ex2 = path2[path2->p_depth].p_ext;
> /* Do we have something to swap ? */
> if (unlikely(!ex2 || !ex1))
> - goto finish;
> + goto errout;
>
> e1_blk = le32_to_cpu(ex1->ee_block);
> e2_blk = le32_to_cpu(ex2->ee_block);
> @@ -5747,7 +5737,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> next2 = e2_blk;
> /* Do we have something to swap */
> if (next1 == EXT_MAX_BLOCKS || next2 == EXT_MAX_BLOCKS)
> - goto finish;
> + goto errout;
> /* Move to the rightest boundary */
> len = next1 - lblk1;
> if (len < next2 - lblk2)
> @@ -5757,28 +5747,32 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> lblk1 += len;
> lblk2 += len;
> count -= len;
> - goto repeat;
> + continue;
> }
>
> /* Prepare left boundary */
> if (e1_blk < lblk1) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode1,
> - &path1, lblk1, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path1 = ext4_force_split_extent_at(handle, inode1,
> + path1, lblk1, 0);
> + if (IS_ERR(path1)) {
> + *erp = PTR_ERR(path1);
> + goto errout;
> + }
> }
> if (e2_blk < lblk2) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode2,
> - &path2, lblk2, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path2 = ext4_force_split_extent_at(handle, inode2,
> + path2, lblk2, 0);
> + if (IS_ERR(path2)) {
> + *erp = PTR_ERR(path2);
> + goto errout;
> + }
> }
> /* ext4_split_extent_at() may result in leaf extent split,
> * path must to be revalidated. */
> if (split)
> - goto repeat;
> + continue;
>
> /* Prepare right boundary */
> len = count;
> @@ -5789,30 +5783,34 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
>
> if (len != e1_len) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode1,
> - &path1, lblk1 + len, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path1 = ext4_force_split_extent_at(handle, inode1,
> + path1, lblk1 + len, 0);
> + if (IS_ERR(path1)) {
> + *erp = PTR_ERR(path1);
> + goto errout;
> + }
> }
> if (len != e2_len) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode2,
> - &path2, lblk2 + len, 0);
> - if (*erp)
> - goto finish;
> + path2 = ext4_force_split_extent_at(handle, inode2,
> + path2, lblk2 + len, 0);
> + if (IS_ERR(path2)) {
> + *erp = PTR_ERR(path2);
> + goto errout;
> + }
> }
> /* ext4_split_extent_at() may result in leaf extent split,
> * path must to be revalidated. */
> if (split)
> - goto repeat;
> + continue;
>
> BUG_ON(e2_len != e1_len);
> *erp = ext4_ext_get_access(handle, inode1, path1 + path1->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> *erp = ext4_ext_get_access(handle, inode2, path2 + path2->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
>
> /* Both extents are fully inside boundaries. Swap it now */
> tmp_ex = *ex1;
> @@ -5830,7 +5828,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> *erp = ext4_ext_dirty(handle, inode2, path2 +
> path2->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> *erp = ext4_ext_dirty(handle, inode1, path1 +
> path1->p_depth);
> /*
> @@ -5840,17 +5838,17 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> * aborted anyway.
> */
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> +
> lblk1 += len;
> lblk2 += len;
> replaced_count += len;
> count -= len;
> -
> - repeat:
> - ext4_free_ext_path(path1);
> - ext4_free_ext_path(path2);
> - path1 = path2 = NULL;
> }
> +
> +errout:
> + ext4_free_ext_path(path1);
> + ext4_free_ext_path(path2);
> return replaced_count;
> }
>
In ext4_swap_extents, maybe we should keep the refactoring to a separate
patch than the changes needed to get rid of ppath in
ext4_force_split_extent_at(), the commits would look a bit cleaner and
easier to read that way. I don't feel too strongly about it tho and
I'll let you take a call.
Regards,
ojaswin
Hi Ojaswin, On 2024/8/3 4:01, Ojaswin Mujoo wrote: > On Wed, Jul 10, 2024 at 12:06:47PM +0800, libaokun@huaweicloud.com wrote: >> From: Baokun Li <libaokun1@huawei.com> >> >> The use of path and ppath is now very confusing, so to make the code more >> readable, pass path between functions uniformly, and get rid of ppath. >> >> To get rid of the ppath in ext4_force_split_extent_at(), the following is >> done here: >> >> * The ext4_find_extent() can update the extent path so it doesn't have to >> allocate and free path repeatedly, thus reducing the consumption of >> memory allocation and freeing in ext4_swap_extents(). >> >> No functional changes. > Looks good Baokun, feel free to add: > > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > > One small comment below.. Thanks for the review! >> Signed-off-by: Baokun Li <libaokun1@huawei.com> >> --- >> fs/ext4/extents.c | 117 ++++++++++++++++++++++++---------------------- >> 1 file changed, 60 insertions(+), 57 deletions(-) >> >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >> index c86b1bb7720f..0bd068ed324f 100644 >> --- a/fs/ext4/extents.c >> +++ b/fs/ext4/extents.c > .. snip .. > >> @@ -5707,25 +5701,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1, > In ext4_swap_extents, maybe we should keep the refactoring to a separate > patch than the changes needed to get rid of ppath in > ext4_force_split_extent_at(), the commits would look a bit cleaner and > easier to read that way. I don't feel too strongly about it tho and > I'll let you take a call. > > Regards, > ojaswin Totally agree! It does seem a bit unclear now to put the refactoring of ext4_swap_extents() into a modification to get rid of the ppath. In the next version I will put the logic for refactoring ext4_swap_extents() into a separate patch. Cheers, Baokun
On Wed 10-07-24 12:06:47, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
>
> The use of path and ppath is now very confusing, so to make the code more
> readable, pass path between functions uniformly, and get rid of ppath.
>
> To get rid of the ppath in ext4_force_split_extent_at(), the following is
> done here:
>
> * The ext4_find_extent() can update the extent path so it doesn't have to
> allocate and free path repeatedly, thus reducing the consumption of
> memory allocation and freeing in ext4_swap_extents().
>
> No functional changes.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 117 ++++++++++++++++++++++++----------------------
> 1 file changed, 60 insertions(+), 57 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index c86b1bb7720f..0bd068ed324f 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -328,27 +328,20 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
> return size;
> }
>
> -static inline int
> +static inline struct ext4_ext_path *
> ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
> - struct ext4_ext_path **ppath, ext4_lblk_t lblk,
> + struct ext4_ext_path *path, ext4_lblk_t lblk,
> int nofail)
> {
> - struct ext4_ext_path *path = *ppath;
> int unwritten = ext4_ext_is_unwritten(path[path->p_depth].p_ext);
> int flags = EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_PRE_IO;
>
> if (nofail)
> flags |= EXT4_GET_BLOCKS_METADATA_NOFAIL | EXT4_EX_NOFAIL;
>
> - path = ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
> + return ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
> EXT4_EXT_MARK_UNWRIT1|EXT4_EXT_MARK_UNWRIT2 : 0,
> flags);
> - if (IS_ERR(path)) {
> - *ppath = NULL;
> - return PTR_ERR(path);
> - }
> - *ppath = path;
> - return 0;
> }
>
> static int
> @@ -2906,11 +2899,12 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> * fail removing space due to ENOSPC so try to use
> * reserved block if that happens.
> */
> - err = ext4_force_split_extent_at(handle, inode, &path,
> - end + 1, 1);
> - if (err < 0)
> + path = ext4_force_split_extent_at(handle, inode, path,
> + end + 1, 1);
> + if (IS_ERR(path)) {
> + err = PTR_ERR(path);
> goto out;
> -
> + }
> } else if (sbi->s_cluster_ratio > 1 && end >= ex_end &&
> partial.state == initial) {
> /*
> @@ -5707,25 +5701,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> int e1_len, e2_len, len;
> int split = 0;
>
> - path1 = ext4_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE);
> + path1 = ext4_find_extent(inode1, lblk1, path1, EXT4_EX_NOCACHE);
> if (IS_ERR(path1)) {
> *erp = PTR_ERR(path1);
> - path1 = NULL;
> - finish:
> - count = 0;
> - goto repeat;
> + goto errout;
> }
> - path2 = ext4_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE);
> + path2 = ext4_find_extent(inode2, lblk2, path2, EXT4_EX_NOCACHE);
> if (IS_ERR(path2)) {
> *erp = PTR_ERR(path2);
> - path2 = NULL;
> - goto finish;
> + goto errout;
> }
> ex1 = path1[path1->p_depth].p_ext;
> ex2 = path2[path2->p_depth].p_ext;
> /* Do we have something to swap ? */
> if (unlikely(!ex2 || !ex1))
> - goto finish;
> + goto errout;
>
> e1_blk = le32_to_cpu(ex1->ee_block);
> e2_blk = le32_to_cpu(ex2->ee_block);
> @@ -5747,7 +5737,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> next2 = e2_blk;
> /* Do we have something to swap */
> if (next1 == EXT_MAX_BLOCKS || next2 == EXT_MAX_BLOCKS)
> - goto finish;
> + goto errout;
> /* Move to the rightest boundary */
> len = next1 - lblk1;
> if (len < next2 - lblk2)
> @@ -5757,28 +5747,32 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> lblk1 += len;
> lblk2 += len;
> count -= len;
> - goto repeat;
> + continue;
> }
>
> /* Prepare left boundary */
> if (e1_blk < lblk1) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode1,
> - &path1, lblk1, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path1 = ext4_force_split_extent_at(handle, inode1,
> + path1, lblk1, 0);
> + if (IS_ERR(path1)) {
> + *erp = PTR_ERR(path1);
> + goto errout;
> + }
> }
> if (e2_blk < lblk2) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode2,
> - &path2, lblk2, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path2 = ext4_force_split_extent_at(handle, inode2,
> + path2, lblk2, 0);
> + if (IS_ERR(path2)) {
> + *erp = PTR_ERR(path2);
> + goto errout;
> + }
> }
> /* ext4_split_extent_at() may result in leaf extent split,
> * path must to be revalidated. */
> if (split)
> - goto repeat;
> + continue;
>
> /* Prepare right boundary */
> len = count;
> @@ -5789,30 +5783,34 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
>
> if (len != e1_len) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode1,
> - &path1, lblk1 + len, 0);
> - if (unlikely(*erp))
> - goto finish;
> + path1 = ext4_force_split_extent_at(handle, inode1,
> + path1, lblk1 + len, 0);
> + if (IS_ERR(path1)) {
> + *erp = PTR_ERR(path1);
> + goto errout;
> + }
> }
> if (len != e2_len) {
> split = 1;
> - *erp = ext4_force_split_extent_at(handle, inode2,
> - &path2, lblk2 + len, 0);
> - if (*erp)
> - goto finish;
> + path2 = ext4_force_split_extent_at(handle, inode2,
> + path2, lblk2 + len, 0);
> + if (IS_ERR(path2)) {
> + *erp = PTR_ERR(path2);
> + goto errout;
> + }
> }
> /* ext4_split_extent_at() may result in leaf extent split,
> * path must to be revalidated. */
> if (split)
> - goto repeat;
> + continue;
>
> BUG_ON(e2_len != e1_len);
> *erp = ext4_ext_get_access(handle, inode1, path1 + path1->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> *erp = ext4_ext_get_access(handle, inode2, path2 + path2->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
>
> /* Both extents are fully inside boundaries. Swap it now */
> tmp_ex = *ex1;
> @@ -5830,7 +5828,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> *erp = ext4_ext_dirty(handle, inode2, path2 +
> path2->p_depth);
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> *erp = ext4_ext_dirty(handle, inode1, path1 +
> path1->p_depth);
> /*
> @@ -5840,17 +5838,17 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> * aborted anyway.
> */
> if (unlikely(*erp))
> - goto finish;
> + goto errout;
> +
> lblk1 += len;
> lblk2 += len;
> replaced_count += len;
> count -= len;
> -
> - repeat:
> - ext4_free_ext_path(path1);
> - ext4_free_ext_path(path2);
> - path1 = path2 = NULL;
> }
> +
> +errout:
> + ext4_free_ext_path(path1);
> + ext4_free_ext_path(path2);
> return replaced_count;
> }
>
> @@ -5965,24 +5963,29 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
> ext4_ext_get_actual_len(ex) != len) {
> /* We need to split this extent to match our extent first */
> down_write(&EXT4_I(inode)->i_data_sem);
> - ret = ext4_force_split_extent_at(NULL, inode, &path, start, 1);
> + path = ext4_force_split_extent_at(NULL, inode, path, start, 1);
> up_write(&EXT4_I(inode)->i_data_sem);
> - if (ret)
> + if (IS_ERR(path)) {
> + ret = PTR_ERR(path);
> goto out;
> + }
>
> path = ext4_find_extent(inode, start, path, 0);
> if (IS_ERR(path))
> return PTR_ERR(path);
> +
> ex = path[path->p_depth].p_ext;
> WARN_ON(le32_to_cpu(ex->ee_block) != start);
>
> if (ext4_ext_get_actual_len(ex) != len) {
> down_write(&EXT4_I(inode)->i_data_sem);
> - ret = ext4_force_split_extent_at(NULL, inode, &path,
> - start + len, 1);
> + path = ext4_force_split_extent_at(NULL, inode, path,
> + start + len, 1);
> up_write(&EXT4_I(inode)->i_data_sem);
> - if (ret)
> + if (IS_ERR(path)) {
> + ret = PTR_ERR(path);
> goto out;
> + }
>
> path = ext4_find_extent(inode, start, path, 0);
> if (IS_ERR(path))
> --
> 2.39.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
© 2016 - 2025 Red Hat, Inc.