xfs: teach scrub to check for sole ownership of metadata objects
authorDarrick J. Wong <djwong@kernel.org>
Wed, 12 Apr 2023 02:00:15 +0000 (19:00 -0700)
committerDarrick J. Wong <djwong@kernel.org>
Wed, 12 Apr 2023 02:00:15 +0000 (19:00 -0700)
Strengthen online scrub's checking even further by enabling us to check
that a range of blocks are owned solely by a given owner.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
fs/xfs/libxfs/xfs_rmap.c
fs/xfs/libxfs/xfs_rmap.h
fs/xfs/scrub/agheader.c
fs/xfs/scrub/btree.c
fs/xfs/scrub/ialloc.c
fs/xfs/scrub/inode.c
fs/xfs/scrub/rmap.c
fs/xfs/scrub/scrub.h

index 308b81f..f4dc23b 100644 (file)
@@ -2735,65 +2735,141 @@ xfs_rmap_has_records(
        return xfs_btree_has_records(cur, &low, &high, &mask, outcome);
 }
 
-/*
- * Is there a record for this owner completely covering a given physical
- * extent?  If so, *has_rmap will be set to true.  If there is no record
- * or the record only covers part of the range, we set *has_rmap to false.
- * This function doesn't perform range lookups or offset checks, so it is
- * not suitable for checking data fork blocks.
- */
-int
-xfs_rmap_record_exists(
-       struct xfs_btree_cur            *cur,
+struct xfs_rmap_ownercount {
+       /* Owner that we're looking for. */
+       struct xfs_rmap_irec    good;
+
+       /* rmap search keys */
+       struct xfs_rmap_irec    low;
+       struct xfs_rmap_irec    high;
+
+       struct xfs_rmap_matches *results;
+
+       /* Stop early if we find a nonmatch? */
+       bool                    stop_on_nonmatch;
+};
+
+/* Does this rmap represent space that can have multiple owners? */
+static inline bool
+xfs_rmap_shareable(
+       struct xfs_mount                *mp,
+       const struct xfs_rmap_irec      *rmap)
+{
+       if (!xfs_has_reflink(mp))
+               return false;
+       if (XFS_RMAP_NON_INODE_OWNER(rmap->rm_owner))
+               return false;
+       if (rmap->rm_flags & (XFS_RMAP_ATTR_FORK |
+                             XFS_RMAP_BMBT_BLOCK))
+               return false;
+       return true;
+}
+
+static inline void
+xfs_rmap_ownercount_init(
+       struct xfs_rmap_ownercount      *roc,
        xfs_agblock_t                   bno,
        xfs_extlen_t                    len,
        const struct xfs_owner_info     *oinfo,
-       bool                            *has_rmap)
+       struct xfs_rmap_matches         *results)
 {
-       uint64_t                        owner;
-       uint64_t                        offset;
-       unsigned int                    flags;
-       int                             has_record;
-       struct xfs_rmap_irec            irec;
-       int                             error;
+       memset(roc, 0, sizeof(*roc));
+       roc->results = results;
+
+       roc->low.rm_startblock = bno;
+       memset(&roc->high, 0xFF, sizeof(roc->high));
+       roc->high.rm_startblock = bno + len - 1;
+
+       memset(results, 0, sizeof(*results));
+       roc->good.rm_startblock = bno;
+       roc->good.rm_blockcount = len;
+       roc->good.rm_owner = oinfo->oi_owner;
+       roc->good.rm_offset = oinfo->oi_offset;
+       if (oinfo->oi_flags & XFS_OWNER_INFO_ATTR_FORK)
+               roc->good.rm_flags |= XFS_RMAP_ATTR_FORK;
+       if (oinfo->oi_flags & XFS_OWNER_INFO_BMBT_BLOCK)
+               roc->good.rm_flags |= XFS_RMAP_BMBT_BLOCK;
+}
 
