| Message ID | 20260918101240.3721805-1-chenyichong@uniontech.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: don't free the tmp_inode tree after a partial migration swap | expand |
> ext4_ext_migrate() cleans up the temporary inode when > ext4_ext_swap_inode_data() fails: > > retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode); > if (retval) > free_ext_block(handle, tmp_inode); > > ext4_ext_swap_inode_data() switches the inode to extents and copies the > tree root into it before it frees the old indirect block map, so it can > return an error after the swap has already happened. The copied root > points at index blocks that tmp_inode allocated, and free_ext_block() > walks that same tree and frees them, leaving the migrated inode with a > tree node that the block bitmap considers free. > > Only free the tmp_inode tree when the inode has not been switched to > extents, which is exactly the case where the swap did not happen. > > Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl") > Signed-off-by: Yichong Chen <chenyichong@uniontech.com> Sashiko has reviewed this patch and found no issues. It looks great!
On Fri 18-09-26 18:12:40, Yichong Chen wrote: > ext4_ext_migrate() cleans up the temporary inode when > ext4_ext_swap_inode_data() fails: > > retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode); > if (retval) > free_ext_block(handle, tmp_inode); > > ext4_ext_swap_inode_data() switches the inode to extents and copies the > tree root into it before it frees the old indirect block map, so it can > return an error after the swap has already happened. The copied root > points at index blocks that tmp_inode allocated, and free_ext_block() > walks that same tree and frees them, leaving the migrated inode with a > tree node that the block bitmap considers free. > > Only free the tmp_inode tree when the inode has not been switched to > extents, which is exactly the case where the swap did not happen. > > Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl") > Signed-off-by: Yichong Chen <chenyichong@uniontech.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/migrate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c > index 2ce587043945..e588b4157604 100644 > --- a/fs/ext4/migrate.c > +++ b/fs/ext4/migrate.c > @@ -562,7 +562,7 @@ int ext4_ext_migrate(struct inode *inode) > free_ext_block(handle, tmp_inode); > else { > retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode); > - if (retval) > + if (retval && !ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) > /* > * if we fail to swap inode data free the extent > * details of the tmp inode > -- > 2.51.0 >
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c index 2ce587043945..e588b4157604 100644 --- a/fs/ext4/migrate.c +++ b/fs/ext4/migrate.c @@ -562,7 +562,7 @@ int ext4_ext_migrate(struct inode *inode) free_ext_block(handle, tmp_inode); else { retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode); - if (retval) + if (retval && !ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) /* * if we fail to swap inode data free the extent * details of the tmp inode
ext4_ext_migrate() cleans up the temporary inode when ext4_ext_swap_inode_data() fails: retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode); if (retval) free_ext_block(handle, tmp_inode); ext4_ext_swap_inode_data() switches the inode to extents and copies the tree root into it before it frees the old indirect block map, so it can return an error after the swap has already happened. The copied root points at index blocks that tmp_inode allocated, and free_ext_block() walks that same tree and frees them, leaving the migrated inode with a tree node that the block bitmap considers free. Only free the tmp_inode tree when the inode has not been switched to extents, which is exactly the case where the swap did not happen. Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl") Signed-off-by: Yichong Chen <chenyichong@uniontech.com> --- fs/ext4/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)