Re: [PATCH 20/25] xfs: verify data checksums during media verification

["Pankaj Raghav (Samsung)" <pankaj.raghav@linux.dev>]

On Wed, Nov 04, 2026 at 09:57:24AM +0100, Christoph Hellwig wrote:
> Wire up reading and verifying data checksums during media verification.
> This is very similar to the file read path in that it kicks of an async
> read for the checksum buffer before reading the data, and then validating
> once both are read in.
> 
> To support this, split the per-read logic in xfs_verify_media into a
> separate helpers for the data checksums vs no checksum cases.
> 
> +
> +/*
> + * Data checksums are per-FSB, so we can only validate FSB-aligned regions.
> + *
> + * When the alignment leads to an empty range, give up.
> + */

A comment here would be helpful about the return values.
0 for success,
1 for media errors
errno for other errors.

> +static int
> +xfs_submit_verify_bio_csum(
> +	struct xfs_mount	*mp,
> +	struct xfs_verify_media	*me,
> +	struct xfs_buftarg	*btp,
> +	struct folio		*folio,
> +	xfs_daddr_t		*daddr)
> +{
> +	xfs_fsblock_t		bno = xfs_daddr_to_rtb(mp, *daddr);
> +	xfs_fsblock_t		end = xfs_daddr_to_rtb(mp, me->me_end_daddr);
> +	struct xfs_buf		*csum_bp = NULL;
> +	struct bvec_iter	saved_iter;
> +	unsigned int		bbcount;
> +	xfs_filblks_t		len;
> +	struct bio		bio;
> +	struct bio_vec		bv;
> +	int			error;
> +
> +	len = min3(end - bno, xfs_rtcsum_max_len(mp, bno),
> +			XFS_B_TO_FSBT(mp, folio_size(folio)));

For large block size systems, if we allocate a single page in the folio
due to fragmentation, len will always be zero (4096 >> (blocklog > 12)).

> +	if (!len) {
> +		*daddr = me->me_end_daddr;
> +		return 0;
> +	}

And we skip those ranges without verifying anything with a success.

I think we should raise the iosize to a single FSB in
xfs_verify_alloc_folio and should return ENOMEM if we cannot allocate
one?

> +
> +	error = xfs_rtcsum_read_async(mp, bno, &csum_bp);
> +	if (error)
> +		return error;
> +
> +	*daddr = xfs_rtb_to_daddr(mp, bno);
> +	bbcount = XFS_FSB_TO_BB(mp, len);
> +
> +	bio_init(&bio, btp->bt_bdev, &bv, 1, REQ_OP_READ);
> +	bio.bi_iter.bi_sector = *daddr;
> +	bio_add_folio_nofail(&bio, folio, bbcount << SECTOR_SHIFT, 0);
> +
> +	saved_iter = bio.bi_iter;
> +	error = submit_bio_wait(&bio);
> +	if (error) {
> +		xfs_verify_media_error(mp, me, btp, *daddr, bbcount, error);
> +		error = 1;
> +		goto out_buf_rele;
> +	}
> +
> +	error = xfs_buf_read_async_wait(csum_bp);
> +	if (error)
> +		goto out_buf_rele;
> +
> +	error = xfs_csum_verify(mp, &bio, &saved_iter,
> +			csum_bp->b_addr + xfs_rtb_to_rtcsumoff(mp, bno), bno,
> +			false);
> +	if (error) {
> +		xfs_verify_media_error(mp, me, btp, *daddr, bbcount, error);
> +		error = 1;
> +		goto out_buf_rele;
> +	}
> +
> +	*daddr = xfs_rtb_to_daddr(mp, bno + len);
> +out_buf_rele:
> +	xfs_buf_rele(csum_bp);
> +	bio_uninit(&bio);
> +	return error;
> +}
> +

-- 
Pankaj