Re: [PATCH 1/5] xfs: cleanup xfs_imap

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

On Mon, Jun 01, 2026 at 02:43:47PM +0200, Christoph Hellwig wrote:
> Reshuffle the code a bit so that the imap_lookup and filling out of the
> xfs_imap structure aren't duplicated.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This is pretty straightforward so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 64 +++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 8d6a4fe4228c..a3fe4e5b1cdd 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2511,44 +2511,38 @@ xfs_imap(
>  	 * inodes in stale state on disk. Hence we have to do a btree lookup
>  	 * in all cases where an untrusted inode number is passed.
>  	 */
> -	if (flags & XFS_IGET_UNTRUSTED) {
> -		error = xfs_imap_lookup(pag, tp, agino, agbno,
> -					&chunk_agbno, &offset_agbno, flags);
> -		if (error)
> -			return error;
> -		goto out_map;
> -	}
> +	if (!(flags & XFS_IGET_UNTRUSTED)) {
> +		/*
> +		 * If the inode cluster size is the same or smaller than the
> +		 * blocksize, get to the buffer by simple arithmetics.
> +		 */
> +		if (M_IGEO(mp)->blocks_per_cluster == 1) {
> +			cluster_agbno = agbno;
> +			offset = XFS_INO_TO_OFFSET(mp, ino);
> +			ASSERT(offset < mp->m_sb.sb_inopblock);
> +			goto out;
> +		}
>  
> -	/*
> -	 * If the inode cluster size is the same as the blocksize or
> -	 * smaller we get to the buffer by simple arithmetics.
> -	 */
> -	if (M_IGEO(mp)->blocks_per_cluster == 1) {
> -		offset = XFS_INO_TO_OFFSET(mp, ino);
> -		ASSERT(offset < mp->m_sb.sb_inopblock);
> -
> -		imap->im_blkno = xfs_agbno_to_daddr(pag, agbno);
> -		imap->im_len = XFS_FSB_TO_BB(mp, 1);
> -		imap->im_boffset = (unsigned short)(offset <<
> -							mp->m_sb.sb_inodelog);
> -		return 0;
> -	}
> +		/*
> +		 * If the inode chunks are aligned, use simple maths to find the
> +		 * location.
> +		 */
> +		if (M_IGEO(mp)->inoalign_mask) {
> +			offset_agbno = agbno & M_IGEO(mp)->inoalign_mask;
> +			chunk_agbno = agbno - offset_agbno;
> +			goto out_map;
> +		}
>  
> -	/*
> -	 * If the inode chunks are aligned then use simple maths to
> -	 * find the location. Otherwise we have to do a btree
> -	 * lookup to find the location.
> -	 */
> -	if (M_IGEO(mp)->inoalign_mask) {
> -		offset_agbno = agbno & M_IGEO(mp)->inoalign_mask;
> -		chunk_agbno = agbno - offset_agbno;
> -	} else {
> -		error = xfs_imap_lookup(pag, tp, agino, agbno,
> -					&chunk_agbno, &offset_agbno, flags);
> -		if (error)
> -			return error;
> +		/*
> +		 * Otherwise we have to do a btree lookup to find the location.
> +		 */
>  	}
>  
> +	error = xfs_imap_lookup(pag, tp, agino, agbno, &chunk_agbno,
> +			&offset_agbno, flags);
> +	if (error)
> +		return error;
> +
>  out_map:
>  	ASSERT(agbno >= chunk_agbno);
>  	cluster_agbno = chunk_agbno +
> @@ -2556,7 +2550,7 @@ xfs_imap(
>  		 M_IGEO(mp)->blocks_per_cluster);
>  	offset = ((agbno - cluster_agbno) * mp->m_sb.sb_inopblock) +
>  		XFS_INO_TO_OFFSET(mp, ino);
> -
> +out:
>  	imap->im_blkno = xfs_agbno_to_daddr(pag, cluster_agbno);
>  	imap->im_len = XFS_FSB_TO_BB(mp, M_IGEO(mp)->blocks_per_cluster);
>  	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
> -- 
> 2.53.0
> 
>