misc zoned allocators fixes and cleanups

misc zoned allocators fixes and cleanups

[Christoph Hellwig]

Hi all,

this series has a few improvements to the zoned code.  The first
is a memory leak fix that might still be useful for 6.16 if possible.

[PATCH 1/7] xfs: don't allocate the xfs_extent_busy structure for zoned RTGs

[Christoph Hellwig]

Busy extent tracking is primarily used to ensure that freed blocks are
not reused for data allocations before the transaction that deleted them
has been committed to stable storage, and secondarily to drive online
discard.  None of the use cases applies to zoned RTGs, as the zoned
allocator can't overwrite blocks before resetting the zone, which already
flushes out all transactions touching the RTGs.

So the busy extent tracking is not needed for zoned RTGs, and also not
called for zoned RTGs.  But somehow the code to skip allocating and
freeing the structure got lost during the zoned XFS upstreaming process.
This not only causes these structures to unessecarily allocated, but can
also lead to memory leaks as the xg_busy_extents pointer in the
xfs_group structure is overlayed with the pointer for the linked list
of to be reset zones.

Stop allocating and freeing the structure to not pointlessly allocate
memory which is then leaked when the zone is reset.

Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_group.c | 14 +++++++++-----
 fs/xfs/xfs_extent_busy.h  |  8 ++++++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_group.c b/fs/xfs/libxfs/xfs_group.c
