On Wed, Jul 23, 2025 at 07:35:44AM +0200, Christoph Hellwig wrote:
> xfs_zone_record_blocks not only records successfully written blocks that
> now back file data, but is also used for blocks speculatively written by
> garbage collection that were never linked to an inode and instantly
> become invalid.
>
> Split the latter functionality out to be easier to understand. This also
> make it clear that we don't need to attach the rmap inode to a
> transaction for the skipped blocks case as we never dirty any peristent
> data structure.
>
> Also make the argument order to xfs_zone_record_blocks a bit more
> natural.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good to me,
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_trace.h | 1 +
> fs/xfs/xfs_zone_alloc.c | 42 ++++++++++++++++++++++++++++-------------
> 2 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 10d4fd671dcf..178b204dee3b 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -455,6 +455,7 @@ DEFINE_EVENT(xfs_zone_alloc_class, name, \
> xfs_extlen_t len), \
> TP_ARGS(oz, rgbno, len))
> DEFINE_ZONE_ALLOC_EVENT(xfs_zone_record_blocks);
> +DEFINE_ZONE_ALLOC_EVENT(xfs_zone_skip_blocks);
> DEFINE_ZONE_ALLOC_EVENT(xfs_zone_alloc_blocks);
>
> TRACE_EVENT(xfs_zone_gc_select_victim,
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 33f7eee521a8..f8bd6d741755 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -166,10 +166,9 @@ xfs_open_zone_mark_full(
> static void
> xfs_zone_record_blocks(
> struct xfs_trans *tp,
> - xfs_fsblock_t fsbno,
> - xfs_filblks_t len,
> struct xfs_open_zone *oz,
> - bool used)
> + xfs_fsblock_t fsbno,
> + xfs_filblks_t len)
> {
> struct xfs_mount *mp = tp->t_mountp;
> struct xfs_rtgroup *rtg = oz->oz_rtg;
> @@ -179,18 +178,37 @@ xfs_zone_record_blocks(
>
> xfs_rtgroup_lock(rtg, XFS_RTGLOCK_RMAP);
> xfs_rtgroup_trans_join(tp, rtg, XFS_RTGLOCK_RMAP);
> - if (used) {
> - rmapip->i_used_blocks += len;
> - ASSERT(rmapip->i_used_blocks <= rtg_blocks(rtg));
> - } else {
> - xfs_add_frextents(mp, len);
> - }
> + rmapip->i_used_blocks += len;
> + ASSERT(rmapip->i_used_blocks <= rtg_blocks(rtg));
> oz->oz_written += len;
> if (oz->oz_written == rtg_blocks(rtg))
> xfs_open_zone_mark_full(oz);
> xfs_trans_log_inode(tp, rmapip, XFS_ILOG_CORE);
> }
>
> +/*
> + * Called for blocks that have been written to disk, but not actually linked to
> + * an inode, which can happen when garbage collection races with user data
> + * writes to a file.
> + */
> +static void
> +xfs_zone_skip_blocks(
> + struct xfs_open_zone *oz,
> + xfs_filblks_t len)
> +{
> + struct xfs_rtgroup *rtg = oz->oz_rtg;
> +
> + trace_xfs_zone_skip_blocks(oz, 0, len);
> +
> + xfs_rtgroup_lock(rtg, XFS_RTGLOCK_RMAP);
> + oz->oz_written += len;
> + if (oz->oz_written == rtg_blocks(rtg))
> + xfs_open_zone_mark_full(oz);
> + xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
> +
> + xfs_add_frextents(rtg_mount(rtg), len);
> +}
> +
> static int
> xfs_zoned_map_extent(
> struct xfs_trans *tp,
> @@ -250,8 +268,7 @@ xfs_zoned_map_extent(
> }
> }
>
> - xfs_zone_record_blocks(tp, new->br_startblock, new->br_blockcount, oz,
> - true);
> + xfs_zone_record_blocks(tp, oz, new->br_startblock, new->br_blockcount);
>
> /* Map the new blocks into the data fork. */
> xfs_bmap_map_extent(tp, ip, XFS_DATA_FORK, new);
> @@ -259,8 +276,7 @@ xfs_zoned_map_extent(
>
> skip:
> trace_xfs_reflink_cow_remap_skip(ip, new);
> - xfs_zone_record_blocks(tp, new->br_startblock, new->br_blockcount, oz,
> - false);
> + xfs_zone_skip_blocks(oz, new->br_blockcount);
> return 0;
> }
>
> --
> 2.47.2
>
>
