On Tue, Dec 14, 2021 at 04:49:45PM -0800, Darrick J. Wong wrote: > On Tue, Dec 14, 2021 at 05:50:58PM +0000, Luís Henriques wrote: > > When migrating to extents, the temporary inode will have it's own checksum > > seed. This means that, when swapping the inodes data, the inode checksums > > will be incorrect. > > > > This can be fixed by recalculating the extents checksums again. Or simply > > by copying the seed into the temporary inode. > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=213357 > > Reported-by: Jeroen van Wolffelaar <jeroen@xxxxxxxxxxxxx> > > Signed-off-by: Luís Henriques <lhenriques@xxxxxxx> > > --- > > fs/ext4/migrate.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > changes since v1: > > > > * Dropped tmp_ei variable > > * ->i_csum_seed is now initialised immediately after tmp_inode is created > > * New comment about the seed initialization and stating that recovery > > needs to be fixed. > > > > Cheers, > > -- > > Luís > > > > diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c > > index 7e0b4f81c6c0..36dfc88ce05b 100644 > > --- a/fs/ext4/migrate.c > > +++ b/fs/ext4/migrate.c > > @@ -459,6 +459,17 @@ int ext4_ext_migrate(struct inode *inode) > > ext4_journal_stop(handle); > > goto out_unlock; > > } > > + /* > > + * Use the correct seed for checksum (i.e. the seed from 'inode'). This > > + * is so that the metadata blocks will have the correct checksum after > > + * the migration. > > + * > > + * Note however that, if a crash occurs during the migration process, > > + * the recovery process is broken because the tmp_inode checksums will > > + * be wrong and the orphans cleanup will fail. > > ...and then what does the user do? I can't really say I know what is the right thing for a user to do. But my understanding is that my patch doesn't change a lot: the recovery process would still fail in a slightly different way, and will need to be fixed at some point. I believe the userspace tools already have support to handle orphan file, but a quick look at the latest version of the patchset show that these tools can't handle this either, and the recovery will also fail. But I may be wrong. Cheers, -- Luís > > --D > > > + */ > > + ei = EXT4_I(inode); > > + EXT4_I(tmp_inode)->i_csum_seed = ei->i_csum_seed; > > i_size_write(tmp_inode, i_size_read(inode)); > > /* > > * Set the i_nlink to zero so it will be deleted later > > @@ -502,7 +513,6 @@ int ext4_ext_migrate(struct inode *inode) > > goto out_tmp_inode; > > } > > > > - ei = EXT4_I(inode); > > i_data = ei->i_data; > > memset(&lb, 0, sizeof(lb)); > >
