On 25 Mar 2022 at 03:23, Dave Chinner wrote:
> On Mon, Mar 21, 2022 at 10:47:45AM +0530, Chandan Babu R wrote:
>> This commit introduces new fields in the on-disk inode format to support
>> 64-bit data fork extent counters and 32-bit attribute fork extent
>> counters. The new fields will be used only when an inode has
>> XFS_DIFLAG2_NREXT64 flag set. Otherwise we continue to use the regular 32-bit
>> data fork extent counters and 16-bit attribute fork extent counters.
>>
>> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
>> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>> Suggested-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Looks good, minor nits below.
>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
>> +STATIC int
>> +xlog_dinode_verify_extent_counts(
>> + struct xfs_mount *mp,
>> + struct xfs_log_dinode *ldip)
>> +{
>> + xfs_extnum_t nextents;
>> + xfs_aextnum_t anextents;
>> +
>> + if (xfs_log_dinode_has_large_extent_counts(ldip)) {
>> + if (!xfs_has_large_extent_counts(mp) ||
>> + (ldip->di_nrext64_pad != 0)) {
>> + XFS_CORRUPTION_ERROR(
>> + "Bad log dinode large extent count format",
>> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
>> + xfs_alert(mp,
>> + "Bad inode 0x%llx, nrext64 %d, padding 0x%x",
>
> "large extent counts" rather than "nrext64"?
>
>> + ldip->di_ino, xfs_has_large_extent_counts(mp),
>> + ldip->di_nrext64_pad);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + nextents = ldip->di_big_nextents;
>> + anextents = ldip->di_big_anextents;
>> + } else {
>> + if (ldip->di_version == 3 && ldip->di_v3_pad != 0) {
>> + XFS_CORRUPTION_ERROR(
>> + "Bad log dinode di_v3_pad",
>> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
>> + xfs_alert(mp,
>> + "Bad inode 0x%llx, di_v3_pad %llu",
>> + ldip->di_ino, ldip->di_v3_pad);
>
> the padding should be dumped as a hexadecimal value - it's much
> easier to see the bit corruption patterns in hex dumps than decimal
> output.
>
I agree. I will make the suggested change.
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + nextents = ldip->di_nextents;
>> + anextents = ldip->di_anextents;
>> + }
>> +
>> + if (unlikely(nextents + anextents > ldip->di_nblocks)) {
>> + XFS_CORRUPTION_ERROR("Bad log dinode extent counts",
>> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
>> + xfs_alert(mp,
>> + "Bad inode 0x%llx, nextents 0x%llx, anextents 0x%x, nblocks 0x%llx",
>> + ldip->di_ino, nextents, anextents, ldip->di_nblocks);
>> + return -EFSCORRUPTED;
>
> Maybe should indicate large extent count status here, too.
>
Yes, I think it will make it easier to determine the inode fields of the two
extent counts.
--
chandan
