On Wed 10-07-24 12:06:53, libaokun@xxxxxxxxxxxxxxx wrote: > From: Baokun Li <libaokun1@xxxxxxxxxx> > > 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@xxxxxxxxxx> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> 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 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR