| Message ID | 20240710040654.1714672-17-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:50, 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_convert_unwritten_extents_endio(), the > following is done here: > > * Free the extents path when an error is encountered. > * 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 | 43 +++++++++++++++++++++++-------------------- > 1 file changed, 23 insertions(+), 20 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index a41cbb8c4475..b7f443f98e9d 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3745,12 +3745,11 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, > allocated); > } > > -static int ext4_convert_unwritten_extents_endio(handle_t *handle, > - struct inode *inode, > - struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath) > +static struct ext4_ext_path * > +ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, > + struct ext4_map_blocks *map, > + struct ext4_ext_path *path) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent *ex; > ext4_lblk_t ee_block; > unsigned int ee_len; > @@ -3780,24 +3779,19 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > #endif > path = ext4_split_convert_extents(handle, inode, map, path, > EXT4_GET_BLOCKS_CONVERT, 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; > } > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - goto out; > + goto errout; > /* first mark the extent as initialized */ > ext4_ext_mark_initialized(ex); > > @@ -3808,9 +3802,15 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > > /* Mark modified extent as dirty */ > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > -out: > + if (err) > + goto errout; > + > ext4_ext_show_leaf(inode, path); > - return err; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static int > @@ -3938,10 +3938,13 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > } > /* IO end_io complete, convert the filled extent to written */ > if (flags & EXT4_GET_BLOCKS_CONVERT) { > - err = ext4_convert_unwritten_extents_endio(handle, inode, map, > - ppath); > - if (err < 0) > + *ppath = ext4_convert_unwritten_extents_endio(handle, inode, > + map, *ppath); > + if (IS_ERR(*ppath)) { > + err = PTR_ERR(*ppath); > + *ppath = NULL; > goto out2; > + } > ext4_update_inode_fsync_trans(handle, inode, 1); > goto map_out; > } > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:50PM +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_convert_unwritten_extents_endio(), the > following is done here: > > * Free the extents path when an error is encountered. > * 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 | 43 +++++++++++++++++++++++-------------------- > 1 file changed, 23 insertions(+), 20 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index a41cbb8c4475..b7f443f98e9d 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3745,12 +3745,11 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, > allocated); > } > > -static int ext4_convert_unwritten_extents_endio(handle_t *handle, > - struct inode *inode, > - struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath) > +static struct ext4_ext_path * > +ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, > + struct ext4_map_blocks *map, > + struct ext4_ext_path *path) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent *ex; > ext4_lblk_t ee_block; > unsigned int ee_len; > @@ -3780,24 +3779,19 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > #endif > path = ext4_split_convert_extents(handle, inode, map, path, > EXT4_GET_BLOCKS_CONVERT, 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; > } > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - goto out; > + goto errout; > /* first mark the extent as initialized */ > ext4_ext_mark_initialized(ex); > > @@ -3808,9 +3802,15 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > > /* Mark modified extent as dirty */ > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > -out: > + if (err) > + goto errout; > + > ext4_ext_show_leaf(inode, path); > - return err; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static int > @@ -3938,10 +3938,13 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > } > /* IO end_io complete, convert the filled extent to written */ > if (flags & EXT4_GET_BLOCKS_CONVERT) { > - err = ext4_convert_unwritten_extents_endio(handle, inode, map, > - ppath); > - if (err < 0) > + *ppath = ext4_convert_unwritten_extents_endio(handle, inode, > + map, *ppath); > + if (IS_ERR(*ppath)) { > + err = PTR_ERR(*ppath); > + *ppath = NULL; > goto out2; > + } > ext4_update_inode_fsync_trans(handle, inode, 1); > goto map_out; > } > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index a41cbb8c4475..b7f443f98e9d 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3745,12 +3745,11 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, allocated); } -static int ext4_convert_unwritten_extents_endio(handle_t *handle, - struct inode *inode, - struct ext4_map_blocks *map, - struct ext4_ext_path **ppath) +static struct ext4_ext_path * +ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode, + struct ext4_map_blocks *map, + struct ext4_ext_path *path) { - struct ext4_ext_path *path = *ppath; struct ext4_extent *ex; ext4_lblk_t ee_block; unsigned int ee_len; @@ -3780,24 +3779,19 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, #endif path = ext4_split_convert_extents(handle, inode, map, path, EXT4_GET_BLOCKS_CONVERT, 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; } err = ext4_ext_get_access(handle, inode, path + depth); if (err) - goto out; + goto errout; /* first mark the extent as initialized */ ext4_ext_mark_initialized(ex); @@ -3808,9 +3802,15 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, /* Mark modified extent as dirty */ err = ext4_ext_dirty(handle, inode, path + path->p_depth); -out: + if (err) + goto errout; + ext4_ext_show_leaf(inode, path); - return err; + return path; + +errout: + ext4_free_ext_path(path); + return ERR_PTR(err); } static int @@ -3938,10 +3938,13 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, } /* IO end_io complete, convert the filled extent to written */ if (flags & EXT4_GET_BLOCKS_CONVERT) { - err = ext4_convert_unwritten_extents_endio(handle, inode, map, - ppath); - if (err < 0) + *ppath = ext4_convert_unwritten_extents_endio(handle, inode, + map, *ppath); + if (IS_ERR(*ppath)) { + err = PTR_ERR(*ppath); + *ppath = NULL; goto out2; + } ext4_update_inode_fsync_trans(handle, inode, 1); goto map_out; }