From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: sandeen@sandeen.net, darrick.wong@oracle.com
Cc: linux-xfs@vger.kernel.org
Subject: [PATCH 5/5] xfs_repair: correctly detect partially written extents
Date: Mon, 26 Oct 2020 16:32:24 -0700 [thread overview]
Message-ID: <160375514426.879169.1166063350727282652.stgit@magnolia> (raw)
In-Reply-To: <160375511371.879169.3659553317719857738.stgit@magnolia>
From: Darrick J. Wong <darrick.wong@oracle.com>
Recently, I was able to create a realtime file with a 16b extent size
and the following data fork mapping:
data offset 0 startblock 144 (0/144) count 3 flag 0
data offset 3 startblock 147 (0/147) count 3 flag 1
data offset 6 startblock 150 (0/150) count 10 flag 0
Notice how we have a written extent, then an unwritten extent, and then
another written extent. The current code in process_rt_rec trips over
that third extent, because repair only knows not to complain about inuse
extents if the mapping was unwritten.
This loop logic is confusing, because it tries to do too many things.
Move the phase3 and phase4 code to separate helper functions, then
isolate the code that handles a mapping that starts in the middle of an
rt extent so that it's clearer what's going on.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
repair/dinode.c | 180 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 112 insertions(+), 68 deletions(-)
diff --git a/repair/dinode.c b/repair/dinode.c
index c89f21e08373..028a23cd5c8c 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -176,76 +176,69 @@ verify_dfsbno_range(
return XR_DFSBNORANGE_VALID;
}
+
static int
-process_rt_rec(
+process_rt_rec_dups(
struct xfs_mount *mp,
- struct xfs_bmbt_irec *irec,
xfs_ino_t ino,
- xfs_rfsblock_t *tot,
- int check_dups)
+ struct xfs_bmbt_irec *irec)
{
- xfs_fsblock_t b, lastb;
+ xfs_fsblock_t b;
xfs_rtblock_t ext;
- int state;
- int pwe; /* partially-written extent */
- /*
- * check numeric validity of the extent
- */
- if (!libxfs_verify_rtbno(mp, irec->br_startblock)) {
- do_warn(
-_("inode %" PRIu64 " - bad rt extent start block number %" PRIu64 ", offset %" PRIu64 "\n"),
- ino,
- irec->br_startblock,
- irec->br_startoff);
- return 1;
- }
-
- lastb = irec->br_startblock + irec->br_blockcount - 1;
- if (!libxfs_verify_rtbno(mp, lastb)) {
- do_warn(
-_("inode %" PRIu64 " - bad rt extent last block number %" PRIu64 ", offset %" PRIu64 "\n"),
- ino,
- lastb,
- irec->br_startoff);
- return 1;
- }
- if (lastb < irec->br_startblock) {
- do_warn(
-_("inode %" PRIu64 " - bad rt extent overflows - start %" PRIu64 ", "
- "end %" PRIu64 ", offset %" PRIu64 "\n"),
- ino,
- irec->br_startblock,
- lastb,
- irec->br_startoff);
- return 1;
- }
-
- /*
- * set the appropriate number of extents
- * this iterates block by block, this can be optimised using extents
- */
- for (b = irec->br_startblock; b < irec->br_startblock +
- irec->br_blockcount; b += mp->m_sb.sb_rextsize) {
+ for (b = rounddown(irec->br_startblock, mp->m_sb.sb_rextsize);
+ b < irec->br_startblock + irec->br_blockcount;
+ b += mp->m_sb.sb_rextsize) {
ext = (xfs_rtblock_t) b / mp->m_sb.sb_rextsize;
- pwe = irec->br_state == XFS_EXT_UNWRITTEN &&
- (b % mp->m_sb.sb_rextsize != 0);
-
- if (check_dups == 1) {
- if (search_rt_dup_extent(mp, ext) && !pwe) {
- do_warn(
+ if (search_rt_dup_extent(mp, ext)) {
+ do_warn(
_("data fork in rt ino %" PRIu64 " claims dup rt extent,"
- "off - %" PRIu64 ", start - %" PRIu64 ", count %" PRIu64 "\n"),
- ino,
- irec->br_startoff,
- irec->br_startblock,
- irec->br_blockcount);
- return 1;
- }
- continue;
+"off - %" PRIu64 ", start - %" PRIu64 ", count %" PRIu64 "\n"),
+ ino,
+ irec->br_startoff,
+ irec->br_startblock,
+ irec->br_blockcount);
+ return 1;
}
+ }
+ return 0;
+}
+
+static int
+process_rt_rec_state(
+ struct xfs_mount *mp,
+ xfs_ino_t ino,
+ struct xfs_bmbt_irec *irec)
+{
+ xfs_fsblock_t b = irec->br_startblock;
+ xfs_rtblock_t ext;
+ int state;
+
+ do {
+ ext = (xfs_rtblock_t)b / mp->m_sb.sb_rextsize;
state = get_rtbmap(ext);
+
+ if ((b % mp->m_sb.sb_rextsize) != 0) {
+ /*
+ * We are midway through a partially written extent.
+ * If we don't find the state that gets set in the
+ * other clause of this loop body, then we have a
+ * partially *mapped* rt extent and should complain.
+ */
+ if (state != XR_E_INUSE)
+ do_error(
+_("data fork in rt inode %" PRIu64 " found invalid rt extent %"PRIu64" state %d at rt block %"PRIu64"\n"),
+ ino, ext, state, b);
+ b = roundup(b, mp->m_sb.sb_rextsize);
+ continue;
+ }
+
+ /*
+ * This is the start of an rt extent. Set the extent state if
+ * nobody else has claimed the extent, or complain if there are
+ * conflicting states.
+ */
switch (state) {
case XR_E_FREE:
case XR_E_UNKNOWN:
@@ -253,32 +246,83 @@ _("data fork in rt ino %" PRIu64 " claims dup rt extent,"
break;
case XR_E_BAD_STATE:
do_error(
-_("bad state in rt block map %" PRIu64 "\n"),
+_("bad state in rt extent map %" PRIu64 "\n"),
ext);
case XR_E_FS_MAP:
case XR_E_INO:
case XR_E_INUSE_FS:
do_error(
-_("data fork in rt inode %" PRIu64 " found metadata block %" PRIu64 " in rt bmap\n"),
+_("data fork in rt inode %" PRIu64 " found rt metadata extent %" PRIu64 " in rt bmap\n"),
ino, ext);
case XR_E_INUSE:
- if (pwe)
- break;
- /* fall through */
case XR_E_MULT:
set_rtbmap(ext, XR_E_MULT);
do_warn(
-_("data fork in rt inode %" PRIu64 " claims used rt block %" PRIu64 "\n"),
- ino, ext);
+_("data fork in rt inode %" PRIu64 " claims used rt extent %" PRIu64 "\n"),
+ ino, b);
return 1;
case XR_E_FREE1:
default:
do_error(
-_("illegal state %d in rt block map %" PRIu64 "\n"),
- state, b);
+_("illegal state %d in rt extent %" PRIu64 "\n"),
+ state, ext);
}
+ b += mp->m_sb.sb_rextsize;
+ } while (b < irec->br_startblock + irec->br_blockcount);
+
+ return 0;
+}
+
+static int
+process_rt_rec(
+ struct xfs_mount *mp,
+ struct xfs_bmbt_irec *irec,
+ xfs_ino_t ino,
+ xfs_rfsblock_t *tot,
+ int check_dups)
+{
+ xfs_fsblock_t lastb;
+ int bad;
+
+ /*
+ * check numeric validity of the extent
+ */
+ if (!libxfs_verify_rtbno(mp, irec->br_startblock)) {
+ do_warn(
+_("inode %" PRIu64 " - bad rt extent start block number %" PRIu64 ", offset %" PRIu64 "\n"),
+ ino,
+ irec->br_startblock,
+ irec->br_startoff);
+ return 1;
}
+ lastb = irec->br_startblock + irec->br_blockcount - 1;
+ if (!libxfs_verify_rtbno(mp, lastb)) {
+ do_warn(
+_("inode %" PRIu64 " - bad rt extent last block number %" PRIu64 ", offset %" PRIu64 "\n"),
+ ino,
+ lastb,
+ irec->br_startoff);
+ return 1;
+ }
+ if (lastb < irec->br_startblock) {
+ do_warn(
+_("inode %" PRIu64 " - bad rt extent overflows - start %" PRIu64 ", "
+ "end %" PRIu64 ", offset %" PRIu64 "\n"),
+ ino,
+ irec->br_startblock,
+ lastb,
+ irec->br_startoff);
+ return 1;
+ }
+
+ if (check_dups)
+ bad = process_rt_rec_dups(mp, ino, irec);
+ else
+ bad = process_rt_rec_state(mp, ino, irec);
+ if (bad)
+ return bad;
+
/*
* bump up the block counter
*/
next prev parent reply other threads:[~2020-10-26 23:34 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-26 23:31 [PATCH 0/5] xfsprogs: fixes for 5.10 Darrick J. Wong
2020-10-26 23:31 ` [PATCH 1/5] mkfs: allow users to specify rtinherit=0 Darrick J. Wong
2020-10-27 5:35 ` Allison Henderson
2020-10-27 17:22 ` Eric Sandeen
2020-10-27 17:24 ` [PATCH 1.5/5] mkfs: clarify valid "inherit" option values Eric Sandeen
2020-10-27 17:40 ` Darrick J. Wong
2020-10-27 17:49 ` Eric Sandeen
2020-10-28 7:32 ` Christoph Hellwig
2020-10-27 17:50 ` [PATCH 1.5/5 V2] " Eric Sandeen
2020-10-27 18:37 ` Darrick J. Wong
2020-10-28 7:32 ` [PATCH 1/5] mkfs: allow users to specify rtinherit=0 Christoph Hellwig
2020-10-26 23:32 ` [PATCH 2/5] xfs: remove unnecessary parameter from scrub_scan_estimate_blocks Darrick J. Wong
2020-10-27 5:35 ` Allison Henderson
2020-10-27 15:33 ` Darrick J. Wong
2020-10-29 18:33 ` Darrick J. Wong
2020-10-29 18:38 ` Allison Henderson
2020-10-28 7:33 ` Christoph Hellwig
2020-10-26 23:32 ` [PATCH 3/5] xfs_db: report ranges of invalid rt blocks Darrick J. Wong
2020-10-27 5:35 ` Allison Henderson
2020-10-28 7:33 ` Christoph Hellwig
2020-10-26 23:32 ` [PATCH 4/5] xfs_repair: skip the rmap and refcount btree checks when the levels are garbage Darrick J. Wong
2020-10-27 5:35 ` Allison Henderson
2020-10-28 7:34 ` Christoph Hellwig
2020-10-26 23:32 ` Darrick J. Wong [this message]
2020-10-27 5:52 ` [PATCH 5/5] xfs_repair: correctly detect partially written extents Allison Henderson
2020-10-28 7:35 ` 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=160375514426.879169.1166063350727282652.stgit@magnolia \
--to=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/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.