Re: [PATCH V9 13/19] xfs: Replace numbered inode recovery error messages with descriptive ones

Darrick J. Wong
2022-04-07T01:50:11Z

On Wed, Apr 06, 2022 at 11:48:57AM +0530, Chandan Babu R wrote:
> This commit also prints inode fields with invalid values instead of printing
> addresses of inode and buffer instances.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>

Much better for diagnosing recovery problems!!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> fs/xfs/xfs_inode_item_recover.c | 52 ++++++++++++++-------------------
> 1 file changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
> index 44b90614859e..96b222e18b0f 100644
> --- a/fs/xfs/xfs_inode_item_recover.c
> +++ b/fs/xfs/xfs_inode_item_recover.c
> @@ -324,13 +324,12 @@ xlog_recover_inode_commit_pass2(
> if (unlikely(S_ISREG(ldip->di_mode))) {
> if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
> (ldip->di_format != XFS_DINODE_FMT_BTREE)) {
> - XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(3)",
> - XFS_ERRLEVEL_LOW, mp, ldip,
> - sizeof(*ldip));
> + XFS_CORRUPTION_ERROR(
> + "Bad log dinode data fork format for regular file",
> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
> xfs_alert(mp,
> - "%s: Bad regular inode log record, rec ptr "PTR_FMT", "
> - "ino ptr = "PTR_FMT", ino bp = "PTR_FMT", ino %Ld",
> - __func__, item, dip, bp, in_f->ilf_ino);
> + "Bad inode 0x%llx, data fork format 0x%x",
> + in_f->ilf_ino, ldip->di_format);
> error = -EFSCORRUPTED;
> goto out_release;
> }
> @@ -338,49 +337,42 @@ xlog_recover_inode_commit_pass2(
> if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
> (ldip->di_format != XFS_DINODE_FMT_BTREE) &&
> (ldip->di_format != XFS_DINODE_FMT_LOCAL)) {
> - XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(4)",
> - XFS_ERRLEVEL_LOW, mp, ldip,
> - sizeof(*ldip));
> + XFS_CORRUPTION_ERROR(
> + "Bad log dinode data fork format for directory",
> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
> xfs_alert(mp,
> - "%s: Bad dir inode log record, rec ptr "PTR_FMT", "
> - "ino ptr = "PTR_FMT", ino bp = "PTR_FMT", ino %Ld",
> - __func__, item, dip, bp, in_f->ilf_ino);
> + "Bad inode 0x%llx, data fork format 0x%x",
> + in_f->ilf_ino, ldip->di_format);
> error = -EFSCORRUPTED;
> goto out_release;
> }
> }
> if (unlikely(ldip->di_nextents + ldip->di_anextents > ldip->di_nblocks)){
> - XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(5)",
> - XFS_ERRLEVEL_LOW, mp, ldip,
> - sizeof(*ldip));
> + XFS_CORRUPTION_ERROR("Bad log dinode extent counts",
> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
> xfs_alert(mp,
> - "%s: Bad inode log record, rec ptr "PTR_FMT", dino ptr "PTR_FMT", "
> - "dino bp "PTR_FMT", ino %Ld, total extents = %d, nblocks = %Ld",
> - __func__, item, dip, bp, in_f->ilf_ino,
> - ldip->di_nextents + ldip->di_anextents,
> + "Bad inode 0x%llx, nextents 0x%x, anextents 0x%x, nblocks 0x%llx",
> + in_f->ilf_ino, ldip->di_nextents, ldip->di_anextents,
> ldip->di_nblocks);
> error = -EFSCORRUPTED;
> goto out_release;
> }
> if (unlikely(ldip->di_forkoff > mp->m_sb.sb_inodesize)) {
> - XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(6)",
> - XFS_ERRLEVEL_LOW, mp, ldip,
> - sizeof(*ldip));
> + XFS_CORRUPTION_ERROR("Bad log dinode fork offset",
> + XFS_ERRLEVEL_LOW, mp, ldip, sizeof(*ldip));
> xfs_alert(mp,
> - "%s: Bad inode log record, rec ptr "PTR_FMT", dino ptr "PTR_FMT", "
> - "dino bp "PTR_FMT", ino %Ld, forkoff 0x%x", __func__,
> - item, dip, bp, in_f->ilf_ino, ldip->di_forkoff);
> + "Bad inode 0x%llx, di_forkoff 0x%x",
> + in_f->ilf_ino, ldip->di_forkoff);
> error = -EFSCORRUPTED;
> goto out_release;
> }
> isize = xfs_log_dinode_size(mp);
> if (unlikely(item->ri_buf[1].i_len > isize)) {
> - XFS_CORRUPTION_ERROR("xlog_recover_inode_pass2(7)",
> - XFS_ERRLEVEL_LOW, mp, ldip,
> - sizeof(*ldip));
> + XFS_CORRUPTION_ERROR("Bad log dinode size", XFS_ERRLEVEL_LOW,
> + mp, ldip, sizeof(*ldip));
> xfs_alert(mp,
> - "%s: Bad inode log record length %d, rec ptr "PTR_FMT,
> - __func__, item->ri_buf[1].i_len, item);
> + "Bad inode 0x%llx log dinode size 0x%x",
> + in_f->ilf_ino, item->ri_buf[1].i_len);
> error = -EFSCORRUPTED;
> goto out_release;
> }
> --
> 2.30.2
>