Re: [PATCH 4/9] xfs: cleanup xfs_verify_media_error

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

On Thu, Sep 24, 2026 at 11:57:05AM +0200, Christoph Hellwig wrote:
> Move all the reporting logic into xfs_verify_report_losses, and pass the
> enum xfs_device instead of the group type to it.  This keeps all the
> reporting related conditions and translation confined to one place.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_verify_media.c | 46 +++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/xfs/xfs_verify_media.c b/fs/xfs/xfs_verify_media.c
> index 5ead3976d511..8ce3773aa875 100644
> --- a/fs/xfs/xfs_verify_media.c
> +++ b/fs/xfs/xfs_verify_media.c
> @@ -84,11 +84,14 @@ xfs_verify_report_data_lost(
>  	return 0;
>  }
>  
> -/* Walk reverse mappings to look for all file data loss */
> -static int
> +/*
> + * Report data loss on the physical sectors, and if possible, walk the reverse
> + * mappings to also report the loss to the files mapped to these sectors.
> + */
> +static void
>  xfs_verify_report_losses(
>  	struct xfs_mount	*mp,
> -	enum xfs_group_type	type,
> +	enum xfs_device		dev,
>  	xfs_daddr_t		daddr,
>  	u64			bblen)
>  {
> @@ -97,13 +100,25 @@ xfs_verify_report_losses(
>  	xfs_fsblock_t		start_bno, end_bno;
>  	uint32_t		start_gno, end_gno;
>  	int			error;
> +	enum xfs_group_type	type;
> +
> +	xfs_healthmon_report_media(mp, dev, daddr, bblen);
> +	if (!xfs_has_rmapbt(mp))
> +		return;
>  
> -	if (type == XG_TYPE_RTG) {
> +	switch (dev) {
> +	case XFS_DEV_RT:
>  		start_bno = xfs_daddr_to_rtb(mp, daddr);
>  		end_bno = xfs_daddr_to_rtb(mp, daddr + bblen - 1);
> -	} else {
> +		type = XG_TYPE_RTG;
> +		break;
> +	case XFS_DEV_DATA:
>  		start_bno = XFS_DADDR_TO_FSB(mp, daddr);
>  		end_bno = XFS_DADDR_TO_FSB(mp, daddr + bblen - 1);
> +		type = XG_TYPE_AG;
> +		break;
> +	default:
> +		return;
>  	}
>  
>  	tp = xfs_trans_alloc_empty(mp);
> @@ -164,7 +179,6 @@ xfs_verify_report_losses(
>  	}
>  
>  	xfs_trans_cancel(tp);
> -	return 0;
>  }
>  
>  /*
> @@ -236,25 +250,11 @@ xfs_verify_media_error(
>  	case BLK_STS_PROTECTION:
>  	case BLK_STS_IOERR:
>  	case BLK_STS_MEDIUM:
> +		if (me->me_flags & XFS_VERIFY_MEDIA_REPORT)
> +			xfs_verify_report_losses(mp, me->me_dev, daddr,
> +					bio_bbcount);
>  		break;
>  	default:
> -		return;
> -	}
> -
> -	if (!(me->me_flags & XFS_VERIFY_MEDIA_REPORT))
> -		return;
> -
> -	xfs_healthmon_report_media(mp, me->me_dev, daddr, bio_bbcount);
> -
> -	if (!xfs_has_rmapbt(mp))
> -		return;
> -
> -	switch (me->me_dev) {
> -	case XFS_DEV_DATA:
> -		xfs_verify_report_losses(mp, XG_TYPE_AG, daddr, bio_bbcount);
> -		break;
> -	case XFS_DEV_RT:
> -		xfs_verify_report_losses(mp, XG_TYPE_RTG, daddr, bio_bbcount);
>  		break;
>  	}
>  }
> -- 
> 2.53.0
> 
>