Merge tag 'scrub-detect-mergeable-records-6.4_2023-04-11' of git://git.kernel.org...
authorDave Chinner <david@fromorbit.com>
Thu, 13 Apr 2023 21:11:02 +0000 (07:11 +1000)
committerDave Chinner <dchinner@redhat.com>
Thu, 13 Apr 2023 21:11:02 +0000 (07:11 +1000)
xfs: detect mergeable and overlapping btree records [v24.5]

While I was doing differential fuzz analysis between xfs_scrub and
xfs_repair, I noticed that xfs_repair was only partially effective at
detecting btree records that can be merged, and xfs_scrub totally didn't
notice at all.

For every interval btree type except for the bmbt, there should never
exist two adjacent records with adjacent keyspaces because the
blockcount field is always large enough to span the entire keyspace of
the domain.  This is because the free space, rmap, and refcount btrees
have a blockcount field large enough to store the maximum AG length, and
there can never be an allocation larger than an AG.

The bmbt is a different story due to its ondisk encoding where the
blockcount is only 21 bits wide.  Because AGs can span up to 2^31 blocks
and the RT volume can span up to 2^52 blocks, a preallocation of 2^22
blocks will be expressed as two records of 2^21 length.  We don't
opportunistically combine records when doing bmbt operations, which is
why the fsck tools have never complained about this scenario.

Offline repair is partially effective at detecting mergeable records
because I taught it to do that for the rmap and refcount btrees.  This
series enhances the free space, rmap, and refcount scrubbers to detect
mergeable records.  For the bmbt, it will flag the file as being
eligible for an optimization to shrink the size of the data structure.

The last patch in this set also enhances the rmap scrubber to detect
records that overlap incorrectly.  This check is done automatically for
non-overlapping btree types, but we have to do it separately for the
rmapbt because there are constraints on which allocation types are
allowed to overlap.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Dave Chinner <david@fromorbit.com>
fs/xfs/libxfs/xfs_bmap.c
fs/xfs/libxfs/xfs_inode_fork.c
fs/xfs/libxfs/xfs_inode_fork.h
fs/xfs/libxfs/xfs_sb.c
fs/xfs/xfs_bmap_util.c
fs/xfs/xfs_buf_item_recover.c
fs/xfs/xfs_dquot.c

index 5e66807..1a4e446 100644 (file)
@@ -1200,6 +1200,12 @@ xfs_iread_extents(
                goto out;
        }
        ASSERT(ir.loaded == xfs_iext_count(ifp));
+       /*
+        * Use release semantics so that we can use acquire semantics in
+        * xfs_need_iread_extents and be guaranteed to see a valid mapping tree
+        * after that load.
+        */
+       smp_store_release(&ifp->if_needextents, 0);
        return 0;
 out:
        xfs_iext_destroy(ifp);
