From: "Darrick J. Wong" <djwong@kernel.org>
To: Carlos Maiolino <cem@kernel.org>
Cc: xfs <linux-xfs@vger.kernel.org>
Subject: [PATCH] xfs_repair: log when buffers fail CRC checks even if we just recompute it
Date: Fri, 31 May 2024 13:13:26 -0700 [thread overview]
Message-ID: <20240531201326.GT52987@frogsfrogsfrogs> (raw)
From: Darrick J. Wong <djwong@kernel.org>
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, xfs_repair -n doesn't say anything about
crc errors.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
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 2d0df492f71a..2324757d4c44 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -971,8 +971,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);
@@ -1252,8 +1257,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 b229422c81e8..7f94f4012062 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 9fd9569ec9f8..c9b54c3a2802 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -1103,8 +1103,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);
@@ -1280,8 +1285,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;
@@ -1444,10 +1455,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) {
next reply other threads:[~2024-05-31 20:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-31 20:13 Darrick J. Wong [this message]
2024-06-01 5:03 ` [PATCH] xfs_repair: log when buffers fail CRC checks even if we just recompute it Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240531201326.GT52987@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.