[LU-13103] e2fsprogs has problem when calculation block number

• Status: Resolved
• Project: Lustre
• Type: Bug
• Priority: Minor
• Reporter: Li Xi
• Assignee: Wang Shilong (Inactive)
• Resolution: Fixed
• Severity: 3

Description

static blkcnt_t blocks_from_inode(ext2_filsys fs,
				  struct ext2_inode_large *inode)
{
	blkcnt_t b;

	b = inode->i_blocks;
	if (ext2fs_has_feature_huge_file(fs->super))
		b += ((long long) inode->osd2.linux2.l_i_blocks_hi) << 32;

	if (!ext2fs_has_feature_huge_file(fs->super) ||
	    !(inode->i_flags & EXT4_HUGE_FILE_FL))
		b *= fs->blocksize / 512;
	b *= EXT2FS_CLUSTER_RATIO(fs);

	return b;
}

This is wrong, it should be something like

static blkcnt_t blocks_from_inode(ext2_filsys fs, struct ext2_inode *inode)
{
	blkcnt_t b;

	b = inode->i_blocks;
	if (ext2fs_has_feature_huge_file(fs->super)) {
		b += ((long long) inode->osd2.linux2.l_i_blocks_hi) << 32;
		if (inode->i_flags & EXT4_HUGE_FILE_FL)
			b *= fs->blocksize / 512;
	}
	b *= EXT2FS_CLUSTER_RATIO(fs);

	return b;
}

ext2fs_iblk_add_blocks(), ext2fs_iblk_sub_blocks() and ext2fs_iblk_set() has similar problems.

Comments

[Andreas Dilger]

It looks to me that the existing code is correct.

There are two parts to the huge file support. One part is the high 16 bits of the block number. The second part is whether the blocks are in 512-byte sector units (old code) or blocksize units (new code). Only if both the HUGE_FILE feature is enabled in the superblock is the high-bits field available. and only if the inode flag is set is it in units of blocksize. Otherwise it has to be converted from sectors to blocks.

[Li Xi]

Dongyang found following:

Lower 32-bits of "block" count. If the huge_file feature flag is not set on the filesystem, the file consumes i_blocks_lo 512-byte blocks on disk. If huge_file is set and EXT4_HUGE_FILE_FL is NOT set in inode.i_flags, then the file consumes i_blocks_lo + (i_blocks_hi << 32) 512-byte blocks on disk. If huge_file is set and EXT4_HUGE_FILE_FL IS set in inode.i_flags, then this file consumes (i_blocks_lo + i_blocks_hi << 32) filesystem blocks on disk.

[Li Xi]

The problem with current code is, if file system has no huge file feature, or if the file has no huge file flag, the block saved in i_blocks will be consider to having the unit of block size. That is not correct, and causes different values between the 512 block number got from stat(2) and from e2fsprogs utilities.

[Wang Shilong (Inactive)]

btw, above codes have another problem that it b *= EXT2FS_CLUSTER_RATIO(fs), this is not right for i_blocks.

[Andreas Dilger]

Please post an email to [e-mail] with bug report and/or patch. It looks like the code in blocks_from_inode() is specific to FUSE, and that may never have been tested with files over 16TB (since that isn't possible with x86 systems, only with PAGE_SIZE over 4KB). The other places where EXT4_HUGE_FILE_FL are used is converting from a blocks count to store into the inode, while the blocks_from_inode() is converting from inode->i_blocks to the 512-byte sector count, so the logic should be reversed.

I also see that ext2fs_inode_i_blocks() is not taking EXT4_HUGE_FILE_FL into account at all, which is likely confusing/broken as well.

[Gerrit Updater]

Wang Shilong ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37109
Subject: LU-13103 e2fsprogs: fix to use inode i_blocks correctly
Project: tools/e2fsprogs
Branch: master
Current Patch Set: 1
Commit: 9450605fc9053d553f7f5980bc747c58d70f9483

[Gerrit Updater]

Wang Shilong ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37110
Subject: LU-13103 e2fsprogs: fix to use inode i_blocks correctly
Project: tools/e2fsprogs
Branch: master-lustre
Current Patch Set: 1
Commit: 5bdf73f1a7055b97342c92d256923a4b32c8ec97

[Wang Shilong (Inactive)]

DY is rebasing e2fsprogs, supposing we will include it soon.