Re: [PATCH 08/20] ext4: get rid of ppath in ext4_find_extent()

From: Ojaswin Mujoo
Date: Tue Jul 30 2024 - 06:04:36 EST

Next message: Manivannan Sadhasivam: "[PATCH] phy: qcom: qmp: Add debug prints for register writes"
Previous message: Jon Hunter: "Re: [RESEND PATCH net-next v3 2/4] net: phy: aquantia: wait for FW reset before checking the vendor ID"
In reply to: Jan Kara: "Re: [PATCH 08/20] ext4: get rid of ppath in ext4_find_extent()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jul 10, 2024 at 12:06:42PM +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.
>
> Getting rid of ppath in ext4_find_extent() requires its caller to update
> ppath. These ppaths will also be dropped later. No functional changes.
>
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/extents.c | 52 +++++++++++++++++++++++--------------------
> fs/ext4/move_extent.c | 6 ++---
> 3 files changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 8007abd4972d..cbe8d6062c52 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3714,7 +3714,7 @@ extern int ext4_ext_insert_extent(handle_t *, struct inode *,
> struct ext4_ext_path **,
> struct ext4_extent *, int);
> extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
> - struct ext4_ext_path **,
> + struct ext4_ext_path *,
> int flags);
> extern void ext4_free_ext_path(struct ext4_ext_path *);
> extern int ext4_ext_check_inode(struct inode *inode);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b1cfce5b57d2..5217e6f53467 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -884,11 +884,10 @@ void ext4_ext_tree_init(handle_t *handle, struct inode *inode)
>
> struct ext4_ext_path *
> ext4_find_extent(struct inode *inode, ext4_lblk_t block,
> - struct ext4_ext_path **orig_path, int flags)
> + struct ext4_ext_path *path, int flags)
> {
> struct ext4_extent_header *eh;
> struct buffer_head *bh;
> - struct ext4_ext_path *path = orig_path ? *orig_path : NULL;
> short int depth, i, ppos = 0;
> int ret;
> gfp_t gfp_flags = GFP_NOFS;
> @@ -909,7 +908,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
> ext4_ext_drop_refs(path);
> if (depth > path[0].p_maxdepth) {
> kfree(path);
> - *orig_path = path = NULL;
> + path = NULL;
> }
> }
> if (!path) {
> @@ -964,8 +963,6 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
>
> err:
> ext4_free_ext_path(path);
> - if (orig_path)
> - *orig_path = NULL;
> return ERR_PTR(ret);
> }
>
> @@ -1430,7 +1427,7 @@ static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
> /* refill path */
> path = ext4_find_extent(inode,
> (ext4_lblk_t)le32_to_cpu(newext->ee_block),
> - ppath, gb_flags);
> + path, gb_flags);
> if (IS_ERR(path))
> err = PTR_ERR(path);
> } else {
> @@ -1442,7 +1439,7 @@ static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
> /* refill path */
> path = ext4_find_extent(inode,
> (ext4_lblk_t)le32_to_cpu(newext->ee_block),
> - ppath, gb_flags);
> + path, gb_flags);
> if (IS_ERR(path)) {
> err = PTR_ERR(path);
> goto out;
> @@ -1458,8 +1455,8 @@ static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
> goto repeat;
> }
> }
> -
> out:
> + *ppath = IS_ERR(path) ? NULL : path;
> return err;
> }
>
> @@ -3260,11 +3257,12 @@ static int ext4_split_extent_at(handle_t *handle,
> * WARN_ON may be triggered in ext4_da_update_reserve_space() due to
> * an incorrect ee_len causing the i_reserved_data_blocks exception.
> */
> - path = ext4_find_extent(inode, ee_block, ppath,
> + path = ext4_find_extent(inode, ee_block, *ppath,
> flags | EXT4_EX_NOFAIL);
> if (IS_ERR(path)) {
> EXT4_ERROR_INODE(inode, "Failed split extent on %u, err %ld",
> split, PTR_ERR(path));
> + *ppath = NULL;
> return PTR_ERR(path);
> }
> depth = ext_depth(inode);
> @@ -3379,9 +3377,12 @@ static int ext4_split_extent(handle_t *handle,
> * Update path is required because previous ext4_split_extent_at() may
> * result in split of original leaf or extent zeroout.
> */
> - path = ext4_find_extent(inode, map->m_lblk, ppath, flags);
> - if (IS_ERR(path))
> + path = ext4_find_extent(inode, map->m_lblk, *ppath, flags);
> + if (IS_ERR(path)) {
> + *ppath = NULL;
> return PTR_ERR(path);
> + }
> + *ppath = path;
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> if (!ex) {
> @@ -3767,9 +3768,12 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
> EXT4_GET_BLOCKS_CONVERT);
> if (err < 0)
> return err;
> - path = ext4_find_extent(inode, map->m_lblk, ppath, 0);
> - if (IS_ERR(path))
> + path = ext4_find_extent(inode, map->m_lblk, *ppath, 0);
> + if (IS_ERR(path)) {
> + *ppath = NULL;
> return PTR_ERR(path);
> + }
> + *ppath = path;
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> }
> @@ -3825,9 +3829,12 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
> EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
> if (err < 0)
> return err;
> - path = ext4_find_extent(inode, map->m_lblk, ppath, 0);
> - if (IS_ERR(path))
> + path = ext4_find_extent(inode, map->m_lblk, *ppath, 0);
> + if (IS_ERR(path)) {
> + *ppath = NULL;
> return PTR_ERR(path);
> + }
> + *ppath = path;
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> if (!ex) {
> @@ -5224,7 +5231,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
> * won't be shifted beyond EXT_MAX_BLOCKS.
> */
> if (SHIFT == SHIFT_LEFT) {
> - path = ext4_find_extent(inode, start - 1, &path,
> + path = ext4_find_extent(inode, start - 1, path,
> EXT4_EX_NOCACHE);
> if (IS_ERR(path))
> return PTR_ERR(path);
> @@ -5273,7 +5280,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
> * becomes NULL to indicate the end of the loop.
> */
> while (iterator && start <= stop) {
> - path = ext4_find_extent(inode, *iterator, &path,
> + path = ext4_find_extent(inode, *iterator, path,
> EXT4_EX_NOCACHE);
> if (IS_ERR(path))
> return PTR_ERR(path);
> @@ -5854,11 +5861,8 @@ int ext4_clu_mapped(struct inode *inode, ext4_lblk_t lclu)
>
> /* search for the extent closest to the first block in the cluster */
> path = ext4_find_extent(inode, EXT4_C2B(sbi, lclu), NULL, 0);
> - if (IS_ERR(path)) {
> - err = PTR_ERR(path);
> - path = NULL;
> - goto out;
> - }
> + if (IS_ERR(path))
> + return PTR_ERR(path);
>
> depth = ext_depth(inode);
>
> @@ -5942,7 +5946,7 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
> if (ret)
> goto out;
>
> - path = ext4_find_extent(inode, start, &path, 0);
> + path = ext4_find_extent(inode, start, path, 0);
> if (IS_ERR(path))
> return PTR_ERR(path);
> ex = path[path->p_depth].p_ext;
> @@ -5956,7 +5960,7 @@ int ext4_ext_replay_update_ex(struct inode *inode, ext4_lblk_t start,
> if (ret)
> goto out;
>
> - path = ext4_find_extent(inode, start, &path, 0);
> + path = ext4_find_extent(inode, start, path, 0);
> if (IS_ERR(path))
> return PTR_ERR(path);
> ex = path[path->p_depth].p_ext;
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 204f53b23622..b0ab19a913bf 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -26,14 +26,14 @@ static inline int
> get_ext_path(struct inode *inode, ext4_lblk_t lblock,
> struct ext4_ext_path **ppath)
> {
> - struct ext4_ext_path *path;
> + struct ext4_ext_path *path = *ppath;
>
> - path = ext4_find_extent(inode, lblock, ppath, EXT4_EX_NOCACHE);
> + *ppath = NULL;
> + path = ext4_find_extent(inode, lblock, path, EXT4_EX_NOCACHE);
> if (IS_ERR(path))
> return PTR_ERR(path);
> if (path[ext_depth(inode)].p_ext == NULL) {
> ext4_free_ext_path(path);
> - *ppath = NULL;
> return -ENODATA;
> }
> *ppath = path;
Hi Baokun,

The changes look good to me, afaict we've handled updating ppath in all
the places correctly.

Feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Regards,
ojaswin
> --
> 2.39.2

>

Next message: Manivannan Sadhasivam: "[PATCH] phy: qcom: qmp: Add debug prints for register writes"
Previous message: Jon Hunter: "Re: [RESEND PATCH net-next v3 2/4] net: phy: aquantia: wait for FW reset before checking the vendor ID"
In reply to: Jan Kara: "Re: [PATCH 08/20] ext4: get rid of ppath in ext4_find_extent()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]