zoned xfs updates v2

zoned xfs updates v2

[Christoph Hellwig]

Hi Carlos,

this series as a bunch of zones fixes, and a trivial code movement to
move the ioend code into it's own file.

Andrey just sent a basically identical version of the third patch, but
I'm including here as this is the series I tested.  Feel free to take his
version and take my indendent reimplementation as a reviewed-by.

Changes since v1:
 - fix a commit message typo
 - clarify the reuse of an old zone in set_zoned_cached by better code
   structure and comments

Diffstat:
 Makefile         |    1 
 xfs_aops.c       |  188 -------------------------------------------------------
 xfs_aops.h       |    1 
 xfs_file.c       |    2 
 xfs_ioend.c      |  184 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 xfs_ioend.h      |   16 ++++
 xfs_iomap.c      |    7 --
 xfs_iomap.h      |   14 ++++
 xfs_zone_alloc.c |   66 +++++++++++++++----
 9 files changed, 273 insertions(+), 206 deletions(-)

[PATCH 1/5] xfs: handle NULL open_zone for merged ioends in xfs_ioend_put_open_zones

[Christoph Hellwig]

In theory we could fail multiple ioends before an open zone was assigned
to them, and the iomap code could merge them.  Check for NULL not only
for the main ioend but also all merged ones on ->io_list to handle this
case.

Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_aops.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 2a0c54256e93..c80f05507373 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -89,8 +89,10 @@ xfs_ioend_put_open_zones(
 	/*
 	 * Put the open zone for all ioends merged into this one (if any).
 	 */
-	list_for_each_entry(tmp, &ioend->io_list, io_list)
-		xfs_open_zone_put(tmp->io_private);
+	list_for_each_entry(tmp, &ioend->io_list, io_list) {
+		if (tmp->io_private)
+			xfs_open_zone_put(tmp->io_private);
+	}
 
 	/*
 	 * The main ioend might not have an open zone if the submission failed
-- 
2.53.0

Re: [PATCH 1/5] xfs: handle NULL open_zone for merged ioends in xfs_ioend_put_open_zones

[Damien Le Moal]

On 2026/08/11 9:48, Christoph Hellwig wrote:
> In theory we could fail multiple ioends before an open zone was assigned
> to them, and the iomap code could merge them.  Check for NULL not only
> for the main ioend but also all merged ones on ->io_list to handle this
> case.
> 
> Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH 2/5] xfs: fix racy open zone caching

[Christoph Hellwig]

When testing on very fast storage devices, I've observed writers using
io_uring creating many open zones with just a few kiB written to it,
which then don't get used.  I tracked this down to multiple io_uring
helper threads finding a full zone in i_private, and then going on to
select a one, with the final one winning the race and leaving it in
i_private.

Fix this by dropping full zones from i_private as soon we find them,
checking cached for a cached zoned when a single writes needs a new zone,
and by keeping an existing cached zone in xfs_set_cached_zone when it
still has space available, dropping the newly found/allocated one
instead.  This uses i_flags_lock as a low-level spinlock for short
hold times to avoid interactions with the ilock, which is used for
completions.

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

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 7d13fa7ab30a..bdbb60cc5d5b 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -793,17 +793,35 @@ xfs_get_cached_zone(
 
 	rcu_read_lock();
 	oz = VFS_I(ip)->i_private;
-	if (oz) {
-		/*
-		 * GC only steals open zones at mount time, so no GC zones
-		 * should end up in the cache.
-		 */
-		ASSERT(!oz->oz_is_gc);
-		if (!atomic_inc_not_zero(&oz->oz_ref))
+	if (!oz)
+		goto out_unlock;
+
+	/*
+	 * GC only steals open zones at mount time, so no GC zones should end up
+	 * in the cache.
+	 */
+	ASSERT(!oz->oz_is_gc);
+
+	/*
+	 * Drop the old cached open zone if it is full.
+	 */
+	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
+		spin_lock(&ip->i_flags_lock);
+		oz = VFS_I(ip)->i_private;
+		if (oz && oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
+			VFS_I(ip)->i_private = NULL;
+			spin_unlock(&ip->i_flags_lock);
+			xfs_open_zone_put(oz);
 			oz = NULL;
+			goto out_unlock;
+		}
+		spin_unlock(&ip->i_flags_lock);
 	}
-	rcu_read_unlock();
 
+	if (!atomic_inc_not_zero(&oz->oz_ref))
+		oz = NULL;
+out_unlock:
+	rcu_read_unlock();
 	return oz;
 }
 
