From 017762cce2da17b83fdc9192dd6e64c0fef7d09c Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Sun, 2 Jun 2024 16:32:50 -0700 Subject: [PATCH] xfs_repair: log when buffers fail CRC checks even if we just recompute it We should always log metadata block CRC validation errors, even if we decide that the block contents are ok and that we'll simply recompute the checksum. Without this patch, xfs_repair -n won't say anything about crc errors on these blocks. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- repair/attr_repair.c | 15 ++++++++++++--- repair/da_util.c | 12 ++++++++---- repair/dir2.c | 28 +++++++++++++++++++++------- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/repair/attr_repair.c b/repair/attr_repair.c index f117f9ae..206a97d6 100644 --- a/repair/attr_repair.c +++ b/repair/attr_repair.c @@ -870,8 +870,13 @@ process_leaf_attr_level(xfs_mount_t *mp, * If block looks ok but CRC didn't match, make sure to * recompute it. */ - if (!no_modify && bp->b_error == -EFSBADCRC) - repair++; + if (bp->b_error == -EFSBADCRC) { + do_warn( + _("bad checksum for block %u in attribute fork for inode %" PRIu64 "\n"), + da_bno, ino); + if (!no_modify) + repair++; + } if (repair && !no_modify) { libxfs_buf_mark_dirty(bp); @@ -1151,8 +1156,12 @@ process_longform_attr( return 1; } - if (bp->b_error == -EFSBADCRC) + if (bp->b_error == -EFSBADCRC) { + do_warn( + _("bad checksum for block 0 in attribute fork for inode %" PRIu64 "\n"), + ino); (*repair)++; + } /* is this block sane? */ if (__check_attr_header(mp, bp, ino)) { diff --git a/repair/da_util.c b/repair/da_util.c index b229422c..7f94f401 100644 --- a/repair/da_util.c +++ b/repair/da_util.c @@ -562,7 +562,7 @@ _("can't read %s block %u for inode %" PRIu64 "\n"), FORKNAME(whichfork), dabno, cursor->ino); return 1; } - if (bp->b_error == -EFSCORRUPTED || bp->b_error == -EFSBADCRC) { + if (bp->b_error == -EFSCORRUPTED) { do_warn( _("corrupt %s tree block %u for inode %" PRIu64 "\n"), FORKNAME(whichfork), dabno, cursor->ino); @@ -625,9 +625,13 @@ _("bad level %d in %s block %u for inode %" PRIu64 "\n"), * If block looks ok but CRC didn't match, make sure to * recompute it. */ - if (!no_modify && - cursor->level[this_level].bp->b_error == -EFSBADCRC) - cursor->level[this_level].dirty = 1; + if (cursor->level[this_level].bp->b_error == -EFSBADCRC) { + do_warn( + _("bad checksum in %s tree block %u for inode %" PRIu64 "\n"), + FORKNAME(whichfork), dabno, cursor->ino); + if (!no_modify) + cursor->level[this_level].dirty = 1; + } if (cursor->level[this_level].dirty && !no_modify) { libxfs_buf_mark_dirty(cursor->level[this_level].bp); diff --git a/repair/dir2.c b/repair/dir2.c index e46ae9ae..bfeaddd0 100644 --- a/repair/dir2.c +++ b/repair/dir2.c @@ -1031,8 +1031,13 @@ _("bad directory block magic # %#x in block %u for directory inode %" PRIu64 "\n rval = process_dir2_data(mp, ino, dip, ino_discovery, dirname, parent, bp, dot, dotdot, mp->m_dir_geo->datablk, (char *)blp, &dirty); /* If block looks ok but CRC didn't match, make sure to recompute it. */ - if (!rval && bp->b_error == -EFSBADCRC) - dirty = 1; + if (bp->b_error == -EFSBADCRC) { + do_warn( + _("corrupt directory block %u for inode %" PRIu64 "\n"), + mp->m_dir_geo->datablk, ino); + if (!rval) + dirty = 1; + } if (dirty && !no_modify) { *repair = 1; libxfs_buf_mark_dirty(bp); @@ -1208,8 +1213,14 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"), * If block looks ok but CRC didn't match, make sure to * recompute it. */ - if (!no_modify && bp->b_error == -EFSBADCRC) - buf_dirty = 1; + if (bp->b_error == -EFSBADCRC) { + do_warn( + _("bad checksum for directory leafn block %u for inode %" PRIu64 "\n"), + da_bno, ino); + if (!no_modify) + buf_dirty = 1; + } + ASSERT(buf_dirty == 0 || (buf_dirty && !no_modify)); if (buf_dirty && !no_modify) { *repair = 1; @@ -1372,10 +1383,13 @@ _("bad directory block magic # %#x in block %" PRIu64 " for directory inode %" P i = process_dir2_data(mp, ino, dip, ino_discovery, dirname, parent, bp, dot, dotdot, (xfs_dablk_t)dbno, (char *)data + mp->m_dir_geo->blksize, &dirty); - if (i == 0) { + if (i == 0) good++; - /* Maybe just CRC is wrong. Make sure we correct it. */ - if (bp->b_error == -EFSBADCRC) + if (bp->b_error == -EFSBADCRC) { + do_warn( + _("bad checksum in directory data block %" PRIu64 " for inode %" PRIu64 "\n"), + dbno, ino); + if (i == 0) dirty = 1; } if (dirty && !no_modify) { -- 2.47.3