xfs: account format bouncing into rmapbt swapext tx reservation
authorBrian Foster <bfoster@redhat.com>
Fri, 9 Mar 2018 22:01:58 +0000 (14:01 -0800)
committerDarrick J. Wong <darrick.wong@oracle.com>
Mon, 12 Mar 2018 03:27:57 +0000 (20:27 -0700)
The extent swap mechanism requires a unique implementation for
rmapbt enabled filesystems. Because the rmapbt tracks extent owner
information, extent swap must individually unmap and remap each
extent between the two inodes.

The rmapbt extent swap transaction block reservation currently
accounts for the worst case bmapbt block and rmapbt block
consumption based on the extent count of each inode. There is a
corner case that exists due to the extent swap implementation that
is not covered by this reservation, however.

If one of the associated inodes is just over the max extent count
used for extent format inodes (i.e., the inode is in btree format by
a single extent), the unmap/remap cycle of the extent swap can
bounce the inode between extent and btree format multiple times,
almost as many times as there are extents in the inode (if the
opposing inode happens to have one less, for example). Each back and
forth cycle involves a block free and allocation, which isn't a
problem except for that the initial transaction reservation must
account for the total number of block allocations performed by the
chain of deferred operations. If not, a block reservation overrun
occurs and the filesystem shuts down.

Update the rmapbt extent swap block reservation to check for this
situation and add some block reservation slop to ensure the entire
operation succeeds. We'd never likely require reservation for both
inodes as fsr wouldn't defrag the file in that case, but the
additional reservation is constrained by the data fork size so be
cautious and check for both.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
fs/xfs/xfs_bmap_util.c

index c83f549..e0a442f 100644 (file)
@@ -1899,17 +1899,28 @@ xfs_swap_extents(
         * performed with log redo items!
         */
        if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
+               int             w       = XFS_DATA_FORK;
+               uint32_t        ipnext  = XFS_IFORK_NEXTENTS(ip, w);
+               uint32_t        tipnext = XFS_IFORK_NEXTENTS(tip, w);
+
+               /*
+                * Conceptually this shouldn't affect the shape of either bmbt,
+                * but since we atomically move extents one by one, we reserve
+                * enough space to rebuild both trees.
+                */
+               resblks = XFS_SWAP_RMAP_SPACE_RES(mp, ipnext, w);
+               resblks +=  XFS_SWAP_RMAP_SPACE_RES(mp, tipnext, w);
+
                /*
-                * Conceptually this shouldn't affect the shape of either
-                * bmbt, but since we atomically move extents one by one,
-                * we reserve enough space to rebuild both trees.
+                * Handle the corner case where either inode might straddle the
+                * btree format boundary. If so, the inode could bounce between
+                * btree <-> extent format on unmap -> remap cycles, freeing and
+                * allocating a bmapbt block each time.
                 */
-               resblks = XFS_SWAP_RMAP_SPACE_RES(mp,
-                               XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK),
-                               XFS_DATA_FORK) +
-                         XFS_SWAP_RMAP_SPACE_RES(mp,
-                               XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK),
-                               XFS_DATA_FORK);
+               if (ipnext == (XFS_IFORK_MAXEXT(ip, w) + 1))
+                       resblks += XFS_IFORK_MAXEXT(ip, w);
+               if (tipnext == (XFS_IFORK_MAXEXT(tip, w) + 1))
+                       resblks += XFS_IFORK_MAXEXT(tip, w);
        }
        error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0, &tp);
        if (error)