On Wed, Jul 10, 2024 at 12:06:52PM +0800, 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 ext4_ext_handle_unwritten_extents(), the
> following is done here:
>
> * Free the extents path when an error is encountered.
> * The 'allocated' is changed from passing a value to passing an address.
>
> No functional changes.
>
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
Looks good Baokun, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
Regards,
ojaswin
> ---
> fs/ext4/extents.c | 82 +++++++++++++++++++++--------------------------
> 1 file changed, 37 insertions(+), 45 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 59e80926fe3a..badadcd64e66 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3887,18 +3887,18 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
> return 0;
> }
>
> -static int
> +static struct ext4_ext_path *
> ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
> struct ext4_map_blocks *map,
> - struct ext4_ext_path **ppath, int flags,
> - unsigned int allocated, ext4_fsblk_t newblock)
> + struct ext4_ext_path *path, int flags,
> + unsigned int *allocated, ext4_fsblk_t newblock)
> {
> int err = 0;
>
> ext_debug(inode, "logical block %llu, max_blocks %u, flags 0x%x, allocated %u\n",
> (unsigned long long)map->m_lblk, map->m_len, flags,
> - allocated);
> - ext4_ext_show_leaf(inode, *ppath);
> + *allocated);
> + ext4_ext_show_leaf(inode, path);
>
> /*
> * When writing into unwritten space, we should not fail to
> @@ -3907,40 +3907,34 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
> flags |= EXT4_GET_BLOCKS_METADATA_NOFAIL;
>
> trace_ext4_ext_handle_unwritten_extents(inode, map, flags,
> - allocated, newblock);
> + *allocated, newblock);
>
> /* get_block() before submitting IO, split the extent */
> if (flags & EXT4_GET_BLOCKS_PRE_IO) {
> - *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;
> - }
> + path = ext4_split_convert_extents(handle, inode, map, path,
> + flags | EXT4_GET_BLOCKS_CONVERT, allocated);
> + if (IS_ERR(path))
> + return path;
> /*
> * shouldn't get a 0 allocated when splitting an extent unless
> * m_len is 0 (bug) or extent has been corrupted
> */
> - if (unlikely(allocated == 0)) {
> + if (unlikely(*allocated == 0)) {
> EXT4_ERROR_INODE(inode,
> "unexpected allocated == 0, m_len = %u",
> map->m_len);
> err = -EFSCORRUPTED;
> - goto out2;
> + goto errout;
> }
> map->m_flags |= EXT4_MAP_UNWRITTEN;
> goto out;
> }
> /* IO end_io complete, convert the filled extent to written */
> if (flags & EXT4_GET_BLOCKS_CONVERT) {
> - *ppath = ext4_convert_unwritten_extents_endio(handle, inode,
> - map, *ppath);
> - if (IS_ERR(*ppath)) {
> - err = PTR_ERR(*ppath);
> - *ppath = NULL;
> - goto out2;
> - }
> + path = ext4_convert_unwritten_extents_endio(handle, inode,
> + map, path);
> + if (IS_ERR(path))
> + return path;
> ext4_update_inode_fsync_trans(handle, inode, 1);
> goto map_out;
> }
> @@ -3972,23 +3966,20 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
> * For buffered writes, at writepage time, etc. Convert a
> * discovered unwritten extent to written.
> */
> - *ppath = ext4_ext_convert_to_initialized(handle, inode, map, *ppath,
> - flags, &allocated);
> - if (IS_ERR(*ppath)) {
> - err = PTR_ERR(*ppath);
> - *ppath = NULL;
> - goto out2;
> - }
> + path = ext4_ext_convert_to_initialized(handle, inode, map, path,
> + flags, allocated);
> + if (IS_ERR(path))
> + return path;
> ext4_update_inode_fsync_trans(handle, inode, 1);
> /*
> * shouldn't get a 0 allocated when converting an unwritten extent
> * unless m_len is 0 (bug) or extent has been corrupted
> */
> - if (unlikely(allocated == 0)) {
> + if (unlikely(*allocated == 0)) {
> EXT4_ERROR_INODE(inode, "unexpected allocated == 0, m_len = %u",
> map->m_len);
> err = -EFSCORRUPTED;
> - goto out2;
> + goto errout;
> }
>
> out:
> @@ -3997,12 +3988,15 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
> map->m_flags |= EXT4_MAP_MAPPED;
> out1:
> map->m_pblk = newblock;
> - if (allocated > map->m_len)
> - allocated = map->m_len;
> - map->m_len = allocated;
> - ext4_ext_show_leaf(inode, *ppath);
> -out2:
> - return err ? err : allocated;
> + if (*allocated > map->m_len)
> + *allocated = map->m_len;
> + map->m_len = *allocated;
> + ext4_ext_show_leaf(inode, path);
> + return path;
> +
> +errout:
> + ext4_free_ext_path(path);
> + return ERR_PTR(err);
> }
>
> /*
> @@ -4199,7 +4193,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> struct ext4_extent newex, *ex, ex2;
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> ext4_fsblk_t newblock = 0, pblk;
> - int err = 0, depth, ret;
> + int err = 0, depth;
> unsigned int allocated = 0, offset = 0;
> unsigned int allocated_clusters = 0;
> struct ext4_allocation_request ar;
> @@ -4273,13 +4267,11 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> goto out;
> }
>
> - ret = ext4_ext_handle_unwritten_extents(
> - handle, inode, map, &path, flags,
> - allocated, newblock);
> - if (ret < 0)
> - err = ret;
> - else
> - allocated = ret;
> + path = ext4_ext_handle_unwritten_extents(
> + handle, inode, map, path, flags,
> + &allocated, newblock);
> + if (IS_ERR(path))
> + err = PTR_ERR(path);
> goto out;
> }
> }
> --
> 2.39.2
>
