Re: [PATCH V8 14/19] xfs: Introduce per-inode 64-bit extent counters

Dave Chinner
2022-03-24T21:53:52Z

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@kernel.org>
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>

Looks good, minor nits below.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

> +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.

> + 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.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com