| Message ID | 20240710040654.1714672-16-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:49, 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_convert_extents(), the following is > done here: > > * 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 | 65 ++++++++++++++++++++++++----------------------- > 1 file changed, 33 insertions(+), 32 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 2a4f6c89858c..a41cbb8c4475 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3699,21 +3699,21 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, > * being filled will be convert to initialized by the end_io callback function > * via ext4_convert_unwritten_extents(). > * > - * Returns the size of unwritten extent to be written on success. > + * The size of unwritten extent to be written is passed to the caller via the > + * allocated pointer. Return an extent path pointer on success, or an error > + * pointer on failure. > */ > -static int ext4_split_convert_extents(handle_t *handle, > +static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, > struct inode *inode, > struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath, > - int flags) > + struct ext4_ext_path *path, > + int flags, unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > ext4_lblk_t eof_block; > ext4_lblk_t ee_block; > 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); > @@ -3741,14 +3741,8 @@ 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; > - 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; > + return ext4_split_extent(handle, inode, path, map, split_flag, flags, > + allocated); > } > > static int ext4_convert_unwritten_extents_endio(handle_t *handle, > @@ -3784,11 +3778,14 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > inode->i_ino, (unsigned long long)ee_block, ee_len, > (unsigned long long)map->m_lblk, map->m_len); > #endif > - err = ext4_split_convert_extents(handle, inode, map, ppath, > - EXT4_GET_BLOCKS_CONVERT); > - if (err < 0) > - return err; > - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); > + path = ext4_split_convert_extents(handle, inode, map, path, > + EXT4_GET_BLOCKS_CONVERT, NULL); > + if (IS_ERR(path)) { > + *ppath = NULL; > + return PTR_ERR(path); > + } > + > + path = ext4_find_extent(inode, map->m_lblk, path, 0); > if (IS_ERR(path)) { > *ppath = NULL; > return PTR_ERR(path); > @@ -3845,11 +3842,14 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > (unsigned long long)ee_block, ee_len); > > if (ee_block != map->m_lblk || ee_len > map->m_len) { > - err = ext4_split_convert_extents(handle, inode, map, ppath, > - EXT4_GET_BLOCKS_CONVERT_UNWRITTEN); > - if (err < 0) > - return err; > - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); > + 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); > + } > + > + path = ext4_find_extent(inode, map->m_lblk, path, 0); > if (IS_ERR(path)) { > *ppath = NULL; > return PTR_ERR(path); > @@ -3915,19 +3915,20 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > > /* get_block() before submitting IO, split the extent */ > if (flags & EXT4_GET_BLOCKS_PRE_IO) { > - ret = ext4_split_convert_extents(handle, inode, map, ppath, > - flags | EXT4_GET_BLOCKS_CONVERT); > - if (ret < 0) { > - err = ret; > + *ppath = ext4_split_convert_extents(handle, inode, map, *ppath, > + flags | EXT4_GET_BLOCKS_CONVERT, &allocated); > + if (IS_ERR(*ppath)) { > + err = PTR_ERR(*ppath); > + *ppath = NULL; > goto out2; > } > /* > - * shouldn't get a 0 return when splitting an extent unless > + * shouldn't get a 0 allocated when splitting an extent unless > * m_len is 0 (bug) or extent has been corrupted > */ > - if (unlikely(ret == 0)) { > + if (unlikely(allocated == 0)) { > EXT4_ERROR_INODE(inode, > - "unexpected ret == 0, m_len = %u", > + "unexpected allocated == 0, m_len = %u", > map->m_len); > err = -EFSCORRUPTED; > goto out2; > @@ -3988,9 +3989,9 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > err = -EFSCORRUPTED; > goto out2; > } > + allocated = ret; > > out: > - allocated = ret; > map->m_flags |= EXT4_MAP_NEW; > map_out: > map->m_flags |= EXT4_MAP_MAPPED; > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:49PM +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_convert_extents(), the following is > done here: > > * 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 | 65 ++++++++++++++++++++++++----------------------- > 1 file changed, 33 insertions(+), 32 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 2a4f6c89858c..a41cbb8c4475 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3699,21 +3699,21 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, > * being filled will be convert to initialized by the end_io callback function > * via ext4_convert_unwritten_extents(). > * > - * Returns the size of unwritten extent to be written on success. > + * The size of unwritten extent to be written is passed to the caller via the > + * allocated pointer. Return an extent path pointer on success, or an error > + * pointer on failure. > */ > -static int ext4_split_convert_extents(handle_t *handle, > +static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, > struct inode *inode, > struct ext4_map_blocks *map, > - struct ext4_ext_path **ppath, > - int flags) > + struct ext4_ext_path *path, > + int flags, unsigned int *allocated) > { > - struct ext4_ext_path *path = *ppath; > ext4_lblk_t eof_block; > ext4_lblk_t ee_block; > 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); > @@ -3741,14 +3741,8 @@ 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; > - 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; > + return ext4_split_extent(handle, inode, path, map, split_flag, flags, > + allocated); > } > > static int ext4_convert_unwritten_extents_endio(handle_t *handle, > @@ -3784,11 +3778,14 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, > inode->i_ino, (unsigned long long)ee_block, ee_len, > (unsigned long long)map->m_lblk, map->m_len); > #endif > - err = ext4_split_convert_extents(handle, inode, map, ppath, > - EXT4_GET_BLOCKS_CONVERT); > - if (err < 0) > - return err; > - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); > + path = ext4_split_convert_extents(handle, inode, map, path, > + EXT4_GET_BLOCKS_CONVERT, NULL); > + if (IS_ERR(path)) { > + *ppath = NULL; > + return PTR_ERR(path); > + } > + > + path = ext4_find_extent(inode, map->m_lblk, path, 0); > if (IS_ERR(path)) { > *ppath = NULL; > return PTR_ERR(path); > @@ -3845,11 +3842,14 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, > (unsigned long long)ee_block, ee_len); > > if (ee_block != map->m_lblk || ee_len > map->m_len) { > - err = ext4_split_convert_extents(handle, inode, map, ppath, > - EXT4_GET_BLOCKS_CONVERT_UNWRITTEN); > - if (err < 0) > - return err; > - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); > + 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); > + } > + > + path = ext4_find_extent(inode, map->m_lblk, path, 0); > if (IS_ERR(path)) { > *ppath = NULL; > return PTR_ERR(path); > @@ -3915,19 +3915,20 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > > /* get_block() before submitting IO, split the extent */ > if (flags & EXT4_GET_BLOCKS_PRE_IO) { > - ret = ext4_split_convert_extents(handle, inode, map, ppath, > - flags | EXT4_GET_BLOCKS_CONVERT); > - if (ret < 0) { > - err = ret; > + *ppath = ext4_split_convert_extents(handle, inode, map, *ppath, > + flags | EXT4_GET_BLOCKS_CONVERT, &allocated); > + if (IS_ERR(*ppath)) { > + err = PTR_ERR(*ppath); > + *ppath = NULL; > goto out2; > } > /* > - * shouldn't get a 0 return when splitting an extent unless > + * shouldn't get a 0 allocated when splitting an extent unless > * m_len is 0 (bug) or extent has been corrupted > */ > - if (unlikely(ret == 0)) { > + if (unlikely(allocated == 0)) { > EXT4_ERROR_INODE(inode, > - "unexpected ret == 0, m_len = %u", > + "unexpected allocated == 0, m_len = %u", > map->m_len); > err = -EFSCORRUPTED; > goto out2; > @@ -3988,9 +3989,9 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, > err = -EFSCORRUPTED; > goto out2; > } > + allocated = ret; > > out: > - allocated = ret; > map->m_flags |= EXT4_MAP_NEW; > map_out: > map->m_flags |= EXT4_MAP_MAPPED; > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 2a4f6c89858c..a41cbb8c4475 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3699,21 +3699,21 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, * being filled will be convert to initialized by the end_io callback function * via ext4_convert_unwritten_extents(). * - * Returns the size of unwritten extent to be written on success. + * The size of unwritten extent to be written is passed to the caller via the + * allocated pointer. Return an extent path pointer on success, or an error + * pointer on failure. */ -static int ext4_split_convert_extents(handle_t *handle, +static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle, struct inode *inode, struct ext4_map_blocks *map, - struct ext4_ext_path **ppath, - int flags) + struct ext4_ext_path *path, + int flags, unsigned int *allocated) { - struct ext4_ext_path *path = *ppath; ext4_lblk_t eof_block; ext4_lblk_t ee_block; 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); @@ -3741,14 +3741,8 @@ 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; - 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; + return ext4_split_extent(handle, inode, path, map, split_flag, flags, + allocated); } static int ext4_convert_unwritten_extents_endio(handle_t *handle, @@ -3784,11 +3778,14 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, inode->i_ino, (unsigned long long)ee_block, ee_len, (unsigned long long)map->m_lblk, map->m_len); #endif - err = ext4_split_convert_extents(handle, inode, map, ppath, - EXT4_GET_BLOCKS_CONVERT); - if (err < 0) - return err; - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); + path = ext4_split_convert_extents(handle, inode, map, path, + EXT4_GET_BLOCKS_CONVERT, NULL); + if (IS_ERR(path)) { + *ppath = NULL; + return PTR_ERR(path); + } + + path = ext4_find_extent(inode, map->m_lblk, path, 0); if (IS_ERR(path)) { *ppath = NULL; return PTR_ERR(path); @@ -3845,11 +3842,14 @@ convert_initialized_extent(handle_t *handle, struct inode *inode, (unsigned long long)ee_block, ee_len); if (ee_block != map->m_lblk || ee_len > map->m_len) { - err = ext4_split_convert_extents(handle, inode, map, ppath, - EXT4_GET_BLOCKS_CONVERT_UNWRITTEN); - if (err < 0) - return err; - path = ext4_find_extent(inode, map->m_lblk, *ppath, 0); + 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); + } + + path = ext4_find_extent(inode, map->m_lblk, path, 0); if (IS_ERR(path)) { *ppath = NULL; return PTR_ERR(path); @@ -3915,19 +3915,20 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, /* get_block() before submitting IO, split the extent */ if (flags & EXT4_GET_BLOCKS_PRE_IO) { - ret = ext4_split_convert_extents(handle, inode, map, ppath, - flags | EXT4_GET_BLOCKS_CONVERT); - if (ret < 0) { - err = ret; + *ppath = ext4_split_convert_extents(handle, inode, map, *ppath, + flags | EXT4_GET_BLOCKS_CONVERT, &allocated); + if (IS_ERR(*ppath)) { + err = PTR_ERR(*ppath); + *ppath = NULL; goto out2; } /* - * shouldn't get a 0 return when splitting an extent unless + * shouldn't get a 0 allocated when splitting an extent unless * m_len is 0 (bug) or extent has been corrupted */ - if (unlikely(ret == 0)) { + if (unlikely(allocated == 0)) { EXT4_ERROR_INODE(inode, - "unexpected ret == 0, m_len = %u", + "unexpected allocated == 0, m_len = %u", map->m_len); err = -EFSCORRUPTED; goto out2; @@ -3988,9 +3989,9 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode, err = -EFSCORRUPTED; goto out2; } + allocated = ret; out: - allocated = ret; map->m_flags |= EXT4_MAP_NEW; map_out: map->m_flags |= EXT4_MAP_MAPPED;