Re: [PATCH 3/6] xfs: pass the write pointer to xfs_init_zone

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Jan 12, 2026 at 11:15:07AM +0100, Damien Le Moal wrote:
> On 1/9/26 18:20, Christoph Hellwig wrote:
> > Move the two methods to query the write pointer out of xfs_init_zone into
> > the callers, so that xfs_init_zone doesn't have to bother with the
> > blk_zone structure and instead operates purely at the XFS realtime group
> > level.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_zone_alloc.c | 66 +++++++++++++++++++++++------------------
> >  1 file changed, 37 insertions(+), 29 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> > index bbcf21704ea0..013228eab0ac 100644
> > --- a/fs/xfs/xfs_zone_alloc.c
> > +++ b/fs/xfs/xfs_zone_alloc.c
> > @@ -981,43 +981,43 @@ struct xfs_init_zones {
> >  	uint64_t		reclaimable;
> >  };
> >  
> > +/*
> > + * For sequential write required zones, we restart writing at the hardware write
> > + * pointer.
> > + *
> > + * For conventional zones or conventional devices we have query the rmap to
> > + * find the highest recorded block and set the write pointer to the block after
> > + * that.  In case of a power loss this misses blocks where the data I/O has
> > + * completed but not recorded in the rmap yet, and it also rewrites blocks if
> > + * the most recently written ones got deleted again before unmount, but this is
> > + * the best we can do without hardware support.
> > + */
> 
> I find this comment and the function name confusing since we are not looking at
> a zone write pointer at all. So maybe rename this to something like:
> 
> xfs_rmap_get_highest_rgbno()
> 
> ? Also, I think the comment block should go...
> 
> > +static xfs_rgblock_t
> > +xfs_rmap_write_pointer(
> > +	struct xfs_rtgroup	*rtg)
> > +{
> > +	xfs_rgblock_t		highest_rgbno;
> > +
> > +	xfs_rtgroup_lock(rtg, XFS_RTGLOCK_RMAP);
> > +	highest_rgbno = xfs_rtrmap_highest_rgbno(rtg);
> > +	xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
> > +
> > +	if (highest_rgbno == NULLRGBLOCK)
> > +		return 0;
> > +	return highest_rgbno + 1;
> > +}
> 
> [...]
> 
> >  	/*
> >  	 * If there are no used blocks, but the zone is not in empty state yet
> >  	 * we lost power before the zoned reset.  In that case finish the work
> > @@ -1066,6 +1066,7 @@ xfs_get_zone_info_cb(
> >  	struct xfs_mount	*mp = iz->mp;
> >  	xfs_fsblock_t		zsbno = xfs_daddr_to_rtb(mp, zone->start);
> >  	xfs_rgnumber_t		rgno;
> > +	xfs_rgblock_t		write_pointer;
> >  	struct xfs_rtgroup	*rtg;
> >  	int			error;
> >  
> > @@ -1080,7 +1081,13 @@ xfs_get_zone_info_cb(
> >  		xfs_warn(mp, "realtime group not found for zone %u.", rgno);
> >  		return -EFSCORRUPTED;
> >  	}
> > -	error = xfs_init_zone(iz, rtg, zone);
> 
> ...here.
> This code is also hard to follow without a comment indicating that write_pointer
> is not set by xfs_zone_validate() for conventional zones. Ideally, we should
> move the call to xfs_rmap_write_pointer() in xfs_zone_validate(). That would be
> cleaner, no ?
> 
> > +	if (!xfs_zone_validate(zone, rtg, &write_pointer)) {

I had wondered by the time I got to the end of this series if this
function should be renamed to xfs_validate_hw_zone() or something like
that?

--D

> > +		xfs_rtgroup_rele(rtg);
> > +		return -EFSCORRUPTED;
> > +	}
> > +	if (zone->cond == BLK_ZONE_COND_NOT_WP)
> > +		write_pointer = xfs_rmap_write_pointer(rtg);
> > +	error = xfs_init_zone(iz, rtg, write_pointer);
> >  	xfs_rtgroup_rele(rtg);
> >  	return error;
> >  }
> > @@ -1290,7 +1297,8 @@ xfs_mount_zones(
> >  		struct xfs_rtgroup	*rtg = NULL;
> >  
> >  		while ((rtg = xfs_rtgroup_next(mp, rtg))) {
> > -			error = xfs_init_zone(&iz, rtg, NULL);
> > +			error = xfs_init_zone(&iz, rtg,
> > +					xfs_rmap_write_pointer(rtg));
> >  			if (error) {
> >  				xfs_rtgroup_rele(rtg);
> >  				goto out_free_zone_info;
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research
>