@@ -818,18 +836,41 @@ xfs_get_cached_zone(
  * that were every written to, but significantly simplifies the cached zone
  * lookup.  Because the open_zone is clearly marked as full when all data
  * in the underlying RTG was written, the caching is always safe.
+ *
+ * Called with a reference on @oz held.  And returns two references on the
+ * returned zone: one for the caller and one for pinning the zone in
+ * inode->i_private.
  */
-static void
+static struct xfs_open_zone *
 xfs_set_cached_zone(
 	struct xfs_inode	*ip,
 	struct xfs_open_zone	*oz)
 {
 	struct xfs_open_zone	*old_oz;
 
+	/*
+	 * If the open zone cached in the inode still has free space, use that
+	 * instead of the new open zone just selected.  This can happen when
+	 * multiple threads race to perform zone selection for an inode.
+	 * io_uring worker threads seem to be good way to trigger this.
+	 *
+	 * We need to grab an extra reference to this open zone as the caller
+	 * owns a reference in addition to the i_private pointer.
+	 */
+	spin_lock(&ip->i_flags_lock);
+	old_oz = VFS_I(ip)->i_private;
+	if (old_oz && old_oz->oz_allocated < rtg_blocks(old_oz->oz_rtg) &&
+	    atomic_inc_not_zero(&old_oz->oz_ref)) {
+		spin_unlock(&ip->i_flags_lock);
+		xfs_open_zone_put(oz);
+		return old_oz;
+	}
+	VFS_I(ip)->i_private = oz;
 	atomic_inc(&oz->oz_ref);
-	old_oz = xchg(&VFS_I(ip)->i_private, oz);
+	spin_unlock(&ip->i_flags_lock);
 	if (old_oz)
 		xfs_open_zone_put(old_oz);
+	return oz;
 }
 
 static void
@@ -873,14 +914,13 @@ xfs_zone_alloc_and_submit(
 	 * the inode is still associated with a zone and use that if so.
 	 */
 	if (!*oz)
+select_zone:
 		*oz = xfs_get_cached_zone(ip);
-
 	if (!*oz) {
-select_zone:
 		*oz = xfs_select_zone(mp, write_hint, pack_tight);
 		if (!*oz)
 			goto out_error;
-		xfs_set_cached_zone(ip, *oz);
+		*oz = xfs_set_cached_zone(ip, *oz);
 	}
 
 	alloc_len = xfs_zone_alloc_blocks(*oz, XFS_B_TO_FSB(mp, ioend->io_size),
-- 
2.53.0

Re: [PATCH 2/5] xfs: fix racy open zone caching

[Damien Le Moal]

On 2026/08/11 9:48, Christoph Hellwig wrote:
> When testing on very fast storage devices, I've observed writers using
> io_uring creating many open zones with just a few kiB written to it,
> which then don't get used.  I tracked this down to multiple io_uring
> helper threads finding a full zone in i_private, and then going on to
> select a one, with the final one winning the race and leaving it in
> i_private.
> 
> Fix this by dropping full zones from i_private as soon we find them,
> checking cached for a cached zoned when a single writes needs a new zone,
> and by keeping an existing cached zone in xfs_set_cached_zone when it
> still has space available, dropping the newly found/allocated one
> instead.  This uses i_flags_lock as a low-level spinlock for short
> hold times to avoid interactions with the ilock, which is used for
> completions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/5] xfs: fix racy open zone caching

[Darrick J. Wong]

On Tue, Aug 11, 2026 at 10:48:38AM -0600, Christoph Hellwig wrote:
> When testing on very fast storage devices, I've observed writers using
> io_uring creating many open zones with just a few kiB written to it,
> which then don't get used.  I tracked this down to multiple io_uring
> helper threads finding a full zone in i_private, and then going on to
> select a one, with the final one winning the race and leaving it in
> i_private.
> 
> Fix this by dropping full zones from i_private as soon we find them,
> checking cached for a cached zoned when a single writes needs a new zone,
> and by keeping an existing cached zone in xfs_set_cached_zone when it
> still has space available, dropping the newly found/allocated one
> instead.  This uses i_flags_lock as a low-level spinlock for short
> hold times to avoid interactions with the ilock, which is used for
> completions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Much clearer now, thanks!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_zone_alloc.c | 66 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 7d13fa7ab30a..bdbb60cc5d5b 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -793,17 +793,35 @@ xfs_get_cached_zone(
>  
>  	rcu_read_lock();
>  	oz = VFS_I(ip)->i_private;
> -	if (oz) {
> -		/*
> -		 * GC only steals open zones at mount time, so no GC zones
> -		 * should end up in the cache.
> -		 */
> -		ASSERT(!oz->oz_is_gc);
> -		if (!atomic_inc_not_zero(&oz->oz_ref))
> +	if (!oz)
> +		goto out_unlock;
> +
> +	/*
> +	 * GC only steals open zones at mount time, so no GC zones should end up
> +	 * in the cache.
> +	 */
> +	ASSERT(!oz->oz_is_gc);
> +
> +	/*
> +	 * Drop the old cached open zone if it is full.
> +	 */
> +	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
> +		spin_lock(&ip->i_flags_lock);
> +		oz = VFS_I(ip)->i_private;
> +		if (oz && oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
> +			VFS_I(ip)->i_private = NULL;
> +			spin_unlock(&ip->i_flags_lock);
> +			xfs_open_zone_put(oz);
>  			oz = NULL;
> +			goto out_unlock;
> +		}
> +		spin_unlock(&ip->i_flags_lock);
>  	}
> -	rcu_read_unlock();
>  
> +	if (!atomic_inc_not_zero(&oz->oz_ref))
> +		oz = NULL;
> +out_unlock:
> +	rcu_read_unlock();
>  	return oz;
>  }
>  
> @@ -818,18 +836,41 @@ xfs_get_cached_zone(
>   * that were every written to, but significantly simplifies the cached zone
>   * lookup.  Because the open_zone is clearly marked as full when all data
>   * in the underlying RTG was written, the caching is always safe.
> + *
> + * Called with a reference on @oz held.  And returns two references on the
> + * returned zone: one for the caller and one for pinning the zone in
> + * inode->i_private.
>   */
> -static void
> +static struct xfs_open_zone *
>  xfs_set_cached_zone(
>  	struct xfs_inode	*ip,
>  	struct xfs_open_zone	*oz)
>  {
>  	struct xfs_open_zone	*old_oz;
>  
> +	/*
> +	 * If the open zone cached in the inode still has free space, use that
> +	 * instead of the new open zone just selected.  This can happen when
> +	 * multiple threads race to perform zone selection for an inode.
> +	 * io_uring worker threads seem to be good way to trigger this.
> +	 *
> +	 * We need to grab an extra reference to this open zone as the caller
> +	 * owns a reference in addition to the i_private pointer.
> +	 */
> +	spin_lock(&ip->i_flags_lock);
> +	old_oz = VFS_I(ip)->i_private;
> +	if (old_oz && old_oz->oz_allocated < rtg_blocks(old_oz->oz_rtg) &&
> +	    atomic_inc_not_zero(&old_oz->oz_ref)) {
> +		spin_unlock(&ip->i_flags_lock);
> +		xfs_open_zone_put(oz);
> +		return old_oz;
> +	}
> +	VFS_I(ip)->i_private = oz;
>  	atomic_inc(&oz->oz_ref);
> -	old_oz = xchg(&VFS_I(ip)->i_private, oz);
> +	spin_unlock(&ip->i_flags_lock);
>  	if (old_oz)
>  		xfs_open_zone_put(old_oz);
> +	return oz;
>  }
>  
>  static void
> @@ -873,14 +914,13 @@ xfs_zone_alloc_and_submit(
>  	 * the inode is still associated with a zone and use that if so.
>  	 */
>  	if (!*oz)
> +select_zone:
>  		*oz = xfs_get_cached_zone(ip);
> -
>  	if (!*oz) {
> -select_zone:
>  		*oz = xfs_select_zone(mp, write_hint, pack_tight);
>  		if (!*oz)
>  			goto out_error;
> -		xfs_set_cached_zone(ip, *oz);
> +		*oz = xfs_set_cached_zone(ip, *oz);
>  	}
>  
>  	alloc_len = xfs_zone_alloc_blocks(*oz, XFS_B_TO_FSB(mp, ioend->io_size),
> -- 
> 2.53.0
> 
> 

[PATCH 3/5] xfs: fix zoned write iomap flags assignments

[Christoph Hellwig]

Don't overwrite IOMAP_F_DIRTY with IOMAP_F_ANON_WRITE, but ensure both
flags are set instead.

Note that in practice this is harmless as all zoned writes force a metadata
transaction anyway, but incorrectly assigned flags are still a landmine
that will cause problems at some point.

Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
Fixes: 2e2383405824 ("xfs: implement direct writes to zoned RT devices")
Cc: <stable@vger.kernel.org> # v6.15
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_aops.c  | 3 +--
 fs/xfs/xfs_iomap.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index c80f05507373..9506cd8d15e6 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -634,11 +634,10 @@ xfs_zoned_map_blocks(
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
 	wpc->iomap.type = IOMAP_MAPPED;
-	wpc->iomap.flags = IOMAP_F_DIRTY;
 	wpc->iomap.bdev = mp->m_rtdev_targp->bt_bdev;
 	wpc->iomap.offset = offset;
 	wpc->iomap.length = XFS_FSB_TO_B(mp, count_fsb);
-	wpc->iomap.flags = IOMAP_F_ANON_WRITE;
+	wpc->iomap.flags = IOMAP_F_ANON_WRITE | IOMAP_F_DIRTY;
 
 	trace_xfs_zoned_map_blocks(ip, offset, wpc->iomap.length);
 	return 0;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 225c3de88d03..9fa9aa33745f 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1081,11 +1081,10 @@ xfs_zoned_direct_write_iomap_begin(
 	}
 
 	iomap->type = IOMAP_MAPPED;
-	iomap->flags = IOMAP_F_DIRTY;
 	iomap->bdev = ip->i_mount->m_rtdev_targp->bt_bdev;
 	iomap->offset = offset;
 	iomap->length = length;
-	iomap->flags = IOMAP_F_ANON_WRITE;
+	iomap->flags = IOMAP_F_ANON_WRITE | IOMAP_F_DIRTY;
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH 3/5] xfs: fix zoned write iomap flags assignments

[Damien Le Moal]

On 2026/08/11 9:48, Christoph Hellwig wrote:
> Don't overwrite IOMAP_F_DIRTY with IOMAP_F_ANON_WRITE, but ensure both
> flags are set instead.
> 
> Note that in practice this is harmless as all zoned writes force a metadata
> transaction anyway, but incorrectly assigned flags are still a landmine
> that will cause problems at some point.
> 
> Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
> Fixes: 2e2383405824 ("xfs: implement direct writes to zoned RT devices")
> Cc: <stable@vger.kernel.org> # v6.15
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH 4/5] xfs: factor out a xfs_iomap_set_anon_write helper

[Christoph Hellwig]

De-duplicate the iomap setup for zoned writes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_aops.c  |  8 ++------
 fs/xfs/xfs_iomap.c |  6 +-----
 fs/xfs/xfs_iomap.h | 14 ++++++++++++++
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 9506cd8d15e6..d46b089f006a 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -633,12 +633,8 @@ xfs_zoned_map_blocks(
 			XFS_BMAPI_REMAP);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
-	wpc->iomap.type = IOMAP_MAPPED;
-	wpc->iomap.bdev = mp->m_rtdev_targp->bt_bdev;
-	wpc->iomap.offset = offset;
-	wpc->iomap.length = XFS_FSB_TO_B(mp, count_fsb);
-	wpc->iomap.flags = IOMAP_F_ANON_WRITE | IOMAP_F_DIRTY;
-
+	xfs_iomap_set_anon_write(ip, &wpc->iomap, offset,
+			XFS_FSB_TO_B(mp, count_fsb));
 	trace_xfs_zoned_map_blocks(ip, offset, wpc->iomap.length);
 	return 0;
 }
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 9fa9aa33745f..e87c26c3e03b 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1080,11 +1080,7 @@ xfs_zoned_direct_write_iomap_begin(
 			return error;
 	}
 
-	iomap->type = IOMAP_MAPPED;
-	iomap->bdev = ip->i_mount->m_rtdev_targp->bt_bdev;
-	iomap->offset = offset;
-	iomap->length = length;
-	iomap->flags = IOMAP_F_ANON_WRITE | IOMAP_F_DIRTY;
+	xfs_iomap_set_anon_write(ip, iomap, offset, length);
 	return 0;
 }
 
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index ebcce7d49446..f697d7e73ab6 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -29,6 +29,20 @@ int xfs_zero_range(struct xfs_inode *ip, loff_t pos, loff_t len,
 int xfs_truncate_page(struct xfs_inode *ip, loff_t pos,
 		struct xfs_zone_alloc_ctx *ac, bool *did_zero);
 
+static inline void
+xfs_iomap_set_anon_write(
+	struct xfs_inode		*ip,
+	struct iomap			*iomap,
+	loff_t				offset,
+	loff_t				length)
+{
+	iomap->type = IOMAP_MAPPED;
+	iomap->bdev = ip->i_mount->m_rtdev_targp->bt_bdev;
+	iomap->offset = offset;
+	iomap->length = length;
+	iomap->flags = IOMAP_F_ANON_WRITE | IOMAP_F_DIRTY;
+}
+
 static inline xfs_filblks_t
 xfs_aligned_fsb_count(
 	xfs_fileoff_t		offset_fsb,
-- 
2.53.0

Re: [PATCH 4/5] xfs: factor out a xfs_iomap_set_anon_write helper

[Damien Le Moal]

On 2026/08/11 9:48, Christoph Hellwig wrote:
> De-duplicate the iomap setup for zoned writes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH 5/5] xfs: split ioend handling into a separate source file

[Christoph Hellwig]

The ioend handling used to be only for buffered writeback, but has been
extended to direct I/O and reads.  Split it into a new source file.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/Makefile    |   1 +
 fs/xfs/xfs_aops.c  | 181 +-------------------------------------------
 fs/xfs/xfs_aops.h  |   1 -
 fs/xfs/xfs_file.c  |   2 +-
 fs/xfs/xfs_ioend.c | 184 +++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_ioend.h |  16 ++++
 6 files changed, 203 insertions(+), 182 deletions(-)
 create mode 100644 fs/xfs/xfs_ioend.c
 create mode 100644 fs/xfs/xfs_ioend.h

diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index 9f7133e02576..399a207f2d0e 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -91,6 +91,7 @@ xfs-y				+= xfs_aops.o \
 				   xfs_healthmon.o \
 				   xfs_icache.o \
 				   xfs_ioctl.o \
+				   xfs_ioend.o \
 				   xfs_iomap.o \
 				   xfs_iops.o \
 				   xfs_inode.o \
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index d46b089f006a..438fff1cee9f 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -20,6 +20,7 @@
 #include "xfs_errortag.h"
 #include "xfs_error.h"
 #include "xfs_icache.h"
+#include "xfs_ioend.h"
 #include "xfs_zone_alloc.h"
 #include "xfs_rtgroup.h"
 #include <linux/bio-integrity.h>
@@ -36,15 +37,6 @@ XFS_WPC(struct iomap_writepage_ctx *ctx)
 	return container_of(ctx, struct xfs_writepage_ctx, ctx);
 }
 
-/*
- * Fast and loose check if this write could update the on-disk inode size.
- */
-static inline bool xfs_ioend_is_append(struct iomap_ioend *ioend)
-{
-	return ioend->io_offset + ioend->io_size >
-		XFS_I(ioend->io_inode)->i_disk_size;
-}
-
 /*
  * Update on-disk file size now that data has been written to disk.
  */
@@ -80,177 +72,6 @@ xfs_setfilesize(
 	return xfs_trans_commit(tp);
 }
 
-static void
-xfs_ioend_put_open_zones(
-	struct iomap_ioend	*ioend)
-{
-	struct iomap_ioend *tmp;
-
-	/*
-	 * Put the open zone for all ioends merged into this one (if any).
-	 */
-	list_for_each_entry(tmp, &ioend->io_list, io_list) {
-		if (tmp->io_private)
-			xfs_open_zone_put(tmp->io_private);
-	}
-
-	/*
-	 * The main ioend might not have an open zone if the submission failed
-	 * before xfs_zone_alloc_and_submit got called.
-	 */
-	if (ioend->io_private)
-		xfs_open_zone_put(ioend->io_private);
-}
-
-/*
- * IO write completion.
- */
-STATIC void
-xfs_end_ioend_write(
-	struct iomap_ioend	*ioend)
-{
-	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
-	struct xfs_mount	*mp = ip->i_mount;
-	bool			is_zoned = xfs_is_zoned_inode(ip);
-	xfs_off_t		offset = ioend->io_offset;
-	size_t			size = ioend->io_size;
-	unsigned int		nofs_flag;
-	int			error;
-
-	/*
-	 * We can allocate memory here while doing writeback on behalf of
-	 * memory reclaim.  To avoid memory allocation deadlocks set the
-	 * task-wide nofs context for the following operations.
-	 */
-	nofs_flag = memalloc_nofs_save();
-
-	/*
-	 * Just clean up the in-memory structures if the fs has been shut down.
-	 */
-	if (xfs_is_shutdown(mp)) {
-		error = -EIO;
-		goto done;
-	}
-
-	/*
-	 * Clean up all COW blocks and underlying data fork delalloc blocks on
-	 * I/O error. The delalloc punch is required because this ioend was
-	 * mapped to blocks in the COW fork and the associated pages are no
-	 * longer dirty. If we don't remove delalloc blocks here, they become
-	 * stale and can corrupt free space accounting on unmount.
-	 */
-	error = blk_status_to_errno(ioend->io_bio.bi_status);
-	if (unlikely(error)) {
-		/*
-		 * Zoned writes update the in-core open zone accounting before
-		 * I/O submission.  A failed write leaves that state
-		 * inconsistent, so shut down the filesystem instead of letting
-		 * later writers wait forever for open zone space to become
-		 * available.
-		 */
-		if (is_zoned) {
-			xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
-			goto done;
-		}
-		if (ioend->io_flags & IOMAP_IOEND_SHARED) {
-			ASSERT(!is_zoned);
-			xfs_reflink_cancel_cow_range(ip, offset, size, true);
-			xfs_bmap_punch_delalloc_range(ip, XFS_DATA_FORK, offset,
-					offset + size, NULL);
-		}
-		goto done;
-	}
-
-	/*
-	 * Success: commit the COW or unwritten blocks if needed.
-	 */
-	if (is_zoned)
-		error = xfs_zoned_end_io(ip, offset, size, ioend->io_sector,
-				ioend->io_private, NULLFSBLOCK);
-	else if (ioend->io_flags & IOMAP_IOEND_SHARED)
-		error = xfs_reflink_end_cow(ip, offset, size);
-	else if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
-		error = xfs_iomap_write_unwritten(ip, offset, size, false);
-
-	if (!error &&
-	    !(ioend->io_flags & IOMAP_IOEND_DIRECT) &&
-	    xfs_ioend_is_append(ioend))
-		error = xfs_setfilesize(ip, offset, size);
-done:
-	if (is_zoned)
-		xfs_ioend_put_open_zones(ioend);
-	iomap_finish_ioends(ioend, error);
-	memalloc_nofs_restore(nofs_flag);
-}
-
-/*
- * Finish all pending IO completions that require transactional modifications.
- *
- * We try to merge physical and logically contiguous ioends before completion to
- * minimise the number of transactions we need to perform during IO completion.
- * Both unwritten extent conversion and COW remapping need to iterate and modify
- * one physical extent at a time, so we gain nothing by merging physically
- * discontiguous extents here.
- *
- * The ioend chain length that we can be processing here is largely unbound in
- * length and we may have to perform significant amounts of work on each ioend
- * to complete it. Hence we have to be careful about holding the CPU for too
- * long in this loop.
- */
-void
-xfs_end_io(
-	struct work_struct	*work)
-{
-	struct xfs_inode	*ip =
-		container_of(work, struct xfs_inode, i_ioend_work);
-	struct iomap_ioend	*ioend;
-	struct list_head	tmp;
-	unsigned long		flags;
-
-	spin_lock_irqsave(&ip->i_ioend_lock, flags);
-	list_replace_init(&ip->i_ioend_list, &tmp);
-	spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
-
-	iomap_sort_ioends(&tmp);
-	while ((ioend = list_first_entry_or_null(&tmp, struct iomap_ioend,
-			io_list))) {
-		list_del_init(&ioend->io_list);
-		iomap_ioend_try_merge(ioend, &tmp);
-		if (bio_op(&ioend->io_bio) == REQ_OP_READ)
-			iomap_finish_ioends(ioend,
-				blk_status_to_errno(ioend->io_bio.bi_status));
-		else
-			xfs_end_ioend_write(ioend);
-		cond_resched();
-	}
-}
-
-void
-xfs_end_bio(
-	struct bio		*bio)
-{
-	struct iomap_ioend	*ioend = iomap_ioend_from_bio(bio);
-	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
-	struct xfs_mount	*mp = ip->i_mount;
-	unsigned long		flags;
-
-	/*
-	 * For Appends record the actually written block number and set the
-	 * boundary flag if needed.
-	 */
-	if (IS_ENABLED(CONFIG_XFS_RT) && bio_is_zone_append(bio)) {
-		ioend->io_sector = bio->bi_iter.bi_sector;
-		xfs_mark_rtg_boundary(ioend);
-	}
-
-	spin_lock_irqsave(&ip->i_ioend_lock, flags);
-	if (list_empty(&ip->i_ioend_list))
-		WARN_ON_ONCE(!queue_work(mp->m_unwritten_workqueue,
-					 &ip->i_ioend_work));
-	list_add_tail(&ioend->io_list, &ip->i_ioend_list);
-	spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
-}
-
 /*
  * We cannot cancel the ioend directly on error.  We may have already set other
  * pages under writeback and hence we have to run I/O completion to mark the
diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h
index 5a7a0f1a0b49..d5ae5c9d4c26 100644
--- a/fs/xfs/xfs_aops.h
+++ b/fs/xfs/xfs_aops.h
@@ -10,6 +10,5 @@ extern const struct address_space_operations xfs_address_space_operations;
 extern const struct address_space_operations xfs_dax_aops;
 
 int xfs_setfilesize(struct xfs_inode *ip, xfs_off_t offset, size_t size);
-void xfs_end_bio(struct bio *bio);
 
 #endif /* __XFS_AOPS_H__ */
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 4745f047193c..3f394791106b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -25,7 +25,7 @@
 #include "xfs_iomap.h"
 #include "xfs_reflink.h"
 #include "xfs_file.h"
-#include "xfs_aops.h"
+#include "xfs_ioend.h"
 #include "xfs_zone_alloc.h"
 #include "xfs_error.h"
 #include "xfs_errortag.h"
diff --git a/fs/xfs/xfs_ioend.c b/fs/xfs/xfs_ioend.c
new file mode 100644
index 000000000000..40695d18dac0
--- /dev/null
+++ b/fs/xfs/xfs_ioend.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2025 Christoph Hellwig.
+ * All Rights Reserved.
+ */
+#include "xfs_platform.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_inode.h"
+#include "xfs_iomap.h"
+#include "xfs_trace.h"
+#include "xfs_bmap_util.h"
+#include "xfs_reflink.h"
+#include "xfs_zone_alloc.h"
+#include "xfs_ioend.h"
+
+static void
+xfs_ioend_put_open_zones(
+	struct iomap_ioend	*ioend)
+{
+	struct iomap_ioend *tmp;
+
+	/*
+	 * Put the open zone for all ioends merged into this one (if any).
+	 */
+	list_for_each_entry(tmp, &ioend->io_list, io_list)
+		xfs_open_zone_put(tmp->io_private);
+
+	/*
+	 * The main ioend might not have an open zone if the submission failed
+	 * before xfs_zone_alloc_and_submit got called.
+	 */
+	if (ioend->io_private)
+		xfs_open_zone_put(ioend->io_private);
+}
+
+static void
+xfs_end_ioend_write(
+	struct iomap_ioend	*ioend)
+{
+	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	bool			is_zoned = xfs_is_zoned_inode(ip);
+	xfs_off_t		offset = ioend->io_offset;
+	size_t			size = ioend->io_size;
+	unsigned int		nofs_flag;
+	int			error;
+
+	/*
+	 * We can allocate memory here while doing writeback on behalf of
+	 * memory reclaim.  To avoid memory allocation deadlocks set the
+	 * task-wide nofs context for the following operations.
+	 */
+	nofs_flag = memalloc_nofs_save();
+
+	/*
+	 * Just clean up the in-memory structures if the fs has been shut down.
+	 */
+	if (xfs_is_shutdown(mp)) {
+		error = -EIO;
+		goto done;
+	}
+
+	/*
+	 * Clean up all COW blocks and underlying data fork delalloc blocks on
+	 * I/O error. The delalloc punch is required because this ioend was
+	 * mapped to blocks in the COW fork and the associated pages are no
+	 * longer dirty. If we don't remove delalloc blocks here, they become
+	 * stale and can corrupt free space accounting on unmount.
+	 */
+	error = blk_status_to_errno(ioend->io_bio.bi_status);
+	if (unlikely(error)) {
+		/*
+		 * Zoned writes update the in-core open zone accounting before
+		 * I/O submission.  A failed write leaves that state
+		 * inconsistent, so shut down the filesystem instead of letting
+		 * later writers wait forever for open zone space to become
+		 * available.
+		 */
+		if (is_zoned) {
+			xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
+			goto done;
+		}
+		if (ioend->io_flags & IOMAP_IOEND_SHARED) {
+			ASSERT(!is_zoned);
+			xfs_reflink_cancel_cow_range(ip, offset, size, true);
+			xfs_bmap_punch_delalloc_range(ip, XFS_DATA_FORK, offset,
+					offset + size, NULL);
+		}
+		goto done;
+	}
+
+	/*
+	 * Success: commit the COW or unwritten blocks if needed.
+	 */
+	if (is_zoned)
+		error = xfs_zoned_end_io(ip, offset, size, ioend->io_sector,
+				ioend->io_private, NULLFSBLOCK);
+	else if (ioend->io_flags & IOMAP_IOEND_SHARED)
+		error = xfs_reflink_end_cow(ip, offset, size);
+	else if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
+		error = xfs_iomap_write_unwritten(ip, offset, size, false);
+
+	if (!error &&
+	    !(ioend->io_flags & IOMAP_IOEND_DIRECT) &&
+	    xfs_ioend_is_append(ioend))
+		error = xfs_setfilesize(ip, offset, size);
+done:
+	if (is_zoned)
+		xfs_ioend_put_open_zones(ioend);
+	iomap_finish_ioends(ioend, error);
+	memalloc_nofs_restore(nofs_flag);
+}
+
+/*
+ * Finish all pending IO completions that require transactional modifications.
+ *
+ * We try to merge physical and logically contiguous ioends before completion to
+ * minimise the number of transactions we need to perform during IO completion.
+ * Both unwritten extent conversion and COW remapping need to iterate and modify
+ * one physical extent at a time, so we gain nothing by merging physically
+ * discontiguous extents here.
+ *
+ * The ioend chain length that we can be processing here is largely unbound in
+ * length and we may have to perform significant amounts of work on each ioend
+ * to complete it. Hence we have to be careful about holding the CPU for too
+ * long in this loop.
+ */
+void
+xfs_end_io(
+	struct work_struct	*work)
+{
+	struct xfs_inode	*ip =
+		container_of(work, struct xfs_inode, i_ioend_work);
+	struct iomap_ioend	*ioend;
+	struct list_head	tmp;
+	unsigned long		flags;
+
+	spin_lock_irqsave(&ip->i_ioend_lock, flags);
+	list_replace_init(&ip->i_ioend_list, &tmp);
+	spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
+
+	iomap_sort_ioends(&tmp);
+	while ((ioend = list_first_entry_or_null(&tmp, struct iomap_ioend,
+			io_list))) {
+		list_del_init(&ioend->io_list);
+		iomap_ioend_try_merge(ioend, &tmp);
+		if (bio_op(&ioend->io_bio) == REQ_OP_READ)
+			iomap_finish_ioends(ioend,
+				blk_status_to_errno(ioend->io_bio.bi_status));
+		else
+			xfs_end_ioend_write(ioend);
+		cond_resched();
+	}
+}
+
+void
+xfs_end_bio(
+	struct bio		*bio)
+{
+	struct iomap_ioend	*ioend = iomap_ioend_from_bio(bio);
+	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	unsigned long		flags;
+
+	/*
+	 * For Appends record the actually written block number and set the
+	 * boundary flag if needed.
+	 */
+	if (IS_ENABLED(CONFIG_XFS_RT) && bio_is_zone_append(bio)) {
+		ioend->io_sector = bio->bi_iter.bi_sector;
+		xfs_mark_rtg_boundary(ioend);
+	}
+
+	spin_lock_irqsave(&ip->i_ioend_lock, flags);
+	if (list_empty(&ip->i_ioend_list))
+		WARN_ON_ONCE(!queue_work(mp->m_unwritten_workqueue,
+					 &ip->i_ioend_work));
+	list_add_tail(&ioend->io_list, &ip->i_ioend_list);
+	spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
+}
diff --git a/fs/xfs/xfs_ioend.h b/fs/xfs/xfs_ioend.h
new file mode 100644
index 000000000000..525865767fca
--- /dev/null
+++ b/fs/xfs/xfs_ioend.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __XFS_IOEND_H
+#define __XFS_IOEND_H
+
+/*
+ * Fast and loose check if this write could update the on-disk inode size.
+ */
+static inline bool xfs_ioend_is_append(struct iomap_ioend *ioend)
+{
+	return ioend->io_offset + ioend->io_size >
+		XFS_I(ioend->io_inode)->i_disk_size;
+}
+
+void xfs_end_bio(struct bio *bio);
+
+#endif /* __XFS_IOEND_H */
-- 
2.53.0

Re: [PATCH 5/5] xfs: split ioend handling into a separate source file

[Damien Le Moal]

On 2026/08/11 9:48, Christoph Hellwig wrote:
> The ioend handling used to be only for buffered writeback, but has been
> extended to direct I/O and reads.  Split it into a new source file.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

zoned xfs updates

[Christoph Hellwig]

Hi Carlos,

this series as a bunch of zones fixes, and a trivial code movement to
move the ioend code into it's own file.

Andrey just sent a basically identical version of the third patch, but
I'm including here as this is the series I tested.  Feel free to take his
version and take my indendent reimplementation as a reviewed-by.

Diffstat:
 Makefile         |    1 
 xfs_aops.c       |  188 -------------------------------------------------------
 xfs_aops.h       |    1 
 xfs_file.c       |    2 
 xfs_ioend.c      |  184 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 xfs_ioend.h      |   16 ++++
 xfs_iomap.c      |    7 --
 xfs_iomap.h      |   14 ++++
 xfs_zone_alloc.c |   56 ++++++++++++----
 9 files changed, 263 insertions(+), 206 deletions(-)

[PATCH 2/5] xfs: fix racy open zone caching

[Christoph Hellwig]

When testing on very fast storage devices, I've observed writers using
io_uring creating many open zones with just a few kiB written to it,
which then don't get used.  I tracked this down to multiple io_uring
helper threads finding a full zone in i_private, and then going on to
select a one, with the final one winning the race and leaving it in
i_private.

Fix this by dropping full zones from i_private as soon we find them,
checking cached for a cached zoned when a single writes needs a new zone,
and by keeping an existing cached zone in xfs_set_cached_zone when it
still has space available, dropping the newly found/allocated one
instead.  This uses i_flags_lock as a low-level spinlock for short
hold times to avoid interactions with the ilock, which is used for
completions.

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

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 7d13fa7ab30a..dee21f65f7b7 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -793,17 +793,35 @@ xfs_get_cached_zone(
 
 	rcu_read_lock();
 	oz = VFS_I(ip)->i_private;
-	if (oz) {
-		/*
-		 * GC only steals open zones at mount time, so no GC zones
-		 * should end up in the cache.
-		 */
-		ASSERT(!oz->oz_is_gc);
-		if (!atomic_inc_not_zero(&oz->oz_ref))
+	if (!oz)
+		goto out_unlock;
+
+	/*
+	 * GC only steals open zones at mount time, so no GC zones should end up
+	 * in the cache.
+	 */
+	ASSERT(!oz->oz_is_gc);
+
+	/*
+	 * Drop the old cached open zone if it is full.
+	 */
+	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
+		spin_lock(&ip->i_flags_lock);
+		oz = VFS_I(ip)->i_private;
+		if (oz && oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
+			VFS_I(ip)->i_private = NULL;
+			spin_unlock(&ip->i_flags_lock);
+			xfs_open_zone_put(oz);
 			oz = NULL;
+			goto out_unlock;
+		}
+		spin_unlock(&ip->i_flags_lock);
 	}
-	rcu_read_unlock();
 
+	if (oz && !atomic_inc_not_zero(&oz->oz_ref))
+		oz = NULL;
+out_unlock:
+	rcu_read_unlock();
 	return oz;
 }
 
@@ -819,17 +837,30 @@ xfs_get_cached_zone(
  * lookup.  Because the open_zone is clearly marked as full when all data
  * in the underlying RTG was written, the caching is always safe.
  */
-static void
+static struct xfs_open_zone *
 xfs_set_cached_zone(
 	struct xfs_inode	*ip,
 	struct xfs_open_zone	*oz)
 {
 	struct xfs_open_zone	*old_oz;
 
+	/*
+	 * If the open zone cached in the inode still has free space, use that
+	 * instead of the inode we just selected.  This can happen when multiple
+	 * threads race to perform zone selection for an inode.  io_uring worker
+	 * threads seem to be good at triggering this.
+	 */
+	spin_lock(&ip->i_flags_lock);
+	old_oz = VFS_I(ip)->i_private;
+	if (old_oz && old_oz->oz_allocated < rtg_blocks(old_oz->oz_rtg))
+		swap(oz, old_oz);
 	atomic_inc(&oz->oz_ref);
-	old_oz = xchg(&VFS_I(ip)->i_private, oz);
+	VFS_I(ip)->i_private = oz;
+	spin_unlock(&ip->i_flags_lock);
+
 	if (old_oz)
 		xfs_open_zone_put(old_oz);
+	return oz;
 }
 
 static void
@@ -873,14 +904,13 @@ xfs_zone_alloc_and_submit(
 	 * the inode is still associated with a zone and use that if so.
 	 */
 	if (!*oz)
+select_zone:
 		*oz = xfs_get_cached_zone(ip);
-
 	if (!*oz) {
-select_zone:
 		*oz = xfs_select_zone(mp, write_hint, pack_tight);
 		if (!*oz)
 			goto out_error;
-		xfs_set_cached_zone(ip, *oz);
+		*oz = xfs_set_cached_zone(ip, *oz);
 	}
 
 	alloc_len = xfs_zone_alloc_blocks(*oz, XFS_B_TO_FSB(mp, ioend->io_size),
-- 
2.53.0

Re: [PATCH 2/5] xfs: fix racy open zone caching

[Darrick J. Wong]

On Mon, Aug 10, 2026 at 08:37:43AM -0700, Christoph Hellwig wrote:
> When testing on very fast storage devices, I've observed writers using
> io_uring creating many open zones with just a few kiB written to it,
> which then don't get used.  I tracked this down to multiple io_uring
> helper threads finding a full zone in i_private, and then going on to
> select a one, with the final one winning the race and leaving it in
> i_private.
> 
> Fix this by dropping full zones from i_private as soon we find them,
> checking cached for a cached zoned when a single writes needs a new zone,
> and by keeping an existing cached zone in xfs_set_cached_zone when it
> still has space available, dropping the newly found/allocated one
> instead.  This uses i_flags_lock as a low-level spinlock for short
> hold times to avoid interactions with the ilock, which is used for
> completions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_alloc.c | 56 +++++++++++++++++++++++++++++++----------
>  1 file changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 7d13fa7ab30a..dee21f65f7b7 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -793,17 +793,35 @@ xfs_get_cached_zone(
>  
>  	rcu_read_lock();
>  	oz = VFS_I(ip)->i_private;
> -	if (oz) {
> -		/*
> -		 * GC only steals open zones at mount time, so no GC zones
> -		 * should end up in the cache.
> -		 */
> -		ASSERT(!oz->oz_is_gc);
> -		if (!atomic_inc_not_zero(&oz->oz_ref))
> +	if (!oz)
> +		goto out_unlock;
> +
> +	/*
> +	 * GC only steals open zones at mount time, so no GC zones should end up
> +	 * in the cache.
> +	 */
> +	ASSERT(!oz->oz_is_gc);
> +
> +	/*
> +	 * Drop the old cached open zone if it is full.
> +	 */
> +	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
> +		spin_lock(&ip->i_flags_lock);
> +		oz = VFS_I(ip)->i_private;
> +		if (oz && oz->oz_allocated == rtg_blocks(oz->oz_rtg)) {
> +			VFS_I(ip)->i_private = NULL;
> +			spin_unlock(&ip->i_flags_lock);
> +			xfs_open_zone_put(oz);
>  			oz = NULL;
> +			goto out_unlock;
> +		}
> +		spin_unlock(&ip->i_flags_lock);
>  	}
> -	rcu_read_unlock();
>  
> +	if (oz && !atomic_inc_not_zero(&oz->oz_ref))
> +		oz = NULL;

Do we still need to test oz for null-ness here?  AFAICT we've already
handled those cases here.

> +out_unlock:
> +	rcu_read_unlock();
>  	return oz;
>  }
>  
> @@ -819,17 +837,30 @@ xfs_get_cached_zone(
>   * lookup.  Because the open_zone is clearly marked as full when all data
>   * in the underlying RTG was written, the caching is always safe.
>   */
> -static void
> +static struct xfs_open_zone *
>  xfs_set_cached_zone(
>  	struct xfs_inode	*ip,
>  	struct xfs_open_zone	*oz)
>  {
>  	struct xfs_open_zone	*old_oz;
>  
> +	/*
> +	 * If the open zone cached in the inode still has free space, use that
> +	 * instead of the inode we just selected.  This can happen when multiple
> +	 * threads race to perform zone selection for an inode.  io_uring worker
> +	 * threads seem to be good at triggering this.
> +	 */
> +	spin_lock(&ip->i_flags_lock);
> +	old_oz = VFS_I(ip)->i_private;
> +	if (old_oz && old_oz->oz_allocated < rtg_blocks(old_oz->oz_rtg))
> +		swap(oz, old_oz);
>  	atomic_inc(&oz->oz_ref);

Hmm, I'm confused about oz_ref handling here.

If old_oz still has space, we swap oz and old_oz, after which oz alias
i_private and old_oz is the zone that the caller passed in.  The above
line then increments oz->oz_ref and puts the zone that the caller passed
in.

Doesn't that cause oz->oz_ref to be too high?  We already had a ref
via i_private, and now we have another one.

--D

> -	old_oz = xchg(&VFS_I(ip)->i_private, oz);
> +	VFS_I(ip)->i_private = oz;
> +	spin_unlock(&ip->i_flags_lock);
> +
>  	if (old_oz)
>  		xfs_open_zone_put(old_oz);
> +	return oz;
>  }
>  
>  static void
> @@ -873,14 +904,13 @@ xfs_zone_alloc_and_submit(
>  	 * the inode is still associated with a zone and use that if so.
>  	 */
>  	if (!*oz)
> +select_zone:
>  		*oz = xfs_get_cached_zone(ip);
> -
>  	if (!*oz) {
> -select_zone:
>  		*oz = xfs_select_zone(mp, write_hint, pack_tight);
>  		if (!*oz)
>  			goto out_error;
> -		xfs_set_cached_zone(ip, *oz);
> +		*oz = xfs_set_cached_zone(ip, *oz);
>  	}
>  
>  	alloc_len = xfs_zone_alloc_blocks(*oz, XFS_B_TO_FSB(mp, ioend->io_size),
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/5] xfs: fix racy open zone caching

[Christoph Hellwig]

On Mon, Aug 10, 2026 at 11:21:42AM -0700, Darrick J. Wong wrote:
> > +	if (oz && !atomic_inc_not_zero(&oz->oz_ref))
> > +		oz = NULL;
> 
> Do we still need to test oz for null-ness here?  AFAICT we've already
> handled those cases here.

Yeah, that can go away.

> > +	spin_lock(&ip->i_flags_lock);
> > +	old_oz = VFS_I(ip)->i_private;
> > +	if (old_oz && old_oz->oz_allocated < rtg_blocks(old_oz->oz_rtg))
> > +		swap(oz, old_oz);
> >  	atomic_inc(&oz->oz_ref);
> 
> Hmm, I'm confused about oz_ref handling here.
> 
> If old_oz still has space, we swap oz and old_oz, after which oz alias
> i_private and old_oz is the zone that the caller passed in.  The above
> line then increments oz->oz_ref and puts the zone that the caller passed
> in.
> 
> Doesn't that cause oz->oz_ref to be too high?  We already had a ref
> via i_private, and now we have another one.

The caller needs a reference, and i_private needs a reference.  But this
is a bit to oclever to understand, we probably should be doing
atomic_inc_not_zero for the i_private case as well.  I'll rework this
to be a bit more clear.