xfs: fix transaction block reservation in xrep_rtbitmap

LOLLM pointed out an inconsistency in the block reservation code in
xrep_rtbitmap.  The first is that the reservation computation is not
consistent between the code that sets up the repair and the code that
tries to avoid exceeding the transaction reservation once we know how
big the rtbitmap really must be.  As a result, the logic doesn't work.

In fixing that, a second problem emerges: if we do readjust, we ask for
the entire reservation all over again.  We really only need the delta,
so ask only for that.

Fix all these problems by hoisting the computation to a trivial helper
so that it gets used in both places.

Cc: stable@vger.kernel.org # v6.14
Fixes: 8defee8dff2b20 ("xfs: online repair of realtime bitmaps for a realtime group")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index dc64902d6c25a4209b53d76a564ccca9039d7385..442a17bf97202908bf760ea0ae5a3764464f82e5 100644 (file)
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -36,6 +36,24 @@
 
 /* rt bitmap content repairs */
 
+/*
+ * Reserve enough blocks to write out a completely new bitmap file, plus twice
+ * as many blocks as we would need if we can only allocate one block per data
+ * fork mapping.  This should cover the preallocation of the temporary file and
+ * exchanging the extent mappings.
+ *
+ * We cannot use xfs_exchmaps_estimate because we have not yet constructed the
+ * replacement bitmap and therefore do not know how many extents it will use.
+ * By the time we do, we will have a dirty transaction (which we cannot drop
+ * because we cannot drop the rtbitmap ILOCK) and cannot ask for more
+ * reservation.
+ */
+static inline unsigned long long
+xrep_rtbitmap_calc_blocks(struct xfs_mount *mp, unsigned long long blocks)
+{
+       return blocks + (xfs_bmbt_calc_size(mp, blocks) * 2);
+}
+
 /* Set up to repair the realtime bitmap for this group. */
 int
 xrep_setup_rtbitmap(
@@ -56,20 +74,7 @@ xrep_setup_rtbitmap(
        if (error)
                return error;
 
-       /*
-        * Reserve enough blocks to write out a completely new bitmap file,
-        * plus twice as many blocks as we would need if we can only allocate
-        * one block per data fork mapping.  This should cover the
-        * preallocation of the temporary file and exchanging the extent
-        * mappings.
-        *
-        * We cannot use xfs_exchmaps_estimate because we have not yet
-        * constructed the replacement bitmap and therefore do not know how
-        * many extents it will use.  By the time we do, we will have a dirty
-        * transaction (which we cannot drop because we cannot drop the
-        * rtbitmap ILOCK) and cannot ask for more reservation.
-        */
-       blocks += xfs_bmbt_calc_size(mp, blocks) * 2;
+       blocks = xrep_rtbitmap_calc_blocks(mp, mp->m_sb.sb_rbmblocks);
        if (blocks > UINT_MAX)
                return -EOPNOTSUPP;
 
@@ -512,7 +517,7 @@ xrep_rtbitmap(
        struct xchk_rtbitmap    *rtb = sc->buf;
        struct xfs_mount        *mp = sc->mp;
        struct xfs_group        *xg = rtg_group(sc->sr.rtg);
-       unsigned long long      blocks = 0;
+       unsigned long long      blocks;
        unsigned int            busy_gen;
        int                     error;
 
@@ -532,15 +537,20 @@ xrep_rtbitmap(
         * figure out if we need to adjust the block reservation in the
         * transaction.
         */
-       blocks = xfs_bmbt_calc_size(mp, rtb->rbmblocks);
+       blocks = xrep_rtbitmap_calc_blocks(mp, rtb->rbmblocks);
        if (blocks > UINT_MAX)
                return -EOPNOTSUPP;
        if (blocks > rtb->resblks) {
-               error = xfs_trans_reserve_more(sc->tp, blocks, 0);
+               uint64_t        delta = blocks - rtb->resblks;
+
+               if (delta > UINT_MAX)
+                       return -EOPNOTSUPP;
+
+               error = xfs_trans_reserve_more(sc->tp, delta, 0);
                if (error)
                        return error;
 
-               rtb->resblks += blocks;
+               rtb->resblks += delta;
        }
 
        /* Fix inode core and forks. */