On Tue, Jan 23, 2018 at 06:19:06PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Refactor inode verifier error reporting into a non-libxfs function so
> that we aren't encoding the message format in libxfs. This also
> changes the kernel dmesg output to resemble buffer verifier errors
> more closely.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_inode_buf.c | 6 ++----
> fs/xfs/xfs_error.c | 37 +++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_error.h | 3 +++
> fs/xfs/xfs_inode.c | 14 ++++++++------
> 4 files changed, 50 insertions(+), 10 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 6e9dcdb..6d05ba6 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -578,10 +578,8 @@ xfs_iread(
> /* even unallocated inodes are verified */
> fa = xfs_dinode_verify(mp, ip->i_ino, dip);
> if (fa) {
> - xfs_alert(mp, "%s: validation failed for inode %lld at %pS",
> - __func__, ip->i_ino, fa);
> -
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, dip);
> + xfs_inode_verifier_error(ip, -EFSCORRUPTED, "record", dip,
> + sizeof(*dip), fa);
What does "record" mean? "dinode" perhaps? Otherwise looks fine:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> error = -EFSCORRUPTED;
> goto out_brelse;
> }
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 980d5f0..ccf520f 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -24,6 +24,7 @@
> #include "xfs_errortag.h"
> #include "xfs_error.h"
> #include "xfs_sysfs.h"
> +#include "xfs_inode.h"
>
> #ifdef DEBUG
>
> @@ -372,3 +373,39 @@ xfs_verifier_error(
> if (xfs_error_level >= XFS_ERRLEVEL_HIGH)
> xfs_stack_trace();
> }
> +
> +/*
> + * Warnings for inode corruption problems. Don't bother with the stack
> + * trace unless the error level is turned up high.
> + */
> +void
> +xfs_inode_verifier_error(
> + struct xfs_inode *ip,
> + int error,
> + const char *name,
> + void *buf,
> + size_t bufsz,
> + xfs_failaddr_t failaddr)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + xfs_failaddr_t fa;
> + int sz;
> +
> + fa = failaddr ? failaddr : __return_address;
> +
> + xfs_alert(mp, "Metadata %s detected at %pS, inode 0x%llx %s",
> + error == -EFSBADCRC ? "CRC error" : "corruption",
> + fa, ip->i_ino, name);
> +
> + xfs_alert(mp, "Unmount and run xfs_repair");
> +
> + if (buf && xfs_error_level >= XFS_ERRLEVEL_LOW) {
> + sz = min_t(size_t, XFS_CORRUPTION_DUMP_LEN, bufsz);
> + xfs_alert(mp, "First %d bytes of corrupted metadata buffer:",
> + sz);
> + xfs_hex_dump(buf, sz);
> + }
> +
> + if (xfs_error_level >= XFS_ERRLEVEL_HIGH)
> + xfs_stack_trace();
> +}
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index a3ba05b..7e728c5 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -28,6 +28,9 @@ extern void xfs_corruption_error(const char *tag, int level,
> int linenum, xfs_failaddr_t failaddr);
> extern void xfs_verifier_error(struct xfs_buf *bp, int error,
> xfs_failaddr_t failaddr);
> +extern void xfs_inode_verifier_error(struct xfs_inode *ip, int error,
> + const char *name, void *buf, size_t bufsz,
> + xfs_failaddr_t failaddr);
>
> #define XFS_ERROR_REPORT(e, lvl, mp) \
> xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index fc118dd..c60efec 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3502,21 +3502,23 @@ bool
> xfs_inode_verify_forks(
> struct xfs_inode *ip)
> {
> + struct xfs_ifork *ifp;
> xfs_failaddr_t fa;
>
> fa = xfs_ifork_verify_data(ip, &xfs_default_ifork_ops);
> if (fa) {
> - xfs_alert(ip->i_mount,
> - "%s: bad inode %llu inline data fork at %pS",
> - __func__, ip->i_ino, fa);
> + ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> + xfs_inode_verifier_error(ip, -EFSCORRUPTED, "data fork",
> + ifp->if_u1.if_data, ifp->if_bytes, fa);
> return false;
> }
>
> fa = xfs_ifork_verify_attr(ip, &xfs_default_ifork_ops);
> if (fa) {
> - xfs_alert(ip->i_mount,
> - "%s: bad inode %llu inline attr fork at %pS",
> - __func__, ip->i_ino, fa);
> + ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
> + xfs_inode_verifier_error(ip, -EFSCORRUPTED, "attr fork",
> + ifp ? ifp->if_u1.if_data : NULL,
> + ifp ? ifp->if_bytes : 0, fa);
> return false;
> }
> return true;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
