| Message ID | 20240710040654.1714672-2-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:35, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > As suggested by Honza in Link，modify ext4_ext_rm_idx() to leave 'path' > alone and just index it like ext4_ext_correct_indexes() does it. This > facilitates adding error handling later. No functional changes. > > Suggested-by: Jan Kara <jack@suse.cz> > Link: https://lore.kernel.org/all/20230216130305.nrbtd42tppxhbynn@quack3/ > Signed-off-by: Baokun Li <libaokun1@huawei.com> Yeah, nice. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/extents.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index e067f2dd0335..bff3666c891a 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2279,27 +2279,26 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, > { > int err; > ext4_fsblk_t leaf; > + int k = depth - 1; > > /* free index block */ > - depth--; > - path = path + depth; > - leaf = ext4_idx_pblock(path->p_idx); > - if (unlikely(path->p_hdr->eh_entries == 0)) { > - EXT4_ERROR_INODE(inode, "path->p_hdr->eh_entries == 0"); > + leaf = ext4_idx_pblock(path[k].p_idx); > + if (unlikely(path[k].p_hdr->eh_entries == 0)) { > + EXT4_ERROR_INODE(inode, "path[%d].p_hdr->eh_entries == 0", k); > return -EFSCORRUPTED; > } > - err = ext4_ext_get_access(handle, inode, path); > + err = ext4_ext_get_access(handle, inode, path + k); > if (err) > return err; > > - if (path->p_idx != EXT_LAST_INDEX(path->p_hdr)) { > - int len = EXT_LAST_INDEX(path->p_hdr) - path->p_idx; > + if (path[k].p_idx != EXT_LAST_INDEX(path[k].p_hdr)) { > + int len = EXT_LAST_INDEX(path[k].p_hdr) - path[k].p_idx; > len *= sizeof(struct ext4_extent_idx); > - memmove(path->p_idx, path->p_idx + 1, len); > + memmove(path[k].p_idx, path[k].p_idx + 1, len); > } > > - le16_add_cpu(&path->p_hdr->eh_entries, -1); > - err = ext4_ext_dirty(handle, inode, path); > + le16_add_cpu(&path[k].p_hdr->eh_entries, -1); > + err = ext4_ext_dirty(handle, inode, path + k); > if (err) > return err; > ext_debug(inode, "index is empty, remove it, free block %llu\n", leaf); > @@ -2308,15 +2307,14 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, > ext4_free_blocks(handle, inode, NULL, leaf, 1, > EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET); > > - while (--depth >= 0) { > - if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr)) > + while (--k >= 0) { > + if (path[k + 1].p_idx != EXT_FIRST_INDEX(path[k + 1].p_hdr)) > break; > - path--; > - err = ext4_ext_get_access(handle, inode, path); > + err = ext4_ext_get_access(handle, inode, path + k); > if (err) > break; > - path->p_idx->ei_block = (path+1)->p_idx->ei_block; > - err = ext4_ext_dirty(handle, inode, path); > + path[k].p_idx->ei_block = path[k + 1].p_idx->ei_block; > + err = ext4_ext_dirty(handle, inode, path + k); > if (err) > break; > } > -- > 2.39.2 >
On Wed, Jul 10, 2024 at 12:06:35PM +0800, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > As suggested by Honza in Link，modify ext4_ext_rm_idx() to leave 'path' > alone and just index it like ext4_ext_correct_indexes() does it. This > facilitates adding error handling later. No functional changes. > > Suggested-by: Jan Kara <jack@suse.cz> > Link: https://lore.kernel.org/all/20230216130305.nrbtd42tppxhbynn@quack3/ > Signed-off-by: Baokun Li <libaokun1@huawei.com> > --- > fs/ext4/extents.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index e067f2dd0335..bff3666c891a 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2279,27 +2279,26 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, > { > int err; > ext4_fsblk_t leaf; > + int k = depth - 1; > > /* free index block */ > - depth--; > - path = path + depth; > - leaf = ext4_idx_pblock(path->p_idx); > - if (unlikely(path->p_hdr->eh_entries == 0)) { > - EXT4_ERROR_INODE(inode, "path->p_hdr->eh_entries == 0"); > + leaf = ext4_idx_pblock(path[k].p_idx); > + if (unlikely(path[k].p_hdr->eh_entries == 0)) { > + EXT4_ERROR_INODE(inode, "path[%d].p_hdr->eh_entries == 0", k); > return -EFSCORRUPTED; > } > - err = ext4_ext_get_access(handle, inode, path); > + err = ext4_ext_get_access(handle, inode, path + k); > if (err) > return err; > > - if (path->p_idx != EXT_LAST_INDEX(path->p_hdr)) { > - int len = EXT_LAST_INDEX(path->p_hdr) - path->p_idx; > + if (path[k].p_idx != EXT_LAST_INDEX(path[k].p_hdr)) { > + int len = EXT_LAST_INDEX(path[k].p_hdr) - path[k].p_idx; > len *= sizeof(struct ext4_extent_idx); > - memmove(path->p_idx, path->p_idx + 1, len); > + memmove(path[k].p_idx, path[k].p_idx + 1, len); > } > > - le16_add_cpu(&path->p_hdr->eh_entries, -1); > - err = ext4_ext_dirty(handle, inode, path); > + le16_add_cpu(&path[k].p_hdr->eh_entries, -1); > + err = ext4_ext_dirty(handle, inode, path + k); > if (err) > return err; > ext_debug(inode, "index is empty, remove it, free block %llu\n", leaf); > @@ -2308,15 +2307,14 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, > ext4_free_blocks(handle, inode, NULL, leaf, 1, > EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET); > > - while (--depth >= 0) { > - if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr)) > + while (--k >= 0) { > + if (path[k + 1].p_idx != EXT_FIRST_INDEX(path[k + 1].p_hdr)) > break; > - path--; > - err = ext4_ext_get_access(handle, inode, path); > + err = ext4_ext_get_access(handle, inode, path + k); > if (err) > break; > - path->p_idx->ei_block = (path+1)->p_idx->ei_block; > - err = ext4_ext_dirty(handle, inode, path); > + path[k].p_idx->ei_block = path[k + 1].p_idx->ei_block; > + err = ext4_ext_dirty(handle, inode, path + k); > if (err) > break; > } Look good to me as well. Feel free to add: Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Regards, ojaswin > -- > 2.39.2 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index e067f2dd0335..bff3666c891a 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -2279,27 +2279,26 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, { int err; ext4_fsblk_t leaf; + int k = depth - 1; /* free index block */ - depth--; - path = path + depth; - leaf = ext4_idx_pblock(path->p_idx); - if (unlikely(path->p_hdr->eh_entries == 0)) { - EXT4_ERROR_INODE(inode, "path->p_hdr->eh_entries == 0"); + leaf = ext4_idx_pblock(path[k].p_idx); + if (unlikely(path[k].p_hdr->eh_entries == 0)) { + EXT4_ERROR_INODE(inode, "path[%d].p_hdr->eh_entries == 0", k); return -EFSCORRUPTED; } - err = ext4_ext_get_access(handle, inode, path); + err = ext4_ext_get_access(handle, inode, path + k); if (err) return err; - if (path->p_idx != EXT_LAST_INDEX(path->p_hdr)) { - int len = EXT_LAST_INDEX(path->p_hdr) - path->p_idx; + if (path[k].p_idx != EXT_LAST_INDEX(path[k].p_hdr)) { + int len = EXT_LAST_INDEX(path[k].p_hdr) - path[k].p_idx; len *= sizeof(struct ext4_extent_idx); - memmove(path->p_idx, path->p_idx + 1, len); + memmove(path[k].p_idx, path[k].p_idx + 1, len); } - le16_add_cpu(&path->p_hdr->eh_entries, -1); - err = ext4_ext_dirty(handle, inode, path); + le16_add_cpu(&path[k].p_hdr->eh_entries, -1); + err = ext4_ext_dirty(handle, inode, path + k); if (err) return err; ext_debug(inode, "index is empty, remove it, free block %llu\n", leaf); @@ -2308,15 +2307,14 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, ext4_free_blocks(handle, inode, NULL, leaf, 1, EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET); - while (--depth >= 0) { - if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr)) + while (--k >= 0) { + if (path[k + 1].p_idx != EXT_FIRST_INDEX(path[k + 1].p_hdr)) break; - path--; - err = ext4_ext_get_access(handle, inode, path); + err = ext4_ext_get_access(handle, inode, path + k); if (err) break; - path->p_idx->ei_block = (path+1)->p_idx->ei_block; - err = ext4_ext_dirty(handle, inode, path); + path[k].p_idx->ei_block = path[k + 1].p_idx->ei_block; + err = ext4_ext_dirty(handle, inode, path + k); if (err) break; }