index ff37eec..5a2e7dd 100644 (file)
@@ -227,10 +227,15 @@ xfs_iformat_data_fork(
 
        /*
         * Initialize the extent count early, as the per-format routines may
-        * depend on it.
+        * depend on it.  Use release semantics to set needextents /after/ we
+        * set the format. This ensures that we can use acquire semantics on
+        * needextents in xfs_need_iread_extents() and be guaranteed to see a
+        * valid format value after that load.
         */
        ip->i_df.if_format = dip->di_format;
        ip->i_df.if_nextents = xfs_dfork_data_extents(dip);
+       smp_store_release(&ip->i_df.if_needextents,
+                          ip->i_df.if_format == XFS_DINODE_FMT_BTREE ? 1 : 0);
 
        switch (inode->i_mode & S_IFMT) {
        case S_IFIFO:
@@ -283,8 +288,17 @@ xfs_ifork_init_attr(
        enum xfs_dinode_fmt     format,
        xfs_extnum_t            nextents)
 {
+       /*
+        * Initialize the extent count early, as the per-format routines may
+        * depend on it.  Use release semantics to set needextents /after/ we
+        * set the format. This ensures that we can use acquire semantics on
+        * needextents in xfs_need_iread_extents() and be guaranteed to see a
+        * valid format value after that load.
+        */
        ip->i_af.if_format = format;
        ip->i_af.if_nextents = nextents;
+       smp_store_release(&ip->i_af.if_needextents,
+                          ip->i_af.if_format == XFS_DINODE_FMT_BTREE ? 1 : 0);
 }
 
 void
index d3943d6..96d3077 100644 (file)
@@ -24,6 +24,7 @@ struct xfs_ifork {
        xfs_extnum_t            if_nextents;    /* # of extents in this fork */
        short                   if_broot_bytes; /* bytes allocated for root */
        int8_t                  if_format;      /* format of this fork */
+       uint8_t                 if_needextents; /* extents have not been read */
 };
 
 /*
@@ -260,9 +261,10 @@ int xfs_iext_count_upgrade(struct xfs_trans *tp, struct xfs_inode *ip,
                uint nr_to_add);
 
 /* returns true if the fork has extents but they are not read in yet. */
-static inline bool xfs_need_iread_extents(struct xfs_ifork *ifp)
+static inline bool xfs_need_iread_extents(const struct xfs_ifork *ifp)
 {
-       return ifp->if_format == XFS_DINODE_FMT_BTREE && ifp->if_height == 0;
+       /* see xfs_iformat_{data,attr}_fork() for needextents semantics */
+       return smp_load_acquire(&ifp->if_needextents) != 0;
 }
 
 #endif /* __XFS_INODE_FORK_H__ */
index 99cc03a..ba0f17b 100644 (file)
@@ -72,7 +72,8 @@ xfs_sb_validate_v5_features(
 }
 
 /*
- * We support all XFS versions newer than a v4 superblock with V2 directories.
+ * We current support XFS v5 formats with known features and v4 superblocks with
+ * at least V2 directories.
  */
 bool
 xfs_sb_good_version(
@@ -86,16 +87,16 @@ xfs_sb_good_version(
        if (xfs_sb_is_v5(sbp))
                return xfs_sb_validate_v5_features(sbp);
 
+       /* versions prior to v4 are not supported */
+       if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_4)
+               return false;
+
        /* We must not have any unknown v4 feature bits set */
        if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) ||
            ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) &&
             (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS)))
                return false;
 
-       /* versions prior to v4 are not supported */
-       if (XFS_SB_VERSION_NUM(sbp) < XFS_SB_VERSION_4)
-               return false;
-
        /* V4 filesystems need v2 directories and unwritten extents */
        if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
                return false;
index a09dd26..f032d3a 100644 (file)
@@ -314,15 +314,13 @@ xfs_getbmap_report_one(
        if (isnullstartblock(got->br_startblock) ||
            got->br_startblock == DELAYSTARTBLOCK) {
                /*
-                * Delalloc extents that start beyond EOF can occur due to
-                * speculative EOF allocation when the delalloc extent is larger
-                * than the largest freespace extent at conversion time.  These
-                * extents cannot be converted by data writeback, so can exist
-                * here even if we are not supposed to be finding delalloc
-                * extents.
+                * Take the flush completion as being a point-in-time snapshot
+                * where there are no delalloc extents, and if any new ones
+                * have been created racily, just skip them as being 'after'
+                * the flush and so don't get reported.
                 */
-               if (got->br_startoff < XFS_B_TO_FSB(ip->i_mount, XFS_ISIZE(ip)))
-                       ASSERT((bmv->bmv_iflags & BMV_IF_DELALLOC) != 0);
+               if (!(bmv->bmv_iflags & BMV_IF_DELALLOC))
+                       return 0;
 
                p->bmv_oflags |= BMV_OF_DELALLOC;
                p->bmv_block = -2;
index ffa9410..43167f5 100644 (file)
@@ -943,6 +943,16 @@ xlog_recover_buf_commit_pass2(
        if (lsn && lsn != -1 && XFS_LSN_CMP(lsn, current_lsn) >= 0) {
                trace_xfs_log_recover_buf_skip(log, buf_f);
                xlog_recover_validate_buf_type(mp, bp, buf_f, NULLCOMMITLSN);
+
+               /*
+                * We're skipping replay of this buffer log item due to the log
+                * item LSN being behind the ondisk buffer.  Verify the buffer
+                * contents since we aren't going to run the write verifier.
+                */
+               if (bp->b_ops) {
+                       bp->b_ops->verify_read(bp);
+                       error = bp->b_error;
+               }
                goto out_release;
        }
 
index 8fb90da..7f07175 100644 (file)
@@ -798,7 +798,6 @@ xfs_qm_dqget_cache_insert(
        error = radix_tree_insert(tree, id, dqp);
        if (unlikely(error)) {
                /* Duplicate found!  Caller must try again. */
-               WARN_ON(error != -EEXIST);
                mutex_unlock(&qi->qi_tree_lock);
                trace_xfs_dqget_dup(dqp);
                return error;