Re: Trouble mounting metadata_csum ext4 filesystems with v4.7.x after c9274d891869880648c4ee9365df3ecc7ba2e285: not enough inode bytes checksummed?

[정대호 <daeho.jeong@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

Hi, Sorry to bother you.

> Basically, a 128-byte inode inside a filesystem that allocated 256 bytes
> for each inode.  As you point out, the old code would checksum the entire
> allocated space, whether or not the inode core used it.  Obviously, you
> want this since inline extended attributes live in that space:
 
> csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)raw,
>                   EXT4_INODE_SIZE(inode->i_sb));
 
> The new code, on the other hand, carefully checksums around the
> i_checksum fields and only bothers to checksum the space between the end
> of i_checksum_hi and the end of the allocated space if the inode core is
> big enough to store i_checksum_hi.  Since we allocated 256 bytes for
> each inode, we checksum the first two bytes after byte 128
> (EXT4_GOOD_OLD_INODE_SIZE), but then we see that i_extra_size == 0 so we
> never bother to checksum anything after that.  This is of course wrong
> since we no longer checksum the xattr space and we've deviated from the
> pre-4.7.4 (documented) on-disk format.

Oops. I had overlooked the case of that i_extra_size is less than 4.
I misunderstood the previous Darrick's codes calculating inode checksum value.
Sorry about that. :-(
 
> if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
>        offset = offsetof(struct ext4_inode, i_checksum_hi);
>        csum = ext4_chksum(sbi, csum, (__u8 *)raw +
>                           EXT4_GOOD_OLD_INODE_SIZE,
>                           offset - EXT4_GOOD_OLD_INODE_SIZE);
>        if (EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi)) {
>                csum = ext4_chksum(sbi, csum, (__u8 *)&dummy_csum,
>                                   csum_size);
>                offset += csum_size;
>        }
>        csum = ext4_chksum(sbi, csum, (__u8 *)raw + offset,
>                           EXT4_INODE_SIZE(inode->i_sb) - offset);
> }
> 
> Can you give that a try?

Darrick, your modification looks good enough to me.

Thank you, guys.