[PATCH] xfs: split xfs_zone_record_blocks

[PATCH] xfs: split xfs_zone_record_blocks

[Christoph Hellwig]

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@lst.de>
---
 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

Re: [PATCH] xfs: split xfs_zone_record_blocks

[Darrick J. Wong]

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@lst.de>

Looks good to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--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
> 
> 

Re: [PATCH] xfs: split xfs_zone_record_blocks

[Carlos Maiolino]

On Wed, 23 Jul 2025 07:35:44 +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.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: split xfs_zone_record_blocks
      commit: f76823e3b284aae30797fded988a807eab2da246

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>