On Fri, Feb 23, 2024 at 08:15:02AM +0100, Christoph Hellwig wrote:
> The code to account fdblocks and frextents in xfs_bmap_del_extent_delay
> is a bit weird in that it accounts frextents before the iext tree
> manipulations and fdblocks after it. Given that the iext tree
> manipulations can fail currently that's not really a problem, but
cannot?
If my correction ^^^ is correct, then:
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> still odd. Move the frextent manipulation to the end, and use a
> fdblocks variable to account of the unconditional indirect blocks and
> the data blocks only freed for !RT. This prepares for following
> updates in the area and already makes the code more readable.
>
> Also remove the !isrt assert given that this code clearly handles
> rt extents correctly, and we'll soon reinstate delalloc support for
> RT inodes.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 95e93534cd1264..074d833e845af3 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4833,6 +4833,7 @@ xfs_bmap_del_extent_delay(
> xfs_fileoff_t del_endoff, got_endoff;
> xfs_filblks_t got_indlen, new_indlen, stolen;
> uint32_t state = xfs_bmap_fork_to_state(whichfork);
> + uint64_t fdblocks;
> int error = 0;
> bool isrt;
>
> @@ -4848,15 +4849,11 @@ xfs_bmap_del_extent_delay(
> ASSERT(got->br_startoff <= del->br_startoff);
> ASSERT(got_endoff >= del_endoff);
>
> - if (isrt)
> - xfs_add_frextents(mp, xfs_rtb_to_rtx(mp, del->br_blockcount));
> -
> /*
> * Update the inode delalloc counter now and wait to update the
> * sb counters as we might have to borrow some blocks for the
> * indirect block accounting.
> */
> - ASSERT(!isrt);
> error = xfs_quota_unreserve_blkres(ip, del->br_blockcount);
> if (error)
> return error;
> @@ -4933,12 +4930,15 @@ xfs_bmap_del_extent_delay(
>
> ASSERT(da_old >= da_new);
> da_diff = da_old - da_new;
> - if (!isrt)
> - da_diff += del->br_blockcount;
> - if (da_diff) {
> - xfs_add_fdblocks(mp, da_diff);
> - xfs_mod_delalloc(mp, -da_diff);
> - }
> + fdblocks = da_diff;
> +
> + if (isrt)
> + xfs_add_frextents(mp, xfs_rtb_to_rtx(mp, del->br_blockcount));
> + else
> + fdblocks += del->br_blockcount;
> +
> + xfs_add_fdblocks(mp, fdblocks);
> + xfs_mod_delalloc(mp, -(int64_t)fdblocks);
> return error;
> }
>
> --
> 2.39.2
>
>
