Message ID | 20240710040654.1714672-15-libaokun@huaweicloud.com |
---|---|
State | Superseded |
Headers | show |
Series | ext4: some bugfixes and cleanups for ext4 extents path | expand |
On Wed 10-07-24 12:06:48, 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_split_extent(), the following is done here: > > * The 'allocated' is changed from passing a value to passing an address. > * Its caller needs to update ppath if it uses ppath. > > 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 | 97 ++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 47 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 0bd068ed324f..2a4f6c89858c 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3345,21 +3345,18 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle, > * c> Splits in three extents: Somone is splitting in middle of the extent > * > */ > -static int ext4_split_extent(handle_t *handle, > - struct inode *inode, > - struct ext4_ext_path **ppath, > - struct ext4_map_blocks *map, > - int split_flag, > - int flags) > +static struct ext4_ext_path *ext4_split_extent(handle_t *handle, > + struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_map_blocks *map, > + int split_flag, int flags, > + unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > ext4_lblk_t ee_block; > struct ext4_extent *ex; > unsigned int ee_len, depth; > - int err = 0; > int unwritten; > int split_flag1, flags1; > - int allocated = map->m_len; > > depth = ext_depth(inode); > ex = path[depth].p_ext; > @@ -3377,33 +3374,25 @@ static int ext4_split_extent(handle_t *handle, > split_flag1 |= EXT4_EXT_DATA_VALID1; > path = ext4_split_extent_at(handle, inode, path, > map->m_lblk + map->m_len, split_flag1, flags1); > - if (IS_ERR(path)) { > - err = PTR_ERR(path); > - *ppath = NULL; > - goto out; > + if (IS_ERR(path)) > + return path; > + /* > + * Update path is required because previous ext4_split_extent_at > + * may result in split of original leaf or extent zeroout. > + */ > + path = ext4_find_extent(inode, map->m_lblk, path, flags); > + if (IS_ERR(path)) > + return path; > + depth = ext_depth(inode); > + ex = path[depth].p_ext; > + if (!ex) { > + EXT4_ERROR_INODE(inode, "unexpected hole at %lu", > + (unsigned long) map->m_lblk); > + ext4_free_ext_path(path); > + return ERR_PTR(-EFSCORRUPTED); > } > - *ppath = path; > - } else { > - allocated = ee_len - (map->m_lblk - ee_block); > - } > - /* > - * Update path is required because previous ext4_split_extent_at() may > - * result in split of original leaf or extent zeroout. > - */ > - path = ext4_find_extent(inode, map->m_lblk, path, flags); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > - *ppath = path; > - depth = ext_depth(inode); > - ex = path[depth].p_ext; > - if (!ex) { > - EXT4_ERROR_INODE(inode, "unexpected hole at %lu", > - (unsigned long) map->m_lblk); > - return -EFSCORRUPTED; > + unwritten = ext4_ext_is_unwritten(ex); > } > - unwritten = ext4_ext_is_unwritten(ex); > > if (map->m_lblk >= ee_block) { > split_flag1 = split_flag & EXT4_EXT_DATA_VALID2; > @@ -3414,17 +3403,18 @@ static int ext4_split_extent(handle_t *handle, > } > path = ext4_split_extent_at(handle, inode, path, > map->m_lblk, split_flag1, flags); > - if (IS_ERR(path)) { > - err = PTR_ERR(path); > - *ppath = NULL; > - goto out; > - } > - *ppath = path; > + if (IS_ERR(path)) > + return path; > } > > + if (allocated) { > + if (map->m_lblk + map->m_len > ee_block + ee_len) > + *allocated = ee_len - (map->m_lblk - ee_block); > + else > + *allocated = map->m_len; > + } > ext4_ext_show_leaf(inode, path); > -out: > - return err ? err : allocated; > + return path; > } > > /* > @@ -3669,10 +3659,15 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, > } > > fallback: > - err = ext4_split_extent(handle, inode, ppath, &split_map, split_flag, > - flags); > - if (err > 0) > - err = 0; > + path = ext4_split_extent(handle, inode, path, &split_map, split_flag, > + flags, NULL); > + if (IS_ERR(path)) { > + err = PTR_ERR(path); > + *ppath = NULL; > + goto out; > + } > + err = 0; > + *ppath = path; > out: > /* If we have gotten a failure, don't zero out status tree */ > if (!err) { > @@ -3718,6 +3713,7 @@ static int ext4_split_convert_extents(handle_t *handle, > struct ext4_extent *ex; > unsigned int ee_len; > int split_flag = 0, depth; > + unsigned int allocated = 0; > > ext_debug(inode, "logical block %llu, max_blocks %u\n", > (unsigned long long)map->m_lblk, map->m_len); > @@ -3745,7 +3741,14 @@ static int ext4_split_convert_extents(handle_t *handle, > split_flag |= (EXT4_EXT_MARK_UNWRIT2 | EXT4_EXT_DATA_VALID2); > } > flags |= EXT4_GET_BLOCKS_PRE_IO; > - return ext4_split_extent(handle, inode, ppath, map, split_flag, flags); > + path = ext4_split_extent(handle, inode, path, map, split_flag, flags, > + &allocated); > + if (IS_ERR(path)) { > + *ppath = NULL; > + return PTR_ERR(path); > + } > + *ppath = path; > + return allocated; > } > > static int ext4_convert_unwritten_extents_endio(handle_t *handle, > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:48PM +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_split_extent(), the following is done here: > > * The 'allocated' is changed from passing a value to passing an address. > * Its caller needs to update ppath if it uses ppath. > > No functional changes. > > Signed-off-by: Baokun Li <libaokun1@huawei.com> Looks good Baokun, feel free to add: Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Regards, ojaswin > --- > fs/ext4/extents.c | 97 ++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 47 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 0bd068ed324f..2a4f6c89858c 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3345,21 +3345,18 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle, > * c> Splits in three extents: Somone is splitting in middle of the extent > * > */ > -static int ext4_split_extent(handle_t *handle, > - struct inode *inode, > - struct ext4_ext_path **ppath, > - struct ext4_map_blocks *map, > - int split_flag, > - int flags) > +static struct ext4_ext_path *ext4_split_extent(handle_t *handle, > + struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_map_blocks *map, > + int split_flag, int flags, > + unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > ext4_lblk_t ee_block; > struct ext4_extent *ex; > unsigned int ee_len, depth; > - int err = 0; > int unwritten; > int split_flag1, flags1; > - int allocated = map->m_len; > > depth = ext_depth(inode); > ex = path[depth].p_ext; > @@ -3377,33 +3374,25 @@ static int ext4_split_extent(handle_t *handle, > split_flag1 |= EXT4_EXT_DATA_VALID1; > path = ext4_split_extent_at(handle, inode, path, > map->m_lblk + map->m_len, split_flag1, flags1); > - if (IS_ERR(path)) { > - err = PTR_ERR(path); > - *ppath = NULL; > - goto out; > + if (IS_ERR(path)) > + return path; > + /* > + * Update path is required because previous ext4_split_extent_at > + * may result in split of original leaf or extent zeroout. > + */ > + path = ext4_find_extent(inode, map->m_lblk, path, flags); > + if (IS_ERR(path)) > + return path; > + depth = ext_depth(inode); > + ex = path[depth].p_ext; > + if (!ex) { > + EXT4_ERROR_INODE(inode, "unexpected hole at %lu", > + (unsigned long) map->m_lblk); > + ext4_free_ext_path(path); > + return ERR_PTR(-EFSCORRUPTED); > } > - *ppath = path; > - } else { > - allocated = ee_len - (map->m_lblk - ee_block); > - } > - /* > - * Update path is required because previous ext4_split_extent_at() may > - * result in split of original leaf or extent zeroout. > - */ > - path = ext4_find_extent(inode, map->m_lblk, path, flags); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > - *ppath = path; > - depth = ext_depth(inode); > - ex = path[depth].p_ext; > - if (!ex) { > - EXT4_ERROR_INODE(inode, "unexpected hole at %lu", > - (unsigned long) map->m_lblk); > - return -EFSCORRUPTED; > + unwritten = ext4_ext_is_unwritten(ex); > } > - unwritten = ext4_ext_is_unwritten(ex); > > if (map->m_lblk >= ee_block) { > split_flag1 = split_flag & EXT4_EXT_DATA_VALID2; > @@ -3414,17 +3403,18 @@ static int ext4_split_extent(handle_t *handle, > } > path = ext4_split_extent_at(handle, inode, path, > map->m_lblk, split_flag1, flags); > - if (IS_ERR(path)) { > - err = PTR_ERR(path); > - *ppath = NULL; > - goto out; > - } > - *ppath = path; > + if (IS_ERR(path)) > + return path; > } > > + if (allocated) { > + if (map->m_lblk + map->m_len > ee_block + ee_len) > + *allocated = ee_len - (map->m_lblk - ee_block); > + else > + *allocated = map->m_len; > + } > ext4_ext_show_leaf(inode, path); > -out: > - return err ? err : allocated; > + return path; > } > > /* > @@ -3669,10 +3659,15 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, > } > > fallback: > - err = ext4_split_extent(handle, inode, ppath, &split_map, split_flag, > - flags); > - if (err > 0) > - err = 0; > + path = ext4_split_extent(handle, inode, path, &split_map, split_flag, > + flags, NULL); > + if (IS_ERR(path)) { > + err = PTR_ERR(path); > + *ppath = NULL; > + goto out; > + } > + err = 0; > + *ppath = path; > out: > /* If we have gotten a failure, don't zero out status tree */ > if (!err) { > @@ -3718,6 +3713,7 @@ static int ext4_split_convert_extents(handle_t *handle, > struct ext4_extent *ex; > unsigned int ee_len; > int split_flag = 0, depth; > + unsigned int allocated = 0; > > ext_debug(inode, "logical block %llu, max_blocks %u\n", > (unsigned long long)map->m_lblk, map->m_len); > @@ -3745,7 +3741,14 @@ static int ext4_split_convert_extents(handle_t *handle, > split_flag |= (EXT4_EXT_MARK_UNWRIT2 | EXT4_EXT_DATA_VALID2); > } > flags |= EXT4_GET_BLOCKS_PRE_IO; > - return ext4_split_extent(handle, inode, ppath, map, split_flag, flags); > + path = ext4_split_extent(handle, inode, path, map, split_flag, flags, > + &allocated); > + if (IS_ERR(path)) { > + *ppath = NULL; > + return PTR_ERR(path); > + } > + *ppath = path; > + return allocated; > } > > static int ext4_convert_unwritten_extents_endio(handle_t *handle, > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 0bd068ed324f..2a4f6c89858c 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3345,21 +3345,18 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle, * c> Splits in three extents: Somone is splitting in middle of the extent * */ -static int ext4_split_extent(handle_t *handle, - struct inode *inode, - struct ext4_ext_path **ppath, - struct ext4_map_blocks *map, - int split_flag, - int flags) +static struct ext4_ext_path *ext4_split_extent(handle_t *handle, + struct inode *inode, + struct ext4_ext_path *path, + struct ext4_map_blocks *map, + int split_flag, int flags, + unsigned int *allocated) { - struct ext4_ext_path *path = *ppath; ext4_lblk_t ee_block; struct ext4_extent *ex; unsigned int ee_len, depth; - int err = 0; int unwritten; int split_flag1, flags1; - int allocated = map->m_len; depth = ext_depth(inode); ex = path[depth].p_ext; @@ -3377,33 +3374,25 @@ static int ext4_split_extent(handle_t *handle, split_flag1 |= EXT4_EXT_DATA_VALID1; path = ext4_split_extent_at(handle, inode, path, map->m_lblk + map->m_len, split_flag1, flags1); - if (IS_ERR(path)) { - err = PTR_ERR(path); - *ppath = NULL; - goto out; + if (IS_ERR(path)) + return path; + /* + * Update path is required because previous ext4_split_extent_at + * may result in split of original leaf or extent zeroout. + */ + path = ext4_find_extent(inode, map->m_lblk, path, flags); + if (IS_ERR(path)) + return path; + depth = ext_depth(inode); + ex = path[depth].p_ext; + if (!ex) { + EXT4_ERROR_INODE(inode, "unexpected hole at %lu", + (unsigned long) map->m_lblk); + ext4_free_ext_path(path); + return ERR_PTR(-EFSCORRUPTED); } - *ppath = path; - } else { - allocated = ee_len - (map->m_lblk - ee_block); - } - /* - * Update path is required because previous ext4_split_extent_at() may - * result in split of original leaf or extent zeroout. - */ - path = ext4_find_extent(inode, map->m_lblk, path, flags); - if (IS_ERR(path)) { - *ppath = NULL; - return PTR_ERR(path); - } - *ppath = path; - depth = ext_depth(inode); - ex = path[depth].p_ext; - if (!ex) { - EXT4_ERROR_INODE(inode, "unexpected hole at %lu", - (unsigned long) map->m_lblk); - return -EFSCORRUPTED; + unwritten = ext4_ext_is_unwritten(ex); } - unwritten = ext4_ext_is_unwritten(ex); if (map->m_lblk >= ee_block) { split_flag1 = split_flag & EXT4_EXT_DATA_VALID2; @@ -3414,17 +3403,18 @@ static int ext4_split_extent(handle_t *handle, } path = ext4_split_extent_at(handle, inode, path, map->m_lblk, split_flag1, flags); - if (IS_ERR(path)) { - err = PTR_ERR(path); - *ppath = NULL; - goto out; - } - *ppath = path; + if (IS_ERR(path)) + return path; } + if (allocated) { + if (map->m_lblk + map->m_len > ee_block + ee_len) + *allocated = ee_len - (map->m_lblk - ee_block); + else + *allocated = map->m_len; + } ext4_ext_show_leaf(inode, path); -out: - return err ? err : allocated; + return path; } /* @@ -3669,10 +3659,15 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, } fallback: - err = ext4_split_extent(handle, inode, ppath, &split_map, split_flag, - flags); - if (err > 0) - err = 0; + path = ext4_split_extent(handle, inode, path, &split_map, split_flag, + flags, NULL); + if (IS_ERR(path)) { + err = PTR_ERR(path); + *ppath = NULL; + goto out; + } + err = 0; + *ppath = path; out: /* If we have gotten a failure, don't zero out status tree */ if (!err) { @@ -3718,6 +3713,7 @@ static int ext4_split_convert_extents(handle_t *handle, struct ext4_extent *ex; unsigned int ee_len; int split_flag = 0, depth; + unsigned int allocated = 0; ext_debug(inode, "logical block %llu, max_blocks %u\n", (unsigned long long)map->m_lblk, map->m_len); @@ -3745,7 +3741,14 @@ static int ext4_split_convert_extents(handle_t *handle, split_flag |= (EXT4_EXT_MARK_UNWRIT2 | EXT4_EXT_DATA_VALID2); } flags |= EXT4_GET_BLOCKS_PRE_IO; - return ext4_split_extent(handle, inode, ppath, map, split_flag, flags); + path = ext4_split_extent(handle, inode, path, map, split_flag, flags, + &allocated); + if (IS_ERR(path)) { + *ppath = NULL; + return PTR_ERR(path); + } + *ppath = path; + return allocated; } static int ext4_convert_unwritten_extents_endio(handle_t *handle,