Re: [PATCH 3/7] xfs: add a xfs_group_type_buftarg helper

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

On Tue, Jul 01, 2025 at 12:40:37PM +0200, Christoph Hellwig wrote:
> Generalize the xfs_group_type helper in the discard code to return a buftarg
> and move it to xfs_mount.h, and use the result in xfs_dax_notify_dev_failure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_discard.c        | 29 +++++++----------------------
>  fs/xfs/xfs_mount.h          | 17 +++++++++++++++++
>  fs/xfs/xfs_notify_failure.c |  3 +--
>  3 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> index 94d0873bcd62..603d51365645 100644
> --- a/fs/xfs/xfs_discard.c
> +++ b/fs/xfs/xfs_discard.c
> @@ -103,24 +103,6 @@ xfs_discard_endio(
>  	bio_put(bio);
>  }
>  
> -static inline struct block_device *
> -xfs_group_bdev(
> -	const struct xfs_group	*xg)
> -{
> -	struct xfs_mount	*mp = xg->xg_mount;
> -
> -	switch (xg->xg_type) {
> -	case XG_TYPE_AG:
> -		return mp->m_ddev_targp->bt_bdev;
> -	case XG_TYPE_RTG:
> -		return mp->m_rtdev_targp->bt_bdev;
> -	default:
> -		ASSERT(0);
> -		break;
> -	}
> -	return NULL;
> -}
> -
>  /*
>   * Walk the discard list and issue discards on all the busy extents in the
>   * list. We plug and chain the bios so that we only need a single completion
> @@ -138,11 +120,14 @@ xfs_discard_extents(
>  
>  	blk_start_plug(&plug);
>  	list_for_each_entry(busyp, &extents->extent_list, list) {
> -		trace_xfs_discard_extent(busyp->group, busyp->bno,
> -				busyp->length);
> +		struct xfs_group	*xg = busyp->group;
> +		struct xfs_buftarg	*btp =
> +			xfs_group_type_buftarg(xg->xg_mount, xg->xg_type);
> +
> +		trace_xfs_discard_extent(xg, busyp->bno, busyp->length);
>  
> -		error = __blkdev_issue_discard(xfs_group_bdev(busyp->group),
> -				xfs_gbno_to_daddr(busyp->group, busyp->bno),
> +		error = __blkdev_issue_discard(btp->bt_bdev,
> +				xfs_gbno_to_daddr(xg, busyp->bno),
>  				XFS_FSB_TO_BB(mp, busyp->length),
>  				GFP_KERNEL, &bio);
>  		if (error && error != -EOPNOTSUPP) {
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index d85084f9f317..97de44c32272 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -802,4 +802,21 @@ static inline void xfs_mod_sb_delalloc(struct xfs_mount *mp, int64_t delta)
>  int xfs_set_max_atomic_write_opt(struct xfs_mount *mp,
>  		unsigned long long new_max_bytes);
>  
> +static inline struct xfs_buftarg *
> +xfs_group_type_buftarg(
> +	struct xfs_mount	*mp,
> +	enum xfs_group_type	type)
> +{
> +	switch (type) {
> +	case XG_TYPE_AG:
> +		return mp->m_ddev_targp;
> +	case XG_TYPE_RTG:
> +		return mp->m_rtdev_targp;
> +	default:
> +		ASSERT(0);
> +		break;
> +	}
> +	return NULL;
> +}
> +
>  #endif	/* __XFS_MOUNT_H__ */
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index 3545dc1d953c..42e9c72b85c0 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -253,8 +253,7 @@ xfs_dax_notify_dev_failure(
>  		return -EOPNOTSUPP;
>  	}
>  
> -	error = xfs_dax_translate_range(type == XG_TYPE_RTG ?
> -			mp->m_rtdev_targp : mp->m_ddev_targp,
> +	error = xfs_dax_translate_range(xfs_group_type_buftarg(mp, type),
>  			offset, len, &daddr, &bblen);
>  	if (error)
>  		return error;
> -- 
> 2.47.2
> 
>