Re: [PATCH 09/10] xfs: stop the steal (of data blocks for RT indirect blocks)

Darrick J. Wong
2024-02-23T17:25:23Z

On Fri, Feb 23, 2024 at 08:15:05AM +0100, Christoph Hellwig wrote:
> When xfs_bmap_del_extent_delay has to split an indirect block it tries
> to steal blocks from the the part that gets unmapped to increase the
> indirect block reservation that now needs to cover for two extents
> instead of one.
>
> This works perfectly fine on the data device, where the data and
> indirect blocks come from the same pool. It has no chance of working
> when the inode sits on the RT device. To support re-enabling delalloc
> for inodes on the RT device, make this behavior conditional on not
> beeing for rt extents. For an RT extent try allocate new blocks or
> otherwise just give up.
>
> Note that split of delalloc extents should only happen on writeback
> failure, as for other kinds of hole punching we first write back all
> data and thus convert the delalloc reservations covering the hole to
> a real allocation.
>
> Note that restoring a quota reservation is always a bit problematic,
> but the force flag should take care of it. That is, if we actually
> supported quota with the RT volume, which seems to not be the case
> at the moment.

...and for anyone following along at home, this should address that
problem:
https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=realtime-quotas_2024-02-21

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index d7fda286a4eaa0..4fa178087073b1 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4912,6 +4912,30 @@ xfs_bmap_del_extent_delay(
> WARN_ON_ONCE(!got_indlen || !new_indlen);
> stolen = xfs_bmap_split_indlen(da_old, &got_indlen, &new_indlen,
> del->br_blockcount);
> + if (isrt && stolen) {
> + /*
> + * Ugg, we can't just steal reservations from the data
> + * blocks as the data blocks come from a different pool.
> + *
> + * So we have to try to increase out reservations here,

"...try to increase our reservation here..."

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> + * and if that fails we have to fail the unmap. To
> + * avoid that as much as possible dip into the reserve
> + * pool.
> + *
> + * Note that in theory the user/group/project could
> + * be over the quota limit in the meantime, thus we
> + * force the quota accounting even if it was over the
> + * limit.
> + */
> + error = xfs_dec_fdblocks(mp, stolen, true);
> + if (error) {
> + ip->i_delayed_blks += del->br_blockcount;
> + xfs_trans_reserve_quota_nblks(NULL, ip, 0,
> + del->br_blockcount, true);
> + return error;
> + }
> + xfs_mod_delalloc(ip, 0, stolen);
> + }
>
> got->br_startblock = nullstartblock((int)got_indlen);
>
> @@ -4924,7 +4948,8 @@ xfs_bmap_del_extent_delay(
> xfs_iext_insert(ip, icur, &new, state);
>
> da_new = got_indlen + new_indlen - stolen;
> - del->br_blockcount -= stolen;
> + if (!isrt)
> + del->br_blockcount -= stolen;
> break;
> }
>
> --
> 2.39.2
>
>