index e9d76bcdc820..20ad7c309489 100644
--- a/fs/xfs/libxfs/xfs_group.c
+++ b/fs/xfs/libxfs/xfs_group.c
@@ -163,7 +163,8 @@ xfs_group_free(
 
 	xfs_defer_drain_free(&xg->xg_intents_drain);
 #ifdef __KERNEL__
-	kfree(xg->xg_busy_extents);
+	if (xfs_group_has_extent_busy(xg->xg_mount, xg->xg_type))
+		kfree(xg->xg_busy_extents);
 #endif
 
 	if (uninit)
@@ -189,9 +190,11 @@ xfs_group_insert(
 	xg->xg_type = type;
 
 #ifdef __KERNEL__
-	xg->xg_busy_extents = xfs_extent_busy_alloc();
-	if (!xg->xg_busy_extents)
-		return -ENOMEM;
+	if (xfs_group_has_extent_busy(mp, type)) {
+		xg->xg_busy_extents = xfs_extent_busy_alloc();
+		if (!xg->xg_busy_extents)
+			return -ENOMEM;
+	}
 	spin_lock_init(&xg->xg_state_lock);
 	xfs_hooks_init(&xg->xg_rmap_update_hooks);
 #endif
@@ -210,7 +213,8 @@ xfs_group_insert(
 out_drain:
 	xfs_defer_drain_free(&xg->xg_intents_drain);
 #ifdef __KERNEL__
-	kfree(xg->xg_busy_extents);
+	if (xfs_group_has_extent_busy(xg->xg_mount, xg->xg_type))
+		kfree(xg->xg_busy_extents);
 #endif
 	return error;
 }
diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h
index f069b04e8ea1..3e6e019b6146 100644
--- a/fs/xfs/xfs_extent_busy.h
+++ b/fs/xfs/xfs_extent_busy.h
@@ -68,4 +68,12 @@ static inline void xfs_extent_busy_sort(struct list_head *list)
 	list_sort(NULL, list, xfs_extent_busy_ag_cmp);
 }
 
+/*
+ * Zoned RTGs don't need to track busy extents, as the actual block freeing only
+ * happens by a zone reset, which forces out all transactions that touched the
+ * to be reset zone first.
+ */
+#define xfs_group_has_extent_busy(mp, type) \
+	((type) == XG_TYPE_AG || !xfs_has_zoned((mp)))
+
 #endif /* __XFS_EXTENT_BUSY_H__ */
-- 
2.47.2

[PATCH 2/7] xfs: use a uint32_t to cache i_used_blocks in xfs_init_zone

[Christoph Hellwig]

i_used_blocks is a uint32_t, so use the same value for the local variable
caching it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 01315ed75502..867465b5b5fe 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1017,7 +1017,7 @@ xfs_init_zone(
 {
 	struct xfs_mount	*mp = rtg_mount(rtg);
 	struct xfs_zone_info	*zi = mp->m_zone_info;
-	uint64_t		used = rtg_rmap(rtg)->i_used_blocks;
+	uint32_t		used = rtg_rmap(rtg)->i_used_blocks;
 	xfs_rgblock_t		write_pointer, highest_rgbno;
 	int			error;
 
-- 
2.47.2

[PATCH 3/7] xfs: rename oz_write_pointer to oz_allocated

[Christoph Hellwig]

This member just tracks how much space we handed out for sequential
write required zones.  Only for conventional space it actually is the
pointer where thing are written at, otherwise zone append manages
that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trace.h      |  8 ++++----
 fs/xfs/xfs_zone_alloc.c | 18 +++++++++---------
 fs/xfs/xfs_zone_gc.c    | 13 ++++++-------
 fs/xfs/xfs_zone_info.c  |  2 +-
 fs/xfs/xfs_zone_priv.h  | 16 ++++++++--------
 5 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 6addebd764b0..10d4fd671dcf 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -425,8 +425,8 @@ DECLARE_EVENT_CLASS(xfs_zone_alloc_class,
 		__field(dev_t, dev)
 		__field(xfs_rgnumber_t, rgno)
 		__field(xfs_rgblock_t, used)
+		__field(xfs_rgblock_t, allocated)
 		__field(xfs_rgblock_t, written)
-		__field(xfs_rgblock_t, write_pointer)
 		__field(xfs_rgblock_t, rgbno)
 		__field(xfs_extlen_t, len)
 	),
@@ -434,17 +434,17 @@ DECLARE_EVENT_CLASS(xfs_zone_alloc_class,
 		__entry->dev = rtg_mount(oz->oz_rtg)->m_super->s_dev;
 		__entry->rgno = rtg_rgno(oz->oz_rtg);
 		__entry->used = rtg_rmap(oz->oz_rtg)->i_used_blocks;
+		__entry->allocated = oz->oz_allocated;
 		__entry->written = oz->oz_written;
-		__entry->write_pointer = oz->oz_write_pointer;
 		__entry->rgbno = rgbno;
 		__entry->len = len;
 	),
-	TP_printk("dev %d:%d rgno 0x%x used 0x%x written 0x%x wp 0x%x rgbno 0x%x len 0x%x",
+	TP_printk("dev %d:%d rgno 0x%x used 0x%x alloced 0x%x written 0x%x rgbno 0x%x len 0x%x",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rgno,
 		  __entry->used,
+		  __entry->allocated,
 		  __entry->written,
-		  __entry->write_pointer,
 		  __entry->rgbno,
 		  __entry->len)
 );
diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 867465b5b5fe..729d80ff52c1 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -434,7 +434,7 @@ xfs_init_open_zone(
 	spin_lock_init(&oz->oz_alloc_lock);
 	atomic_set(&oz->oz_ref, 1);
 	oz->oz_rtg = rtg;
-	oz->oz_write_pointer = write_pointer;
+	oz->oz_allocated = write_pointer;
 	oz->oz_written = write_pointer;
 	oz->oz_write_hint = write_hint;
 	oz->oz_is_gc = is_gc;
@@ -569,7 +569,7 @@ xfs_try_use_zone(
 	struct xfs_open_zone	*oz,
 	bool			lowspace)
 {
-	if (oz->oz_write_pointer == rtg_blocks(oz->oz_rtg))
+	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg))
 		return false;
 	if (!lowspace && !xfs_good_hint_match(oz, file_hint))
 		return false;
@@ -744,25 +744,25 @@ xfs_zone_alloc_blocks(
 {
 	struct xfs_rtgroup	*rtg = oz->oz_rtg;
 	struct xfs_mount	*mp = rtg_mount(rtg);
-	xfs_rgblock_t		rgbno;
+	xfs_rgblock_t		allocated;
 
 	spin_lock(&oz->oz_alloc_lock);
 	count_fsb = min3(count_fsb, XFS_MAX_BMBT_EXTLEN,
-		(xfs_filblks_t)rtg_blocks(rtg) - oz->oz_write_pointer);
+		(xfs_filblks_t)rtg_blocks(rtg) - oz->oz_allocated);
 	if (!count_fsb) {
 		spin_unlock(&oz->oz_alloc_lock);
 		return 0;
 	}
-	rgbno = oz->oz_write_pointer;
-	oz->oz_write_pointer += count_fsb;
+	allocated = oz->oz_allocated;
+	oz->oz_allocated += count_fsb;
 	spin_unlock(&oz->oz_alloc_lock);
 
-	trace_xfs_zone_alloc_blocks(oz, rgbno, count_fsb);
+	trace_xfs_zone_alloc_blocks(oz, allocated, count_fsb);
 
 	*sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
 	*is_seq = bdev_zone_is_seq(mp->m_rtdev_targp->bt_bdev, *sector);
 	if (!*is_seq)
-		*sector += XFS_FSB_TO_BB(mp, rgbno);
+		*sector += XFS_FSB_TO_BB(mp, allocated);
 	return XFS_FSB_TO_B(mp, count_fsb);
 }
 
@@ -983,7 +983,7 @@ xfs_zone_rgbno_is_valid(
 	lockdep_assert_held(&rtg_rmap(rtg)->i_lock);
 
 	if (rtg->rtg_open_zone)
-		return rgbno < rtg->rtg_open_zone->oz_write_pointer;
+		return rgbno < rtg->rtg_open_zone->oz_allocated;
 	return !xa_get_mark(&rtg_mount(rtg)->m_groups[XG_TYPE_RTG].xa,
 			rtg_rgno(rtg), XFS_RTG_FREE);
 }
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 9c00fc5baa30..7ea9fa77b061 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -533,8 +533,7 @@ xfs_zone_gc_steal_open(
 
 	spin_lock(&zi->zi_open_zones_lock);
 	list_for_each_entry(oz, &zi->zi_open_zones, oz_entry) {
-		if (!found ||
-		    oz->oz_write_pointer < found->oz_write_pointer)
+		if (!found || oz->oz_allocated < found->oz_allocated)
 			found = oz;
 	}
 
@@ -584,7 +583,7 @@ xfs_zone_gc_ensure_target(
 {
 	struct xfs_open_zone	*oz = mp->m_zone_info->zi_open_gc_zone;
 
-	if (!oz || oz->oz_write_pointer == rtg_blocks(oz->oz_rtg))
+	if (!oz || oz->oz_allocated == rtg_blocks(oz->oz_rtg))
 		return xfs_zone_gc_select_target(mp);
 	return oz;
 }
@@ -605,7 +604,7 @@ xfs_zone_gc_space_available(
 	oz = xfs_zone_gc_ensure_target(data->mp);
 	if (!oz)
 		return false;
-	return oz->oz_write_pointer < rtg_blocks(oz->oz_rtg) &&
+	return oz->oz_allocated < rtg_blocks(oz->oz_rtg) &&
 		xfs_zone_gc_scratch_available(data);
 }
 
@@ -647,7 +646,7 @@ xfs_zone_gc_alloc_blocks(
 	 */
 	spin_lock(&mp->m_sb_lock);
 	*count_fsb = min(*count_fsb,
-			rtg_blocks(oz->oz_rtg) - oz->oz_write_pointer);
+			rtg_blocks(oz->oz_rtg) - oz->oz_allocated);
 	*count_fsb = min3(*count_fsb,
 			mp->m_free[XC_FREE_RTEXTENTS].res_avail,
 			mp->m_free[XC_FREE_RTAVAILABLE].res_avail);
@@ -661,8 +660,8 @@ xfs_zone_gc_alloc_blocks(
 	*daddr = xfs_gbno_to_daddr(&oz->oz_rtg->rtg_group, 0);
 	*is_seq = bdev_zone_is_seq(mp->m_rtdev_targp->bt_bdev, *daddr);
 	if (!*is_seq)
-		*daddr += XFS_FSB_TO_BB(mp, oz->oz_write_pointer);
-	oz->oz_write_pointer += *count_fsb;
+		*daddr += XFS_FSB_TO_BB(mp, oz->oz_allocated);
+	oz->oz_allocated += *count_fsb;
 	atomic_inc(&oz->oz_ref);
 	return oz;
 }
diff --git a/fs/xfs/xfs_zone_info.c b/fs/xfs/xfs_zone_info.c
index 733bcc2f8645..07e30c596975 100644
--- a/fs/xfs/xfs_zone_info.c
+++ b/fs/xfs/xfs_zone_info.c
@@ -32,7 +32,7 @@ xfs_show_open_zone(
 {
 	seq_printf(m, "\t  zone %d, wp %u, written %u, used %u, hint %s\n",
 		rtg_rgno(oz->oz_rtg),
-		oz->oz_write_pointer, oz->oz_written,
+		oz->oz_allocated, oz->oz_written,
 		rtg_rmap(oz->oz_rtg)->i_used_blocks,
 		xfs_write_hint_to_str(oz->oz_write_hint));
 }
diff --git a/fs/xfs/xfs_zone_priv.h b/fs/xfs/xfs_zone_priv.h
index ab696975a993..35e6de3d25ed 100644
--- a/fs/xfs/xfs_zone_priv.h
+++ b/fs/xfs/xfs_zone_priv.h
@@ -11,18 +11,18 @@ struct xfs_open_zone {
 	atomic_t		oz_ref;
 
 	/*
-	 * oz_write_pointer is the write pointer at which space is handed out
-	 * for conventional zones, or simple the count of blocks handed out
-	 * so far for sequential write required zones and is protected by
-	 * oz_alloc_lock/
+	 * oz_allocated is the amount of space already allocated out of the zone
+	 * and is protected by oz_alloc_lock.
+	 *
+	 * For conventional zones it also is the offset of the next write.
 	 */
 	spinlock_t		oz_alloc_lock;
-	xfs_rgblock_t		oz_write_pointer;
+	xfs_rgblock_t		oz_allocated;
 
 	/*
-	 * oz_written is the number of blocks for which we've received a
-	 * write completion.  oz_written must always be <= oz_write_pointer
-	 * and is protected by the ILOCK of the rmap inode.
+	 * oz_written is the number of blocks for which we've received a write
+	 * completion.  oz_written must always be <= oz_allocated and is
+	 * protected by the ILOCK of the rmap inode.
 	 */
 	xfs_rgblock_t		oz_written;
 
-- 
2.47.2

[PATCH 4/7] xfs: stop passing an inode to the zone space reservation helpers

[Christoph Hellwig]

None of them actually needs the inode, the mount is enough.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_file.c            | 24 ++++++++++++------------
 fs/xfs/xfs_iops.c            |  4 ++--
 fs/xfs/xfs_zone_alloc.h      |  4 ++--
 fs/xfs/xfs_zone_space_resv.c | 17 ++++++-----------
 4 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 38e365b16348..ed69a65f56d7 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -497,7 +497,7 @@ xfs_file_write_checks(
 
 static ssize_t
 xfs_zoned_write_space_reserve(
-	struct xfs_inode		*ip,
+	struct xfs_mount		*mp,
 	struct kiocb			*iocb,
 	struct iov_iter			*from,
 	unsigned int			flags,
@@ -533,8 +533,8 @@ xfs_zoned_write_space_reserve(
 	 *
 	 * Any remaining block will be returned after the write.
 	 */
-	return xfs_zoned_space_reserve(ip,
-			XFS_B_TO_FSB(ip->i_mount, count) + 1 + 2, flags, ac);
+	return xfs_zoned_space_reserve(mp, XFS_B_TO_FSB(mp, count) + 1 + 2,
+			flags, ac);
 }
 
 static int
@@ -718,13 +718,13 @@ xfs_file_dio_write_zoned(
 	struct xfs_zone_alloc_ctx ac = { };
 	ssize_t			ret;
 
-	ret = xfs_zoned_write_space_reserve(ip, iocb, from, 0, &ac);
+	ret = xfs_zoned_write_space_reserve(ip->i_mount, iocb, from, 0, &ac);
 	if (ret < 0)
 		return ret;
 	ret = xfs_file_dio_write_aligned(ip, iocb, from,
 			&xfs_zoned_direct_write_iomap_ops,
 			&xfs_dio_zoned_write_ops, &ac);
-	xfs_zoned_space_unreserve(ip, &ac);
+	xfs_zoned_space_unreserve(ip->i_mount, &ac);
 	return ret;
 }
 
@@ -1032,7 +1032,7 @@ xfs_file_buffered_write_zoned(
 	struct xfs_zone_alloc_ctx ac = { };
 	ssize_t			ret;
 
-	ret = xfs_zoned_write_space_reserve(ip, iocb, from, XFS_ZR_GREEDY, &ac);
+	ret = xfs_zoned_write_space_reserve(mp, iocb, from, XFS_ZR_GREEDY, &ac);
 	if (ret < 0)
 		return ret;
 
@@ -1073,7 +1073,7 @@ xfs_file_buffered_write_zoned(
 out_unlock:
 	xfs_iunlock(ip, iolock);
 out_unreserve:
-	xfs_zoned_space_unreserve(ip, &ac);
+	xfs_zoned_space_unreserve(ip->i_mount, &ac);
 	if (ret > 0) {
 		XFS_STATS_ADD(mp, xs_write_bytes, ret);
 		ret = generic_write_sync(iocb, ret);
@@ -1414,11 +1414,11 @@ xfs_file_zoned_fallocate(
 	struct xfs_inode	*ip = XFS_I(file_inode(file));
 	int			error;
 
-	error = xfs_zoned_space_reserve(ip, 2, XFS_ZR_RESERVED, &ac);
+	error = xfs_zoned_space_reserve(ip->i_mount, 2, XFS_ZR_RESERVED, &ac);
 	if (error)
 		return error;
 	error = __xfs_file_fallocate(file, mode, offset, len, &ac);
-	xfs_zoned_space_unreserve(ip, &ac);
+	xfs_zoned_space_unreserve(ip->i_mount, &ac);
 	return error;
 }
 
@@ -1828,12 +1828,12 @@ xfs_write_fault_zoned(
 	 * But as the overallocation is limited to less than a folio and will be
 	 * release instantly that's just fine.
 	 */
-	error = xfs_zoned_space_reserve(ip, XFS_B_TO_FSB(ip->i_mount, len), 0,
-			&ac);
+	error = xfs_zoned_space_reserve(ip->i_mount,
+			XFS_B_TO_FSB(ip->i_mount, len), 0, &ac);
 	if (error < 0)
 		return vmf_fs_error(error);
 	ret = __xfs_write_fault(vmf, order, &ac);
-	xfs_zoned_space_unreserve(ip, &ac);
+	xfs_zoned_space_unreserve(ip->i_mount, &ac);
 	return ret;
 }
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 01e597290eb5..149b5460fbfd 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -970,7 +970,7 @@ xfs_setattr_size(
 	 * change.
 	 */
 	if (xfs_is_zoned_inode(ip)) {
-		error = xfs_zoned_space_reserve(ip, 1,
+		error = xfs_zoned_space_reserve(mp, 1,
 				XFS_ZR_NOWAIT | XFS_ZR_RESERVED, &ac);
 		if (error) {
 			if (error == -EAGAIN)
@@ -998,7 +998,7 @@ xfs_setattr_size(
 	}
 
 	if (xfs_is_zoned_inode(ip))
-		xfs_zoned_space_unreserve(ip, &ac);
+		xfs_zoned_space_unreserve(mp, &ac);
 
 	if (error)
 		return error;
diff --git a/fs/xfs/xfs_zone_alloc.h b/fs/xfs/xfs_zone_alloc.h
index ecf39106704c..4db02816d0fd 100644
--- a/fs/xfs/xfs_zone_alloc.h
+++ b/fs/xfs/xfs_zone_alloc.h
@@ -23,9 +23,9 @@ struct xfs_zone_alloc_ctx {
  */
 #define XFS_ZR_RESERVED		(1U << 2)
 
-int xfs_zoned_space_reserve(struct xfs_inode *ip, xfs_filblks_t count_fsb,
+int xfs_zoned_space_reserve(struct xfs_mount *mp, xfs_filblks_t count_fsb,
 		unsigned int flags, struct xfs_zone_alloc_ctx *ac);
-void xfs_zoned_space_unreserve(struct xfs_inode *ip,
+void xfs_zoned_space_unreserve(struct xfs_mount *mp,
 		struct xfs_zone_alloc_ctx *ac);
 void xfs_zoned_add_available(struct xfs_mount *mp, xfs_filblks_t count_fsb);
 
diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
index 93c9a7721139..1313c55b8cbe 100644
--- a/fs/xfs/xfs_zone_space_resv.c
+++ b/fs/xfs/xfs_zone_space_resv.c
@@ -117,11 +117,10 @@ xfs_zoned_space_wait_error(
 
 static int
 xfs_zoned_reserve_available(
-	struct xfs_inode		*ip,
+	struct xfs_mount		*mp,
 	xfs_filblks_t			count_fsb,
 	unsigned int			flags)
 {
-	struct xfs_mount		*mp = ip->i_mount;
 	struct xfs_zone_info		*zi = mp->m_zone_info;
 	struct xfs_zone_reservation	reservation = {
 		.task		= current,
@@ -198,11 +197,10 @@ xfs_zoned_reserve_available(
  */
 static int
 xfs_zoned_reserve_extents_greedy(
-	struct xfs_inode		*ip,
+	struct xfs_mount		*mp,
 	xfs_filblks_t			*count_fsb,
 	unsigned int			flags)
 {
-	struct xfs_mount		*mp = ip->i_mount;
 	struct xfs_zone_info		*zi = mp->m_zone_info;
 	s64				len = *count_fsb;
 	int				error = -ENOSPC;
@@ -220,12 +218,11 @@ xfs_zoned_reserve_extents_greedy(
 
 int
 xfs_zoned_space_reserve(
-	struct xfs_inode		*ip,
+	struct xfs_mount		*mp,
 	xfs_filblks_t			count_fsb,
 	unsigned int			flags,
 	struct xfs_zone_alloc_ctx	*ac)
 {
-	struct xfs_mount		*mp = ip->i_mount;
 	int				error;
 
 	ASSERT(ac->reserved_blocks == 0);
@@ -234,11 +231,11 @@ xfs_zoned_space_reserve(
 	error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
 			flags & XFS_ZR_RESERVED);
 	if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
-		error = xfs_zoned_reserve_extents_greedy(ip, &count_fsb, flags);
+		error = xfs_zoned_reserve_extents_greedy(mp, &count_fsb, flags);
 	if (error)
 		return error;
 
-	error = xfs_zoned_reserve_available(ip, count_fsb, flags);
+	error = xfs_zoned_reserve_available(mp, count_fsb, flags);
 	if (error) {
 		xfs_add_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb);
 		return error;
@@ -249,12 +246,10 @@ xfs_zoned_space_reserve(
 
 void
 xfs_zoned_space_unreserve(
-	struct xfs_inode		*ip,
+	struct xfs_mount		*mp,
 	struct xfs_zone_alloc_ctx	*ac)
 {
 	if (ac->reserved_blocks > 0) {
-		struct xfs_mount	*mp = ip->i_mount;
-
 		xfs_zoned_add_available(mp, ac->reserved_blocks);
 		xfs_add_freecounter(mp, XC_FREE_RTEXTENTS, ac->reserved_blocks);
 	}
-- 
2.47.2

[PATCH 5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()

[Christoph Hellwig]

From: George Hu <integral@archlinux.org>

Refactor the xfs_max_open_zones() function by replacing the usage of
min() and max() macro with clamp() to simplify the code and improve
readability.

Signed-off-by: George Hu <integral@archlinux.org>
Reviewed-by: John Garry <john.g.garry@oracle.com>
---
 fs/xfs/xfs_zone_alloc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 729d80ff52c1..d9e2b1411434 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1133,9 +1133,7 @@ xfs_max_open_zones(
 	/*
 	 * Cap the max open limit to 1/4 of available space
 	 */
-	max_open = min(max_open, mp->m_sb.sb_rgcount / 4);
-
-	return max(XFS_MIN_OPEN_ZONES, max_open);
+	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
 }
 
 /*
-- 
2.47.2

[PATCH 6/7] xfs: improve the comments in xfs_max_open_zones

[Christoph Hellwig]

Describe the rationale for the decisions a bit better.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_alloc.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index d9e2b1411434..c1f053f4a82a 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1114,24 +1114,27 @@ xfs_get_zone_info_cb(
 }
 
 /*
- * Calculate the max open zone limit based on the of number of
- * backing zones available
+ * Calculate the max open zone limit based on the of number of backing zones
+ * available.
  */
 static inline uint32_t
 xfs_max_open_zones(
 	struct xfs_mount	*mp)
 {
 	unsigned int		max_open, max_open_data_zones;
+
 	/*
-	 * We need two zones for every open data zone,
-	 * one in reserve as we don't reclaim open zones. One data zone
-	 * and its spare is included in XFS_MIN_ZONES.
+	 * We need two zones for every open data zone, one in reserve as we
+	 * don't reclaim open zones.  One data zone and its spare is included
+	 * in XFS_MIN_ZONES to support at least one user data writer.
 	 */
 	max_open_data_zones = (mp->m_sb.sb_rgcount - XFS_MIN_ZONES) / 2 + 1;
 	max_open = max_open_data_zones + XFS_OPEN_GC_ZONES;
 
 	/*
-	 * Cap the max open limit to 1/4 of available space
+	 * Cap the max open limit to 1/4 of available space.  Without this we'd
+	 * run out of easy reclaim targets too quickly and storage devices don't
+	 * handle huge numbers of concurrent write streams overly well.
 	 */
 	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
 }
-- 
2.47.2

[PATCH 7/7] xfs: improve the comments in xfs_select_zone_nowait

[Christoph Hellwig]

The top of the function comment is outdated, and the parts still correct
duplicate information in comment inside the function.  Remove the top of
the function comment and instead improve a comment inside the function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_alloc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index c1f053f4a82a..4e4ca5bbfc47 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -654,13 +654,6 @@ static inline bool xfs_zoned_pack_tight(struct xfs_inode *ip)
 		!(ip->i_diflags & XFS_DIFLAG_APPEND);
 }
 
-/*
- * Pick a new zone for writes.
- *
- * If we aren't using up our budget of open zones just open a new one from the
- * freelist.  Else try to find one that matches the expected data lifetime.  If
- * we don't find one that is good pick any zone that is available.
- */
 static struct xfs_open_zone *
 xfs_select_zone_nowait(
 	struct xfs_mount	*mp,
@@ -688,7 +681,8 @@ xfs_select_zone_nowait(
 		goto out_unlock;
 
 	/*
-	 * See if we can open a new zone and use that.
+	 * See if we can open a new zone and use that so that data for different
+	 * files is mixed as little as possible.
 	 */
 	oz = xfs_try_open_zone(mp, write_hint);
 	if (oz)
-- 
2.47.2

Re: [PATCH 1/7] xfs: don't allocate the xfs_extent_busy structure for zoned RTGs

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:01PM +0200, Christoph Hellwig wrote:
> Busy extent tracking is primarily used to ensure that freed blocks are
> not reused for data allocations before the transaction that deleted them
> has been committed to stable storage, and secondarily to drive online
> discard.  None of the use cases applies to zoned RTGs, as the zoned
> allocator can't overwrite blocks before resetting the zone, which already
> flushes out all transactions touching the RTGs.
> 
> So the busy extent tracking is not needed for zoned RTGs, and also not
> called for zoned RTGs.  But somehow the code to skip allocating and
> freeing the structure got lost during the zoned XFS upstreaming process.
> This not only causes these structures to unessecarily allocated, but can

nit: unnecessarily

> also lead to memory leaks as the xg_busy_extents pointer in the
> xfs_group structure is overlayed with the pointer for the linked list
> of to be reset zones.
> 
> Stop allocating and freeing the structure to not pointlessly allocate
> memory which is then leaked when the zone is reset.

Yikes!

Cc: <stable@vger.kernel.org> # v6.15

> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

With those added,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_group.c | 14 +++++++++-----
>  fs/xfs/xfs_extent_busy.h  |  8 ++++++++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_group.c b/fs/xfs/libxfs/xfs_group.c
> index e9d76bcdc820..20ad7c309489 100644
> --- a/fs/xfs/libxfs/xfs_group.c
> +++ b/fs/xfs/libxfs/xfs_group.c
> @@ -163,7 +163,8 @@ xfs_group_free(
>  
>  	xfs_defer_drain_free(&xg->xg_intents_drain);
>  #ifdef __KERNEL__
> -	kfree(xg->xg_busy_extents);
> +	if (xfs_group_has_extent_busy(xg->xg_mount, xg->xg_type))
> +		kfree(xg->xg_busy_extents);
>  #endif
>  
>  	if (uninit)
> @@ -189,9 +190,11 @@ xfs_group_insert(
>  	xg->xg_type = type;
>  
>  #ifdef __KERNEL__
> -	xg->xg_busy_extents = xfs_extent_busy_alloc();
> -	if (!xg->xg_busy_extents)
> -		return -ENOMEM;
> +	if (xfs_group_has_extent_busy(mp, type)) {
> +		xg->xg_busy_extents = xfs_extent_busy_alloc();
> +		if (!xg->xg_busy_extents)
> +			return -ENOMEM;
> +	}
>  	spin_lock_init(&xg->xg_state_lock);
>  	xfs_hooks_init(&xg->xg_rmap_update_hooks);
>  #endif
> @@ -210,7 +213,8 @@ xfs_group_insert(
>  out_drain:
>  	xfs_defer_drain_free(&xg->xg_intents_drain);
>  #ifdef __KERNEL__
> -	kfree(xg->xg_busy_extents);
> +	if (xfs_group_has_extent_busy(xg->xg_mount, xg->xg_type))
> +		kfree(xg->xg_busy_extents);
>  #endif
>  	return error;
>  }
> diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h
> index f069b04e8ea1..3e6e019b6146 100644
> --- a/fs/xfs/xfs_extent_busy.h
> +++ b/fs/xfs/xfs_extent_busy.h
> @@ -68,4 +68,12 @@ static inline void xfs_extent_busy_sort(struct list_head *list)
>  	list_sort(NULL, list, xfs_extent_busy_ag_cmp);
>  }
>  
> +/*
> + * Zoned RTGs don't need to track busy extents, as the actual block freeing only
> + * happens by a zone reset, which forces out all transactions that touched the
> + * to be reset zone first.
> + */
> +#define xfs_group_has_extent_busy(mp, type) \
> +	((type) == XG_TYPE_AG || !xfs_has_zoned((mp)))
> +
>  #endif /* __XFS_EXTENT_BUSY_H__ */
> -- 
> 2.47.2
> 
> 

Re: [PATCH 2/7] xfs: use a uint32_t to cache i_used_blocks in xfs_init_zone

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:02PM +0200, Christoph Hellwig wrote:
> i_used_blocks is a uint32_t, so use the same value for the local variable
> caching it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_zone_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 01315ed75502..867465b5b5fe 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -1017,7 +1017,7 @@ xfs_init_zone(
>  {
>  	struct xfs_mount	*mp = rtg_mount(rtg);
>  	struct xfs_zone_info	*zi = mp->m_zone_info;
> -	uint64_t		used = rtg_rmap(rtg)->i_used_blocks;
> +	uint32_t		used = rtg_rmap(rtg)->i_used_blocks;
>  	xfs_rgblock_t		write_pointer, highest_rgbno;
>  	int			error;
>  
> -- 
> 2.47.2
> 
> 

Re: [PATCH 3/7] xfs: rename oz_write_pointer to oz_allocated

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:03PM +0200, Christoph Hellwig wrote:
> This member just tracks how much space we handed out for sequential
> write required zones.  Only for conventional space it actually is the
> pointer where thing are written at, otherwise zone append manages
> that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_trace.h      |  8 ++++----
>  fs/xfs/xfs_zone_alloc.c | 18 +++++++++---------
>  fs/xfs/xfs_zone_gc.c    | 13 ++++++-------
>  fs/xfs/xfs_zone_info.c  |  2 +-
>  fs/xfs/xfs_zone_priv.h  | 16 ++++++++--------
>  5 files changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 6addebd764b0..10d4fd671dcf 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -425,8 +425,8 @@ DECLARE_EVENT_CLASS(xfs_zone_alloc_class,
>  		__field(dev_t, dev)
>  		__field(xfs_rgnumber_t, rgno)
>  		__field(xfs_rgblock_t, used)
> +		__field(xfs_rgblock_t, allocated)
>  		__field(xfs_rgblock_t, written)
> -		__field(xfs_rgblock_t, write_pointer)
>  		__field(xfs_rgblock_t, rgbno)
>  		__field(xfs_extlen_t, len)
>  	),
> @@ -434,17 +434,17 @@ DECLARE_EVENT_CLASS(xfs_zone_alloc_class,
>  		__entry->dev = rtg_mount(oz->oz_rtg)->m_super->s_dev;
>  		__entry->rgno = rtg_rgno(oz->oz_rtg);
>  		__entry->used = rtg_rmap(oz->oz_rtg)->i_used_blocks;
> +		__entry->allocated = oz->oz_allocated;
>  		__entry->written = oz->oz_written;
> -		__entry->write_pointer = oz->oz_write_pointer;
>  		__entry->rgbno = rgbno;
>  		__entry->len = len;
>  	),
> -	TP_printk("dev %d:%d rgno 0x%x used 0x%x written 0x%x wp 0x%x rgbno 0x%x len 0x%x",
> +	TP_printk("dev %d:%d rgno 0x%x used 0x%x alloced 0x%x written 0x%x rgbno 0x%x len 0x%x",

Looks fine though I'd prefer this be the full word ^^^ "allocated"

With that changed,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->rgno,
>  		  __entry->used,
> +		  __entry->allocated,
>  		  __entry->written,
> -		  __entry->write_pointer,
>  		  __entry->rgbno,
>  		  __entry->len)
>  );
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 867465b5b5fe..729d80ff52c1 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -434,7 +434,7 @@ xfs_init_open_zone(
>  	spin_lock_init(&oz->oz_alloc_lock);
>  	atomic_set(&oz->oz_ref, 1);
>  	oz->oz_rtg = rtg;
> -	oz->oz_write_pointer = write_pointer;
> +	oz->oz_allocated = write_pointer;
>  	oz->oz_written = write_pointer;
>  	oz->oz_write_hint = write_hint;
>  	oz->oz_is_gc = is_gc;
> @@ -569,7 +569,7 @@ xfs_try_use_zone(
>  	struct xfs_open_zone	*oz,
>  	bool			lowspace)
>  {
> -	if (oz->oz_write_pointer == rtg_blocks(oz->oz_rtg))
> +	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg))
>  		return false;
>  	if (!lowspace && !xfs_good_hint_match(oz, file_hint))
>  		return false;
> @@ -744,25 +744,25 @@ xfs_zone_alloc_blocks(
>  {
>  	struct xfs_rtgroup	*rtg = oz->oz_rtg;
>  	struct xfs_mount	*mp = rtg_mount(rtg);
> -	xfs_rgblock_t		rgbno;
> +	xfs_rgblock_t		allocated;
>  
>  	spin_lock(&oz->oz_alloc_lock);
>  	count_fsb = min3(count_fsb, XFS_MAX_BMBT_EXTLEN,
> -		(xfs_filblks_t)rtg_blocks(rtg) - oz->oz_write_pointer);
> +		(xfs_filblks_t)rtg_blocks(rtg) - oz->oz_allocated);
>  	if (!count_fsb) {
>  		spin_unlock(&oz->oz_alloc_lock);
>  		return 0;
>  	}
> -	rgbno = oz->oz_write_pointer;
> -	oz->oz_write_pointer += count_fsb;
> +	allocated = oz->oz_allocated;
> +	oz->oz_allocated += count_fsb;
>  	spin_unlock(&oz->oz_alloc_lock);
>  
> -	trace_xfs_zone_alloc_blocks(oz, rgbno, count_fsb);
> +	trace_xfs_zone_alloc_blocks(oz, allocated, count_fsb);
>  
>  	*sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	*is_seq = bdev_zone_is_seq(mp->m_rtdev_targp->bt_bdev, *sector);
>  	if (!*is_seq)
> -		*sector += XFS_FSB_TO_BB(mp, rgbno);
> +		*sector += XFS_FSB_TO_BB(mp, allocated);
>  	return XFS_FSB_TO_B(mp, count_fsb);
>  }
>  
> @@ -983,7 +983,7 @@ xfs_zone_rgbno_is_valid(
>  	lockdep_assert_held(&rtg_rmap(rtg)->i_lock);
>  
>  	if (rtg->rtg_open_zone)
> -		return rgbno < rtg->rtg_open_zone->oz_write_pointer;
> +		return rgbno < rtg->rtg_open_zone->oz_allocated;
>  	return !xa_get_mark(&rtg_mount(rtg)->m_groups[XG_TYPE_RTG].xa,
>  			rtg_rgno(rtg), XFS_RTG_FREE);
>  }
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 9c00fc5baa30..7ea9fa77b061 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -533,8 +533,7 @@ xfs_zone_gc_steal_open(
>  
>  	spin_lock(&zi->zi_open_zones_lock);
>  	list_for_each_entry(oz, &zi->zi_open_zones, oz_entry) {
> -		if (!found ||
> -		    oz->oz_write_pointer < found->oz_write_pointer)
> +		if (!found || oz->oz_allocated < found->oz_allocated)
>  			found = oz;
>  	}
>  
> @@ -584,7 +583,7 @@ xfs_zone_gc_ensure_target(
>  {
>  	struct xfs_open_zone	*oz = mp->m_zone_info->zi_open_gc_zone;
>  
> -	if (!oz || oz->oz_write_pointer == rtg_blocks(oz->oz_rtg))
> +	if (!oz || oz->oz_allocated == rtg_blocks(oz->oz_rtg))
>  		return xfs_zone_gc_select_target(mp);
>  	return oz;
>  }
> @@ -605,7 +604,7 @@ xfs_zone_gc_space_available(
>  	oz = xfs_zone_gc_ensure_target(data->mp);
>  	if (!oz)
>  		return false;
> -	return oz->oz_write_pointer < rtg_blocks(oz->oz_rtg) &&
> +	return oz->oz_allocated < rtg_blocks(oz->oz_rtg) &&
>  		xfs_zone_gc_scratch_available(data);
>  }
>  
> @@ -647,7 +646,7 @@ xfs_zone_gc_alloc_blocks(
>  	 */
>  	spin_lock(&mp->m_sb_lock);
>  	*count_fsb = min(*count_fsb,
> -			rtg_blocks(oz->oz_rtg) - oz->oz_write_pointer);
> +			rtg_blocks(oz->oz_rtg) - oz->oz_allocated);
>  	*count_fsb = min3(*count_fsb,
>  			mp->m_free[XC_FREE_RTEXTENTS].res_avail,
>  			mp->m_free[XC_FREE_RTAVAILABLE].res_avail);
> @@ -661,8 +660,8 @@ xfs_zone_gc_alloc_blocks(
>  	*daddr = xfs_gbno_to_daddr(&oz->oz_rtg->rtg_group, 0);
>  	*is_seq = bdev_zone_is_seq(mp->m_rtdev_targp->bt_bdev, *daddr);
>  	if (!*is_seq)
> -		*daddr += XFS_FSB_TO_BB(mp, oz->oz_write_pointer);
> -	oz->oz_write_pointer += *count_fsb;
> +		*daddr += XFS_FSB_TO_BB(mp, oz->oz_allocated);
> +	oz->oz_allocated += *count_fsb;
>  	atomic_inc(&oz->oz_ref);
>  	return oz;
>  }
> diff --git a/fs/xfs/xfs_zone_info.c b/fs/xfs/xfs_zone_info.c
> index 733bcc2f8645..07e30c596975 100644
> --- a/fs/xfs/xfs_zone_info.c
> +++ b/fs/xfs/xfs_zone_info.c
> @@ -32,7 +32,7 @@ xfs_show_open_zone(
>  {
>  	seq_printf(m, "\t  zone %d, wp %u, written %u, used %u, hint %s\n",
>  		rtg_rgno(oz->oz_rtg),
> -		oz->oz_write_pointer, oz->oz_written,
> +		oz->oz_allocated, oz->oz_written,
>  		rtg_rmap(oz->oz_rtg)->i_used_blocks,
>  		xfs_write_hint_to_str(oz->oz_write_hint));
>  }
> diff --git a/fs/xfs/xfs_zone_priv.h b/fs/xfs/xfs_zone_priv.h
> index ab696975a993..35e6de3d25ed 100644
> --- a/fs/xfs/xfs_zone_priv.h
> +++ b/fs/xfs/xfs_zone_priv.h
> @@ -11,18 +11,18 @@ struct xfs_open_zone {
>  	atomic_t		oz_ref;
>  
>  	/*
> -	 * oz_write_pointer is the write pointer at which space is handed out
> -	 * for conventional zones, or simple the count of blocks handed out
> -	 * so far for sequential write required zones and is protected by
> -	 * oz_alloc_lock/
> +	 * oz_allocated is the amount of space already allocated out of the zone
> +	 * and is protected by oz_alloc_lock.
> +	 *
> +	 * For conventional zones it also is the offset of the next write.
>  	 */
>  	spinlock_t		oz_alloc_lock;
> -	xfs_rgblock_t		oz_write_pointer;
> +	xfs_rgblock_t		oz_allocated;
>  
>  	/*
> -	 * oz_written is the number of blocks for which we've received a
> -	 * write completion.  oz_written must always be <= oz_write_pointer
> -	 * and is protected by the ILOCK of the rmap inode.
> +	 * oz_written is the number of blocks for which we've received a write
> +	 * completion.  oz_written must always be <= oz_allocated and is
> +	 * protected by the ILOCK of the rmap inode.
>  	 */
>  	xfs_rgblock_t		oz_written;
>  
> -- 
> 2.47.2
> 
> 

Re: [PATCH 4/7] xfs: stop passing an inode to the zone space reservation helpers

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:04PM +0200, Christoph Hellwig wrote:
> None of them actually needs the inode, the mount is enough.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_file.c            | 24 ++++++++++++------------
>  fs/xfs/xfs_iops.c            |  4 ++--
>  fs/xfs/xfs_zone_alloc.h      |  4 ++--
>  fs/xfs/xfs_zone_space_resv.c | 17 ++++++-----------
>  4 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 38e365b16348..ed69a65f56d7 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -497,7 +497,7 @@ xfs_file_write_checks(
>  
>  static ssize_t
>  xfs_zoned_write_space_reserve(
> -	struct xfs_inode		*ip,
> +	struct xfs_mount		*mp,
>  	struct kiocb			*iocb,
>  	struct iov_iter			*from,
>  	unsigned int			flags,
> @@ -533,8 +533,8 @@ xfs_zoned_write_space_reserve(
>  	 *
>  	 * Any remaining block will be returned after the write.
>  	 */
> -	return xfs_zoned_space_reserve(ip,
> -			XFS_B_TO_FSB(ip->i_mount, count) + 1 + 2, flags, ac);
> +	return xfs_zoned_space_reserve(mp, XFS_B_TO_FSB(mp, count) + 1 + 2,
> +			flags, ac);
>  }
>  
>  static int
> @@ -718,13 +718,13 @@ xfs_file_dio_write_zoned(
>  	struct xfs_zone_alloc_ctx ac = { };
>  	ssize_t			ret;
>  
> -	ret = xfs_zoned_write_space_reserve(ip, iocb, from, 0, &ac);
> +	ret = xfs_zoned_write_space_reserve(ip->i_mount, iocb, from, 0, &ac);
>  	if (ret < 0)
>  		return ret;
>  	ret = xfs_file_dio_write_aligned(ip, iocb, from,
>  			&xfs_zoned_direct_write_iomap_ops,
>  			&xfs_dio_zoned_write_ops, &ac);
> -	xfs_zoned_space_unreserve(ip, &ac);
> +	xfs_zoned_space_unreserve(ip->i_mount, &ac);
>  	return ret;
>  }
>  
> @@ -1032,7 +1032,7 @@ xfs_file_buffered_write_zoned(
>  	struct xfs_zone_alloc_ctx ac = { };
>  	ssize_t			ret;
>  
> -	ret = xfs_zoned_write_space_reserve(ip, iocb, from, XFS_ZR_GREEDY, &ac);
> +	ret = xfs_zoned_write_space_reserve(mp, iocb, from, XFS_ZR_GREEDY, &ac);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -1073,7 +1073,7 @@ xfs_file_buffered_write_zoned(
>  out_unlock:
>  	xfs_iunlock(ip, iolock);
>  out_unreserve:
> -	xfs_zoned_space_unreserve(ip, &ac);
> +	xfs_zoned_space_unreserve(ip->i_mount, &ac);
>  	if (ret > 0) {
>  		XFS_STATS_ADD(mp, xs_write_bytes, ret);
>  		ret = generic_write_sync(iocb, ret);
> @@ -1414,11 +1414,11 @@ xfs_file_zoned_fallocate(
>  	struct xfs_inode	*ip = XFS_I(file_inode(file));
>  	int			error;
>  
> -	error = xfs_zoned_space_reserve(ip, 2, XFS_ZR_RESERVED, &ac);
> +	error = xfs_zoned_space_reserve(ip->i_mount, 2, XFS_ZR_RESERVED, &ac);
>  	if (error)
>  		return error;
>  	error = __xfs_file_fallocate(file, mode, offset, len, &ac);
> -	xfs_zoned_space_unreserve(ip, &ac);
> +	xfs_zoned_space_unreserve(ip->i_mount, &ac);
>  	return error;
>  }
>  
> @@ -1828,12 +1828,12 @@ xfs_write_fault_zoned(
>  	 * But as the overallocation is limited to less than a folio and will be
>  	 * release instantly that's just fine.
>  	 */
> -	error = xfs_zoned_space_reserve(ip, XFS_B_TO_FSB(ip->i_mount, len), 0,
> -			&ac);
> +	error = xfs_zoned_space_reserve(ip->i_mount,
> +			XFS_B_TO_FSB(ip->i_mount, len), 0, &ac);
>  	if (error < 0)
>  		return vmf_fs_error(error);
>  	ret = __xfs_write_fault(vmf, order, &ac);
> -	xfs_zoned_space_unreserve(ip, &ac);
> +	xfs_zoned_space_unreserve(ip->i_mount, &ac);
>  	return ret;
>  }
>  
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 01e597290eb5..149b5460fbfd 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -970,7 +970,7 @@ xfs_setattr_size(
>  	 * change.
>  	 */
>  	if (xfs_is_zoned_inode(ip)) {
> -		error = xfs_zoned_space_reserve(ip, 1,
> +		error = xfs_zoned_space_reserve(mp, 1,
>  				XFS_ZR_NOWAIT | XFS_ZR_RESERVED, &ac);
>  		if (error) {
>  			if (error == -EAGAIN)
> @@ -998,7 +998,7 @@ xfs_setattr_size(
>  	}
>  
>  	if (xfs_is_zoned_inode(ip))
> -		xfs_zoned_space_unreserve(ip, &ac);
> +		xfs_zoned_space_unreserve(mp, &ac);
>  
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/xfs_zone_alloc.h b/fs/xfs/xfs_zone_alloc.h
> index ecf39106704c..4db02816d0fd 100644
> --- a/fs/xfs/xfs_zone_alloc.h
> +++ b/fs/xfs/xfs_zone_alloc.h
> @@ -23,9 +23,9 @@ struct xfs_zone_alloc_ctx {
>   */
>  #define XFS_ZR_RESERVED		(1U << 2)
>  
> -int xfs_zoned_space_reserve(struct xfs_inode *ip, xfs_filblks_t count_fsb,
> +int xfs_zoned_space_reserve(struct xfs_mount *mp, xfs_filblks_t count_fsb,
>  		unsigned int flags, struct xfs_zone_alloc_ctx *ac);
> -void xfs_zoned_space_unreserve(struct xfs_inode *ip,
> +void xfs_zoned_space_unreserve(struct xfs_mount *mp,
>  		struct xfs_zone_alloc_ctx *ac);
>  void xfs_zoned_add_available(struct xfs_mount *mp, xfs_filblks_t count_fsb);
>  
> diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
> index 93c9a7721139..1313c55b8cbe 100644
> --- a/fs/xfs/xfs_zone_space_resv.c
> +++ b/fs/xfs/xfs_zone_space_resv.c
> @@ -117,11 +117,10 @@ xfs_zoned_space_wait_error(
>  
>  static int
>  xfs_zoned_reserve_available(
> -	struct xfs_inode		*ip,
> +	struct xfs_mount		*mp,
>  	xfs_filblks_t			count_fsb,
>  	unsigned int			flags)
>  {
> -	struct xfs_mount		*mp = ip->i_mount;
>  	struct xfs_zone_info		*zi = mp->m_zone_info;
>  	struct xfs_zone_reservation	reservation = {
>  		.task		= current,
> @@ -198,11 +197,10 @@ xfs_zoned_reserve_available(
>   */
>  static int
>  xfs_zoned_reserve_extents_greedy(
> -	struct xfs_inode		*ip,
> +	struct xfs_mount		*mp,
>  	xfs_filblks_t			*count_fsb,
>  	unsigned int			flags)
>  {
> -	struct xfs_mount		*mp = ip->i_mount;
>  	struct xfs_zone_info		*zi = mp->m_zone_info;
>  	s64				len = *count_fsb;
>  	int				error = -ENOSPC;
> @@ -220,12 +218,11 @@ xfs_zoned_reserve_extents_greedy(
>  
>  int
>  xfs_zoned_space_reserve(
> -	struct xfs_inode		*ip,
> +	struct xfs_mount		*mp,
>  	xfs_filblks_t			count_fsb,
>  	unsigned int			flags,
>  	struct xfs_zone_alloc_ctx	*ac)
>  {
> -	struct xfs_mount		*mp = ip->i_mount;
>  	int				error;
>  
>  	ASSERT(ac->reserved_blocks == 0);
> @@ -234,11 +231,11 @@ xfs_zoned_space_reserve(
>  	error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
>  			flags & XFS_ZR_RESERVED);
>  	if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
> -		error = xfs_zoned_reserve_extents_greedy(ip, &count_fsb, flags);
> +		error = xfs_zoned_reserve_extents_greedy(mp, &count_fsb, flags);
>  	if (error)
>  		return error;
>  
> -	error = xfs_zoned_reserve_available(ip, count_fsb, flags);
> +	error = xfs_zoned_reserve_available(mp, count_fsb, flags);
>  	if (error) {
>  		xfs_add_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb);
>  		return error;
> @@ -249,12 +246,10 @@ xfs_zoned_space_reserve(
>  
>  void
>  xfs_zoned_space_unreserve(
> -	struct xfs_inode		*ip,
> +	struct xfs_mount		*mp,
>  	struct xfs_zone_alloc_ctx	*ac)
>  {
>  	if (ac->reserved_blocks > 0) {
> -		struct xfs_mount	*mp = ip->i_mount;
> -
>  		xfs_zoned_add_available(mp, ac->reserved_blocks);
>  		xfs_add_freecounter(mp, XC_FREE_RTEXTENTS, ac->reserved_blocks);
>  	}
> -- 
> 2.47.2
> 
> 

Re: [PATCH 5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:05PM +0200, Christoph Hellwig wrote:
> From: George Hu <integral@archlinux.org>
> 
> Refactor the xfs_max_open_zones() function by replacing the usage of
> min() and max() macro with clamp() to simplify the code and improve
> readability.
> 
> Signed-off-by: George Hu <integral@archlinux.org>
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> ---
>  fs/xfs/xfs_zone_alloc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 729d80ff52c1..d9e2b1411434 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -1133,9 +1133,7 @@ xfs_max_open_zones(
>  	/*
>  	 * Cap the max open limit to 1/4 of available space
>  	 */
> -	max_open = min(max_open, mp->m_sb.sb_rgcount / 4);
> -
> -	return max(XFS_MIN_OPEN_ZONES, max_open);
> +	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);

Does clamp() handle the case where @max < @min properly?
I'm worried about shenanigans on a runt 7-zone drive, though I can't
remember off the top of my head if we actually prohibit that...

--D

>  }
>  
>  /*
> -- 
> 2.47.2
> 
> 

Re: [PATCH 6/7] xfs: improve the comments in xfs_max_open_zones

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:06PM +0200, Christoph Hellwig wrote:
> Describe the rationale for the decisions a bit better.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_zone_alloc.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index d9e2b1411434..c1f053f4a82a 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -1114,24 +1114,27 @@ xfs_get_zone_info_cb(
>  }
>  
>  /*
> - * Calculate the max open zone limit based on the of number of
> - * backing zones available
> + * Calculate the max open zone limit based on the of number of backing zones
> + * available.
>   */
>  static inline uint32_t
>  xfs_max_open_zones(
>  	struct xfs_mount	*mp)
>  {
>  	unsigned int		max_open, max_open_data_zones;
> +
>  	/*
> -	 * We need two zones for every open data zone,
> -	 * one in reserve as we don't reclaim open zones. One data zone
> -	 * and its spare is included in XFS_MIN_ZONES.
> +	 * We need two zones for every open data zone, one in reserve as we
> +	 * don't reclaim open zones.  One data zone and its spare is included
> +	 * in XFS_MIN_ZONES to support at least one user data writer.
>  	 */
>  	max_open_data_zones = (mp->m_sb.sb_rgcount - XFS_MIN_ZONES) / 2 + 1;
>  	max_open = max_open_data_zones + XFS_OPEN_GC_ZONES;
>  
>  	/*
> -	 * Cap the max open limit to 1/4 of available space
> +	 * Cap the max open limit to 1/4 of available space.  Without this we'd
> +	 * run out of easy reclaim targets too quickly and storage devices don't
> +	 * handle huge numbers of concurrent write streams overly well.
>  	 */
>  	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
>  }
> -- 
> 2.47.2
> 
> 

Re: [PATCH 7/7] xfs: improve the comments in xfs_select_zone_nowait

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:54:07PM +0200, Christoph Hellwig wrote:
> The top of the function comment is outdated, and the parts still correct
> duplicate information in comment inside the function.  Remove the top of
> the function comment and instead improve a comment inside the function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Makes sense,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_zone_alloc.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index c1f053f4a82a..4e4ca5bbfc47 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -654,13 +654,6 @@ static inline bool xfs_zoned_pack_tight(struct xfs_inode *ip)
>  		!(ip->i_diflags & XFS_DIFLAG_APPEND);
>  }
>  
> -/*
> - * Pick a new zone for writes.
> - *
> - * If we aren't using up our budget of open zones just open a new one from the
> - * freelist.  Else try to find one that matches the expected data lifetime.  If
> - * we don't find one that is good pick any zone that is available.
> - */
>  static struct xfs_open_zone *
>  xfs_select_zone_nowait(
>  	struct xfs_mount	*mp,
> @@ -688,7 +681,8 @@ xfs_select_zone_nowait(
>  		goto out_unlock;
>  
>  	/*
> -	 * See if we can open a new zone and use that.
> +	 * See if we can open a new zone and use that so that data for different
> +	 * files is mixed as little as possible.
>  	 */
>  	oz = xfs_try_open_zone(mp, write_hint);
>  	if (oz)
> -- 
> 2.47.2
> 
> 

Re: [PATCH 5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()

[Alan Huang]

On Jul 17, 2025, at 00:02, Darrick J. Wong <djwong@kernel.org> wrote:
> 
> On Wed, Jul 16, 2025 at 02:54:05PM +0200, Christoph Hellwig wrote:
>> From: George Hu <integral@archlinux.org>
>> 
>> Refactor the xfs_max_open_zones() function by replacing the usage of
>> min() and max() macro with clamp() to simplify the code and improve
>> readability.
>> 
>> Signed-off-by: George Hu <integral@archlinux.org>
>> Reviewed-by: John Garry <john.g.garry@oracle.com>
>> ---
>> fs/xfs/xfs_zone_alloc.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
>> index 729d80ff52c1..d9e2b1411434 100644
>> --- a/fs/xfs/xfs_zone_alloc.c
>> +++ b/fs/xfs/xfs_zone_alloc.c
>> @@ -1133,9 +1133,7 @@ xfs_max_open_zones(
>> /*
>>  * Cap the max open limit to 1/4 of available space
>>  */
>> - max_open = min(max_open, mp->m_sb.sb_rgcount / 4);
>> -
>> - return max(XFS_MIN_OPEN_ZONES, max_open);
>> + return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
> 
> Does clamp() handle the case where @max < @min properly?

No, it only has BUILD_BUG_ON_MSG(statically_true(ulo > uhi), “xxx")


> I'm worried about shenanigans on a runt 7-zone drive, though I can't
> remember off the top of my head if we actually prohibit that...
> 
> --D
> 
>> }
>> 
>> /*
>> -- 
>> 2.47.2

Re: [PATCH 5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()

[Christoph Hellwig]

On Thu, Jul 17, 2025 at 12:52:14AM +0800, Alan Huang wrote:
> > Does clamp() handle the case where @max < @min properly?
> 
> No, it only has BUILD_BUG_ON_MSG(statically_true(ulo > uhi), “xxx")

Well, I guess we'll just need to drop this patch then, right?

Re: [PATCH 5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()

[Alan Huang]

On Jul 17, 2025, at 13:16, Christoph Hellwig <hch@lst.de> wrote:
> 
> On Thu, Jul 17, 2025 at 12:52:14AM +0800, Alan Huang wrote:
>>> Does clamp() handle the case where @max < @min properly?
>> 
>> No, it only has BUILD_BUG_ON_MSG(statically_true(ulo > uhi), “xxx")
> 
> Well, I guess we'll just need to drop this patch then, right?
> 

Yeah, I think so.

Re: [PATCH 1/7] xfs: don't allocate the xfs_extent_busy structure for zoned RTGs

[Carlos Maiolino]

On Wed, 16 Jul 2025 14:54:01 +0200, Christoph Hellwig wrote:
> Busy extent tracking is primarily used to ensure that freed blocks are
> not reused for data allocations before the transaction that deleted them
> has been committed to stable storage, and secondarily to drive online
> discard.  None of the use cases applies to zoned RTGs, as the zoned
> allocator can't overwrite blocks before resetting the zone, which already
> flushes out all transactions touching the RTGs.
> 
> [...]

Applied to for-next, thanks!

[1/7] xfs: don't allocate the xfs_extent_busy structure for zoned RTGs
      commit: 48510b0d85220f61337ad497dfc26ac2beb50413
[2/7] xfs: use a uint32_t to cache i_used_blocks in xfs_init_zone
      commit: c11330b4ac1141ac6c3bfce93333405dc3bb5628
[3/7] xfs: rename oz_write_pointer to oz_allocated
      commit: 8a034f984dfe32b4bf23d6729b04d37b14a76cbd
[4/7] xfs: stop passing an inode to the zone space reservation helpers
      commit: 7a779c6867faf3e2e06e60d2774af648da468c7d
[5/7] xfs: replace min & max with clamp() in xfs_max_open_zones()
      (no commit info)
[6/7] xfs: improve the comments in xfs_max_open_zones
      commit: 8196283484ec758b067adb9405299da675e2411d
[7/7] xfs: improve the comments in xfs_select_zone_nowait
      commit: e381d3e2c80d4b163302e53d0a0879c59ccff148

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