-       xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
-       ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
-              (flags & XFS_RMAP_BMBT_BLOCK));
+/* Figure out if this is a match for the owner. */
+STATIC int
+xfs_rmap_count_owners_helper(
+       struct xfs_btree_cur            *cur,
+       const struct xfs_rmap_irec      *rec,
+       void                            *priv)
+{
+       struct xfs_rmap_ownercount      *roc = priv;
+       struct xfs_rmap_irec            check = *rec;
+       unsigned int                    keyflags;
+       bool                            filedata;
+       int64_t                         delta;
+
+       filedata = !XFS_RMAP_NON_INODE_OWNER(check.rm_owner) &&
+                  !(check.rm_flags & XFS_RMAP_BMBT_BLOCK);
+
+       /* Trim the part of check that comes before the comparison range. */
+       delta = (int64_t)roc->good.rm_startblock - check.rm_startblock;
+       if (delta > 0) {
+               check.rm_startblock += delta;
+               check.rm_blockcount -= delta;
+               if (filedata)
+                       check.rm_offset += delta;
+       }
 
-       error = xfs_rmap_lookup_le(cur, bno, owner, offset, flags, &irec,
-                       &has_record);
-       if (error)
-               return error;
-       if (!has_record) {
-               *has_rmap = false;
-               return 0;
+       /* Trim the part of check that comes after the comparison range. */
+       delta = (check.rm_startblock + check.rm_blockcount) -
+               (roc->good.rm_startblock + roc->good.rm_blockcount);
+       if (delta > 0)
+               check.rm_blockcount -= delta;
+
+       /* Don't care about unwritten status for establishing ownership. */
+       keyflags = check.rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK);
+
+       if (check.rm_startblock == roc->good.rm_startblock &&
+           check.rm_blockcount == roc->good.rm_blockcount &&
+           check.rm_owner      == roc->good.rm_owner &&
+           check.rm_offset     == roc->good.rm_offset &&
+           keyflags            == roc->good.rm_flags) {
+               roc->results->matches++;
+       } else {
+               roc->results->non_owner_matches++;
+               if (xfs_rmap_shareable(cur->bc_mp, &roc->good) ^
+                   xfs_rmap_shareable(cur->bc_mp, &check))
+                       roc->results->bad_non_owner_matches++;
        }
 
-       *has_rmap = (irec.rm_owner == owner && irec.rm_startblock <= bno &&
-                    irec.rm_startblock + irec.rm_blockcount >= bno + len);
+       if (roc->results->non_owner_matches && roc->stop_on_nonmatch)
+               return -ECANCELED;
+
        return 0;
 }
 
-struct xfs_rmap_key_state {
-       uint64_t                        owner;
-       uint64_t                        offset;
-       unsigned int                    flags;
-};
-
-/* For each rmap given, figure out if it doesn't match the key we want. */
-STATIC int
-xfs_rmap_has_other_keys_helper(
+/* Count the number of owners and non-owners of this range of blocks. */
+int
+xfs_rmap_count_owners(
        struct xfs_btree_cur            *cur,
-       const struct xfs_rmap_irec      *rec,
-       void                            *priv)
+       xfs_agblock_t                   bno,
+       xfs_extlen_t                    len,
+       const struct xfs_owner_info     *oinfo,
+       struct xfs_rmap_matches         *results)
 {
-       struct xfs_rmap_key_state       *rks = priv;
+       struct xfs_rmap_ownercount      roc;
+       int                             error;
 
-       if (rks->owner == rec->rm_owner && rks->offset == rec->rm_offset &&
-           ((rks->flags & rec->rm_flags) & XFS_RMAP_KEY_FLAGS) == rks->flags)
-               return 0;
-       return -ECANCELED;
+       xfs_rmap_ownercount_init(&roc, bno, len, oinfo, results);
+       error = xfs_rmap_query_range(cur, &roc.low, &roc.high,
+                       xfs_rmap_count_owners_helper, &roc);
+       if (error)
+               return error;
+
+       /*
+        * There can't be any non-owner rmaps that conflict with the given
+        * owner if we didn't find any rmaps matching the owner.
+        */
+       if (!results->matches)
+               results->bad_non_owner_matches = 0;
+
+       return 0;
 }
 
 /*
@@ -2806,28 +2882,26 @@ xfs_rmap_has_other_keys(
        xfs_agblock_t                   bno,
        xfs_extlen_t                    len,
        const struct xfs_owner_info     *oinfo,
-       bool                            *has_rmap)
+       bool                            *has_other)
 {
-       struct xfs_rmap_irec            low = {0};
-       struct xfs_rmap_irec            high;
-       struct xfs_rmap_key_state       rks;
+       struct xfs_rmap_matches         res;
+       struct xfs_rmap_ownercount      roc;
        int                             error;
 
-       xfs_owner_info_unpack(oinfo, &rks.owner, &rks.offset, &rks.flags);
-       *has_rmap = false;
-
-       low.rm_startblock = bno;
-       memset(&high, 0xFF, sizeof(high));
-       high.rm_startblock = bno + len - 1;
+       xfs_rmap_ownercount_init(&roc, bno, len, oinfo, &res);
+       roc.stop_on_nonmatch = true;
 
-       error = xfs_rmap_query_range(cur, &low, &high,
-                       xfs_rmap_has_other_keys_helper, &rks);
+       error = xfs_rmap_query_range(cur, &roc.low, &roc.high,
+                       xfs_rmap_count_owners_helper, &roc);
        if (error == -ECANCELED) {
-               *has_rmap = true;
+               *has_other = true;
                return 0;
        }
+       if (error)
+               return error;
 
-       return error;
+       *has_other = false;
+       return 0;
 }
 
 const struct xfs_owner_info XFS_RMAP_OINFO_SKIP_UPDATE = {
index 4cbe50c..3c98d9d 100644 (file)
@@ -200,12 +200,24 @@ xfs_failaddr_t xfs_rmap_check_irec(struct xfs_btree_cur *cur,
 
 int xfs_rmap_has_records(struct xfs_btree_cur *cur, xfs_agblock_t bno,
                xfs_extlen_t len, enum xbtree_recpacking *outcome);
-int xfs_rmap_record_exists(struct xfs_btree_cur *cur, xfs_agblock_t bno,
+
+struct xfs_rmap_matches {
+       /* Number of owner matches. */
+       unsigned long long      matches;
+
+       /* Number of non-owner matches. */
+       unsigned long long      non_owner_matches;
+
+       /* Number of non-owner matches that conflict with the owner matches. */
+       unsigned long long      bad_non_owner_matches;
+};
+
+int xfs_rmap_count_owners(struct xfs_btree_cur *cur, xfs_agblock_t bno,
                xfs_extlen_t len, const struct xfs_owner_info *oinfo,
-               bool *has_rmap);
+               struct xfs_rmap_matches *rmatch);
 int xfs_rmap_has_other_keys(struct xfs_btree_cur *cur, xfs_agblock_t bno,
                xfs_extlen_t len, const struct xfs_owner_info *oinfo,
-               bool *has_rmap);
+               bool *has_other);
 int xfs_rmap_map_raw(struct xfs_btree_cur *cur, struct xfs_rmap_irec *rmap);
 
 extern const struct xfs_owner_info XFS_RMAP_OINFO_SKIP_UPDATE;
