Re: [PATCH 16/21] xfs: add data checksum support to zoned garbage collection

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

On Thu, Sep 24, 2026 at 11:59:48AM +0200, Christoph Hellwig wrote:
> Transfer the checksum from the old location to the new one.  The
> implementation mirrors that of user data reads and writes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_gc.c | 65 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 53 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 5fdcf98a2133..1cebd2ee2136 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -21,6 +21,7 @@
>  #include "xfs_zone_alloc.h"
>  #include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
> +#include "xfs_rtcsum.h"
>  #include "xfs_trace.h"
>  
>  /*
> @@ -106,6 +107,10 @@ struct xfs_gc_bio {
>  	/* Realtime group currently being reclaimed */
>  	struct xfs_rtgroup		*victim_rtg;
>  
> +	/* Buffer for data checksums */
> +	struct xfs_buf			*csum_bp;
> +	void				*csum_buf;

Where is csum_buf used?

> +
>  	/* Bio used for reads and writes, including the bvec used by it */
>  	struct bio			bio;	/* must be last */
>  };
> @@ -660,6 +665,20 @@ xfs_zone_gc_alloc_blocks(
>  	return true;
>  }
>  
> +static void
> +xfs_zone_gc_free_chunk(
> +	struct xfs_gc_bio	*chunk)
> +{
> +	atomic_dec(&chunk->victim_rtg->rtg_gccount);
> +	xfs_rtgroup_rele(chunk->victim_rtg);
> +	list_del(&chunk->entry);
> +	xfs_open_zone_put(chunk->oz);
> +	if (chunk->csum_bp)
> +		xfs_buf_rele(chunk->csum_bp);
> +	xfs_irele(chunk->ip);
> +	bio_put(&chunk->bio);
> +}
> +
>  static void
>  xfs_zone_gc_add_data(
>  	struct xfs_gc_bio	*chunk)
> @@ -725,6 +744,11 @@ xfs_zone_gc_start_chunk(
>  	if (!xfs_zone_gc_iter_irec(mp, iter, &irec, &ip))
>  		return false;
>  
> +	if (xfs_has_rtcsum(mp)) {
> +		irec.rm_blockcount = min(irec.rm_blockcount,
> +			xfs_rtcsum_max_len(mp, irec.rm_startblock));
> +	}
> +
>  	if (!xfs_zone_gc_alloc_blocks(data, &irec.rm_blockcount, &daddr,
>  			&is_seq)) {
>  		xfs_irele(ip);
> @@ -748,6 +772,7 @@ xfs_zone_gc_start_chunk(
>  	chunk->data = data;
>  	chunk->oz = data->oz;
>  	chunk->victim_rtg = iter->victim_rtg;
> +	chunk->csum_bp = NULL;
>  	atomic_inc(&rtg_group(chunk->victim_rtg)->xg_active_ref);
>  	atomic_inc(&chunk->victim_rtg->rtg_gccount);
>  
> @@ -764,22 +789,22 @@ xfs_zone_gc_start_chunk(
>  	list_add_tail(&chunk->entry, &data->reading);
>  	xfs_zone_gc_iter_advance(iter, irec.rm_blockcount);
>  
> +	if (xfs_is_rtcsum_inode(ip)) {
> +		int error;
> +
> +		error = xfs_rtcsum_read_async(mp, chunk->old_startblock,
> +				&chunk->csum_bp);
> +		if (error) {
> +			xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
> +			xfs_zone_gc_free_chunk(chunk);
> +			return false;
> +		}

Ok so this starts the csum block read asynchronously, and then submits
the read bio.  chunk->csum_bp is the checksum buffer of the rtcsum file
of the zone we're cleaning out, right?

> +	}
> +
>  	submit_bio(bio);
>  	return true;
>  }
>  
> -static void
> -xfs_zone_gc_free_chunk(
> -	struct xfs_gc_bio	*chunk)
> -{
> -	atomic_dec(&chunk->victim_rtg->rtg_gccount);
> -	xfs_rtgroup_rele(chunk->victim_rtg);
> -	list_del(&chunk->entry);
> -	xfs_open_zone_put(chunk->oz);
> -	xfs_irele(chunk->ip);
> -	bio_put(&chunk->bio);
> -}
> -
>  static void
>  xfs_zone_gc_submit_write(
>  	struct xfs_zone_gc_data	*data,
> @@ -832,6 +857,9 @@ xfs_zone_gc_split_write(
>  	split_chunk->old_startblock = chunk->old_startblock;
>  	split_chunk->new_daddr = chunk->new_daddr;
>  	split_chunk->oz = chunk->oz;
> +	split_chunk->csum_bp = chunk->csum_bp;
> +	if (split_chunk->csum_bp)
> +		xfs_buf_hold(split_chunk->csum_bp);
>  	atomic_inc(&chunk->oz->oz_ref);
>  
>  	split_chunk->victim_rtg = chunk->victim_rtg;
> @@ -919,6 +947,19 @@ xfs_zone_gc_finish_chunk(
>  
>  	if (chunk->is_seq)
>  		chunk->new_daddr = chunk->bio.bi_iter.bi_sector;
> +
> +	if (xfs_is_rtcsum_inode(ip)) {
> +		unsigned int	boff;
> +
> +		error = xfs_buf_read_async_wait(chunk->csum_bp);
> +		if (error)
> +			goto free;
> +
> +		boff = xfs_rtb_to_rtcsumoff(mp, chunk->old_startblock);
> +		error = xfs_rtcsum_log(chunk->oz, chunk->new_daddr, chunk->len,
> +				chunk->csum_bp->b_addr + boff);

...and then write the same checksum to the new daddr using the old
checksum.  There's no explicit validation of the checksum in the garbage
collector itself, but I gather the read ioend did this for us already,
right?

--D

> +	}
> +
>  	error = xfs_zoned_end_io(ip, chunk->offset, chunk->len,
>  			chunk->new_daddr, chunk->oz, chunk->old_startblock);
>  free:
> -- 
> 2.53.0
> 
>