diff mbox series

[02/10] xfs: refactor xfs_dabuf_map

Message ID 20191120111727.16119-3-hch@lst.de (mailing list archive)
State Accepted
Headers show
Series [01/10] xfs: simplify mappedbno handling in xfs_da_{get,read}_buf | expand

Commit Message

Christoph Hellwig Nov. 20, 2019, 11:17 a.m. UTC 
Merge xfs_buf_map_from_irec and xfs_da_map_covers_blocks into a single
loop in the caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_da_btree.c | 156 ++++++++++++-----------------------
 1 file changed, 54 insertions(+), 102 deletions(-)

Comments

Darrick J. Wong Nov. 20, 2019, 6:24 p.m. UTC | #1 
On Wed, Nov 20, 2019 at 12:17:19PM +0100, Christoph Hellwig wrote:
> Merge xfs_buf_map_from_irec and xfs_da_map_covers_blocks into a single
> loop in the caller.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 156 ++++++++++++-----------------------
>  1 file changed, 54 insertions(+), 102 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index f3087f061a48..e078817fc26c 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2460,74 +2460,6 @@ xfs_da_shrink_inode(
>  	return error;
>  }
>  
> -/*
> - * See if the mapping(s) for this btree block are valid, i.e.
> - * don't contain holes, are logically contiguous, and cover the whole range.
> - */
> -STATIC int
> -xfs_da_map_covers_blocks(
> -	int		nmap,
> -	xfs_bmbt_irec_t	*mapp,
> -	xfs_dablk_t	bno,
> -	int		count)
> -{
> -	int		i;
> -	xfs_fileoff_t	off;
> -
> -	for (i = 0, off = bno; i < nmap; i++) {
> -		if (mapp[i].br_startblock == HOLESTARTBLOCK ||
> -		    mapp[i].br_startblock == DELAYSTARTBLOCK) {
> -			return 0;
> -		}
> -		if (off != mapp[i].br_startoff) {
> -			return 0;
> -		}
> -		off += mapp[i].br_blockcount;
> -	}
> -	return off == bno + count;
> -}
> -
> -/*
> - * Convert a struct xfs_bmbt_irec to a struct xfs_buf_map.
> - *
> - * For the single map case, it is assumed that the caller has provided a pointer
> - * to a valid xfs_buf_map.  For the multiple map case, this function will
> - * allocate the xfs_buf_map to hold all the maps and replace the caller's single
> - * map pointer with the allocated map.
> - */
> -static int
> -xfs_buf_map_from_irec(
> -	struct xfs_mount	*mp,
> -	struct xfs_buf_map	**mapp,
> -	int			*nmaps,
> -	struct xfs_bmbt_irec	*irecs,
> -	int			nirecs)
> -{
> -	struct xfs_buf_map	*map;
> -	int			i;
> -
> -	ASSERT(*nmaps == 1);
> -	ASSERT(nirecs >= 1);
> -
> -	if (nirecs > 1) {
> -		map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map),
> -				  KM_NOFS);
> -		if (!map)
> -			return -ENOMEM;
> -		*mapp = map;
> -	}
> -
> -	*nmaps = nirecs;
> -	map = *mapp;
> -	for (i = 0; i < *nmaps; i++) {
> -		ASSERT(irecs[i].br_startblock != DELAYSTARTBLOCK &&
> -		       irecs[i].br_startblock != HOLESTARTBLOCK);
> -		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, irecs[i].br_startblock);
> -		map[i].bm_len = XFS_FSB_TO_BB(mp, irecs[i].br_blockcount);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * Map the block we are given ready for reading. There are three possible return
>   * values:
> @@ -2542,58 +2474,78 @@ xfs_dabuf_map(
>  	xfs_dablk_t		bno,
>  	xfs_daddr_t		mappedbno,
>  	int			whichfork,
> -	struct xfs_buf_map	**map,
> +	struct xfs_buf_map	**mapp,
>  	int			*nmaps)
>  {
>  	struct xfs_mount	*mp = dp->i_mount;
>  	int			nfsb = xfs_dabuf_nfsb(mp, whichfork);
> -	int			error = 0;
> -	struct xfs_bmbt_irec	irec;
> -	struct xfs_bmbt_irec	*irecs = &irec;
> -	int			nirecs;
> -
> -	ASSERT(map && *map);
> -	ASSERT(*nmaps == 1);
> +	struct xfs_bmbt_irec	irec, *irecs = &irec;
> +	struct xfs_buf_map	*map = *mapp;
> +	xfs_fileoff_t		off = bno;
> +	int			error = 0, nirecs, i;
>  
> -	if (nfsb != 1)
> +	if (nfsb > 1)
>  		irecs = kmem_zalloc(sizeof(irec) * nfsb, KM_NOFS);
> +
>  	nirecs = nfsb;
> -	error = xfs_bmapi_read(dp, (xfs_fileoff_t)bno, nfsb, irecs,
> -			       &nirecs, xfs_bmapi_aflag(whichfork));
> +	error = xfs_bmapi_read(dp, bno, nfsb, irecs, &nirecs,
> +			xfs_bmapi_aflag(whichfork));
>  	if (error)
> -		goto out;
> +		goto out_free_irecs;
>  
> -	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
> -		/* Caller ok with no mapping. */
> -		if (!XFS_IS_CORRUPT(mp, mappedbno != -2)) {
> -			error = -1;
> -			goto out;
> -		}
> +	/*
> +	 * Use the caller provided map for the single map case, else allocate a
> +	 * larger one that needs to be free by the caller.
> +	 */
> +	if (nirecs > 1) {
> +		map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map), KM_NOFS);
> +		if (!map)
> +			goto out_free_irecs;
> +		*mapp = map;
> +	}
>  
> -		/* Caller expected a mapping, so abort. */
> +	for (i = 0; i < nirecs; i++) {
> +		if (irecs[i].br_startblock == HOLESTARTBLOCK ||
> +		    irecs[i].br_startblock == DELAYSTARTBLOCK)
> +			goto invalid_mapping;
> +		if (off != irecs[i].br_startoff)
> +			goto invalid_mapping;
> +
> +		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, irecs[i].br_startblock);
> +		map[i].bm_len = XFS_FSB_TO_BB(mp, irecs[i].br_blockcount);
> +		off += irecs[i].br_blockcount;
> +	}
> +
> +	if (off != bno + nfsb)
> +		goto invalid_mapping;
> +
> +	*nmaps = nirecs;
> +out_free_irecs:
> +	if (irecs != &irec)
> +		kmem_free(irecs);
> +	return error;
> +
> +invalid_mapping:
> +	/* Caller ok with no mapping. */
> +	if (XFS_IS_CORRUPT(mp, mappedbno != -2)) {
> +		error = -EFSCORRUPTED;
>  		if (xfs_error_level >= XFS_ERRLEVEL_LOW) {
> -			int i;
> +			xfs_alert(mp, "%s: bno %u inode %llu",
> +					__func__, bno, dp->i_ino);
>  
> -			xfs_alert(mp, "%s: bno %lld dir: inode %lld", __func__,
> -					(long long)bno, (long long)dp->i_ino);
> -			for (i = 0; i < *nmaps; i++) {
> +			for (i = 0; i < nirecs; i++) {
>  				xfs_alert(mp,
>  "[%02d] br_startoff %lld br_startblock %lld br_blockcount %lld br_state %d",
> -					i,
> -					(long long)irecs[i].br_startoff,
> -					(long long)irecs[i].br_startblock,
> -					(long long)irecs[i].br_blockcount,
> +					i, irecs[i].br_startoff,
> +					irecs[i].br_startblock,
> +					irecs[i].br_blockcount,
>  					irecs[i].br_state);
>  			}
>  		}
> -		error = -EFSCORRUPTED;
> -		goto out;
> +	} else {
> +		*nmaps = 0;
>  	}
> -	error = xfs_buf_map_from_irec(mp, map, nmaps, irecs, nirecs);
> -out:
> -	if (irecs != &irec)
> -		kmem_free(irecs);
> -	return error;
> +	goto out_free_irecs;
>  }
>  
>  /*
> -- 
> 2.20.1
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index f3087f061a48..e078817fc26c 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2460,74 +2460,6 @@  xfs_da_shrink_inode(
 	return error;
 }
 
-/*
- * See if the mapping(s) for this btree block are valid, i.e.
- * don't contain holes, are logically contiguous, and cover the whole range.
- */
-STATIC int
-xfs_da_map_covers_blocks(
-	int		nmap,
-	xfs_bmbt_irec_t	*mapp,
-	xfs_dablk_t	bno,
-	int		count)
-{
-	int		i;
-	xfs_fileoff_t	off;
-
-	for (i = 0, off = bno; i < nmap; i++) {
-		if (mapp[i].br_startblock == HOLESTARTBLOCK ||
-		    mapp[i].br_startblock == DELAYSTARTBLOCK) {
-			return 0;
-		}
-		if (off != mapp[i].br_startoff) {
-			return 0;
-		}
-		off += mapp[i].br_blockcount;
-	}
-	return off == bno + count;
-}
-
-/*
- * Convert a struct xfs_bmbt_irec to a struct xfs_buf_map.
- *
- * For the single map case, it is assumed that the caller has provided a pointer
- * to a valid xfs_buf_map.  For the multiple map case, this function will
- * allocate the xfs_buf_map to hold all the maps and replace the caller's single
- * map pointer with the allocated map.
- */
-static int
-xfs_buf_map_from_irec(
-	struct xfs_mount	*mp,
-	struct xfs_buf_map	**mapp,
-	int			*nmaps,
-	struct xfs_bmbt_irec	*irecs,
-	int			nirecs)
-{
-	struct xfs_buf_map	*map;
-	int			i;
-
-	ASSERT(*nmaps == 1);
-	ASSERT(nirecs >= 1);
-
-	if (nirecs > 1) {
-		map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map),
-				  KM_NOFS);
-		if (!map)
-			return -ENOMEM;
-		*mapp = map;
-	}
-
-	*nmaps = nirecs;
-	map = *mapp;
-	for (i = 0; i < *nmaps; i++) {
-		ASSERT(irecs[i].br_startblock != DELAYSTARTBLOCK &&
-		       irecs[i].br_startblock != HOLESTARTBLOCK);
-		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, irecs[i].br_startblock);
-		map[i].bm_len = XFS_FSB_TO_BB(mp, irecs[i].br_blockcount);
-	}
-	return 0;
-}
-
 /*
  * Map the block we are given ready for reading. There are three possible return
  * values:
@@ -2542,58 +2474,78 @@  xfs_dabuf_map(
 	xfs_dablk_t		bno,
 	xfs_daddr_t		mappedbno,
 	int			whichfork,
-	struct xfs_buf_map	**map,
+	struct xfs_buf_map	**mapp,
 	int			*nmaps)
 {
 	struct xfs_mount	*mp = dp->i_mount;
 	int			nfsb = xfs_dabuf_nfsb(mp, whichfork);
-	int			error = 0;
-	struct xfs_bmbt_irec	irec;
-	struct xfs_bmbt_irec	*irecs = &irec;
-	int			nirecs;
-
-	ASSERT(map && *map);
-	ASSERT(*nmaps == 1);
+	struct xfs_bmbt_irec	irec, *irecs = &irec;
+	struct xfs_buf_map	*map = *mapp;
+	xfs_fileoff_t		off = bno;
+	int			error = 0, nirecs, i;
 
-	if (nfsb != 1)
+	if (nfsb > 1)
 		irecs = kmem_zalloc(sizeof(irec) * nfsb, KM_NOFS);
+
 	nirecs = nfsb;
-	error = xfs_bmapi_read(dp, (xfs_fileoff_t)bno, nfsb, irecs,
-			       &nirecs, xfs_bmapi_aflag(whichfork));
+	error = xfs_bmapi_read(dp, bno, nfsb, irecs, &nirecs,
+			xfs_bmapi_aflag(whichfork));
 	if (error)
-		goto out;
+		goto out_free_irecs;
 
-	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
-		/* Caller ok with no mapping. */
-		if (!XFS_IS_CORRUPT(mp, mappedbno != -2)) {
-			error = -1;
-			goto out;
-		}
+	/*
+	 * Use the caller provided map for the single map case, else allocate a
+	 * larger one that needs to be free by the caller.
+	 */
+	if (nirecs > 1) {
+		map = kmem_zalloc(nirecs * sizeof(struct xfs_buf_map), KM_NOFS);
+		if (!map)
+			goto out_free_irecs;
+		*mapp = map;
+	}
 
-		/* Caller expected a mapping, so abort. */
+	for (i = 0; i < nirecs; i++) {
+		if (irecs[i].br_startblock == HOLESTARTBLOCK ||
+		    irecs[i].br_startblock == DELAYSTARTBLOCK)
+			goto invalid_mapping;
+		if (off != irecs[i].br_startoff)
+			goto invalid_mapping;
+
+		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, irecs[i].br_startblock);
+		map[i].bm_len = XFS_FSB_TO_BB(mp, irecs[i].br_blockcount);
+		off += irecs[i].br_blockcount;
+	}
+
+	if (off != bno + nfsb)
+		goto invalid_mapping;
+
+	*nmaps = nirecs;
+out_free_irecs:
+	if (irecs != &irec)
+		kmem_free(irecs);
+	return error;
+
+invalid_mapping:
+	/* Caller ok with no mapping. */
+	if (XFS_IS_CORRUPT(mp, mappedbno != -2)) {
+		error = -EFSCORRUPTED;
 		if (xfs_error_level >= XFS_ERRLEVEL_LOW) {
-			int i;
+			xfs_alert(mp, "%s: bno %u inode %llu",
+					__func__, bno, dp->i_ino);
 
-			xfs_alert(mp, "%s: bno %lld dir: inode %lld", __func__,
-					(long long)bno, (long long)dp->i_ino);
-			for (i = 0; i < *nmaps; i++) {
+			for (i = 0; i < nirecs; i++) {
 				xfs_alert(mp,
 "[%02d] br_startoff %lld br_startblock %lld br_blockcount %lld br_state %d",
-					i,
-					(long long)irecs[i].br_startoff,
-					(long long)irecs[i].br_startblock,
-					(long long)irecs[i].br_blockcount,
+					i, irecs[i].br_startoff,
+					irecs[i].br_startblock,
+					irecs[i].br_blockcount,
 					irecs[i].br_state);
 			}
 		}
-		error = -EFSCORRUPTED;
-		goto out;
+	} else {
+		*nmaps = 0;
 	}
-	error = xfs_buf_map_from_irec(mp, map, nmaps, irecs, nirecs);
-out:
-	if (irecs != &irec)
-		kmem_free(irecs);
-	return error;
+	goto out_free_irecs;
 }
 
 /*