Re: [PATCH] repair: simplify bmap_next_offset

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Jul 15, 2020 at 08:34:17PM +0200, Christoph Hellwig wrote:
> The tp argument is always NULL, and the whichfork argument is always
> XFS_DATA_FORK, so simplify and cleanup the function based on those
> assumptions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

/me has nearly the same code change in his 5.8 sync branch.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  repair/phase6.c | 51 +++++++++++++++++++++++--------------------------
>  1 file changed, 24 insertions(+), 27 deletions(-)
> 
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 446bcfcb7..952af590f 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -406,46 +406,43 @@ dir_hash_dup_names(dir_hash_tab_t *hashtab)
>  }
>  
>  /*
> - * Given a block number in a fork, return the next valid block number
> - * (not a hole).
> - * If this is the last block number then NULLFILEOFF is returned.
> - *
> - * This was originally in the kernel, but only used in xfs_repair.
> + * Given a block number in a fork, return the next valid block number (not a
> + * hole).  If this is the last block number then NULLFILEOFF is returned.
>   */
>  static int
>  bmap_next_offset(
> -	xfs_trans_t	*tp,			/* transaction pointer */
> -	xfs_inode_t	*ip,			/* incore inode */
> -	xfs_fileoff_t	*bnop,			/* current block */
> -	int		whichfork)		/* data or attr fork */
> +	struct xfs_inode	*ip,
> +	xfs_fileoff_t		*bnop)
>  {
> -	xfs_fileoff_t	bno;			/* current block */
> -	int		error;			/* error return value */
> -	xfs_bmbt_irec_t got;			/* current extent value */
> -	struct xfs_ifork	*ifp;		/* inode fork pointer */
> +	xfs_fileoff_t		bno;
> +	int			error;
> +	struct xfs_bmbt_irec	got;	
>  	struct xfs_iext_cursor	icur;
>  
> -	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> -	       return EIO;
> -	if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> +	switch (ip->i_d.di_format) {
> +	case XFS_DINODE_FMT_LOCAL:
>  		*bnop = NULLFILEOFF;
>  		return 0;
> +	case XFS_DINODE_FMT_BTREE:
> +	case XFS_DINODE_FMT_EXTENTS:
> +		break;
> +	default:
> +		return EIO;
> +	}
> +
> +	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> +		error = -libxfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> +		if (error)
> +			return error;
>  	}
> -	ifp = XFS_IFORK_PTR(ip, whichfork);
> -	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> -	    (error = -libxfs_iread_extents(tp, ip, whichfork)))
> -		return error;
>  	bno = *bnop + 1;
> -	if (!libxfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
> +	if (!libxfs_iext_lookup_extent(ip, &ip->i_df, bno, &icur, &got))
>  		*bnop = NULLFILEOFF;
>  	else
>  		*bnop = got.br_startoff < bno ? bno : got.br_startoff;
>  	return 0;
>  }
>  
> -
>  static void
>  res_failed(
>  	int	err)
> @@ -2054,7 +2051,7 @@ longform_dir2_check_node(
>  			next_da_bno != NULLFILEOFF && da_bno < mp->m_dir_geo->freeblk;
>  			da_bno = (xfs_dablk_t)next_da_bno) {
>  		next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> -		if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
> +		if (bmap_next_offset(ip, &next_da_bno))
>  			break;
>  
>  		/*
> @@ -2129,7 +2126,7 @@ longform_dir2_check_node(
>  	     next_da_bno != NULLFILEOFF;
>  	     da_bno = (xfs_dablk_t)next_da_bno) {
>  		next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> -		if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
> +		if (bmap_next_offset(ip, &next_da_bno))
>  			break;
>  
>  		error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops,
> @@ -2261,7 +2258,7 @@ longform_dir2_entry_check(xfs_mount_t	*mp,
>  		struct xfs_dir2_data_hdr *d;
>  
>  		next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> -		if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK)) {
> +		if (bmap_next_offset(ip, &next_da_bno)) {
>  			/*
>  			 * if this is the first block, there isn't anything we
>  			 * can recover so we just trash it.
> -- 
> 2.27.0
>