Re: [PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents

From: Jan Kara <jack@suse.cz>
To: Li Lingfeng <lilingfeng3@huawei.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu,
	adilger.kernel@dilger.ca, linux-kernel@vger.kernel.org,
	jack@suse.cz, yi.zhang@huawei.com, yukuai3@huawei.com,
	libaokun2@huawei.com
Subject: Re: [PATCH -next] ext4: recover csum seed of tmp_inode after migrating to extents
Date: Fri, 17 Jun 2022 12:44:41 +0200	[thread overview]
Message-ID: <20220617104441.lfhuca6tflg2oxah@quack3.lan> (raw)
In-Reply-To: <20220617062515.2113438-1-lilingfeng3@huawei.com>

On Fri 17-06-22 14:25:15, Li Lingfeng wrote:
> When migrating to extents, the checksum seed of temporary inode
> need to be replaced by inode's, otherwise the inode checksums
> will be incorrect when swapping the inodes data.
> 
> However, the temporary inode can not match it's checksum to
> itself since it has lost it's own checksum seed.
> 
> mkfs.ext4 -F /dev/sdc
> mount /dev/sdc /mnt/sdc
> xfs_io -fc "pwrite 4k 4k" -c "fsync" /mnt/sdc/testfile
> chattr -e /mnt/sdc/testfile
> chattr +e /mnt/sdc/testfile
> umount /dev/sdc
> fsck -fn /dev/sdc
> 
> ========
> ...
> Pass 1: Checking inodes, blocks, and sizes
> Inode 13 passes checks, but checksum does not match inode.  Fix? no
> ...
> ========
> 
> The fix is simple, save the checksum seed of temporary inode, and
> recover it after migrating to extents.
> 
> Fixes: e81c9302a6c3 ("ext4: set csum seed in tmp inode while migrating to extents")
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>

Yeah, the migration code and checksums never quite worked together. That's
the reason why we'd decided to deprecate this code. But I guess this fix is
simple enough and improves things so feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/migrate.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 49912814f3d8..04320715d61f 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -417,7 +417,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	struct inode *tmp_inode = NULL;
>  	struct migrate_struct lb;
>  	unsigned long max_entries;
> -	__u32 goal;
> +	__u32 goal, tmp_csum_seed;
>  	uid_t owner[2];
>  
>  	/*
> @@ -465,6 +465,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * the migration.
>  	 */
>  	ei = EXT4_I(inode);
> +	tmp_csum_seed = EXT4_I(tmp_inode)->i_csum_seed;
>  	EXT4_I(tmp_inode)->i_csum_seed = ei->i_csum_seed;
>  	i_size_write(tmp_inode, i_size_read(inode));
>  	/*
> @@ -575,6 +576,7 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * the inode is not visible to user space.
>  	 */
>  	tmp_inode->i_blocks = 0;
> +	EXT4_I(tmp_inode)->i_csum_seed = tmp_csum_seed;
>  
>  	/* Reset the extent details */
>  	ext4_ext_tree_init(handle, tmp_inode);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR