Re: [PATCH v2] ext4: drop tmp inode ownership before migration cleanup

[Jan Kara]

On Thu 22-10-26 15:02:35, Yichong Chen wrote:
> After the extent tree is transferred to the original inode,
> ext4_journal_ensure_credits() can fail before tmp_inode is cleared. The
> subsequent iput(tmp_inode) may then free the transferred tree.
> 
> Move the existing resets of tmp_inode's size, block count and checksum
> seed before ext4_journal_ensure_credits(). This leaves the temporary inode
> ready for eviction even if obtaining journal credits fails. Keep
> ext4_ext_tree_init(), which marks the inode dirty, after the credits check.
> 
> Fixes: a413036791d0 ("ext4: Provide function to handle transaction restarts")
> Signed-off-by: Yichong Chen <chenyichong@xxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> 
> Notes:
>     Changes in v2:
>     - Move the existing i_size, i_blocks and i_csum_seed resets before
>       ext4_journal_ensure_credits() instead of adding a conditional duplicate,
>       as suggested by Jan Kara.
>     
>     v1: https://lore.kernel.org/all/20260930080241.414973-1-chenyichong@xxxxxxxxxxxxx/
> 
>  fs/ext4/migrate.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 09931d3ba2c6..cf5239c42693 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -566,10 +566,6 @@ int ext4_ext_migrate(struct inode *inode)
>  			free_ext_block(handle, tmp_inode);
>  	}
>  
> -	/* We mark the tmp_inode dirty via ext4_ext_tree_init. */
> -	retval = ext4_journal_ensure_credits(handle, 1, 0);
> -	if (retval < 0)
> -		goto out_stop;
>  	/*
>  	 * Mark the tmp_inode as of size zero
>  	 */
> @@ -586,6 +582,11 @@ int ext4_ext_migrate(struct inode *inode)
>  	tmp_inode->i_blocks = 0;
>  	EXT4_I(tmp_inode)->i_csum_seed = tmp_csum_seed;
>  
> +	/* We mark the tmp_inode dirty via ext4_ext_tree_init. */
> +	retval = ext4_journal_ensure_credits(handle, 1, 0);
> +	if (retval < 0)
> +		goto out_stop;
> +
>  	/* Reset the extent details */
>  	ext4_ext_tree_init(handle, tmp_inode);
>  out_stop:
> 
> base-commit: 9a482184d910775cf648863148944945b74e2815
> -- 
> 2.51.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR