Message ID | 20240710040654.1714672-21-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:54, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > The ext4_find_extent() can update the extent path so that it does not have > to allocate and free the path repeatedly, thus reducing the consumption of > memory allocation and freeing in the following functions: > > ext4_ext_clear_bb > ext4_ext_replay_set_iblocks > ext4_fc_replay_add_range > ext4_fc_set_bitmaps_and_counters > > No functional changes. Note that ext4_find_extent() does not support error > pointers, so in this case set path to NULL first. > > 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 | 51 +++++++++++++++++++------------------------ > fs/ext4/fast_commit.c | 11 ++++++---- > 2 files changed, 29 insertions(+), 33 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 737432bb316e..5ff92cd50dc8 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -6068,12 +6068,9 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > if (IS_ERR(path)) > return PTR_ERR(path); > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > + if (!ex) > goto out; > - } > end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); > - ext4_free_ext_path(path); > > /* Count the number of data blocks */ > cur = 0; > @@ -6099,32 +6096,28 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > ret = skip_hole(inode, &cur); > if (ret < 0) > goto out; > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > goto out; > numblks += path->p_depth; > - ext4_free_ext_path(path); > while (cur < end) { > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > break; > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > - return 0; > - } > + if (!ex) > + goto cleanup; > + > cur = max(cur + 1, le32_to_cpu(ex->ee_block) + > ext4_ext_get_actual_len(ex)); > ret = skip_hole(inode, &cur); > - if (ret < 0) { > - ext4_free_ext_path(path); > + if (ret < 0) > break; > - } > - path2 = ext4_find_extent(inode, cur, NULL, 0); > - if (IS_ERR(path2)) { > - ext4_free_ext_path(path); > + > + path2 = ext4_find_extent(inode, cur, path2, 0); > + if (IS_ERR(path2)) > break; > - } > + > for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) { > cmp1 = cmp2 = 0; > if (i <= path->p_depth) > @@ -6136,13 +6129,14 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > if (cmp1 != cmp2 && cmp2 != 0) > numblks++; > } > - ext4_free_ext_path(path); > - ext4_free_ext_path(path2); > } > > out: > inode->i_blocks = numblks << (inode->i_sb->s_blocksize_bits - 9); > ext4_mark_inode_dirty(NULL, inode); > +cleanup: > + ext4_free_ext_path(path); > + ext4_free_ext_path(path2); > return 0; > } > > @@ -6163,12 +6157,9 @@ int ext4_ext_clear_bb(struct inode *inode) > if (IS_ERR(path)) > return PTR_ERR(path); > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > - return 0; > - } > + if (!ex) > + goto out; > end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); > - ext4_free_ext_path(path); > > cur = 0; > while (cur < end) { > @@ -6178,16 +6169,16 @@ int ext4_ext_clear_bb(struct inode *inode) > if (ret < 0) > break; > if (ret > 0) { > - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); > - if (!IS_ERR_OR_NULL(path)) { > + path = ext4_find_extent(inode, map.m_lblk, path, 0); > + if (!IS_ERR(path)) { > for (j = 0; j < path->p_depth; j++) { > - > ext4_mb_mark_bb(inode->i_sb, > path[j].p_block, 1, false); > ext4_fc_record_regions(inode->i_sb, inode->i_ino, > 0, path[j].p_block, 1, 1); > } > - ext4_free_ext_path(path); > + } else { > + path = NULL; > } > ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); > ext4_fc_record_regions(inode->i_sb, inode->i_ino, > @@ -6196,5 +6187,7 @@ int ext4_ext_clear_bb(struct inode *inode) > cur = cur + map.m_len; > } > > +out: > + ext4_free_ext_path(path); > return 0; > } > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 1dee40477727..1426d595fab7 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1766,7 +1766,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > > if (ret == 0) { > /* Range is not mapped */ > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > goto out; > memset(&newex, 0, sizeof(newex)); > @@ -1782,7 +1782,6 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > up_write((&EXT4_I(inode)->i_data_sem)); > if (IS_ERR(path)) > goto out; > - ext4_free_ext_path(path); > goto next; > } > > @@ -1830,6 +1829,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >> > sb->s_blocksize_bits); > out: > + ext4_free_ext_path(path); > iput(inode); > return 0; > } > @@ -1930,12 +1930,13 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) > break; > > if (ret > 0) { > - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); > + path = ext4_find_extent(inode, map.m_lblk, path, 0); > if (!IS_ERR(path)) { > for (j = 0; j < path->p_depth; j++) > ext4_mb_mark_bb(inode->i_sb, > path[j].p_block, 1, true); > - ext4_free_ext_path(path); > + } else { > + path = NULL; > } > cur += ret; > ext4_mb_mark_bb(inode->i_sb, map.m_pblk, > @@ -1946,6 +1947,8 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) > } > iput(inode); > } > + > + ext4_free_ext_path(path); > } > > /* > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:54PM +0800, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > The ext4_find_extent() can update the extent path so that it does not have > to allocate and free the path repeatedly, thus reducing the consumption of > memory allocation and freeing in the following functions: > > ext4_ext_clear_bb > ext4_ext_replay_set_iblocks > ext4_fc_replay_add_range > ext4_fc_set_bitmaps_and_counters > > No functional changes. Note that ext4_find_extent() does not support error > pointers, so in this case set path to NULL first. > > 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 | 51 +++++++++++++++++++------------------------ > fs/ext4/fast_commit.c | 11 ++++++---- > 2 files changed, 29 insertions(+), 33 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 737432bb316e..5ff92cd50dc8 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -6068,12 +6068,9 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > if (IS_ERR(path)) > return PTR_ERR(path); > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > + if (!ex) > goto out; > - } > end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); > - ext4_free_ext_path(path); > > /* Count the number of data blocks */ > cur = 0; > @@ -6099,32 +6096,28 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > ret = skip_hole(inode, &cur); > if (ret < 0) > goto out; > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > goto out; > numblks += path->p_depth; > - ext4_free_ext_path(path); > while (cur < end) { > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > break; > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > - return 0; > - } > + if (!ex) > + goto cleanup; > + > cur = max(cur + 1, le32_to_cpu(ex->ee_block) + > ext4_ext_get_actual_len(ex)); > ret = skip_hole(inode, &cur); > - if (ret < 0) { > - ext4_free_ext_path(path); > + if (ret < 0) > break; > - } > - path2 = ext4_find_extent(inode, cur, NULL, 0); > - if (IS_ERR(path2)) { > - ext4_free_ext_path(path); > + > + path2 = ext4_find_extent(inode, cur, path2, 0); > + if (IS_ERR(path2)) > break; > - } > + > for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) { > cmp1 = cmp2 = 0; > if (i <= path->p_depth) > @@ -6136,13 +6129,14 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) > if (cmp1 != cmp2 && cmp2 != 0) > numblks++; > } > - ext4_free_ext_path(path); > - ext4_free_ext_path(path2); > } > > out: > inode->i_blocks = numblks << (inode->i_sb->s_blocksize_bits - 9); > ext4_mark_inode_dirty(NULL, inode); > +cleanup: > + ext4_free_ext_path(path); > + ext4_free_ext_path(path2); > return 0; > } > > @@ -6163,12 +6157,9 @@ int ext4_ext_clear_bb(struct inode *inode) > if (IS_ERR(path)) > return PTR_ERR(path); > ex = path[path->p_depth].p_ext; > - if (!ex) { > - ext4_free_ext_path(path); > - return 0; > - } > + if (!ex) > + goto out; > end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); > - ext4_free_ext_path(path); > > cur = 0; > while (cur < end) { > @@ -6178,16 +6169,16 @@ int ext4_ext_clear_bb(struct inode *inode) > if (ret < 0) > break; > if (ret > 0) { > - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); > - if (!IS_ERR_OR_NULL(path)) { > + path = ext4_find_extent(inode, map.m_lblk, path, 0); > + if (!IS_ERR(path)) { > for (j = 0; j < path->p_depth; j++) { > - > ext4_mb_mark_bb(inode->i_sb, > path[j].p_block, 1, false); > ext4_fc_record_regions(inode->i_sb, inode->i_ino, > 0, path[j].p_block, 1, 1); > } > - ext4_free_ext_path(path); > + } else { > + path = NULL; > } > ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); > ext4_fc_record_regions(inode->i_sb, inode->i_ino, > @@ -6196,5 +6187,7 @@ int ext4_ext_clear_bb(struct inode *inode) > cur = cur + map.m_len; > } > > +out: > + ext4_free_ext_path(path); > return 0; > } > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 1dee40477727..1426d595fab7 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1766,7 +1766,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > > if (ret == 0) { > /* Range is not mapped */ > - path = ext4_find_extent(inode, cur, NULL, 0); > + path = ext4_find_extent(inode, cur, path, 0); > if (IS_ERR(path)) > goto out; > memset(&newex, 0, sizeof(newex)); > @@ -1782,7 +1782,6 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > up_write((&EXT4_I(inode)->i_data_sem)); > if (IS_ERR(path)) > goto out; > - ext4_free_ext_path(path); > goto next; > } > > @@ -1830,6 +1829,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, > ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >> > sb->s_blocksize_bits); > out: > + ext4_free_ext_path(path); > iput(inode); > return 0; > } > @@ -1930,12 +1930,13 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) > break; > > if (ret > 0) { > - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); > + path = ext4_find_extent(inode, map.m_lblk, path, 0); > if (!IS_ERR(path)) { > for (j = 0; j < path->p_depth; j++) > ext4_mb_mark_bb(inode->i_sb, > path[j].p_block, 1, true); > - ext4_free_ext_path(path); > + } else { > + path = NULL; > } > cur += ret; > ext4_mb_mark_bb(inode->i_sb, map.m_pblk, > @@ -1946,6 +1947,8 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) > } > iput(inode); > } > + > + ext4_free_ext_path(path); > } > > /* > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 737432bb316e..5ff92cd50dc8 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -6068,12 +6068,9 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) if (IS_ERR(path)) return PTR_ERR(path); ex = path[path->p_depth].p_ext; - if (!ex) { - ext4_free_ext_path(path); + if (!ex) goto out; - } end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); - ext4_free_ext_path(path); /* Count the number of data blocks */ cur = 0; @@ -6099,32 +6096,28 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) ret = skip_hole(inode, &cur); if (ret < 0) goto out; - path = ext4_find_extent(inode, cur, NULL, 0); + path = ext4_find_extent(inode, cur, path, 0); if (IS_ERR(path)) goto out; numblks += path->p_depth; - ext4_free_ext_path(path); while (cur < end) { - path = ext4_find_extent(inode, cur, NULL, 0); + path = ext4_find_extent(inode, cur, path, 0); if (IS_ERR(path)) break; ex = path[path->p_depth].p_ext; - if (!ex) { - ext4_free_ext_path(path); - return 0; - } + if (!ex) + goto cleanup; + cur = max(cur + 1, le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex)); ret = skip_hole(inode, &cur); - if (ret < 0) { - ext4_free_ext_path(path); + if (ret < 0) break; - } - path2 = ext4_find_extent(inode, cur, NULL, 0); - if (IS_ERR(path2)) { - ext4_free_ext_path(path); + + path2 = ext4_find_extent(inode, cur, path2, 0); + if (IS_ERR(path2)) break; - } + for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) { cmp1 = cmp2 = 0; if (i <= path->p_depth) @@ -6136,13 +6129,14 @@ int ext4_ext_replay_set_iblocks(struct inode *inode) if (cmp1 != cmp2 && cmp2 != 0) numblks++; } - ext4_free_ext_path(path); - ext4_free_ext_path(path2); } out: inode->i_blocks = numblks << (inode->i_sb->s_blocksize_bits - 9); ext4_mark_inode_dirty(NULL, inode); +cleanup: + ext4_free_ext_path(path); + ext4_free_ext_path(path2); return 0; } @@ -6163,12 +6157,9 @@ int ext4_ext_clear_bb(struct inode *inode) if (IS_ERR(path)) return PTR_ERR(path); ex = path[path->p_depth].p_ext; - if (!ex) { - ext4_free_ext_path(path); - return 0; - } + if (!ex) + goto out; end = le32_to_cpu(ex->ee_block) + ext4_ext_get_actual_len(ex); - ext4_free_ext_path(path); cur = 0; while (cur < end) { @@ -6178,16 +6169,16 @@ int ext4_ext_clear_bb(struct inode *inode) if (ret < 0) break; if (ret > 0) { - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); - if (!IS_ERR_OR_NULL(path)) { + path = ext4_find_extent(inode, map.m_lblk, path, 0); + if (!IS_ERR(path)) { for (j = 0; j < path->p_depth; j++) { - ext4_mb_mark_bb(inode->i_sb, path[j].p_block, 1, false); ext4_fc_record_regions(inode->i_sb, inode->i_ino, 0, path[j].p_block, 1, 1); } - ext4_free_ext_path(path); + } else { + path = NULL; } ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); ext4_fc_record_regions(inode->i_sb, inode->i_ino, @@ -6196,5 +6187,7 @@ int ext4_ext_clear_bb(struct inode *inode) cur = cur + map.m_len; } +out: + ext4_free_ext_path(path); return 0; } diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 1dee40477727..1426d595fab7 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -1766,7 +1766,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, if (ret == 0) { /* Range is not mapped */ - path = ext4_find_extent(inode, cur, NULL, 0); + path = ext4_find_extent(inode, cur, path, 0); if (IS_ERR(path)) goto out; memset(&newex, 0, sizeof(newex)); @@ -1782,7 +1782,6 @@ static int ext4_fc_replay_add_range(struct super_block *sb, up_write((&EXT4_I(inode)->i_data_sem)); if (IS_ERR(path)) goto out; - ext4_free_ext_path(path); goto next; } @@ -1830,6 +1829,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >> sb->s_blocksize_bits); out: + ext4_free_ext_path(path); iput(inode); return 0; } @@ -1930,12 +1930,13 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) break; if (ret > 0) { - path = ext4_find_extent(inode, map.m_lblk, NULL, 0); + path = ext4_find_extent(inode, map.m_lblk, path, 0); if (!IS_ERR(path)) { for (j = 0; j < path->p_depth; j++) ext4_mb_mark_bb(inode->i_sb, path[j].p_block, 1, true); - ext4_free_ext_path(path); + } else { + path = NULL; } cur += ret; ext4_mb_mark_bb(inode->i_sb, map.m_pblk, @@ -1946,6 +1947,8 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) } iput(inode); } + + ext4_free_ext_path(path); } /*