index 1a84153..6c6e5eb 100644 (file)
@@ -51,7 +51,7 @@ xchk_superblock_xref(
 
        xchk_xref_is_used_space(sc, agbno, 1);
        xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
 
@@ -515,7 +515,7 @@ xchk_agf_xref(
        xchk_agf_xref_freeblks(sc);
        xchk_agf_xref_cntbt(sc);
        xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
        xchk_agf_xref_btreeblks(sc);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
@@ -644,7 +644,7 @@ xchk_agfl_block_xref(
 
        xchk_xref_is_used_space(sc, agbno, 1);
        xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_AG);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_AG);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
 }
@@ -701,7 +701,7 @@ xchk_agfl_xref(
 
        xchk_xref_is_used_space(sc, agbno, 1);
        xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
 
@@ -857,7 +857,7 @@ xchk_agi_xref(
        xchk_xref_is_used_space(sc, agbno, 1);
        xchk_xref_is_not_inode_chunk(sc, agbno, 1);
        xchk_agi_xref_icounts(sc);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
        xchk_agi_xref_fiblocks(sc);
index 1165dc0..1935b9c 100644 (file)
@@ -402,7 +402,7 @@ xchk_btree_check_block_owner(
        if (!bs->sc->sa.bno_cur && btnum == XFS_BTNUM_BNO)
                bs->cur = NULL;
 
-       xchk_xref_is_owned_by(bs->sc, agbno, 1, bs->oinfo);
+       xchk_xref_is_only_owned_by(bs->sc, agbno, 1, bs->oinfo);
        if (!bs->sc->sa.rmap_cur && btnum == XFS_BTNUM_RMAP)
                bs->cur = NULL;
 
index fda96b5..575f22a 100644 (file)
@@ -276,7 +276,7 @@ xchk_iallocbt_chunk(
                xchk_inobt_chunk_xref_finobt(sc, irec, agino, nr_inodes);
        else
                xchk_finobt_chunk_xref_inobt(sc, irec, agino, nr_inodes);
-       xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
+       xchk_xref_is_only_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
        xchk_xref_is_not_shared(sc, agbno, len);
        xchk_xref_is_not_cow_staging(sc, agbno, len);
        return true;
@@ -428,7 +428,7 @@ xchk_iallocbt_check_cluster(
                return 0;
        }
 
-       xchk_xref_is_owned_by(bs->sc, agbno, M_IGEO(mp)->blocks_per_cluster,
+       xchk_xref_is_only_owned_by(bs->sc, agbno, M_IGEO(mp)->blocks_per_cluster,
                        &XFS_RMAP_OINFO_INODES);
 
        /* Grab the inode cluster buffer. */
index 50ebd72..2db96c8 100644 (file)
@@ -556,7 +556,7 @@ xchk_inode_xref(
 
        xchk_xref_is_used_space(sc, agbno, 1);
        xchk_inode_xref_finobt(sc, ino);
-       xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_INODES);
+       xchk_xref_is_only_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_INODES);
        xchk_xref_is_not_shared(sc, agbno, 1);
        xchk_xref_is_not_cow_staging(sc, agbno, 1);
        xchk_inode_xref_bmap(sc, dip);
index 2f9e4f7..18b6428 100644 (file)
@@ -167,38 +167,29 @@ xchk_rmapbt(
                        &XFS_RMAP_OINFO_AG, NULL);
 }
 
-/* xref check that the extent is owned by a given owner */
-static inline void
-xchk_xref_check_owner(
+/* xref check that the extent is owned only by a given owner */
+void
+xchk_xref_is_only_owned_by(
        struct xfs_scrub                *sc,
        xfs_agblock_t                   bno,
        xfs_extlen_t                    len,
-       const struct xfs_owner_info     *oinfo,
-       bool                            should_have_rmap)
+       const struct xfs_owner_info     *oinfo)
 {
-       bool                            has_rmap;
+       struct xfs_rmap_matches         res;
        int                             error;
 
        if (!sc->sa.rmap_cur || xchk_skip_xref(sc->sm))
                return;
 
-       error = xfs_rmap_record_exists(sc->sa.rmap_cur, bno, len, oinfo,
-                       &has_rmap);
+       error = xfs_rmap_count_owners(sc->sa.rmap_cur, bno, len, oinfo, &res);
        if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
                return;
-       if (has_rmap != should_have_rmap)
+       if (res.matches != 1)
+               xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
+       if (res.bad_non_owner_matches)
+               xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
+       if (res.non_owner_matches)
                xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
-}
-
-/* xref check that the extent is owned by a given owner */
-void
-xchk_xref_is_owned_by(
-       struct xfs_scrub                *sc,
-       xfs_agblock_t                   bno,
-       xfs_extlen_t                    len,
-       const struct xfs_owner_info     *oinfo)
-{
-       xchk_xref_check_owner(sc, bno, len, oinfo, true);
 }
 
 /* xref check that the extent is not owned by a given owner */
@@ -209,7 +200,19 @@ xchk_xref_is_not_owned_by(
        xfs_extlen_t                    len,
        const struct xfs_owner_info     *oinfo)
 {
-       xchk_xref_check_owner(sc, bno, len, oinfo, false);
+       struct xfs_rmap_matches         res;
+       int                             error;
+
+       if (!sc->sa.rmap_cur || xchk_skip_xref(sc->sm))
+               return;
+
+       error = xfs_rmap_count_owners(sc->sa.rmap_cur, bno, len, oinfo, &res);
+       if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
+               return;
+       if (res.matches != 0)
+               xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
+       if (res.bad_non_owner_matches)
+               xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
 }
 
 /* xref check that the extent has no reverse mapping at all */
index b6f452e..c519927 100644 (file)
@@ -162,7 +162,7 @@ void xchk_xref_is_not_inode_chunk(struct xfs_scrub *sc, xfs_agblock_t agbno,
                xfs_extlen_t len);
 void xchk_xref_is_inode_chunk(struct xfs_scrub *sc, xfs_agblock_t agbno,
                xfs_extlen_t len);
-void xchk_xref_is_owned_by(struct xfs_scrub *sc, xfs_agblock_t agbno,
+void xchk_xref_is_only_owned_by(struct xfs_scrub *sc, xfs_agblock_t agbno,
                xfs_extlen_t len, const struct xfs_owner_info *oinfo);
 void xchk_xref_is_not_owned_by(struct xfs_scrub *sc, xfs_agblock_t agbno,
                xfs_extlen_t len, const struct xfs_owner_info *oinfo);