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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox