| Message ID | 20240710040654.1714672-20-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:53, 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 convert_initialized_extent(), the following is > done here: > > * Free the extents path when an error is encountered. > > 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 | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index badadcd64e66..737432bb316e 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3810,13 +3810,12 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, > return ERR_PTR(err); > } > > -static int > +static struct ext4_ext_path * > convert_initialized_extent(handle_t *handle, struct inode *inode, > struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath, > + struct ext4_ext_path *path, > unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent *ex; > ext4_lblk_t ee_block; > unsigned int ee_len; > @@ -3841,29 +3840,25 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > if (ee_block != map->m_lblk || ee_len > map->m_len) { > path = ext4_split_convert_extents(handle, inode, map, path, > EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, NULL); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > + if (IS_ERR(path)) > + return path; > > path = ext4_find_extent(inode, map->m_lblk, path, 0); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > - *ppath = path; > + 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); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > } > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - return err; > + goto errout; > /* first mark the extent as unwritten */ > ext4_ext_mark_unwritten(ex); > > @@ -3875,7 +3870,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > /* Mark modified extent as dirty */ > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > if (err) > - return err; > + goto errout; > ext4_ext_show_leaf(inode, path); > > ext4_update_inode_fsync_trans(handle, inode, 1); > @@ -3884,7 +3879,11 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > if (*allocated > map->m_len) > *allocated = map->m_len; > map->m_len = *allocated; > - return 0; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static struct ext4_ext_path * > @@ -4254,8 +4253,10 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > */ > if ((!ext4_ext_is_unwritten(ex)) && > (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN)) { > - err = convert_initialized_extent(handle, > - inode, map, &path, &allocated); > + path = convert_initialized_extent(handle, > + inode, map, path, &allocated); > + if (IS_ERR(path)) > + err = PTR_ERR(path); > goto out; > } else if (!ext4_ext_is_unwritten(ex)) { > map->m_flags |= EXT4_MAP_MAPPED; > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:53PM +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 convert_initialized_extent(), the following is > done here: > > * Free the extents path when an error is encountered. > > 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 | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index badadcd64e66..737432bb316e 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3810,13 +3810,12 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, > return ERR_PTR(err); > } > > -static int > +static struct ext4_ext_path * > convert_initialized_extent(handle_t *handle, struct inode *inode, > struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath, > + struct ext4_ext_path *path, > unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent *ex; > ext4_lblk_t ee_block; > unsigned int ee_len; > @@ -3841,29 +3840,25 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > if (ee_block != map->m_lblk || ee_len > map->m_len) { > path = ext4_split_convert_extents(handle, inode, map, path, > EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, NULL); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > + if (IS_ERR(path)) > + return path; > > path = ext4_find_extent(inode, map->m_lblk, path, 0); > - if (IS_ERR(path)) { > - *ppath = NULL; > - return PTR_ERR(path); > - } > - *ppath = path; > + 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); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > } > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - return err; > + goto errout; > /* first mark the extent as unwritten */ > ext4_ext_mark_unwritten(ex); > > @@ -3875,7 +3870,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > /* Mark modified extent as dirty */ > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > if (err) > - return err; > + goto errout; > ext4_ext_show_leaf(inode, path); > > ext4_update_inode_fsync_trans(handle, inode, 1); > @@ -3884,7 +3879,11 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > if (*allocated > map->m_len) > *allocated = map->m_len; > map->m_len = *allocated; > - return 0; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static struct ext4_ext_path * > @@ -4254,8 +4253,10 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > */ > if ((!ext4_ext_is_unwritten(ex)) && > (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN)) { > - err = convert_initialized_extent(handle, > - inode, map, &path, &allocated); > + path = convert_initialized_extent(handle, > + inode, map, path, &allocated); > + if (IS_ERR(path)) > + err = PTR_ERR(path); > goto out; > } else if (!ext4_ext_is_unwritten(ex)) { > map->m_flags |= EXT4_MAP_MAPPED; > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index badadcd64e66..737432bb316e 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3810,13 +3810,12 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, return ERR_PTR(err); } -static int +static struct ext4_ext_path * convert_initialized_extent(handle_t *handle, struct inode *inode, struct ext4_map_blocks *map, - struct ext4_ext_path **ppath, + struct ext4_ext_path *path, unsigned int *allocated) { - struct ext4_ext_path *path = *ppath; struct ext4_extent *ex; ext4_lblk_t ee_block; unsigned int ee_len; @@ -3841,29 +3840,25 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, if (ee_block != map->m_lblk || ee_len > map->m_len) { path = ext4_split_convert_extents(handle, inode, map, path, EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, NULL); - if (IS_ERR(path)) { - *ppath = NULL; - return PTR_ERR(path); - } + if (IS_ERR(path)) + return path; path = ext4_find_extent(inode, map->m_lblk, path, 0); - if (IS_ERR(path)) { - *ppath = NULL; - return PTR_ERR(path); - } - *ppath = path; + 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); - return -EFSCORRUPTED; + err = -EFSCORRUPTED; + goto errout; } } err = ext4_ext_get_access(handle, inode, path + depth); if (err) - return err; + goto errout; /* first mark the extent as unwritten */ ext4_ext_mark_unwritten(ex); @@ -3875,7 +3870,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, /* Mark modified extent as dirty */ err = ext4_ext_dirty(handle, inode, path + path->p_depth); if (err) - return err; + goto errout; ext4_ext_show_leaf(inode, path); ext4_update_inode_fsync_trans(handle, inode, 1); @@ -3884,7 +3879,11 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, if (*allocated > map->m_len) *allocated = map->m_len; map->m_len = *allocated; - return 0; + return path; + +errout: + ext4_free_ext_path(path); + return ERR_PTR(err); } static struct ext4_ext_path * @@ -4254,8 +4253,10 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, */ if ((!ext4_ext_is_unwritten(ex)) && (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN)) { - err = convert_initialized_extent(handle, - inode, map, &path, &allocated); + path = convert_initialized_extent(handle, + inode, map, path, &allocated); + if (IS_ERR(path)) + err = PTR_ERR(path); goto out; } else if (!ext4_ext_is_unwritten(ex)) { map->m_flags |= EXT4_MAP_MAPPED;