Message ID | 20240710040654.1714672-12-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:45, 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_ext_insert_extent(), the following is done > here: > > * Free the extents path when an error is encountered. > * Its caller needs to update ppath if it uses ppath. > * Free path when npath is used, free npath when it is not used. > * The got_allocated_blocks label in ext4_ext_map_blocks() does not > update err now, so err is updated to 0 if the err returned by > ext4_ext_search_right() is greater than 0 and is about to enter > got_allocated_blocks. > > 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/ext4.h | 7 ++-- > fs/ext4/extents.c | 88 ++++++++++++++++++++++++------------------- > fs/ext4/fast_commit.c | 8 ++-- > fs/ext4/migrate.c | 5 ++- > 4 files changed, 61 insertions(+), 47 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index cbe8d6062c52..53b4c1f454e6 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -3710,9 +3710,10 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode, > extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode, > int num, > struct ext4_ext_path *path); > -extern int ext4_ext_insert_extent(handle_t *, struct inode *, > - struct ext4_ext_path **, > - struct ext4_extent *, int); > +extern struct ext4_ext_path *ext4_ext_insert_extent( > + handle_t *handle, struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_extent *newext, int gb_flags); > extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t, > struct ext4_ext_path *, > int flags); > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 0d6ce9e74b01..fc75390d591a 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1974,16 +1974,15 @@ static unsigned int ext4_ext_check_overlap(struct ext4_sb_info *sbi, > * inserts requested extent as new one into the tree, > * creating new leaf in the no-space case. > */ > -int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > - struct ext4_ext_path **ppath, > - struct ext4_extent *newext, int gb_flags) > +struct ext4_ext_path * > +ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_extent *newext, int gb_flags) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent_header *eh; > struct ext4_extent *ex, *fex; > struct ext4_extent *nearex; /* nearest extent */ > - struct ext4_ext_path *npath = NULL; > - int depth, len, err; > + int depth, len, err = 0; > ext4_lblk_t next; > int mb_flags = 0, unwritten; > > @@ -1991,14 +1990,16 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > mb_flags |= EXT4_MB_DELALLOC_RESERVED; > if (unlikely(ext4_ext_get_actual_len(newext) == 0)) { > EXT4_ERROR_INODE(inode, "ext4_ext_get_actual_len(newext) == 0"); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > depth = ext_depth(inode); > ex = path[depth].p_ext; > eh = path[depth].p_hdr; > if (unlikely(path[depth].p_hdr == NULL)) { > EXT4_ERROR_INODE(inode, "path[%d].p_hdr == NULL", depth); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > > /* try to insert block into found extent and return */ > @@ -2036,7 +2037,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > err = ext4_ext_get_access(handle, inode, > path + depth); > if (err) > - return err; > + goto errout; > unwritten = ext4_ext_is_unwritten(ex); > ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex) > + ext4_ext_get_actual_len(newext)); > @@ -2061,7 +2062,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > err = ext4_ext_get_access(handle, inode, > path + depth); > if (err) > - return err; > + goto errout; > > unwritten = ext4_ext_is_unwritten(ex); > ex->ee_block = newext->ee_block; > @@ -2086,21 +2087,26 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > if (le32_to_cpu(newext->ee_block) > le32_to_cpu(fex->ee_block)) > next = ext4_ext_next_leaf_block(path); > if (next != EXT_MAX_BLOCKS) { > + struct ext4_ext_path *npath; > + > ext_debug(inode, "next leaf block - %u\n", next); > - BUG_ON(npath != NULL); > npath = ext4_find_extent(inode, next, NULL, gb_flags); > - if (IS_ERR(npath)) > - return PTR_ERR(npath); > + if (IS_ERR(npath)) { > + err = PTR_ERR(npath); > + goto errout; > + } > BUG_ON(npath->p_depth != path->p_depth); > eh = npath[depth].p_hdr; > if (le16_to_cpu(eh->eh_entries) < le16_to_cpu(eh->eh_max)) { > ext_debug(inode, "next leaf isn't full(%d)\n", > le16_to_cpu(eh->eh_entries)); > + ext4_free_ext_path(path); > path = npath; > goto has_space; > } > ext_debug(inode, "next leaf has no free space(%d,%d)\n", > le16_to_cpu(eh->eh_entries), le16_to_cpu(eh->eh_max)); > + ext4_free_ext_path(npath); > } > > /* > @@ -2111,12 +2117,8 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > mb_flags |= EXT4_MB_USE_RESERVED; > path = ext4_ext_create_new_leaf(handle, inode, mb_flags, gb_flags, > path, newext); > - if (IS_ERR(path)) { > - *ppath = NULL; > - err = PTR_ERR(path); > - goto cleanup; > - } > - *ppath = path; > + if (IS_ERR(path)) > + return path; > depth = ext_depth(inode); > eh = path[depth].p_hdr; > > @@ -2125,7 +2127,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - goto cleanup; > + goto errout; > > if (!nearex) { > /* there is no extent in this leaf, create first one */ > @@ -2183,17 +2185,20 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > if (!(gb_flags & EXT4_GET_BLOCKS_PRE_IO)) > ext4_ext_try_to_merge(handle, inode, path, nearex); > > - > /* time to correct all indexes above */ > err = ext4_ext_correct_indexes(handle, inode, path); > if (err) > - goto cleanup; > + goto errout; > > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > + if (err) > + goto errout; > > -cleanup: > - ext4_free_ext_path(npath); > - return err; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static int ext4_fill_es_cache_info(struct inode *inode, > @@ -3248,24 +3253,29 @@ static int ext4_split_extent_at(handle_t *handle, > if (split_flag & EXT4_EXT_MARK_UNWRIT2) > ext4_ext_mark_unwritten(ex2); > > - err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags); > - if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) > + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); > + if (!IS_ERR(path)) { > + *ppath = path; > goto out; > + } > + *ppath = NULL; > + err = PTR_ERR(path); > + if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) > + return err; > > /* > - * Update path is required because previous ext4_ext_insert_extent() > - * may have freed or reallocated the path. Using EXT4_EX_NOFAIL > - * guarantees that ext4_find_extent() will not return -ENOMEM, > - * otherwise -ENOMEM will cause a retry in do_writepages(), and a > - * WARN_ON may be triggered in ext4_da_update_reserve_space() due to > - * an incorrect ee_len causing the i_reserved_data_blocks exception. > + * Get a new path to try to zeroout or fix the extent length. > + * Using EXT4_EX_NOFAIL guarantees that ext4_find_extent() > + * will not return -ENOMEM, otherwise -ENOMEM will cause a > + * retry in do_writepages(), and a 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, NULL, > 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); > @@ -3324,7 +3334,7 @@ static int ext4_split_extent_at(handle_t *handle, > ext4_ext_dirty(handle, inode, path + path->p_depth); > return err; > out: > - ext4_ext_show_leaf(inode, *ppath); > + ext4_ext_show_leaf(inode, path); > return err; > } > > @@ -4313,6 +4323,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > get_implied_cluster_alloc(inode->i_sb, map, &ex2, path)) { > ar.len = allocated = map->m_len; > newblock = map->m_pblk; > + err = 0; > goto got_allocated_blocks; > } > > @@ -4385,8 +4396,9 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > map->m_flags |= EXT4_MAP_UNWRITTEN; > } > > - err = ext4_ext_insert_extent(handle, inode, &path, &newex, flags); > - if (err) { > + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); > + if (IS_ERR(path)) { > + err = PTR_ERR(path); > if (allocated_clusters) { > int fb_flags = 0; > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 87c009e0c59a..1dee40477727 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1777,12 +1777,12 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > if (ext4_ext_is_unwritten(ex)) > ext4_ext_mark_unwritten(&newex); > down_write(&EXT4_I(inode)->i_data_sem); > - ret = ext4_ext_insert_extent( > - NULL, inode, &path, &newex, 0); > + path = ext4_ext_insert_extent(NULL, inode, > + path, &newex, 0); > up_write((&EXT4_I(inode)->i_data_sem)); > - ext4_free_ext_path(path); > - if (ret) > + if (IS_ERR(path)) > goto out; > + ext4_free_ext_path(path); > goto next; > } > > diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c > index d98ac2af8199..0f68b8a14560 100644 > --- a/fs/ext4/migrate.c > +++ b/fs/ext4/migrate.c > @@ -37,7 +37,6 @@ static int finish_range(handle_t *handle, struct inode *inode, > path = ext4_find_extent(inode, lb->first_block, NULL, 0); > if (IS_ERR(path)) { > retval = PTR_ERR(path); > - path = NULL; > goto err_out; > } > > @@ -53,7 +52,9 @@ static int finish_range(handle_t *handle, struct inode *inode, > retval = ext4_datasem_ensure_credits(handle, inode, needed, needed, 0); > if (retval < 0) > goto err_out; > - retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0); > + path = ext4_ext_insert_extent(handle, inode, path, &newext, 0); > + if (IS_ERR(path)) > + retval = PTR_ERR(path); > err_out: > up_write((&EXT4_I(inode)->i_data_sem)); > ext4_free_ext_path(path); > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:45PM +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_ext_insert_extent(), the following is done > here: > > * Free the extents path when an error is encountered. > * Its caller needs to update ppath if it uses ppath. > * Free path when npath is used, free npath when it is not used. > * The got_allocated_blocks label in ext4_ext_map_blocks() does not > update err now, so err is updated to 0 if the err returned by > ext4_ext_search_right() is greater than 0 and is about to enter > got_allocated_blocks. > > No functional changes. > > Signed-off-by: Baokun Li <libaokun1@huawei.com> Looks good, feel free to add: Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Regards, ojaswin > --- > fs/ext4/ext4.h | 7 ++-- > fs/ext4/extents.c | 88 ++++++++++++++++++++++++------------------- > fs/ext4/fast_commit.c | 8 ++-- > fs/ext4/migrate.c | 5 ++- > 4 files changed, 61 insertions(+), 47 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index cbe8d6062c52..53b4c1f454e6 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -3710,9 +3710,10 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode, > extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode, > int num, > struct ext4_ext_path *path); > -extern int ext4_ext_insert_extent(handle_t *, struct inode *, > - struct ext4_ext_path **, > - struct ext4_extent *, int); > +extern struct ext4_ext_path *ext4_ext_insert_extent( > + handle_t *handle, struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_extent *newext, int gb_flags); > extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t, > struct ext4_ext_path *, > int flags); > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 0d6ce9e74b01..fc75390d591a 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1974,16 +1974,15 @@ static unsigned int ext4_ext_check_overlap(struct ext4_sb_info *sbi, > * inserts requested extent as new one into the tree, > * creating new leaf in the no-space case. > */ > -int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > - struct ext4_ext_path **ppath, > - struct ext4_extent *newext, int gb_flags) > +struct ext4_ext_path * > +ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_extent *newext, int gb_flags) > { > - struct ext4_ext_path *path = *ppath; > struct ext4_extent_header *eh; > struct ext4_extent *ex, *fex; > struct ext4_extent *nearex; /* nearest extent */ > - struct ext4_ext_path *npath = NULL; > - int depth, len, err; > + int depth, len, err = 0; > ext4_lblk_t next; > int mb_flags = 0, unwritten; > > @@ -1991,14 +1990,16 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > mb_flags |= EXT4_MB_DELALLOC_RESERVED; > if (unlikely(ext4_ext_get_actual_len(newext) == 0)) { > EXT4_ERROR_INODE(inode, "ext4_ext_get_actual_len(newext) == 0"); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > depth = ext_depth(inode); > ex = path[depth].p_ext; > eh = path[depth].p_hdr; > if (unlikely(path[depth].p_hdr == NULL)) { > EXT4_ERROR_INODE(inode, "path[%d].p_hdr == NULL", depth); > - return -EFSCORRUPTED; > + err = -EFSCORRUPTED; > + goto errout; > } > > /* try to insert block into found extent and return */ > @@ -2036,7 +2037,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > err = ext4_ext_get_access(handle, inode, > path + depth); > if (err) > - return err; > + goto errout; > unwritten = ext4_ext_is_unwritten(ex); > ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex) > + ext4_ext_get_actual_len(newext)); > @@ -2061,7 +2062,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > err = ext4_ext_get_access(handle, inode, > path + depth); > if (err) > - return err; > + goto errout; > > unwritten = ext4_ext_is_unwritten(ex); > ex->ee_block = newext->ee_block; > @@ -2086,21 +2087,26 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > if (le32_to_cpu(newext->ee_block) > le32_to_cpu(fex->ee_block)) > next = ext4_ext_next_leaf_block(path); > if (next != EXT_MAX_BLOCKS) { > + struct ext4_ext_path *npath; > + > ext_debug(inode, "next leaf block - %u\n", next); > - BUG_ON(npath != NULL); > npath = ext4_find_extent(inode, next, NULL, gb_flags); > - if (IS_ERR(npath)) > - return PTR_ERR(npath); > + if (IS_ERR(npath)) { > + err = PTR_ERR(npath); > + goto errout; > + } > BUG_ON(npath->p_depth != path->p_depth); > eh = npath[depth].p_hdr; > if (le16_to_cpu(eh->eh_entries) < le16_to_cpu(eh->eh_max)) { > ext_debug(inode, "next leaf isn't full(%d)\n", > le16_to_cpu(eh->eh_entries)); > + ext4_free_ext_path(path); > path = npath; > goto has_space; > } > ext_debug(inode, "next leaf has no free space(%d,%d)\n", > le16_to_cpu(eh->eh_entries), le16_to_cpu(eh->eh_max)); > + ext4_free_ext_path(npath); > } > > /* > @@ -2111,12 +2117,8 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > mb_flags |= EXT4_MB_USE_RESERVED; > path = ext4_ext_create_new_leaf(handle, inode, mb_flags, gb_flags, > path, newext); > - if (IS_ERR(path)) { > - *ppath = NULL; > - err = PTR_ERR(path); > - goto cleanup; > - } > - *ppath = path; > + if (IS_ERR(path)) > + return path; > depth = ext_depth(inode); > eh = path[depth].p_hdr; > > @@ -2125,7 +2127,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > > err = ext4_ext_get_access(handle, inode, path + depth); > if (err) > - goto cleanup; > + goto errout; > > if (!nearex) { > /* there is no extent in this leaf, create first one */ > @@ -2183,17 +2185,20 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > if (!(gb_flags & EXT4_GET_BLOCKS_PRE_IO)) > ext4_ext_try_to_merge(handle, inode, path, nearex); > > - > /* time to correct all indexes above */ > err = ext4_ext_correct_indexes(handle, inode, path); > if (err) > - goto cleanup; > + goto errout; > > err = ext4_ext_dirty(handle, inode, path + path->p_depth); > + if (err) > + goto errout; > > -cleanup: > - ext4_free_ext_path(npath); > - return err; > + return path; > + > +errout: > + ext4_free_ext_path(path); > + return ERR_PTR(err); > } > > static int ext4_fill_es_cache_info(struct inode *inode, > @@ -3248,24 +3253,29 @@ static int ext4_split_extent_at(handle_t *handle, > if (split_flag & EXT4_EXT_MARK_UNWRIT2) > ext4_ext_mark_unwritten(ex2); > > - err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags); > - if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) > + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); > + if (!IS_ERR(path)) { > + *ppath = path; > goto out; > + } > + *ppath = NULL; > + err = PTR_ERR(path); > + if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) > + return err; > > /* > - * Update path is required because previous ext4_ext_insert_extent() > - * may have freed or reallocated the path. Using EXT4_EX_NOFAIL > - * guarantees that ext4_find_extent() will not return -ENOMEM, > - * otherwise -ENOMEM will cause a retry in do_writepages(), and a > - * WARN_ON may be triggered in ext4_da_update_reserve_space() due to > - * an incorrect ee_len causing the i_reserved_data_blocks exception. > + * Get a new path to try to zeroout or fix the extent length. > + * Using EXT4_EX_NOFAIL guarantees that ext4_find_extent() > + * will not return -ENOMEM, otherwise -ENOMEM will cause a > + * retry in do_writepages(), and a 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, NULL, > 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); > @@ -3324,7 +3334,7 @@ static int ext4_split_extent_at(handle_t *handle, > ext4_ext_dirty(handle, inode, path + path->p_depth); > return err; > out: > - ext4_ext_show_leaf(inode, *ppath); > + ext4_ext_show_leaf(inode, path); > return err; > } > > @@ -4313,6 +4323,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > get_implied_cluster_alloc(inode->i_sb, map, &ex2, path)) { > ar.len = allocated = map->m_len; > newblock = map->m_pblk; > + err = 0; > goto got_allocated_blocks; > } > > @@ -4385,8 +4396,9 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > map->m_flags |= EXT4_MAP_UNWRITTEN; > } > > - err = ext4_ext_insert_extent(handle, inode, &path, &newex, flags); > - if (err) { > + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); > + if (IS_ERR(path)) { > + err = PTR_ERR(path); > if (allocated_clusters) { > int fb_flags = 0; > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 87c009e0c59a..1dee40477727 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1777,12 +1777,12 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > if (ext4_ext_is_unwritten(ex)) > ext4_ext_mark_unwritten(&newex); > down_write(&EXT4_I(inode)->i_data_sem); > - ret = ext4_ext_insert_extent( > - NULL, inode, &path, &newex, 0); > + path = ext4_ext_insert_extent(NULL, inode, > + path, &newex, 0); > up_write((&EXT4_I(inode)->i_data_sem)); > - ext4_free_ext_path(path); > - if (ret) > + if (IS_ERR(path)) > goto out; > + ext4_free_ext_path(path); > goto next; > } > > diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c > index d98ac2af8199..0f68b8a14560 100644 > --- a/fs/ext4/migrate.c > +++ b/fs/ext4/migrate.c > @@ -37,7 +37,6 @@ static int finish_range(handle_t *handle, struct inode *inode, > path = ext4_find_extent(inode, lb->first_block, NULL, 0); > if (IS_ERR(path)) { > retval = PTR_ERR(path); > - path = NULL; > goto err_out; > } > > @@ -53,7 +52,9 @@ static int finish_range(handle_t *handle, struct inode *inode, > retval = ext4_datasem_ensure_credits(handle, inode, needed, needed, 0); > if (retval < 0) > goto err_out; > - retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0); > + path = ext4_ext_insert_extent(handle, inode, path, &newext, 0); > + if (IS_ERR(path)) > + retval = PTR_ERR(path); > err_out: > up_write((&EXT4_I(inode)->i_data_sem)); > ext4_free_ext_path(path); > -- > 2.39.2 >
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index cbe8d6062c52..53b4c1f454e6 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -3710,9 +3710,10 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode, extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num, struct ext4_ext_path *path); -extern int ext4_ext_insert_extent(handle_t *, struct inode *, - struct ext4_ext_path **, - struct ext4_extent *, int); +extern struct ext4_ext_path *ext4_ext_insert_extent( + handle_t *handle, struct inode *inode, + struct ext4_ext_path *path, + struct ext4_extent *newext, int gb_flags); extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t, struct ext4_ext_path *, int flags); diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 0d6ce9e74b01..fc75390d591a 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1974,16 +1974,15 @@ static unsigned int ext4_ext_check_overlap(struct ext4_sb_info *sbi, * inserts requested extent as new one into the tree, * creating new leaf in the no-space case. */ -int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, - struct ext4_ext_path **ppath, - struct ext4_extent *newext, int gb_flags) +struct ext4_ext_path * +ext4_ext_insert_extent(handle_t *handle, struct inode *inode, + struct ext4_ext_path *path, + struct ext4_extent *newext, int gb_flags) { - struct ext4_ext_path *path = *ppath; struct ext4_extent_header *eh; struct ext4_extent *ex, *fex; struct ext4_extent *nearex; /* nearest extent */ - struct ext4_ext_path *npath = NULL; - int depth, len, err; + int depth, len, err = 0; ext4_lblk_t next; int mb_flags = 0, unwritten; @@ -1991,14 +1990,16 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, mb_flags |= EXT4_MB_DELALLOC_RESERVED; if (unlikely(ext4_ext_get_actual_len(newext) == 0)) { EXT4_ERROR_INODE(inode, "ext4_ext_get_actual_len(newext) == 0"); - return -EFSCORRUPTED; + err = -EFSCORRUPTED; + goto errout; } depth = ext_depth(inode); ex = path[depth].p_ext; eh = path[depth].p_hdr; if (unlikely(path[depth].p_hdr == NULL)) { EXT4_ERROR_INODE(inode, "path[%d].p_hdr == NULL", depth); - return -EFSCORRUPTED; + err = -EFSCORRUPTED; + goto errout; } /* try to insert block into found extent and return */ @@ -2036,7 +2037,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, err = ext4_ext_get_access(handle, inode, path + depth); if (err) - return err; + goto errout; unwritten = ext4_ext_is_unwritten(ex); ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex) + ext4_ext_get_actual_len(newext)); @@ -2061,7 +2062,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, err = ext4_ext_get_access(handle, inode, path + depth); if (err) - return err; + goto errout; unwritten = ext4_ext_is_unwritten(ex); ex->ee_block = newext->ee_block; @@ -2086,21 +2087,26 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, if (le32_to_cpu(newext->ee_block) > le32_to_cpu(fex->ee_block)) next = ext4_ext_next_leaf_block(path); if (next != EXT_MAX_BLOCKS) { + struct ext4_ext_path *npath; + ext_debug(inode, "next leaf block - %u\n", next); - BUG_ON(npath != NULL); npath = ext4_find_extent(inode, next, NULL, gb_flags); - if (IS_ERR(npath)) - return PTR_ERR(npath); + if (IS_ERR(npath)) { + err = PTR_ERR(npath); + goto errout; + } BUG_ON(npath->p_depth != path->p_depth); eh = npath[depth].p_hdr; if (le16_to_cpu(eh->eh_entries) < le16_to_cpu(eh->eh_max)) { ext_debug(inode, "next leaf isn't full(%d)\n", le16_to_cpu(eh->eh_entries)); + ext4_free_ext_path(path); path = npath; goto has_space; } ext_debug(inode, "next leaf has no free space(%d,%d)\n", le16_to_cpu(eh->eh_entries), le16_to_cpu(eh->eh_max)); + ext4_free_ext_path(npath); } /* @@ -2111,12 +2117,8 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, mb_flags |= EXT4_MB_USE_RESERVED; path = ext4_ext_create_new_leaf(handle, inode, mb_flags, gb_flags, path, newext); - if (IS_ERR(path)) { - *ppath = NULL; - err = PTR_ERR(path); - goto cleanup; - } - *ppath = path; + if (IS_ERR(path)) + return path; depth = ext_depth(inode); eh = path[depth].p_hdr; @@ -2125,7 +2127,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, err = ext4_ext_get_access(handle, inode, path + depth); if (err) - goto cleanup; + goto errout; if (!nearex) { /* there is no extent in this leaf, create first one */ @@ -2183,17 +2185,20 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, if (!(gb_flags & EXT4_GET_BLOCKS_PRE_IO)) ext4_ext_try_to_merge(handle, inode, path, nearex); - /* time to correct all indexes above */ err = ext4_ext_correct_indexes(handle, inode, path); if (err) - goto cleanup; + goto errout; err = ext4_ext_dirty(handle, inode, path + path->p_depth); + if (err) + goto errout; -cleanup: - ext4_free_ext_path(npath); - return err; + return path; + +errout: + ext4_free_ext_path(path); + return ERR_PTR(err); } static int ext4_fill_es_cache_info(struct inode *inode, @@ -3248,24 +3253,29 @@ static int ext4_split_extent_at(handle_t *handle, if (split_flag & EXT4_EXT_MARK_UNWRIT2) ext4_ext_mark_unwritten(ex2); - err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags); - if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); + if (!IS_ERR(path)) { + *ppath = path; goto out; + } + *ppath = NULL; + err = PTR_ERR(path); + if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM) + return err; /* - * Update path is required because previous ext4_ext_insert_extent() - * may have freed or reallocated the path. Using EXT4_EX_NOFAIL - * guarantees that ext4_find_extent() will not return -ENOMEM, - * otherwise -ENOMEM will cause a retry in do_writepages(), and a - * WARN_ON may be triggered in ext4_da_update_reserve_space() due to - * an incorrect ee_len causing the i_reserved_data_blocks exception. + * Get a new path to try to zeroout or fix the extent length. + * Using EXT4_EX_NOFAIL guarantees that ext4_find_extent() + * will not return -ENOMEM, otherwise -ENOMEM will cause a + * retry in do_writepages(), and a 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, NULL, 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); @@ -3324,7 +3334,7 @@ static int ext4_split_extent_at(handle_t *handle, ext4_ext_dirty(handle, inode, path + path->p_depth); return err; out: - ext4_ext_show_leaf(inode, *ppath); + ext4_ext_show_leaf(inode, path); return err; } @@ -4313,6 +4323,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, get_implied_cluster_alloc(inode->i_sb, map, &ex2, path)) { ar.len = allocated = map->m_len; newblock = map->m_pblk; + err = 0; goto got_allocated_blocks; } @@ -4385,8 +4396,9 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, map->m_flags |= EXT4_MAP_UNWRITTEN; } - err = ext4_ext_insert_extent(handle, inode, &path, &newex, flags); - if (err) { + path = ext4_ext_insert_extent(handle, inode, path, &newex, flags); + if (IS_ERR(path)) { + err = PTR_ERR(path); if (allocated_clusters) { int fb_flags = 0; diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 87c009e0c59a..1dee40477727 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -1777,12 +1777,12 @@ static int ext4_fc_replay_add_range(struct super_block *sb, if (ext4_ext_is_unwritten(ex)) ext4_ext_mark_unwritten(&newex); down_write(&EXT4_I(inode)->i_data_sem); - ret = ext4_ext_insert_extent( - NULL, inode, &path, &newex, 0); + path = ext4_ext_insert_extent(NULL, inode, + path, &newex, 0); up_write((&EXT4_I(inode)->i_data_sem)); - ext4_free_ext_path(path); - if (ret) + if (IS_ERR(path)) goto out; + ext4_free_ext_path(path); goto next; } diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c index d98ac2af8199..0f68b8a14560 100644 --- a/fs/ext4/migrate.c +++ b/fs/ext4/migrate.c @@ -37,7 +37,6 @@ static int finish_range(handle_t *handle, struct inode *inode, path = ext4_find_extent(inode, lb->first_block, NULL, 0); if (IS_ERR(path)) { retval = PTR_ERR(path); - path = NULL; goto err_out; } @@ -53,7 +52,9 @@ static int finish_range(handle_t *handle, struct inode *inode, retval = ext4_datasem_ensure_credits(handle, inode, needed, needed, 0); if (retval < 0) goto err_out; - retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0); + path = ext4_ext_insert_extent(handle, inode, path, &newext, 0); + if (IS_ERR(path)) + retval = PTR_ERR(path); err_out: up_write((&EXT4_I(inode)->i_data_sem)); ext4_free_ext_path(path);