RFC: clean up the dabuf mappedbno interface

RFC: clean up the dabuf mappedbno interface

[Christoph Hellwig]

Hi all,

this (lightly tested) series cleanups the strange mappedbno argument
to various dabuf helpers.  If that argument has a positive value
it is used as xfs_daddr_t to get/read the block, and otherwise
it is overloaded for a flag.  This series splits out the users that
have a xfs_daddr_t to just use the buffer interfaces directly, and
the replaces the magic with a flags argument and one properly
named flag.

[PATCH 1/9] xfs: simplify mappedbno case from xfs_da_get_buf and xfs_da_read_buf

[Christoph Hellwig]

Shortcut the creation of xfs_bmbt_irec and xfs_buf_map for the case
where the callers passed an already mapped xfs_daddr_t.  This is in
preparation for splitting these cases out entirely later.  Also reject
the mappedbno case for xfs_da_reada_buf as no callers currently uses
it and it will be removed soon.

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

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 46b1c3fb305c..681fba5731c2 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -110,6 +110,13 @@ xfs_da_state_free(xfs_da_state_t *state)
 	kmem_zone_free(xfs_da_state_zone, state);
 }
 
+static inline int xfs_dabuf_nfsb(struct xfs_mount *mp, int whichfork)
+{
+	if (whichfork == XFS_DATA_FORK)
+		return mp->m_dir_geo->fsbcount;
+	return mp->m_attr_geo->fsbcount;
+}
+
 void
 xfs_da3_node_hdr_from_disk(
 	struct xfs_mount		*mp,
@@ -2570,7 +2577,7 @@ xfs_dabuf_map(
 	int			*nmaps)
 {
 	struct xfs_mount	*mp = dp->i_mount;
-	int			nfsb;
+	int			nfsb = xfs_dabuf_nfsb(mp, whichfork);
 	int			error = 0;
 	struct xfs_bmbt_irec	irec;
 	struct xfs_bmbt_irec	*irecs = &irec;
@@ -2579,35 +2586,13 @@ xfs_dabuf_map(
 	ASSERT(map && *map);
 	ASSERT(*nmaps == 1);
 
-	if (whichfork == XFS_DATA_FORK)
-		nfsb = mp->m_dir_geo->fsbcount;
-	else
-		nfsb = mp->m_attr_geo->fsbcount;
-
-	/*
-	 * Caller doesn't have a mapping.  -2 means don't complain
-	 * if we land in a hole.
-	 */
-	if (mappedbno == -1 || mappedbno == -2) {
-		/*
-		 * Optimize the one-block case.
-		 */
-		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));
-		if (error)
-			goto out;
-	} else {
-		irecs->br_startblock = XFS_DADDR_TO_FSB(mp, mappedbno);
-		irecs->br_startoff = (xfs_fileoff_t)bno;
-		irecs->br_blockcount = nfsb;
-		irecs->br_state = 0;
-		nirecs = 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));
+	if (error)
+		goto out;
 
 	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
 		/* Caller ok with no mapping. */
@@ -2648,24 +2633,29 @@ xfs_dabuf_map(
  */
 int
 xfs_da_get_buf(
-	struct xfs_trans	*trans,
+	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
 	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp,
 	int			whichfork)
 {
+	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_buf		*bp;
-	struct xfs_buf_map	map;
-	struct xfs_buf_map	*mapp;
-	int			nmap;
+	struct xfs_buf_map	map, *mapp = &map;
+	int			nmap = 1;
 	int			error;
 
 	*bpp = NULL;
-	mapp = &map;
-	nmap = 1;
-	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
-				&mapp, &nmap);
+
+	if (mappedbno >= 0) {
+		bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, mappedbno,
+				XFS_FSB_TO_BB(mp,
+					xfs_dabuf_nfsb(mp, whichfork)), 0);
+		goto done;
+	}
+
+	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
 		if (error == -1)
@@ -2673,12 +2663,12 @@ xfs_da_get_buf(
 		goto out_free;
 	}
 
-	bp = xfs_trans_get_buf_map(trans, dp->i_mount->m_ddev_targp,
-				    mapp, nmap, 0);
+	bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
+done:
 	error = bp ? bp->b_error : -EIO;
 	if (error) {
 		if (bp)
-			xfs_trans_brelse(trans, bp);
+			xfs_trans_brelse(tp, bp);
 		goto out_free;
 	}
 
@@ -2696,7 +2686,7 @@ xfs_da_get_buf(
  */
 int
 xfs_da_read_buf(
-	struct xfs_trans	*trans,
+	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
 	xfs_daddr_t		mappedbno,
@@ -2704,17 +2694,23 @@ xfs_da_read_buf(
 	int			whichfork,
 	const struct xfs_buf_ops *ops)
 {
+	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_buf		*bp;
-	struct xfs_buf_map	map;
-	struct xfs_buf_map	*mapp;
-	int			nmap;
+	struct xfs_buf_map	map, *mapp = &map;
+	int			nmap = 1;
 	int			error;
 
 	*bpp = NULL;
-	mapp = &map;
-	nmap = 1;
-	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
-				&mapp, &nmap);
+
+	if (mappedbno >= 0) {
+		error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
+				mappedbno, XFS_FSB_TO_BB(mp,
+					xfs_dabuf_nfsb(mp, whichfork)),
+				0, &bp, ops);
+		goto done;
+	}
+
+	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
 		if (error == -1)
@@ -2722,9 +2718,9 @@ xfs_da_read_buf(
 		goto out_free;
 	}
 
-	error = xfs_trans_read_buf_map(dp->i_mount, trans,
-					dp->i_mount->m_ddev_targp,
-					mapp, nmap, 0, &bp, ops);
+	error = xfs_trans_read_buf_map(mp, tp, mp->m_ddev_targp, mapp, nmap, 0,
+			&bp, ops);
+done:
 	if (error)
 		goto out_free;
 
@@ -2756,6 +2752,9 @@ xfs_da_reada_buf(
 	int			nmap;
 	int			error;
 
+	if (mappedbno >= 0)
+		return -EINVAL;
+
 	mapp = &map;
 	nmap = 1;
 	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
-- 
2.20.1

Re: [PATCH 1/9] xfs: simplify mappedbno case from xfs_da_get_buf and xfs_da_read_buf

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:06PM +0100, Christoph Hellwig wrote:
> Shortcut the creation of xfs_bmbt_irec and xfs_buf_map for the case
> where the callers passed an already mapped xfs_daddr_t.  This is in
> preparation for splitting these cases out entirely later.  Also reject
> the mappedbno case for xfs_da_reada_buf as no callers currently uses
> it and it will be removed soon.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 103 +++++++++++++++++------------------
>  1 file changed, 51 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 46b1c3fb305c..681fba5731c2 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -110,6 +110,13 @@ xfs_da_state_free(xfs_da_state_t *state)
>  	kmem_zone_free(xfs_da_state_zone, state);
>  }
>  
> +static inline int xfs_dabuf_nfsb(struct xfs_mount *mp, int whichfork)
> +{
> +	if (whichfork == XFS_DATA_FORK)
> +		return mp->m_dir_geo->fsbcount;
> +	return mp->m_attr_geo->fsbcount;
> +}
> +
>  void
>  xfs_da3_node_hdr_from_disk(
>  	struct xfs_mount		*mp,
> @@ -2570,7 +2577,7 @@ xfs_dabuf_map(
>  	int			*nmaps)
>  {
>  	struct xfs_mount	*mp = dp->i_mount;
> -	int			nfsb;
> +	int			nfsb = xfs_dabuf_nfsb(mp, whichfork);
>  	int			error = 0;
>  	struct xfs_bmbt_irec	irec;
>  	struct xfs_bmbt_irec	*irecs = &irec;
> @@ -2579,35 +2586,13 @@ xfs_dabuf_map(
>  	ASSERT(map && *map);
>  	ASSERT(*nmaps == 1);
>  
> -	if (whichfork == XFS_DATA_FORK)
> -		nfsb = mp->m_dir_geo->fsbcount;
> -	else
> -		nfsb = mp->m_attr_geo->fsbcount;
> -
> -	/*
> -	 * Caller doesn't have a mapping.  -2 means don't complain
> -	 * if we land in a hole.
> -	 */
> -	if (mappedbno == -1 || mappedbno == -2) {
> -		/*
> -		 * Optimize the one-block case.
> -		 */
> -		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));
> -		if (error)
> -			goto out;
> -	} else {
> -		irecs->br_startblock = XFS_DADDR_TO_FSB(mp, mappedbno);
> -		irecs->br_startoff = (xfs_fileoff_t)bno;
> -		irecs->br_blockcount = nfsb;
> -		irecs->br_state = 0;
> -		nirecs = 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));
> +	if (error)
> +		goto out;
>  
>  	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
>  		/* Caller ok with no mapping. */
> @@ -2648,24 +2633,29 @@ xfs_dabuf_map(
>   */
>  int
>  xfs_da_get_buf(
> -	struct xfs_trans	*trans,
> +	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
>  	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp,
>  	int			whichfork)
>  {
> +	struct xfs_mount	*mp = dp->i_mount;
>  	struct xfs_buf		*bp;
> -	struct xfs_buf_map	map;
> -	struct xfs_buf_map	*mapp;
> -	int			nmap;
> +	struct xfs_buf_map	map, *mapp = &map;
> +	int			nmap = 1;
>  	int			error;
>  
>  	*bpp = NULL;
> -	mapp = &map;
> -	nmap = 1;
> -	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
> -				&mapp, &nmap);
> +
> +	if (mappedbno >= 0) {
> +		bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, mappedbno,
> +				XFS_FSB_TO_BB(mp,
> +					xfs_dabuf_nfsb(mp, whichfork)), 0);
> +		goto done;
> +	}
> +
> +	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
>  		if (error == -1)
> @@ -2673,12 +2663,12 @@ xfs_da_get_buf(
>  		goto out_free;
>  	}
>  
> -	bp = xfs_trans_get_buf_map(trans, dp->i_mount->m_ddev_targp,
> -				    mapp, nmap, 0);
> +	bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
> +done:
>  	error = bp ? bp->b_error : -EIO;
>  	if (error) {
>  		if (bp)
> -			xfs_trans_brelse(trans, bp);
> +			xfs_trans_brelse(tp, bp);
>  		goto out_free;
>  	}
>  
> @@ -2696,7 +2686,7 @@ xfs_da_get_buf(
>   */
>  int
>  xfs_da_read_buf(
> -	struct xfs_trans	*trans,
> +	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
>  	xfs_daddr_t		mappedbno,
> @@ -2704,17 +2694,23 @@ xfs_da_read_buf(
>  	int			whichfork,
>  	const struct xfs_buf_ops *ops)
>  {
> +	struct xfs_mount	*mp = dp->i_mount;
>  	struct xfs_buf		*bp;
> -	struct xfs_buf_map	map;
> -	struct xfs_buf_map	*mapp;
> -	int			nmap;
> +	struct xfs_buf_map	map, *mapp = &map;
> +	int			nmap = 1;
>  	int			error;
>  
>  	*bpp = NULL;
> -	mapp = &map;
> -	nmap = 1;
> -	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
> -				&mapp, &nmap);
> +
> +	if (mappedbno >= 0) {
> +		error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
> +				mappedbno, XFS_FSB_TO_BB(mp,
> +					xfs_dabuf_nfsb(mp, whichfork)),
> +				0, &bp, ops);
> +		goto done;
> +	}
> +
> +	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
>  		if (error == -1)
> @@ -2722,9 +2718,9 @@ xfs_da_read_buf(
>  		goto out_free;
>  	}
>  
> -	error = xfs_trans_read_buf_map(dp->i_mount, trans,
> -					dp->i_mount->m_ddev_targp,
> -					mapp, nmap, 0, &bp, ops);
> +	error = xfs_trans_read_buf_map(mp, tp, mp->m_ddev_targp, mapp, nmap, 0,
> +			&bp, ops);
> +done:
>  	if (error)
>  		goto out_free;
>  
> @@ -2756,6 +2752,9 @@ xfs_da_reada_buf(
>  	int			nmap;
>  	int			error;
>  
> +	if (mappedbno >= 0)
> +		return -EINVAL;
> +
>  	mapp = &map;
>  	nmap = 1;
>  	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
> -- 
> 2.20.1
> 

[PATCH 2/9] xfs: improve the xfs_dabuf_map calling conventions

[Christoph Hellwig]

Use a flags argument with the XFS_DABUF_MAP_HOLE_OK flag to signal that
a hole is okay and not corruption, and return -ENOENT instead of the
nameless -1 to signal that case in the return value.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_da_btree.c | 25 +++++++++++++++----------
 fs/xfs/libxfs/xfs_da_btree.h |  3 +++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 681fba5731c2..c26f139bcf00 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2571,7 +2571,7 @@ static int
 xfs_dabuf_map(
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
+	unsigned int		flags,
 	int			whichfork,
 	struct xfs_buf_map	**map,
 	int			*nmaps)
@@ -2596,8 +2596,8 @@ xfs_dabuf_map(
 
 	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
 		/* Caller ok with no mapping. */
-		if (mappedbno == -2) {
-			error = -1;
+		if (flags & XFS_DABUF_MAP_HOLE_OK) {
+			error = -ENOENT;
 			goto out;
 		}
 
@@ -2655,10 +2655,12 @@ xfs_da_get_buf(
 		goto done;
 	}
 
-	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno,
+			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
+			whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
-		if (error == -1)
+		if (error == -ENOENT)
 			error = 0;
 		goto out_free;
 	}
@@ -2710,10 +2712,12 @@ xfs_da_read_buf(
 		goto done;
 	}
 
-	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno,
+			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
+			whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
-		if (error == -1)
+		if (error == -ENOENT)
 			error = 0;
 		goto out_free;
 	}
@@ -2757,11 +2761,12 @@ xfs_da_reada_buf(
 
 	mapp = &map;
 	nmap = 1;
-	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
-				&mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno,
+			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
+			whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
-		if (error == -1)
+		if (error == -ENOENT)
 			error = 0;
 		goto out_free;
 	}
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 5af4df71e92b..9ec0d0243e96 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -204,6 +204,9 @@ int	xfs_da3_node_read(struct xfs_trans *tp, struct xfs_inode *dp,
 /*
  * Utility routines.
  */
+
+#define XFS_DABUF_MAP_HOLE_OK	(1 << 0)
+
 int	xfs_da_grow_inode(xfs_da_args_t *args, xfs_dablk_t *new_blkno);
 int	xfs_da_grow_inode_int(struct xfs_da_args *args, xfs_fileoff_t *bno,
 			      int count);
-- 
2.20.1

Re: [PATCH 2/9] xfs: improve the xfs_dabuf_map calling conventions

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:07PM +0100, Christoph Hellwig wrote:
> Use a flags argument with the XFS_DABUF_MAP_HOLE_OK flag to signal that
> a hole is okay and not corruption, and return -ENOENT instead of the
> nameless -1 to signal that case in the return value.

Why not set *nirecs = 0 and return 0 like we sometimes do for bmap
lookups?

--D

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 25 +++++++++++++++----------
>  fs/xfs/libxfs/xfs_da_btree.h |  3 +++
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 681fba5731c2..c26f139bcf00 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2571,7 +2571,7 @@ static int
>  xfs_dabuf_map(
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
> +	unsigned int		flags,
>  	int			whichfork,
>  	struct xfs_buf_map	**map,
>  	int			*nmaps)
> @@ -2596,8 +2596,8 @@ xfs_dabuf_map(
>  
>  	if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
>  		/* Caller ok with no mapping. */
> -		if (mappedbno == -2) {
> -			error = -1;
> +		if (flags & XFS_DABUF_MAP_HOLE_OK) {
> +			error = -ENOENT;
>  			goto out;
>  		}
>  
> @@ -2655,10 +2655,12 @@ xfs_da_get_buf(
>  		goto done;
>  	}
>  
> -	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno,
> +			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> +			whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
> -		if (error == -1)
> +		if (error == -ENOENT)
>  			error = 0;
>  		goto out_free;
>  	}
> @@ -2710,10 +2712,12 @@ xfs_da_read_buf(
>  		goto done;
>  	}
>  
> -	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork, &mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno,
> +			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> +			whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
> -		if (error == -1)
> +		if (error == -ENOENT)
>  			error = 0;
>  		goto out_free;
>  	}
> @@ -2757,11 +2761,12 @@ xfs_da_reada_buf(
>  
>  	mapp = &map;
>  	nmap = 1;
> -	error = xfs_dabuf_map(dp, bno, mappedbno, whichfork,
> -				&mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno,
> +			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> +			whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
> -		if (error == -1)
> +		if (error == -ENOENT)
>  			error = 0;
>  		goto out_free;
>  	}
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 5af4df71e92b..9ec0d0243e96 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -204,6 +204,9 @@ int	xfs_da3_node_read(struct xfs_trans *tp, struct xfs_inode *dp,
>  /*
>   * Utility routines.
>   */
> +
> +#define XFS_DABUF_MAP_HOLE_OK	(1 << 0)
> +
>  int	xfs_da_grow_inode(xfs_da_args_t *args, xfs_dablk_t *new_blkno);
>  int	xfs_da_grow_inode_int(struct xfs_da_args *args, xfs_fileoff_t *bno,
>  			      int count);
> -- 
> 2.20.1
> 

Re: [PATCH 2/9] xfs: improve the xfs_dabuf_map calling conventions

[Christoph Hellwig]

On Sun, Nov 17, 2019 at 10:35:21AM -0800, Darrick J. Wong wrote:
> On Sat, Nov 16, 2019 at 07:22:07PM +0100, Christoph Hellwig wrote:
> > Use a flags argument with the XFS_DABUF_MAP_HOLE_OK flag to signal that
> > a hole is okay and not corruption, and return -ENOENT instead of the
> > nameless -1 to signal that case in the return value.
> 
> Why not set *nirecs = 0 and return 0 like we sometimes do for bmap
> lookups?

Sure, I can change it to that for the next version.

Re: [PATCH 2/9] xfs: improve the xfs_dabuf_map calling conventions

[Darrick J. Wong]

On Mon, Nov 18, 2019 at 07:25:05AM +0100, Christoph Hellwig wrote:
> On Sun, Nov 17, 2019 at 10:35:21AM -0800, Darrick J. Wong wrote:
> > On Sat, Nov 16, 2019 at 07:22:07PM +0100, Christoph Hellwig wrote:
> > > Use a flags argument with the XFS_DABUF_MAP_HOLE_OK flag to signal that
> > > a hole is okay and not corruption, and return -ENOENT instead of the
> > > nameless -1 to signal that case in the return value.
> > 
> > Why not set *nirecs = 0 and return 0 like we sometimes do for bmap
> > lookups?
> 
> Sure, I can change it to that for the next version.

Also, I forgot to mention that some of the comments (particularly
xfs_dabuf_map) need to be updated to reflect the new "no mapping" return
style since there's no more @mappedbno, etc.

--D

Re: [PATCH 2/9] xfs: improve the xfs_dabuf_map calling conventions

[Christoph Hellwig]

On Tue, Nov 19, 2019 at 09:12:05AM -0800, Darrick J. Wong wrote:
> Also, I forgot to mention that some of the comments (particularly
> xfs_dabuf_map) need to be updated to reflect the new "no mapping" return
> style since there's no more @mappedbno, etc.

That is the one comment on xfs_dabuf_map, which in fact is already
wrong in the current tree.  When applying your changes I just removed
the comment entirely as it is a static function with just two callers
right below it.

[PATCH 3/9] xfs: remove the mappedbno argument to xfs_da_reada_buf

[Christoph Hellwig]

Replace the mappedbno argument with the simple flags for xfs_da_reada_buf
and xfs_dir3_data_readahead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_da_btree.c  | 10 ++--------
 fs/xfs/libxfs/xfs_da_btree.h  |  4 ++--
 fs/xfs/libxfs/xfs_dir2_data.c |  6 +++---
 fs/xfs/libxfs/xfs_dir2_priv.h |  4 ++--
 fs/xfs/scrub/parent.c         |  2 +-
 fs/xfs/xfs_dir2_readdir.c     |  3 ++-
 fs/xfs/xfs_file.c             |  2 +-
 7 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index c26f139bcf00..b1b0b38d7747 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2747,7 +2747,7 @@ int
 xfs_da_reada_buf(
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
+	unsigned int		flags,
 	int			whichfork,
 	const struct xfs_buf_ops *ops)
 {
@@ -2756,14 +2756,9 @@ xfs_da_reada_buf(
 	int			nmap;
 	int			error;
 
-	if (mappedbno >= 0)
-		return -EINVAL;
-
 	mapp = &map;
 	nmap = 1;
-	error = xfs_dabuf_map(dp, bno,
-			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
-			whichfork, &mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno, flags, whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
 		if (error == -ENOENT)
@@ -2771,7 +2766,6 @@ xfs_da_reada_buf(
 		goto out_free;
 	}
 
-	mappedbno = mapp[0].bm_bn;
 	xfs_buf_readahead_map(dp->i_mount->m_ddev_targp, mapp, nmap, ops);
 
 out_free:
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 9ec0d0243e96..4ba0ded7b973 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -218,8 +218,8 @@ int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
 			       struct xfs_buf **bpp, int whichfork,
 			       const struct xfs_buf_ops *ops);
 int	xfs_da_reada_buf(struct xfs_inode *dp, xfs_dablk_t bno,
-				xfs_daddr_t mapped_bno, int whichfork,
-				const struct xfs_buf_ops *ops);
+		unsigned int flags, int whichfork,
+		const struct xfs_buf_ops *ops);
 int	xfs_da_shrink_inode(xfs_da_args_t *args, xfs_dablk_t dead_blkno,
 					  struct xfs_buf *dead_buf);
 
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 9e471a28b6c6..10680f6422c2 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -417,10 +417,10 @@ int
 xfs_dir3_data_readahead(
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mapped_bno)
+	unsigned int		flags)
 {
-	return xfs_da_reada_buf(dp, bno, mapped_bno,
-				XFS_DATA_FORK, &xfs_dir3_data_reada_buf_ops);
+	return xfs_da_reada_buf(dp, bno, flags, XFS_DATA_FORK,
+				&xfs_dir3_data_reada_buf_ops);
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index a22222df4bf2..a730c5223c64 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -76,8 +76,8 @@ extern xfs_failaddr_t __xfs_dir3_data_check(struct xfs_inode *dp,
 		struct xfs_buf *bp);
 extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
 		xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
-extern int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
-		xfs_daddr_t mapped_bno);
+int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
+		unsigned int flags);
 
 extern struct xfs_dir2_data_free *
 xfs_dir2_data_freeinsert(struct xfs_dir2_data_hdr *hdr,
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index c962bd534690..17100a83e23e 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -80,7 +80,7 @@ xchk_parent_count_parent_dentries(
 	 */
 	lock_mode = xfs_ilock_data_map_shared(parent);
 	if (parent->i_d.di_nextents > 0)
-		error = xfs_dir3_data_readahead(parent, 0, -1);
+		error = xfs_dir3_data_readahead(parent, 0, 0);
 	xfs_iunlock(parent, lock_mode);
 	if (error)
 		return error;
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index b149cb4a4d86..f23f3b23ec37 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -315,7 +315,8 @@ xfs_dir2_leaf_readbuf(
 				break;
 			}
 			if (next_ra > *ra_blk) {
-				xfs_dir3_data_readahead(dp, next_ra, -2);
+				xfs_dir3_data_readahead(dp, next_ra,
+						        XFS_DABUF_MAP_HOLE_OK);
 				*ra_blk = next_ra;
 			}
 			ra_want -= geo->fsbcount;
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 865543e41fb4..c93250108952 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1104,7 +1104,7 @@ xfs_dir_open(
 	 */
 	mode = xfs_ilock_data_map_shared(ip);
 	if (ip->i_d.di_nextents > 0)
-		error = xfs_dir3_data_readahead(ip, 0, -1);
+		error = xfs_dir3_data_readahead(ip, 0, 0);
 	xfs_iunlock(ip, mode);
 	return error;
 }
-- 
2.20.1

Re: [PATCH 3/9] xfs: remove the mappedbno argument to xfs_da_reada_buf

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:08PM +0100, Christoph Hellwig wrote:
> Replace the mappedbno argument with the simple flags for xfs_da_reada_buf
> and xfs_dir3_data_readahead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_da_btree.c  | 10 ++--------
>  fs/xfs/libxfs/xfs_da_btree.h  |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_data.c |  6 +++---
>  fs/xfs/libxfs/xfs_dir2_priv.h |  4 ++--
>  fs/xfs/scrub/parent.c         |  2 +-
>  fs/xfs/xfs_dir2_readdir.c     |  3 ++-
>  fs/xfs/xfs_file.c             |  2 +-
>  7 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index c26f139bcf00..b1b0b38d7747 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2747,7 +2747,7 @@ int
>  xfs_da_reada_buf(
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
> +	unsigned int		flags,
>  	int			whichfork,
>  	const struct xfs_buf_ops *ops)
>  {
> @@ -2756,14 +2756,9 @@ xfs_da_reada_buf(
>  	int			nmap;
>  	int			error;
>  
> -	if (mappedbno >= 0)
> -		return -EINVAL;
> -
>  	mapp = &map;
>  	nmap = 1;
> -	error = xfs_dabuf_map(dp, bno,
> -			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> -			whichfork, &mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno, flags, whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
>  		if (error == -ENOENT)
> @@ -2771,7 +2766,6 @@ xfs_da_reada_buf(
>  		goto out_free;
>  	}
>  
> -	mappedbno = mapp[0].bm_bn;
>  	xfs_buf_readahead_map(dp->i_mount->m_ddev_targp, mapp, nmap, ops);
>  
>  out_free:
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 9ec0d0243e96..4ba0ded7b973 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -218,8 +218,8 @@ int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
>  			       struct xfs_buf **bpp, int whichfork,
>  			       const struct xfs_buf_ops *ops);
>  int	xfs_da_reada_buf(struct xfs_inode *dp, xfs_dablk_t bno,
> -				xfs_daddr_t mapped_bno, int whichfork,
> -				const struct xfs_buf_ops *ops);
> +		unsigned int flags, int whichfork,
> +		const struct xfs_buf_ops *ops);
>  int	xfs_da_shrink_inode(xfs_da_args_t *args, xfs_dablk_t dead_blkno,
>  					  struct xfs_buf *dead_buf);
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 9e471a28b6c6..10680f6422c2 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -417,10 +417,10 @@ int
>  xfs_dir3_data_readahead(
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mapped_bno)
> +	unsigned int		flags)
>  {
> -	return xfs_da_reada_buf(dp, bno, mapped_bno,
> -				XFS_DATA_FORK, &xfs_dir3_data_reada_buf_ops);
> +	return xfs_da_reada_buf(dp, bno, flags, XFS_DATA_FORK,
> +				&xfs_dir3_data_reada_buf_ops);
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index a22222df4bf2..a730c5223c64 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -76,8 +76,8 @@ extern xfs_failaddr_t __xfs_dir3_data_check(struct xfs_inode *dp,
>  		struct xfs_buf *bp);
>  extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
>  		xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
> -extern int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
> -		xfs_daddr_t mapped_bno);
> +int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
> +		unsigned int flags);
>  
>  extern struct xfs_dir2_data_free *
>  xfs_dir2_data_freeinsert(struct xfs_dir2_data_hdr *hdr,
> diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
> index c962bd534690..17100a83e23e 100644
> --- a/fs/xfs/scrub/parent.c
> +++ b/fs/xfs/scrub/parent.c
> @@ -80,7 +80,7 @@ xchk_parent_count_parent_dentries(
>  	 */
>  	lock_mode = xfs_ilock_data_map_shared(parent);
>  	if (parent->i_d.di_nextents > 0)
> -		error = xfs_dir3_data_readahead(parent, 0, -1);
> +		error = xfs_dir3_data_readahead(parent, 0, 0);
>  	xfs_iunlock(parent, lock_mode);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index b149cb4a4d86..f23f3b23ec37 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -315,7 +315,8 @@ xfs_dir2_leaf_readbuf(
>  				break;
>  			}
>  			if (next_ra > *ra_blk) {
> -				xfs_dir3_data_readahead(dp, next_ra, -2);
> +				xfs_dir3_data_readahead(dp, next_ra,
> +						        XFS_DABUF_MAP_HOLE_OK);
>  				*ra_blk = next_ra;
>  			}
>  			ra_want -= geo->fsbcount;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 865543e41fb4..c93250108952 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1104,7 +1104,7 @@ xfs_dir_open(
>  	 */
>  	mode = xfs_ilock_data_map_shared(ip);
>  	if (ip->i_d.di_nextents > 0)
> -		error = xfs_dir3_data_readahead(ip, 0, -1);
> +		error = xfs_dir3_data_readahead(ip, 0, 0);
>  	xfs_iunlock(ip, mode);
>  	return error;
>  }
> -- 
> 2.20.1
> 

[PATCH 4/9] xfs: remove the mappedbno argument to xfs_attr3_leaf_read

[Christoph Hellwig]

This argument is always hard coded to -1, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr.c      | 10 +++++-----
 fs/xfs/libxfs/xfs_attr_leaf.c | 17 ++++++++---------
 fs/xfs/libxfs/xfs_attr_leaf.h |  3 +--
 fs/xfs/xfs_attr_list.c        |  5 +++--
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 510ca6974604..ebe6b0575f40 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -589,7 +589,7 @@ xfs_attr_leaf_addname(
 	 */
 	dp = args->dp;
 	args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
 	if (error)
 		return error;
 
@@ -715,7 +715,7 @@ xfs_attr_leaf_addname(
 		 * remove the "old" attr from that block (neat, huh!)
 		 */
 		error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
-					   -1, &bp);
+					   &bp);
 		if (error)
 			return error;
 
@@ -769,7 +769,7 @@ xfs_attr_leaf_removename(
 	 */
 	dp = args->dp;
 	args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
 	if (error)
 		return error;
 
@@ -813,7 +813,7 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
 	trace_xfs_attr_leaf_get(args);
 
 	args->blkno = 0;
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
 	if (error)
 		return error;
 
@@ -1173,7 +1173,7 @@ xfs_attr_node_removename(
 		ASSERT(state->path.blk[0].bp);
 		state->path.blk[0].bp = NULL;
 
-		error = xfs_attr3_leaf_read(args->trans, args->dp, 0, -1, &bp);
+		error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
 		if (error)
 			goto out;
 
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 85ec5945d29f..9c0cdb51955e 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -430,13 +430,12 @@ xfs_attr3_leaf_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp)
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
-				XFS_ATTR_FORK, &xfs_attr3_leaf_buf_ops);
+	err = xfs_da_read_buf(tp, dp, bno, -1, bpp, XFS_ATTR_FORK,
+			&xfs_attr3_leaf_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_ATTR_LEAF_BUF);
 	return err;
@@ -1159,7 +1158,7 @@ xfs_attr3_leaf_to_node(
 	error = xfs_da_grow_inode(args, &blkno);
 	if (error)
 		goto out;
-	error = xfs_attr3_leaf_read(args->trans, dp, 0, -1, &bp1);
+	error = xfs_attr3_leaf_read(args->trans, dp, 0, &bp1);
 	if (error)
 		goto out;
 
@@ -1994,7 +1993,7 @@ xfs_attr3_leaf_toosmall(
 		if (blkno == 0)
 			continue;
 		error = xfs_attr3_leaf_read(state->args->trans, state->args->dp,
-					blkno, -1, &bp);
+					blkno, &bp);
 		if (error)
 			return error;
 
@@ -2730,7 +2729,7 @@ xfs_attr3_leaf_clearflag(
 	/*
 	 * Set up the operation.
 	 */
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
 	if (error)
 		return error;
 
@@ -2797,7 +2796,7 @@ xfs_attr3_leaf_setflag(
 	/*
 	 * Set up the operation.
 	 */
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
 	if (error)
 		return error;
 
@@ -2859,7 +2858,7 @@ xfs_attr3_leaf_flipflags(
 	/*
 	 * Read the block containing the "old" attr
 	 */
-	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp1);
+	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp1);
 	if (error)
 		return error;
 
@@ -2868,7 +2867,7 @@ xfs_attr3_leaf_flipflags(
 	 */
 	if (args->blkno2 != args->blkno) {
 		error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno2,
-					   -1, &bp2);
+					   &bp2);
 		if (error)
 			return error;
 	} else {
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 16208a7743df..f4a188e28b7b 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -108,8 +108,7 @@ int	xfs_attr_leaf_order(struct xfs_buf *leaf1_bp,
 				   struct xfs_buf *leaf2_bp);
 int	xfs_attr_leaf_newentsize(struct xfs_da_args *args, int *local);
 int	xfs_attr3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
-			xfs_dablk_t bno, xfs_daddr_t mappedbno,
-			struct xfs_buf **bpp);
+			xfs_dablk_t bno, struct xfs_buf **bpp);
 void	xfs_attr3_leaf_hdr_from_disk(struct xfs_da_geometry *geo,
 				     struct xfs_attr3_icleaf_hdr *to,
 				     struct xfs_attr_leafblock *from);
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 0ec6606149a2..426f93cfb2ea 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -380,7 +380,8 @@ xfs_attr_node_list(
 			break;
 		cursor->blkno = leafhdr.forw;
 		xfs_trans_brelse(context->tp, bp);
-		error = xfs_attr3_leaf_read(context->tp, dp, cursor->blkno, -1, &bp);
+		error = xfs_attr3_leaf_read(context->tp, dp, cursor->blkno,
+					    &bp);
 		if (error)
 			return error;
 	}
@@ -500,7 +501,7 @@ xfs_attr_leaf_list(xfs_attr_list_context_t *context)
 	trace_xfs_attr_leaf_list(context);
 
 	context->cursor->blkno = 0;
-	error = xfs_attr3_leaf_read(context->tp, context->dp, 0, -1, &bp);
+	error = xfs_attr3_leaf_read(context->tp, context->dp, 0, &bp);
 	if (error)
 		return error;
 
-- 
2.20.1

Re: [PATCH 4/9] xfs: remove the mappedbno argument to xfs_attr3_leaf_read

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:09PM +0100, Christoph Hellwig wrote:
> This argument is always hard coded to -1, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Heh, neat!
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c      | 10 +++++-----
>  fs/xfs/libxfs/xfs_attr_leaf.c | 17 ++++++++---------
>  fs/xfs/libxfs/xfs_attr_leaf.h |  3 +--
>  fs/xfs/xfs_attr_list.c        |  5 +++--
>  4 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 510ca6974604..ebe6b0575f40 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -589,7 +589,7 @@ xfs_attr_leaf_addname(
>  	 */
>  	dp = args->dp;
>  	args->blkno = 0;
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
>  	if (error)
>  		return error;
>  
> @@ -715,7 +715,7 @@ xfs_attr_leaf_addname(
>  		 * remove the "old" attr from that block (neat, huh!)
>  		 */
>  		error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno,
> -					   -1, &bp);
> +					   &bp);
>  		if (error)
>  			return error;
>  
> @@ -769,7 +769,7 @@ xfs_attr_leaf_removename(
>  	 */
>  	dp = args->dp;
>  	args->blkno = 0;
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
>  	if (error)
>  		return error;
>  
> @@ -813,7 +813,7 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
>  	trace_xfs_attr_leaf_get(args);
>  
>  	args->blkno = 0;
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
>  	if (error)
>  		return error;
>  
> @@ -1173,7 +1173,7 @@ xfs_attr_node_removename(
>  		ASSERT(state->path.blk[0].bp);
>  		state->path.blk[0].bp = NULL;
>  
> -		error = xfs_attr3_leaf_read(args->trans, args->dp, 0, -1, &bp);
> +		error = xfs_attr3_leaf_read(args->trans, args->dp, 0, &bp);
>  		if (error)
>  			goto out;
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 85ec5945d29f..9c0cdb51955e 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -430,13 +430,12 @@ xfs_attr3_leaf_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp)
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
> -				XFS_ATTR_FORK, &xfs_attr3_leaf_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, bno, -1, bpp, XFS_ATTR_FORK,
> +			&xfs_attr3_leaf_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_ATTR_LEAF_BUF);
>  	return err;
> @@ -1159,7 +1158,7 @@ xfs_attr3_leaf_to_node(
>  	error = xfs_da_grow_inode(args, &blkno);
>  	if (error)
>  		goto out;
> -	error = xfs_attr3_leaf_read(args->trans, dp, 0, -1, &bp1);
> +	error = xfs_attr3_leaf_read(args->trans, dp, 0, &bp1);
>  	if (error)
>  		goto out;
>  
> @@ -1994,7 +1993,7 @@ xfs_attr3_leaf_toosmall(
>  		if (blkno == 0)
>  			continue;
>  		error = xfs_attr3_leaf_read(state->args->trans, state->args->dp,
> -					blkno, -1, &bp);
> +					blkno, &bp);
>  		if (error)
>  			return error;
>  
> @@ -2730,7 +2729,7 @@ xfs_attr3_leaf_clearflag(
>  	/*
>  	 * Set up the operation.
>  	 */
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
>  	if (error)
>  		return error;
>  
> @@ -2797,7 +2796,7 @@ xfs_attr3_leaf_setflag(
>  	/*
>  	 * Set up the operation.
>  	 */
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp);
>  	if (error)
>  		return error;
>  
> @@ -2859,7 +2858,7 @@ xfs_attr3_leaf_flipflags(
>  	/*
>  	 * Read the block containing the "old" attr
>  	 */
> -	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, -1, &bp1);
> +	error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, &bp1);
>  	if (error)
>  		return error;
>  
> @@ -2868,7 +2867,7 @@ xfs_attr3_leaf_flipflags(
>  	 */
>  	if (args->blkno2 != args->blkno) {
>  		error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno2,
> -					   -1, &bp2);
> +					   &bp2);
>  		if (error)
>  			return error;
>  	} else {
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index 16208a7743df..f4a188e28b7b 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -108,8 +108,7 @@ int	xfs_attr_leaf_order(struct xfs_buf *leaf1_bp,
>  				   struct xfs_buf *leaf2_bp);
>  int	xfs_attr_leaf_newentsize(struct xfs_da_args *args, int *local);
>  int	xfs_attr3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -			xfs_dablk_t bno, xfs_daddr_t mappedbno,
> -			struct xfs_buf **bpp);
> +			xfs_dablk_t bno, struct xfs_buf **bpp);
>  void	xfs_attr3_leaf_hdr_from_disk(struct xfs_da_geometry *geo,
>  				     struct xfs_attr3_icleaf_hdr *to,
>  				     struct xfs_attr_leafblock *from);
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 0ec6606149a2..426f93cfb2ea 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -380,7 +380,8 @@ xfs_attr_node_list(
>  			break;
>  		cursor->blkno = leafhdr.forw;
>  		xfs_trans_brelse(context->tp, bp);
> -		error = xfs_attr3_leaf_read(context->tp, dp, cursor->blkno, -1, &bp);
> +		error = xfs_attr3_leaf_read(context->tp, dp, cursor->blkno,
> +					    &bp);
>  		if (error)
>  			return error;
>  	}
> @@ -500,7 +501,7 @@ xfs_attr_leaf_list(xfs_attr_list_context_t *context)
>  	trace_xfs_attr_leaf_list(context);
>  
>  	context->cursor->blkno = 0;
> -	error = xfs_attr3_leaf_read(context->tp, context->dp, 0, -1, &bp);
> +	error = xfs_attr3_leaf_read(context->tp, context->dp, 0, &bp);
>  	if (error)
>  		return error;
>  
> -- 
> 2.20.1
> 

[PATCH 5/9] xfs: remove the mappedbno argument to xfs_dir3_leaf_read

[Christoph Hellwig]

This argument is always hard coded to -1, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_dir2_leaf.c | 9 ++++-----
 fs/xfs/libxfs/xfs_dir2_priv.h | 4 ++--
 fs/xfs/scrub/dir.c            | 2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index e2e4b2c6d6c2..2eee4e299e19 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -262,13 +262,12 @@ xfs_dir3_leaf_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		fbno,
-	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp)
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
-				XFS_DATA_FORK, &xfs_dir3_leaf1_buf_ops);
+	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
+			&xfs_dir3_leaf1_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAF1_BUF);
 	return err;
@@ -639,7 +638,7 @@ xfs_dir2_leaf_addname(
 
 	trace_xfs_dir2_leaf_addname(args);
 
-	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
+	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
 	if (error)
 		return error;
 
@@ -1230,7 +1229,7 @@ xfs_dir2_leaf_lookup_int(
 	tp = args->trans;
 	mp = dp->i_mount;
 
-	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
+	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index a730c5223c64..ade41556901a 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -91,8 +91,8 @@ void xfs_dir2_leaf_hdr_from_disk(struct xfs_mount *mp,
 		struct xfs_dir3_icleaf_hdr *to, struct xfs_dir2_leaf *from);
 void xfs_dir2_leaf_hdr_to_disk(struct xfs_mount *mp, struct xfs_dir2_leaf *to,
 		struct xfs_dir3_icleaf_hdr *from);
-extern int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
-		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
+int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
+		xfs_dablk_t fbno, struct xfs_buf **bpp);
 extern int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
 		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
 extern int xfs_dir2_block_to_leaf(struct xfs_da_args *args,
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 7983ea40668a..910e0bf85bd7 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -497,7 +497,7 @@ xchk_directory_leaf1_bestfree(
 	int				error;
 
 	/* Read the free space block. */
-	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, -1, &bp);
+	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, &bp);
 	if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
 		goto out;
 	xchk_buffer_recheck(sc, bp);
-- 
2.20.1

Re: [PATCH 5/9] xfs: remove the mappedbno argument to xfs_dir3_leaf_read

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:10PM +0100, Christoph Hellwig wrote:
> This argument is always hard coded to -1, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_dir2_leaf.c | 9 ++++-----
>  fs/xfs/libxfs/xfs_dir2_priv.h | 4 ++--
>  fs/xfs/scrub/dir.c            | 2 +-
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index e2e4b2c6d6c2..2eee4e299e19 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -262,13 +262,12 @@ xfs_dir3_leaf_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		fbno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp)
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
> -				XFS_DATA_FORK, &xfs_dir3_leaf1_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
> +			&xfs_dir3_leaf1_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAF1_BUF);
>  	return err;
> @@ -639,7 +638,7 @@ xfs_dir2_leaf_addname(
>  
>  	trace_xfs_dir2_leaf_addname(args);
>  
> -	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
> +	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
>  	if (error)
>  		return error;
>  
> @@ -1230,7 +1229,7 @@ xfs_dir2_leaf_lookup_int(
>  	tp = args->trans;
>  	mp = dp->i_mount;
>  
> -	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
> +	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index a730c5223c64..ade41556901a 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -91,8 +91,8 @@ void xfs_dir2_leaf_hdr_from_disk(struct xfs_mount *mp,
>  		struct xfs_dir3_icleaf_hdr *to, struct xfs_dir2_leaf *from);
>  void xfs_dir2_leaf_hdr_to_disk(struct xfs_mount *mp, struct xfs_dir2_leaf *to,
>  		struct xfs_dir3_icleaf_hdr *from);
> -extern int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
> +int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
> +		xfs_dablk_t fbno, struct xfs_buf **bpp);
>  extern int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
>  		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
>  extern int xfs_dir2_block_to_leaf(struct xfs_da_args *args,
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index 7983ea40668a..910e0bf85bd7 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -497,7 +497,7 @@ xchk_directory_leaf1_bestfree(
>  	int				error;
>  
>  	/* Read the free space block. */
> -	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, -1, &bp);
> +	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, &bp);
>  	if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
>  		goto out;
>  	xchk_buffer_recheck(sc, bp);
> -- 
> 2.20.1
> 

Re: [PATCH 5/9] xfs: remove the mappedbno argument to xfs_dir3_leaf_read

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:10PM +0100, Christoph Hellwig wrote:
> This argument is always hard coded to -1, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_dir2_leaf.c | 9 ++++-----
>  fs/xfs/libxfs/xfs_dir2_priv.h | 4 ++--
>  fs/xfs/scrub/dir.c            | 2 +-
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index e2e4b2c6d6c2..2eee4e299e19 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -262,13 +262,12 @@ xfs_dir3_leaf_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		fbno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp)
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
> -				XFS_DATA_FORK, &xfs_dir3_leaf1_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
> +			&xfs_dir3_leaf1_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAF1_BUF);
>  	return err;
> @@ -639,7 +638,7 @@ xfs_dir2_leaf_addname(
>  
>  	trace_xfs_dir2_leaf_addname(args);
>  
> -	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
> +	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
>  	if (error)
>  		return error;
>  
> @@ -1230,7 +1229,7 @@ xfs_dir2_leaf_lookup_int(
>  	tp = args->trans;
>  	mp = dp->i_mount;
>  
> -	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, &lbp);
> +	error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, &lbp);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index a730c5223c64..ade41556901a 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -91,8 +91,8 @@ void xfs_dir2_leaf_hdr_from_disk(struct xfs_mount *mp,
>  		struct xfs_dir3_icleaf_hdr *to, struct xfs_dir2_leaf *from);
>  void xfs_dir2_leaf_hdr_to_disk(struct xfs_mount *mp, struct xfs_dir2_leaf *to,
>  		struct xfs_dir3_icleaf_hdr *from);
> -extern int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
> +int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
> +		xfs_dablk_t fbno, struct xfs_buf **bpp);
>  extern int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
>  		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
>  extern int xfs_dir2_block_to_leaf(struct xfs_da_args *args,
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index 7983ea40668a..910e0bf85bd7 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -497,7 +497,7 @@ xchk_directory_leaf1_bestfree(
>  	int				error;
>  
>  	/* Read the free space block. */
> -	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, -1, &bp);
> +	error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, &bp);
>  	if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
>  		goto out;
>  	xchk_buffer_recheck(sc, bp);
> -- 
> 2.20.1
> 

[PATCH 6/9] xfs: remove the mappedbno argument to xfs_dir3_leafn_read

[Christoph Hellwig]

This argument is always hard coded to -1, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_dir2_leaf.c | 5 ++---
 fs/xfs/libxfs/xfs_dir2_node.c | 3 +--
 fs/xfs/libxfs/xfs_dir2_priv.h | 4 ++--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 2eee4e299e19..a1fe45db61c3 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -278,13 +278,12 @@ xfs_dir3_leafn_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		fbno,
-	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp)
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
-				XFS_DATA_FORK, &xfs_dir3_leafn_buf_ops);
+	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
+			&xfs_dir3_leafn_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAFN_BUF);
 	return err;
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 5f30a1953a52..a5450229a7ef 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -1554,8 +1554,7 @@ xfs_dir2_leafn_toosmall(
 		/*
 		 * Read the sibling leaf block.
 		 */
-		error = xfs_dir3_leafn_read(state->args->trans, dp,
-					    blkno, -1, &bp);
+		error = xfs_dir3_leafn_read(state->args->trans, dp, blkno, &bp);
 		if (error)
 			return error;
 
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index ade41556901a..3001cf82baa6 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -93,8 +93,8 @@ void xfs_dir2_leaf_hdr_to_disk(struct xfs_mount *mp, struct xfs_dir2_leaf *to,
 		struct xfs_dir3_icleaf_hdr *from);
 int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
 		xfs_dablk_t fbno, struct xfs_buf **bpp);
-extern int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
-		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
+int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
+		xfs_dablk_t fbno, struct xfs_buf **bpp);
 extern int xfs_dir2_block_to_leaf(struct xfs_da_args *args,
 		struct xfs_buf *dbp);
 extern int xfs_dir2_leaf_addname(struct xfs_da_args *args);
-- 
2.20.1

Re: [PATCH 6/9] xfs: remove the mappedbno argument to xfs_dir3_leafn_read

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:11PM +0100, Christoph Hellwig wrote:
> This argument is always hard coded to -1, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_dir2_leaf.c | 5 ++---
>  fs/xfs/libxfs/xfs_dir2_node.c | 3 +--
>  fs/xfs/libxfs/xfs_dir2_priv.h | 4 ++--
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 2eee4e299e19..a1fe45db61c3 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -278,13 +278,12 @@ xfs_dir3_leafn_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		fbno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp)
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
> -				XFS_DATA_FORK, &xfs_dir3_leafn_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
> +			&xfs_dir3_leafn_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAFN_BUF);
>  	return err;
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 5f30a1953a52..a5450229a7ef 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -1554,8 +1554,7 @@ xfs_dir2_leafn_toosmall(
>  		/*
>  		 * Read the sibling leaf block.
>  		 */
> -		error = xfs_dir3_leafn_read(state->args->trans, dp,
> -					    blkno, -1, &bp);
> +		error = xfs_dir3_leafn_read(state->args->trans, dp, blkno, &bp);
>  		if (error)
>  			return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index ade41556901a..3001cf82baa6 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -93,8 +93,8 @@ void xfs_dir2_leaf_hdr_to_disk(struct xfs_mount *mp, struct xfs_dir2_leaf *to,
>  		struct xfs_dir3_icleaf_hdr *from);
>  int xfs_dir3_leaf_read(struct xfs_trans *tp, struct xfs_inode *dp,
>  		xfs_dablk_t fbno, struct xfs_buf **bpp);
> -extern int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -		xfs_dablk_t fbno, xfs_daddr_t mappedbno, struct xfs_buf **bpp);
> +int xfs_dir3_leafn_read(struct xfs_trans *tp, struct xfs_inode *dp,
> +		xfs_dablk_t fbno, struct xfs_buf **bpp);
>  extern int xfs_dir2_block_to_leaf(struct xfs_da_args *args,
>  		struct xfs_buf *dbp);
>  extern int xfs_dir2_leaf_addname(struct xfs_da_args *args);
> -- 
> 2.20.1
> 

[PATCH 7/9] xfs: split xfs_da3_node_read

[Christoph Hellwig]

Split xfs_da3_node_read into one variant that always looks up the daddr
and doesn't accept holes, and one that already has a daddr at hand.
This is in preparation of splitting up xfs_da_read_buf in a similar way.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr.c     |  14 ++---
 fs/xfs/libxfs/xfs_da_btree.c | 111 ++++++++++++++++++++---------------
 fs/xfs/libxfs/xfs_da_btree.h |   6 +-
 fs/xfs/xfs_attr_inactive.c   |   8 +--
 fs/xfs/xfs_attr_list.c       |   6 +-
 5 files changed, 82 insertions(+), 63 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index ebe6b0575f40..0d7fcc983b3d 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1266,10 +1266,9 @@ xfs_attr_refillstate(xfs_da_state_t *state)
 	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
 	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
 		if (blk->disk_blkno) {
-			error = xfs_da3_node_read(state->args->trans,
-						state->args->dp,
-						blk->blkno, blk->disk_blkno,
-						&blk->bp, XFS_ATTR_FORK);
+			error = xfs_da3_node_read_mapped(state->args->trans,
+					state->args->dp, blk->disk_blkno,
+					&blk->bp, XFS_ATTR_FORK);
 			if (error)
 				return error;
 		} else {
@@ -1285,10 +1284,9 @@ xfs_attr_refillstate(xfs_da_state_t *state)
 	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
 	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
 		if (blk->disk_blkno) {
-			error = xfs_da3_node_read(state->args->trans,
-						state->args->dp,
-						blk->blkno, blk->disk_blkno,
-						&blk->bp, XFS_ATTR_FORK);
+			error = xfs_da3_node_read_mapped(state->args->trans,
+					state->args->dp, blk->disk_blkno,
+					&blk->bp, XFS_ATTR_FORK);
 			if (error)
 				return error;
 		} else {
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index b1b0b38d7747..489936e01c33 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -331,46 +331,66 @@ const struct xfs_buf_ops xfs_da3_node_buf_ops = {
 	.verify_struct = xfs_da3_node_verify_struct,
 };
 
+static int
+xfs_da3_node_set_type(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
+{
+	struct xfs_da_blkinfo	*info = bp->b_addr;
+
+	switch (be16_to_cpu(info->magic)) {
+	case XFS_DA_NODE_MAGIC:
+	case XFS_DA3_NODE_MAGIC:
+		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DA_NODE_BUF);
+		return 0;
+	case XFS_ATTR_LEAF_MAGIC:
+	case XFS_ATTR3_LEAF_MAGIC:
+		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_ATTR_LEAF_BUF);
+		return 0;
+	case XFS_DIR2_LEAFN_MAGIC:
+	case XFS_DIR3_LEAFN_MAGIC:
+		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DIR_LEAFN_BUF);
+		return 0;
+	default:
+		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, tp->t_mountp,
+				info, sizeof(*info));
+		xfs_trans_brelse(tp, bp);
+		return -EFSCORRUPTED;
+	}
+}
+
 int
 xfs_da3_node_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp,
-	int			which_fork)
+	int			whichfork)
 {
-	int			err;
+	int			error;
 
-	err = xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
-					which_fork, &xfs_da3_node_buf_ops);
-	if (!err && tp && *bpp) {
-		struct xfs_da_blkinfo	*info = (*bpp)->b_addr;
-		int			type;
+	error = xfs_da_read_buf(tp, dp, bno, -1, bpp, whichfork,
+			&xfs_da3_node_buf_ops);
+	if (error || !*bpp || !tp)
+		return error;
+	return xfs_da3_node_set_type(tp, *bpp);
+}
 
-		switch (be16_to_cpu(info->magic)) {
-		case XFS_DA_NODE_MAGIC:
-		case XFS_DA3_NODE_MAGIC:
-			type = XFS_BLFT_DA_NODE_BUF;
-			break;
-		case XFS_ATTR_LEAF_MAGIC:
-		case XFS_ATTR3_LEAF_MAGIC:
-			type = XFS_BLFT_ATTR_LEAF_BUF;
-			break;
-		case XFS_DIR2_LEAFN_MAGIC:
-		case XFS_DIR3_LEAFN_MAGIC:
-			type = XFS_BLFT_DIR_LEAFN_BUF;
-			break;
-		default:
-			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-					tp->t_mountp, info, sizeof(*info));
-			xfs_trans_brelse(tp, *bpp);
-			*bpp = NULL;
-			return -EFSCORRUPTED;
-		}
-		xfs_trans_buf_set_type(tp, *bpp, type);
-	}
-	return err;
+int
+xfs_da3_node_read_mapped(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*dp,
+	xfs_daddr_t		mappedbno,
+	struct xfs_buf		**bpp,
+	int			whichfork)
+{
+	int			error;
+
+	error = xfs_da_read_buf(tp, dp, 0, mappedbno, bpp, whichfork,
+			&xfs_da3_node_buf_ops);
+	if (error || !*bpp || !tp)
+		return error;
+	return xfs_da3_node_set_type(tp, *bpp);
 }
 
 /*========================================================================
@@ -1166,8 +1186,7 @@ xfs_da3_root_join(
 	 */
 	child = be32_to_cpu(oldroothdr.btree[0].before);
 	ASSERT(child != 0);
-	error = xfs_da3_node_read(args->trans, dp, child, -1, &bp,
-					     args->whichfork);
+	error = xfs_da3_node_read(args->trans, dp, child, &bp, args->whichfork);
 	if (error)
 		return error;
 	xfs_da_blkinfo_onlychild_validate(bp->b_addr, oldroothdr.level);
@@ -1281,8 +1300,8 @@ xfs_da3_node_toosmall(
 			blkno = nodehdr.back;
 		if (blkno == 0)
 			continue;
-		error = xfs_da3_node_read(state->args->trans, dp,
-					blkno, -1, &bp, state->args->whichfork);
+		error = xfs_da3_node_read(state->args->trans, dp, blkno, &bp,
+				state->args->whichfork);
 		if (error)
 			return error;
 
@@ -1570,7 +1589,7 @@ xfs_da3_node_lookup_int(
 		 */
 		blk->blkno = blkno;
 		error = xfs_da3_node_read(args->trans, args->dp, blkno,
-					-1, &blk->bp, args->whichfork);
+					&blk->bp, args->whichfork);
 		if (error) {
 			blk->blkno = 0;
 			state->path.active--;
@@ -1809,7 +1828,7 @@ xfs_da3_blk_link(
 		if (old_info->back) {
 			error = xfs_da3_node_read(args->trans, dp,
 						be32_to_cpu(old_info->back),
-						-1, &bp, args->whichfork);
+						&bp, args->whichfork);
 			if (error)
 				return error;
 			ASSERT(bp != NULL);
@@ -1830,7 +1849,7 @@ xfs_da3_blk_link(
 		if (old_info->forw) {
 			error = xfs_da3_node_read(args->trans, dp,
 						be32_to_cpu(old_info->forw),
-						-1, &bp, args->whichfork);
+						&bp, args->whichfork);
 			if (error)
 				return error;
 			ASSERT(bp != NULL);
@@ -1889,7 +1908,7 @@ xfs_da3_blk_unlink(
 		if (drop_info->back) {
 			error = xfs_da3_node_read(args->trans, args->dp,
 						be32_to_cpu(drop_info->back),
-						-1, &bp, args->whichfork);
+						&bp, args->whichfork);
 			if (error)
 				return error;
 			ASSERT(bp != NULL);
@@ -1906,7 +1925,7 @@ xfs_da3_blk_unlink(
 		if (drop_info->forw) {
 			error = xfs_da3_node_read(args->trans, args->dp,
 						be32_to_cpu(drop_info->forw),
-						-1, &bp, args->whichfork);
+						&bp, args->whichfork);
 			if (error)
 				return error;
 			ASSERT(bp != NULL);
@@ -1990,7 +2009,7 @@ xfs_da3_path_shift(
 		/*
 		 * Read the next child block into a local buffer.
 		 */
-		error = xfs_da3_node_read(args->trans, dp, blkno, -1, &bp,
+		error = xfs_da3_node_read(args->trans, dp, blkno, &bp,
 					  args->whichfork);
 		if (error)
 			return error;
@@ -2283,7 +2302,7 @@ xfs_da3_swap_lastblock(
 	 * Read the last block in the btree space.
 	 */
 	last_blkno = (xfs_dablk_t)lastoff - args->geo->fsbcount;
-	error = xfs_da3_node_read(tp, dp, last_blkno, -1, &last_buf, w);
+	error = xfs_da3_node_read(tp, dp, last_blkno, &last_buf, w);
 	if (error)
 		return error;
 	/*
@@ -2320,7 +2339,7 @@ xfs_da3_swap_lastblock(
 	 * If the moved block has a left sibling, fix up the pointers.
 	 */
 	if ((sib_blkno = be32_to_cpu(dead_info->back))) {
-		error = xfs_da3_node_read(tp, dp, sib_blkno, -1, &sib_buf, w);
+		error = xfs_da3_node_read(tp, dp, sib_blkno, &sib_buf, w);
 		if (error)
 			goto done;
 		sib_info = sib_buf->b_addr;
@@ -2342,7 +2361,7 @@ xfs_da3_swap_lastblock(
 	 * If the moved block has a right sibling, fix up the pointers.
 	 */
 	if ((sib_blkno = be32_to_cpu(dead_info->forw))) {
-		error = xfs_da3_node_read(tp, dp, sib_blkno, -1, &sib_buf, w);
+		error = xfs_da3_node_read(tp, dp, sib_blkno, &sib_buf, w);
 		if (error)
 			goto done;
 		sib_info = sib_buf->b_addr;
@@ -2366,7 +2385,7 @@ xfs_da3_swap_lastblock(
 	 * Walk down the tree looking for the parent of the moved block.
 	 */
 	for (;;) {
-		error = xfs_da3_node_read(tp, dp, par_blkno, -1, &par_buf, w);
+		error = xfs_da3_node_read(tp, dp, par_blkno, &par_buf, w);
 		if (error)
 			goto done;
 		par_node = par_buf->b_addr;
@@ -2417,7 +2436,7 @@ xfs_da3_swap_lastblock(
 			error = -EFSCORRUPTED;
 			goto done;
 		}
-		error = xfs_da3_node_read(tp, dp, par_blkno, -1, &par_buf, w);
+		error = xfs_da3_node_read(tp, dp, par_blkno, &par_buf, w);
 		if (error)
 			goto done;
 		par_node = par_buf->b_addr;
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 4ba0ded7b973..74eeb97852d8 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -198,8 +198,10 @@ int	xfs_da3_path_shift(xfs_da_state_t *state, xfs_da_state_path_t *path,
 int	xfs_da3_blk_link(xfs_da_state_t *state, xfs_da_state_blk_t *old_blk,
 				       xfs_da_state_blk_t *new_blk);
 int	xfs_da3_node_read(struct xfs_trans *tp, struct xfs_inode *dp,
-			 xfs_dablk_t bno, xfs_daddr_t mappedbno,
-			 struct xfs_buf **bpp, int which_fork);
+			xfs_dablk_t bno, struct xfs_buf **bpp, int whichfork);
+int	xfs_da3_node_read_mapped(struct xfs_trans *tp, struct xfs_inode *dp,
+			xfs_daddr_t mappedbno, struct xfs_buf **bpp,
+			int whichfork);
 
 /*
  * Utility routines.
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index a78c501f6fb1..f1cafd82ec75 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -233,7 +233,7 @@ xfs_attr3_node_inactive(
 		 * traversal of the tree so we may deal with many blocks
 		 * before we come back to this one.
 		 */
-		error = xfs_da3_node_read(*trans, dp, child_fsb, -1, &child_bp,
+		error = xfs_da3_node_read(*trans, dp, child_fsb, &child_bp,
 					  XFS_ATTR_FORK);
 		if (error)
 			return error;
@@ -280,8 +280,8 @@ xfs_attr3_node_inactive(
 		if (i + 1 < ichdr.count) {
 			struct xfs_da3_icnode_hdr phdr;
 
-			error = xfs_da3_node_read(*trans, dp, 0, parent_blkno,
-						 &bp, XFS_ATTR_FORK);
+			error = xfs_da3_node_read_mapped(*trans, dp,
+					parent_blkno, &bp, XFS_ATTR_FORK);
 			if (error)
 				return error;
 			xfs_da3_node_hdr_from_disk(dp->i_mount, &phdr,
@@ -322,7 +322,7 @@ xfs_attr3_root_inactive(
 	 * the extents in reverse order the extent containing
 	 * block 0 must still be there.
 	 */
-	error = xfs_da3_node_read(*trans, dp, 0, -1, &bp, XFS_ATTR_FORK);
+	error = xfs_da3_node_read(*trans, dp, 0, &bp, XFS_ATTR_FORK);
 	if (error)
 		return error;
 	blkno = bp->b_bn;
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 426f93cfb2ea..8afbb30be002 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -224,7 +224,7 @@ xfs_attr_node_list_lookup(
 	ASSERT(*pbp == NULL);
 	cursor->blkno = 0;
 	for (;;) {
-		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
+		error = xfs_da3_node_read(tp, dp, cursor->blkno, &bp,
 				XFS_ATTR_FORK);
 		if (error)
 			return error;
@@ -312,8 +312,8 @@ xfs_attr_node_list(
 	 */
 	bp = NULL;
 	if (cursor->blkno > 0) {
-		error = xfs_da3_node_read(context->tp, dp, cursor->blkno, -1,
-					      &bp, XFS_ATTR_FORK);
+		error = xfs_da3_node_read(context->tp, dp, cursor->blkno, &bp,
+				XFS_ATTR_FORK);
 		if ((error != 0) && (error != -EFSCORRUPTED))
 			return error;
 		if (bp) {
-- 
2.20.1

Re: [PATCH 7/9] xfs: split xfs_da3_node_read

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:12PM +0100, Christoph Hellwig wrote:
> Split xfs_da3_node_read into one variant that always looks up the daddr
> and doesn't accept holes, and one that already has a daddr at hand.
> This is in preparation of splitting up xfs_da_read_buf in a similar way.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c     |  14 ++---
>  fs/xfs/libxfs/xfs_da_btree.c | 111 ++++++++++++++++++++---------------
>  fs/xfs/libxfs/xfs_da_btree.h |   6 +-
>  fs/xfs/xfs_attr_inactive.c   |   8 +--
>  fs/xfs/xfs_attr_list.c       |   6 +-
>  5 files changed, 82 insertions(+), 63 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index ebe6b0575f40..0d7fcc983b3d 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1266,10 +1266,9 @@ xfs_attr_refillstate(xfs_da_state_t *state)
>  	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
>  	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
>  		if (blk->disk_blkno) {
> -			error = xfs_da3_node_read(state->args->trans,
> -						state->args->dp,
> -						blk->blkno, blk->disk_blkno,
> -						&blk->bp, XFS_ATTR_FORK);
> +			error = xfs_da3_node_read_mapped(state->args->trans,
> +					state->args->dp, blk->disk_blkno,
> +					&blk->bp, XFS_ATTR_FORK);
>  			if (error)
>  				return error;
>  		} else {
> @@ -1285,10 +1284,9 @@ xfs_attr_refillstate(xfs_da_state_t *state)
>  	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
>  	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
>  		if (blk->disk_blkno) {
> -			error = xfs_da3_node_read(state->args->trans,
> -						state->args->dp,
> -						blk->blkno, blk->disk_blkno,
> -						&blk->bp, XFS_ATTR_FORK);
> +			error = xfs_da3_node_read_mapped(state->args->trans,
> +					state->args->dp, blk->disk_blkno,
> +					&blk->bp, XFS_ATTR_FORK);
>  			if (error)
>  				return error;
>  		} else {
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index b1b0b38d7747..489936e01c33 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -331,46 +331,66 @@ const struct xfs_buf_ops xfs_da3_node_buf_ops = {
>  	.verify_struct = xfs_da3_node_verify_struct,
>  };
>  
> +static int
> +xfs_da3_node_set_type(
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp)
> +{
> +	struct xfs_da_blkinfo	*info = bp->b_addr;
> +
> +	switch (be16_to_cpu(info->magic)) {
> +	case XFS_DA_NODE_MAGIC:
> +	case XFS_DA3_NODE_MAGIC:
> +		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DA_NODE_BUF);
> +		return 0;
> +	case XFS_ATTR_LEAF_MAGIC:
> +	case XFS_ATTR3_LEAF_MAGIC:
> +		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_ATTR_LEAF_BUF);
> +		return 0;
> +	case XFS_DIR2_LEAFN_MAGIC:
> +	case XFS_DIR3_LEAFN_MAGIC:
> +		xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DIR_LEAFN_BUF);
> +		return 0;
> +	default:
> +		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, tp->t_mountp,
> +				info, sizeof(*info));
> +		xfs_trans_brelse(tp, bp);
> +		return -EFSCORRUPTED;
> +	}
> +}
> +
>  int
>  xfs_da3_node_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp,
> -	int			which_fork)
> +	int			whichfork)
>  {
> -	int			err;
> +	int			error;
>  
> -	err = xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
> -					which_fork, &xfs_da3_node_buf_ops);
> -	if (!err && tp && *bpp) {
> -		struct xfs_da_blkinfo	*info = (*bpp)->b_addr;
> -		int			type;
> +	error = xfs_da_read_buf(tp, dp, bno, -1, bpp, whichfork,
> +			&xfs_da3_node_buf_ops);
> +	if (error || !*bpp || !tp)
> +		return error;
> +	return xfs_da3_node_set_type(tp, *bpp);
> +}
>  
> -		switch (be16_to_cpu(info->magic)) {
> -		case XFS_DA_NODE_MAGIC:
> -		case XFS_DA3_NODE_MAGIC:
> -			type = XFS_BLFT_DA_NODE_BUF;
> -			break;
> -		case XFS_ATTR_LEAF_MAGIC:
> -		case XFS_ATTR3_LEAF_MAGIC:
> -			type = XFS_BLFT_ATTR_LEAF_BUF;
> -			break;
> -		case XFS_DIR2_LEAFN_MAGIC:
> -		case XFS_DIR3_LEAFN_MAGIC:
> -			type = XFS_BLFT_DIR_LEAFN_BUF;
> -			break;
> -		default:
> -			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> -					tp->t_mountp, info, sizeof(*info));
> -			xfs_trans_brelse(tp, *bpp);
> -			*bpp = NULL;
> -			return -EFSCORRUPTED;
> -		}
> -		xfs_trans_buf_set_type(tp, *bpp, type);
> -	}
> -	return err;
> +int
> +xfs_da3_node_read_mapped(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*dp,
> +	xfs_daddr_t		mappedbno,
> +	struct xfs_buf		**bpp,
> +	int			whichfork)
> +{
> +	int			error;
> +
> +	error = xfs_da_read_buf(tp, dp, 0, mappedbno, bpp, whichfork,
> +			&xfs_da3_node_buf_ops);
> +	if (error || !*bpp || !tp)
> +		return error;
> +	return xfs_da3_node_set_type(tp, *bpp);
>  }
>  
>  /*========================================================================
> @@ -1166,8 +1186,7 @@ xfs_da3_root_join(
>  	 */
>  	child = be32_to_cpu(oldroothdr.btree[0].before);
>  	ASSERT(child != 0);
> -	error = xfs_da3_node_read(args->trans, dp, child, -1, &bp,
> -					     args->whichfork);
> +	error = xfs_da3_node_read(args->trans, dp, child, &bp, args->whichfork);
>  	if (error)
>  		return error;
>  	xfs_da_blkinfo_onlychild_validate(bp->b_addr, oldroothdr.level);
> @@ -1281,8 +1300,8 @@ xfs_da3_node_toosmall(
>  			blkno = nodehdr.back;
>  		if (blkno == 0)
>  			continue;
> -		error = xfs_da3_node_read(state->args->trans, dp,
> -					blkno, -1, &bp, state->args->whichfork);
> +		error = xfs_da3_node_read(state->args->trans, dp, blkno, &bp,
> +				state->args->whichfork);
>  		if (error)
>  			return error;
>  
> @@ -1570,7 +1589,7 @@ xfs_da3_node_lookup_int(
>  		 */
>  		blk->blkno = blkno;
>  		error = xfs_da3_node_read(args->trans, args->dp, blkno,
> -					-1, &blk->bp, args->whichfork);
> +					&blk->bp, args->whichfork);
>  		if (error) {
>  			blk->blkno = 0;
>  			state->path.active--;
> @@ -1809,7 +1828,7 @@ xfs_da3_blk_link(
>  		if (old_info->back) {
>  			error = xfs_da3_node_read(args->trans, dp,
>  						be32_to_cpu(old_info->back),
> -						-1, &bp, args->whichfork);
> +						&bp, args->whichfork);
>  			if (error)
>  				return error;
>  			ASSERT(bp != NULL);
> @@ -1830,7 +1849,7 @@ xfs_da3_blk_link(
>  		if (old_info->forw) {
>  			error = xfs_da3_node_read(args->trans, dp,
>  						be32_to_cpu(old_info->forw),
> -						-1, &bp, args->whichfork);
> +						&bp, args->whichfork);
>  			if (error)
>  				return error;
>  			ASSERT(bp != NULL);
> @@ -1889,7 +1908,7 @@ xfs_da3_blk_unlink(
>  		if (drop_info->back) {
>  			error = xfs_da3_node_read(args->trans, args->dp,
>  						be32_to_cpu(drop_info->back),
> -						-1, &bp, args->whichfork);
> +						&bp, args->whichfork);
>  			if (error)
>  				return error;
>  			ASSERT(bp != NULL);
> @@ -1906,7 +1925,7 @@ xfs_da3_blk_unlink(
>  		if (drop_info->forw) {
>  			error = xfs_da3_node_read(args->trans, args->dp,
>  						be32_to_cpu(drop_info->forw),
> -						-1, &bp, args->whichfork);
> +						&bp, args->whichfork);
>  			if (error)
>  				return error;
>  			ASSERT(bp != NULL);
> @@ -1990,7 +2009,7 @@ xfs_da3_path_shift(
>  		/*
>  		 * Read the next child block into a local buffer.
>  		 */
> -		error = xfs_da3_node_read(args->trans, dp, blkno, -1, &bp,
> +		error = xfs_da3_node_read(args->trans, dp, blkno, &bp,
>  					  args->whichfork);
>  		if (error)
>  			return error;
> @@ -2283,7 +2302,7 @@ xfs_da3_swap_lastblock(
>  	 * Read the last block in the btree space.
>  	 */
>  	last_blkno = (xfs_dablk_t)lastoff - args->geo->fsbcount;
> -	error = xfs_da3_node_read(tp, dp, last_blkno, -1, &last_buf, w);
> +	error = xfs_da3_node_read(tp, dp, last_blkno, &last_buf, w);
>  	if (error)
>  		return error;
>  	/*
> @@ -2320,7 +2339,7 @@ xfs_da3_swap_lastblock(
>  	 * If the moved block has a left sibling, fix up the pointers.
>  	 */
>  	if ((sib_blkno = be32_to_cpu(dead_info->back))) {
> -		error = xfs_da3_node_read(tp, dp, sib_blkno, -1, &sib_buf, w);
> +		error = xfs_da3_node_read(tp, dp, sib_blkno, &sib_buf, w);
>  		if (error)
>  			goto done;
>  		sib_info = sib_buf->b_addr;
> @@ -2342,7 +2361,7 @@ xfs_da3_swap_lastblock(
>  	 * If the moved block has a right sibling, fix up the pointers.
>  	 */
>  	if ((sib_blkno = be32_to_cpu(dead_info->forw))) {
> -		error = xfs_da3_node_read(tp, dp, sib_blkno, -1, &sib_buf, w);
> +		error = xfs_da3_node_read(tp, dp, sib_blkno, &sib_buf, w);
>  		if (error)
>  			goto done;
>  		sib_info = sib_buf->b_addr;
> @@ -2366,7 +2385,7 @@ xfs_da3_swap_lastblock(
>  	 * Walk down the tree looking for the parent of the moved block.
>  	 */
>  	for (;;) {
> -		error = xfs_da3_node_read(tp, dp, par_blkno, -1, &par_buf, w);
> +		error = xfs_da3_node_read(tp, dp, par_blkno, &par_buf, w);
>  		if (error)
>  			goto done;
>  		par_node = par_buf->b_addr;
> @@ -2417,7 +2436,7 @@ xfs_da3_swap_lastblock(
>  			error = -EFSCORRUPTED;
>  			goto done;
>  		}
> -		error = xfs_da3_node_read(tp, dp, par_blkno, -1, &par_buf, w);
> +		error = xfs_da3_node_read(tp, dp, par_blkno, &par_buf, w);
>  		if (error)
>  			goto done;
>  		par_node = par_buf->b_addr;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 4ba0ded7b973..74eeb97852d8 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -198,8 +198,10 @@ int	xfs_da3_path_shift(xfs_da_state_t *state, xfs_da_state_path_t *path,
>  int	xfs_da3_blk_link(xfs_da_state_t *state, xfs_da_state_blk_t *old_blk,
>  				       xfs_da_state_blk_t *new_blk);
>  int	xfs_da3_node_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -			 xfs_dablk_t bno, xfs_daddr_t mappedbno,
> -			 struct xfs_buf **bpp, int which_fork);
> +			xfs_dablk_t bno, struct xfs_buf **bpp, int whichfork);
> +int	xfs_da3_node_read_mapped(struct xfs_trans *tp, struct xfs_inode *dp,
> +			xfs_daddr_t mappedbno, struct xfs_buf **bpp,
> +			int whichfork);
>  
>  /*
>   * Utility routines.
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index a78c501f6fb1..f1cafd82ec75 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -233,7 +233,7 @@ xfs_attr3_node_inactive(
>  		 * traversal of the tree so we may deal with many blocks
>  		 * before we come back to this one.
>  		 */
> -		error = xfs_da3_node_read(*trans, dp, child_fsb, -1, &child_bp,
> +		error = xfs_da3_node_read(*trans, dp, child_fsb, &child_bp,
>  					  XFS_ATTR_FORK);
>  		if (error)
>  			return error;
> @@ -280,8 +280,8 @@ xfs_attr3_node_inactive(
>  		if (i + 1 < ichdr.count) {
>  			struct xfs_da3_icnode_hdr phdr;
>  
> -			error = xfs_da3_node_read(*trans, dp, 0, parent_blkno,
> -						 &bp, XFS_ATTR_FORK);
> +			error = xfs_da3_node_read_mapped(*trans, dp,
> +					parent_blkno, &bp, XFS_ATTR_FORK);
>  			if (error)
>  				return error;
>  			xfs_da3_node_hdr_from_disk(dp->i_mount, &phdr,
> @@ -322,7 +322,7 @@ xfs_attr3_root_inactive(
>  	 * the extents in reverse order the extent containing
>  	 * block 0 must still be there.
>  	 */
> -	error = xfs_da3_node_read(*trans, dp, 0, -1, &bp, XFS_ATTR_FORK);
> +	error = xfs_da3_node_read(*trans, dp, 0, &bp, XFS_ATTR_FORK);
>  	if (error)
>  		return error;
>  	blkno = bp->b_bn;
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 426f93cfb2ea..8afbb30be002 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -224,7 +224,7 @@ xfs_attr_node_list_lookup(
>  	ASSERT(*pbp == NULL);
>  	cursor->blkno = 0;
>  	for (;;) {
> -		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
> +		error = xfs_da3_node_read(tp, dp, cursor->blkno, &bp,
>  				XFS_ATTR_FORK);
>  		if (error)
>  			return error;
> @@ -312,8 +312,8 @@ xfs_attr_node_list(
>  	 */
>  	bp = NULL;
>  	if (cursor->blkno > 0) {
> -		error = xfs_da3_node_read(context->tp, dp, cursor->blkno, -1,
> -					      &bp, XFS_ATTR_FORK);
> +		error = xfs_da3_node_read(context->tp, dp, cursor->blkno, &bp,
> +				XFS_ATTR_FORK);
>  		if ((error != 0) && (error != -EFSCORRUPTED))
>  			return error;
>  		if (bp) {
> -- 
> 2.20.1
> 

[PATCH 8/9] xfs: remove the mappedbno argument to xfs_da_read_buf

[Christoph Hellwig]

Move the code for reading an already mapped block into
xfs_da3_node_read_mapped, which is the only caller ever passing a block
number in the mappedbno argument and replace the mappedbno argument with
the simple xfs_dabuf_get flags.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr_leaf.c  |  2 +-
 fs/xfs/libxfs/xfs_da_btree.c   | 34 ++++++++++++++++------------------
 fs/xfs/libxfs/xfs_da_btree.h   |  5 ++---
 fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
 fs/xfs/libxfs/xfs_dir2_data.c  |  6 +++---
 fs/xfs/libxfs/xfs_dir2_leaf.c  | 13 ++++++-------
 fs/xfs/libxfs/xfs_dir2_node.c  | 14 +++++++-------
 fs/xfs/libxfs/xfs_dir2_priv.h  |  4 ++--
 fs/xfs/scrub/dabtree.c         |  4 ++--
 fs/xfs/scrub/dir.c             |  9 +++++----
 fs/xfs/xfs_dir2_readdir.c      |  2 +-
 11 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 9c0cdb51955e..450e75cc7c93 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -434,7 +434,7 @@ xfs_attr3_leaf_read(
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, bno, -1, bpp, XFS_ATTR_FORK,
+	err = xfs_da_read_buf(tp, dp, bno, 0, bpp, XFS_ATTR_FORK,
 			&xfs_attr3_leaf_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_ATTR_LEAF_BUF);
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 489936e01c33..34d0ce93bcc3 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -369,7 +369,7 @@ xfs_da3_node_read(
 {
 	int			error;
 
-	error = xfs_da_read_buf(tp, dp, bno, -1, bpp, whichfork,
+	error = xfs_da_read_buf(tp, dp, bno, 0, bpp, whichfork,
 			&xfs_da3_node_buf_ops);
 	if (error || !*bpp || !tp)
 		return error;
@@ -384,12 +384,22 @@ xfs_da3_node_read_mapped(
 	struct xfs_buf		**bpp,
 	int			whichfork)
 {
+	struct xfs_mount	*mp = dp->i_mount;
 	int			error;
 
-	error = xfs_da_read_buf(tp, dp, 0, mappedbno, bpp, whichfork,
-			&xfs_da3_node_buf_ops);
-	if (error || !*bpp || !tp)
+	error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp, mappedbno,
+			XFS_FSB_TO_BB(mp, xfs_dabuf_nfsb(mp, whichfork)), 0,
+			bpp, &xfs_da3_node_buf_ops);
+	if (error || !*bpp)
 		return error;
+
+	if (whichfork == XFS_ATTR_FORK)
+		xfs_buf_set_ref(*bpp, XFS_ATTR_BTREE_REF);
+	else
+		xfs_buf_set_ref(*bpp, XFS_DIR_BTREE_REF);
+
+	if (!tp)
+		return 0;
 	return xfs_da3_node_set_type(tp, *bpp);
 }
 
@@ -2710,7 +2720,7 @@ xfs_da_read_buf(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
+	unsigned int		flags,
 	struct xfs_buf		**bpp,
 	int			whichfork,
 	const struct xfs_buf_ops *ops)
@@ -2722,18 +2732,7 @@ xfs_da_read_buf(
 	int			error;
 
 	*bpp = NULL;
-
-	if (mappedbno >= 0) {
-		error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
-				mappedbno, XFS_FSB_TO_BB(mp,
-					xfs_dabuf_nfsb(mp, whichfork)),
-				0, &bp, ops);
-		goto done;
-	}
-
-	error = xfs_dabuf_map(dp, bno,
-			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
-			whichfork, &mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno, flags, whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
 		if (error == -ENOENT)
@@ -2743,7 +2742,6 @@ xfs_da_read_buf(
 
 	error = xfs_trans_read_buf_map(mp, tp, mp->m_ddev_targp, mapp, nmap, 0,
 			&bp, ops);
-done:
 	if (error)
 		goto out_free;
 
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 74eeb97852d8..1c8347af8071 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -216,9 +216,8 @@ int	xfs_da_get_buf(struct xfs_trans *trans, struct xfs_inode *dp,
 			      xfs_dablk_t bno, xfs_daddr_t mappedbno,
 			      struct xfs_buf **bp, int whichfork);
 int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
-			       xfs_dablk_t bno, xfs_daddr_t mappedbno,
-			       struct xfs_buf **bpp, int whichfork,
-			       const struct xfs_buf_ops *ops);
+		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp,
+		int whichfork, const struct xfs_buf_ops *ops);
 int	xfs_da_reada_buf(struct xfs_inode *dp, xfs_dablk_t bno,
 		unsigned int flags, int whichfork,
 		const struct xfs_buf_ops *ops);
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 358151ddfa75..e287b3b87006 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -123,7 +123,7 @@ xfs_dir3_block_read(
 	struct xfs_mount	*mp = dp->i_mount;
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, mp->m_dir_geo->datablk, -1, bpp,
+	err = xfs_da_read_buf(tp, dp, mp->m_dir_geo->datablk, 0, bpp,
 				XFS_DATA_FORK, &xfs_dir3_block_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_BLOCK_BUF);
@@ -952,7 +952,7 @@ xfs_dir2_leaf_to_block(
 	 * Read the data block if we don't already have it, give up if it fails.
 	 */
 	if (!dbp) {
-		error = xfs_dir3_data_read(tp, dp, args->geo->datablk, -1, &dbp);
+		error = xfs_dir3_data_read(tp, dp, args->geo->datablk, 0, &dbp);
 		if (error)
 			return error;
 	}
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 10680f6422c2..9ac08df96b3f 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -401,13 +401,13 @@ xfs_dir3_data_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mapped_bno,
+	unsigned int		flags,
 	struct xfs_buf		**bpp)
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, bno, mapped_bno, bpp,
-				XFS_DATA_FORK, &xfs_dir3_data_buf_ops);
+	err = xfs_da_read_buf(tp, dp, bno, flags, bpp, XFS_DATA_FORK,
+			&xfs_dir3_data_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_DATA_BUF);
 	return err;
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index a1fe45db61c3..0107a661acd8 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -266,7 +266,7 @@ xfs_dir3_leaf_read(
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
+	err = xfs_da_read_buf(tp, dp, fbno, 0, bpp, XFS_DATA_FORK,
 			&xfs_dir3_leaf1_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAF1_BUF);
@@ -282,7 +282,7 @@ xfs_dir3_leafn_read(
 {
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
+	err = xfs_da_read_buf(tp, dp, fbno, 0, bpp, XFS_DATA_FORK,
 			&xfs_dir3_leafn_buf_ops);
 	if (!err && tp && *bpp)
 		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAFN_BUF);
@@ -826,7 +826,7 @@ xfs_dir2_leaf_addname(
 		 */
 		error = xfs_dir3_data_read(tp, dp,
 				   xfs_dir2_db_to_da(args->geo, use_block),
-				   -1, &dbp);
+				   0, &dbp);
 		if (error) {
 			xfs_trans_brelse(tp, lbp);
 			return error;
@@ -1268,7 +1268,7 @@ xfs_dir2_leaf_lookup_int(
 				xfs_trans_brelse(tp, dbp);
 			error = xfs_dir3_data_read(tp, dp,
 					   xfs_dir2_db_to_da(args->geo, newdb),
-					   -1, &dbp);
+					   0, &dbp);
 			if (error) {
 				xfs_trans_brelse(tp, lbp);
 				return error;
@@ -1310,7 +1310,7 @@ xfs_dir2_leaf_lookup_int(
 			xfs_trans_brelse(tp, dbp);
 			error = xfs_dir3_data_read(tp, dp,
 					   xfs_dir2_db_to_da(args->geo, cidb),
-					   -1, &dbp);
+					   0, &dbp);
 			if (error) {
 				xfs_trans_brelse(tp, lbp);
 				return error;
@@ -1602,8 +1602,7 @@ xfs_dir2_leaf_trim_data(
 	/*
 	 * Read the offending data block.  We need its buffer.
 	 */
-	error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(geo, db), -1,
-				   &dbp);
+	error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(geo, db), 0, &dbp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index a5450229a7ef..cc1a20b69215 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -212,14 +212,14 @@ __xfs_dir3_free_read(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		fbno,
-	xfs_daddr_t		mappedbno,
+	unsigned int		flags,
 	struct xfs_buf		**bpp)
 {
 	xfs_failaddr_t		fa;
 	int			err;
 
-	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
-				XFS_DATA_FORK, &xfs_dir3_free_buf_ops);
+	err = xfs_da_read_buf(tp, dp, fbno, flags, bpp, XFS_DATA_FORK,
+			&xfs_dir3_free_buf_ops);
 	if (err || !*bpp)
 		return err;
 
@@ -297,7 +297,7 @@ xfs_dir2_free_read(
 	xfs_dablk_t		fbno,
 	struct xfs_buf		**bpp)
 {
-	return __xfs_dir3_free_read(tp, dp, fbno, -1, bpp);
+	return __xfs_dir3_free_read(tp, dp, fbno, 0, bpp);
 }
 
 static int
@@ -307,7 +307,7 @@ xfs_dir2_free_try_read(
 	xfs_dablk_t		fbno,
 	struct xfs_buf		**bpp)
 {
-	return __xfs_dir3_free_read(tp, dp, fbno, -2, bpp);
+	return __xfs_dir3_free_read(tp, dp, fbno, XFS_DABUF_MAP_HOLE_OK, bpp);
 }
 
 static int
@@ -858,7 +858,7 @@ xfs_dir2_leafn_lookup_for_entry(
 				error = xfs_dir3_data_read(tp, dp,
 						xfs_dir2_db_to_da(args->geo,
 								  newdb),
-						-1, &curbp);
+						0, &curbp);
 				if (error)
 					return error;
 			}
@@ -1940,7 +1940,7 @@ xfs_dir2_node_addname_int(
 		/* Read the data block in. */
 		error = xfs_dir3_data_read(tp, dp,
 					   xfs_dir2_db_to_da(args->geo, dbno),
-					   -1, &dbp);
+					   0, &dbp);
 	}
 	if (error)
 		return error;
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index 3001cf82baa6..13b80d0d264b 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -74,8 +74,8 @@ extern void xfs_dir3_data_check(struct xfs_inode *dp, struct xfs_buf *bp);
 
 extern xfs_failaddr_t __xfs_dir3_data_check(struct xfs_inode *dp,
 		struct xfs_buf *bp);
-extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
-		xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
+int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
+		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp);
 int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
 		unsigned int flags);
 
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index 85b9207359ec..97a15b6f2865 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -331,8 +331,8 @@ xchk_da_btree_block(
 		goto out_nobuf;
 
 	/* Read the buffer. */
-	error = xfs_da_read_buf(dargs->trans, dargs->dp, blk->blkno, -2,
-			&blk->bp, dargs->whichfork,
+	error = xfs_da_read_buf(dargs->trans, dargs->dp, blk->blkno,
+			XFS_DABUF_MAP_HOLE_OK, &blk->bp, dargs->whichfork,
 			&xchk_da_btree_buf_ops);
 	if (!xchk_da_process_error(ds, level, &error))
 		goto out_nobuf;
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 910e0bf85bd7..266da4e4bde6 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -229,7 +229,8 @@ xchk_dir_rec(
 		xchk_da_set_corrupt(ds, level);
 		goto out;
 	}
-	error = xfs_dir3_data_read(ds->dargs.trans, dp, rec_bno, -2, &bp);
+	error = xfs_dir3_data_read(ds->dargs.trans, dp, rec_bno,
+			XFS_DABUF_MAP_HOLE_OK, &bp);
 	if (!xchk_fblock_process_error(ds->sc, XFS_DATA_FORK, rec_bno,
 			&error))
 		goto out;
@@ -346,7 +347,7 @@ xchk_directory_data_bestfree(
 		error = xfs_dir3_block_read(sc->tp, sc->ip, &bp);
 	} else {
 		/* dir data format */
-		error = xfs_dir3_data_read(sc->tp, sc->ip, lblk, -1, &bp);
+		error = xfs_dir3_data_read(sc->tp, sc->ip, lblk, 0, &bp);
 	}
 	if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
 		goto out;
@@ -557,7 +558,7 @@ xchk_directory_leaf1_bestfree(
 		if (best == NULLDATAOFF)
 			continue;
 		error = xfs_dir3_data_read(sc->tp, sc->ip,
-				i * args->geo->fsbcount, -1, &dbp);
+				i * args->geo->fsbcount, 0, &dbp);
 		if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk,
 				&error))
 			break;
@@ -608,7 +609,7 @@ xchk_directory_free_bestfree(
 		}
 		error = xfs_dir3_data_read(sc->tp, sc->ip,
 				(freehdr.firstdb + i) * args->geo->fsbcount,
-				-1, &dbp);
+				0, &dbp);
 		if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk,
 				&error))
 			break;
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index f23f3b23ec37..a01d4bb45cee 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -280,7 +280,7 @@ xfs_dir2_leaf_readbuf(
 	new_off = xfs_dir2_da_to_byte(geo, map.br_startoff);
 	if (new_off > *cur_off)
 		*cur_off = new_off;
-	error = xfs_dir3_data_read(args->trans, dp, map.br_startoff, -1, &bp);
+	error = xfs_dir3_data_read(args->trans, dp, map.br_startoff, 0, &bp);
 	if (error)
 		goto out;
 
-- 
2.20.1

Re: [PATCH 8/9] xfs: remove the mappedbno argument to xfs_da_read_buf

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:13PM +0100, Christoph Hellwig wrote:
> Move the code for reading an already mapped block into
> xfs_da3_node_read_mapped, which is the only caller ever passing a block
> number in the mappedbno argument and replace the mappedbno argument with
> the simple xfs_dabuf_get flags.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c  |  2 +-
>  fs/xfs/libxfs/xfs_da_btree.c   | 34 ++++++++++++++++------------------
>  fs/xfs/libxfs/xfs_da_btree.h   |  5 ++---
>  fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_data.c  |  6 +++---
>  fs/xfs/libxfs/xfs_dir2_leaf.c  | 13 ++++++-------
>  fs/xfs/libxfs/xfs_dir2_node.c  | 14 +++++++-------
>  fs/xfs/libxfs/xfs_dir2_priv.h  |  4 ++--
>  fs/xfs/scrub/dabtree.c         |  4 ++--
>  fs/xfs/scrub/dir.c             |  9 +++++----
>  fs/xfs/xfs_dir2_readdir.c      |  2 +-
>  11 files changed, 47 insertions(+), 50 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 9c0cdb51955e..450e75cc7c93 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -434,7 +434,7 @@ xfs_attr3_leaf_read(
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, bno, -1, bpp, XFS_ATTR_FORK,
> +	err = xfs_da_read_buf(tp, dp, bno, 0, bpp, XFS_ATTR_FORK,
>  			&xfs_attr3_leaf_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_ATTR_LEAF_BUF);
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 489936e01c33..34d0ce93bcc3 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -369,7 +369,7 @@ xfs_da3_node_read(
>  {
>  	int			error;
>  
> -	error = xfs_da_read_buf(tp, dp, bno, -1, bpp, whichfork,
> +	error = xfs_da_read_buf(tp, dp, bno, 0, bpp, whichfork,
>  			&xfs_da3_node_buf_ops);
>  	if (error || !*bpp || !tp)
>  		return error;
> @@ -384,12 +384,22 @@ xfs_da3_node_read_mapped(
>  	struct xfs_buf		**bpp,
>  	int			whichfork)
>  {
> +	struct xfs_mount	*mp = dp->i_mount;
>  	int			error;
>  
> -	error = xfs_da_read_buf(tp, dp, 0, mappedbno, bpp, whichfork,
> -			&xfs_da3_node_buf_ops);
> -	if (error || !*bpp || !tp)
> +	error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp, mappedbno,
> +			XFS_FSB_TO_BB(mp, xfs_dabuf_nfsb(mp, whichfork)), 0,
> +			bpp, &xfs_da3_node_buf_ops);
> +	if (error || !*bpp)
>  		return error;
> +
> +	if (whichfork == XFS_ATTR_FORK)
> +		xfs_buf_set_ref(*bpp, XFS_ATTR_BTREE_REF);
> +	else
> +		xfs_buf_set_ref(*bpp, XFS_DIR_BTREE_REF);
> +
> +	if (!tp)
> +		return 0;
>  	return xfs_da3_node_set_type(tp, *bpp);
>  }
>  
> @@ -2710,7 +2720,7 @@ xfs_da_read_buf(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
> +	unsigned int		flags,
>  	struct xfs_buf		**bpp,
>  	int			whichfork,
>  	const struct xfs_buf_ops *ops)
> @@ -2722,18 +2732,7 @@ xfs_da_read_buf(
>  	int			error;
>  
>  	*bpp = NULL;
> -
> -	if (mappedbno >= 0) {
> -		error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
> -				mappedbno, XFS_FSB_TO_BB(mp,
> -					xfs_dabuf_nfsb(mp, whichfork)),
> -				0, &bp, ops);
> -		goto done;
> -	}
> -
> -	error = xfs_dabuf_map(dp, bno,
> -			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> -			whichfork, &mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno, flags, whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
>  		if (error == -ENOENT)
> @@ -2743,7 +2742,6 @@ xfs_da_read_buf(
>  
>  	error = xfs_trans_read_buf_map(mp, tp, mp->m_ddev_targp, mapp, nmap, 0,
>  			&bp, ops);
> -done:
>  	if (error)
>  		goto out_free;
>  
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 74eeb97852d8..1c8347af8071 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -216,9 +216,8 @@ int	xfs_da_get_buf(struct xfs_trans *trans, struct xfs_inode *dp,
>  			      xfs_dablk_t bno, xfs_daddr_t mappedbno,
>  			      struct xfs_buf **bp, int whichfork);
>  int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
> -			       xfs_dablk_t bno, xfs_daddr_t mappedbno,
> -			       struct xfs_buf **bpp, int whichfork,
> -			       const struct xfs_buf_ops *ops);
> +		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp,
> +		int whichfork, const struct xfs_buf_ops *ops);
>  int	xfs_da_reada_buf(struct xfs_inode *dp, xfs_dablk_t bno,
>  		unsigned int flags, int whichfork,
>  		const struct xfs_buf_ops *ops);
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 358151ddfa75..e287b3b87006 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -123,7 +123,7 @@ xfs_dir3_block_read(
>  	struct xfs_mount	*mp = dp->i_mount;
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, mp->m_dir_geo->datablk, -1, bpp,
> +	err = xfs_da_read_buf(tp, dp, mp->m_dir_geo->datablk, 0, bpp,
>  				XFS_DATA_FORK, &xfs_dir3_block_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_BLOCK_BUF);
> @@ -952,7 +952,7 @@ xfs_dir2_leaf_to_block(
>  	 * Read the data block if we don't already have it, give up if it fails.
>  	 */
>  	if (!dbp) {
> -		error = xfs_dir3_data_read(tp, dp, args->geo->datablk, -1, &dbp);
> +		error = xfs_dir3_data_read(tp, dp, args->geo->datablk, 0, &dbp);
>  		if (error)
>  			return error;
>  	}
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 10680f6422c2..9ac08df96b3f 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -401,13 +401,13 @@ xfs_dir3_data_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mapped_bno,
> +	unsigned int		flags,
>  	struct xfs_buf		**bpp)
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, bno, mapped_bno, bpp,
> -				XFS_DATA_FORK, &xfs_dir3_data_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, bno, flags, bpp, XFS_DATA_FORK,
> +			&xfs_dir3_data_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_DATA_BUF);
>  	return err;
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index a1fe45db61c3..0107a661acd8 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -266,7 +266,7 @@ xfs_dir3_leaf_read(
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
> +	err = xfs_da_read_buf(tp, dp, fbno, 0, bpp, XFS_DATA_FORK,
>  			&xfs_dir3_leaf1_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAF1_BUF);
> @@ -282,7 +282,7 @@ xfs_dir3_leafn_read(
>  {
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, -1, bpp, XFS_DATA_FORK,
> +	err = xfs_da_read_buf(tp, dp, fbno, 0, bpp, XFS_DATA_FORK,
>  			&xfs_dir3_leafn_buf_ops);
>  	if (!err && tp && *bpp)
>  		xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_LEAFN_BUF);
> @@ -826,7 +826,7 @@ xfs_dir2_leaf_addname(
>  		 */
>  		error = xfs_dir3_data_read(tp, dp,
>  				   xfs_dir2_db_to_da(args->geo, use_block),
> -				   -1, &dbp);
> +				   0, &dbp);
>  		if (error) {
>  			xfs_trans_brelse(tp, lbp);
>  			return error;
> @@ -1268,7 +1268,7 @@ xfs_dir2_leaf_lookup_int(
>  				xfs_trans_brelse(tp, dbp);
>  			error = xfs_dir3_data_read(tp, dp,
>  					   xfs_dir2_db_to_da(args->geo, newdb),
> -					   -1, &dbp);
> +					   0, &dbp);
>  			if (error) {
>  				xfs_trans_brelse(tp, lbp);
>  				return error;
> @@ -1310,7 +1310,7 @@ xfs_dir2_leaf_lookup_int(
>  			xfs_trans_brelse(tp, dbp);
>  			error = xfs_dir3_data_read(tp, dp,
>  					   xfs_dir2_db_to_da(args->geo, cidb),
> -					   -1, &dbp);
> +					   0, &dbp);
>  			if (error) {
>  				xfs_trans_brelse(tp, lbp);
>  				return error;
> @@ -1602,8 +1602,7 @@ xfs_dir2_leaf_trim_data(
>  	/*
>  	 * Read the offending data block.  We need its buffer.
>  	 */
> -	error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(geo, db), -1,
> -				   &dbp);
> +	error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(geo, db), 0, &dbp);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index a5450229a7ef..cc1a20b69215 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -212,14 +212,14 @@ __xfs_dir3_free_read(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		fbno,
> -	xfs_daddr_t		mappedbno,
> +	unsigned int		flags,
>  	struct xfs_buf		**bpp)
>  {
>  	xfs_failaddr_t		fa;
>  	int			err;
>  
> -	err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
> -				XFS_DATA_FORK, &xfs_dir3_free_buf_ops);
> +	err = xfs_da_read_buf(tp, dp, fbno, flags, bpp, XFS_DATA_FORK,
> +			&xfs_dir3_free_buf_ops);
>  	if (err || !*bpp)
>  		return err;
>  
> @@ -297,7 +297,7 @@ xfs_dir2_free_read(
>  	xfs_dablk_t		fbno,
>  	struct xfs_buf		**bpp)
>  {
> -	return __xfs_dir3_free_read(tp, dp, fbno, -1, bpp);
> +	return __xfs_dir3_free_read(tp, dp, fbno, 0, bpp);
>  }
>  
>  static int
> @@ -307,7 +307,7 @@ xfs_dir2_free_try_read(
>  	xfs_dablk_t		fbno,
>  	struct xfs_buf		**bpp)
>  {
> -	return __xfs_dir3_free_read(tp, dp, fbno, -2, bpp);
> +	return __xfs_dir3_free_read(tp, dp, fbno, XFS_DABUF_MAP_HOLE_OK, bpp);
>  }
>  
>  static int
> @@ -858,7 +858,7 @@ xfs_dir2_leafn_lookup_for_entry(
>  				error = xfs_dir3_data_read(tp, dp,
>  						xfs_dir2_db_to_da(args->geo,
>  								  newdb),
> -						-1, &curbp);
> +						0, &curbp);
>  				if (error)
>  					return error;
>  			}
> @@ -1940,7 +1940,7 @@ xfs_dir2_node_addname_int(
>  		/* Read the data block in. */
>  		error = xfs_dir3_data_read(tp, dp,
>  					   xfs_dir2_db_to_da(args->geo, dbno),
> -					   -1, &dbp);
> +					   0, &dbp);
>  	}
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index 3001cf82baa6..13b80d0d264b 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -74,8 +74,8 @@ extern void xfs_dir3_data_check(struct xfs_inode *dp, struct xfs_buf *bp);
>  
>  extern xfs_failaddr_t __xfs_dir3_data_check(struct xfs_inode *dp,
>  		struct xfs_buf *bp);
> -extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
> -		xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
> +int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
> +		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp);
>  int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
>  		unsigned int flags);
>  
> diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
> index 85b9207359ec..97a15b6f2865 100644
> --- a/fs/xfs/scrub/dabtree.c
> +++ b/fs/xfs/scrub/dabtree.c
> @@ -331,8 +331,8 @@ xchk_da_btree_block(
>  		goto out_nobuf;
>  
>  	/* Read the buffer. */
> -	error = xfs_da_read_buf(dargs->trans, dargs->dp, blk->blkno, -2,
> -			&blk->bp, dargs->whichfork,
> +	error = xfs_da_read_buf(dargs->trans, dargs->dp, blk->blkno,
> +			XFS_DABUF_MAP_HOLE_OK, &blk->bp, dargs->whichfork,
>  			&xchk_da_btree_buf_ops);
>  	if (!xchk_da_process_error(ds, level, &error))
>  		goto out_nobuf;
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index 910e0bf85bd7..266da4e4bde6 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -229,7 +229,8 @@ xchk_dir_rec(
>  		xchk_da_set_corrupt(ds, level);
>  		goto out;
>  	}
> -	error = xfs_dir3_data_read(ds->dargs.trans, dp, rec_bno, -2, &bp);
> +	error = xfs_dir3_data_read(ds->dargs.trans, dp, rec_bno,
> +			XFS_DABUF_MAP_HOLE_OK, &bp);
>  	if (!xchk_fblock_process_error(ds->sc, XFS_DATA_FORK, rec_bno,
>  			&error))
>  		goto out;
> @@ -346,7 +347,7 @@ xchk_directory_data_bestfree(
>  		error = xfs_dir3_block_read(sc->tp, sc->ip, &bp);
>  	} else {
>  		/* dir data format */
> -		error = xfs_dir3_data_read(sc->tp, sc->ip, lblk, -1, &bp);
> +		error = xfs_dir3_data_read(sc->tp, sc->ip, lblk, 0, &bp);
>  	}
>  	if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
>  		goto out;
> @@ -557,7 +558,7 @@ xchk_directory_leaf1_bestfree(
>  		if (best == NULLDATAOFF)
>  			continue;
>  		error = xfs_dir3_data_read(sc->tp, sc->ip,
> -				i * args->geo->fsbcount, -1, &dbp);
> +				i * args->geo->fsbcount, 0, &dbp);
>  		if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk,
>  				&error))
>  			break;
> @@ -608,7 +609,7 @@ xchk_directory_free_bestfree(
>  		}
>  		error = xfs_dir3_data_read(sc->tp, sc->ip,
>  				(freehdr.firstdb + i) * args->geo->fsbcount,
> -				-1, &dbp);
> +				0, &dbp);
>  		if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, lblk,
>  				&error))
>  			break;
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index f23f3b23ec37..a01d4bb45cee 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -280,7 +280,7 @@ xfs_dir2_leaf_readbuf(
>  	new_off = xfs_dir2_da_to_byte(geo, map.br_startoff);
>  	if (new_off > *cur_off)
>  		*cur_off = new_off;
> -	error = xfs_dir3_data_read(args->trans, dp, map.br_startoff, -1, &bp);
> +	error = xfs_dir3_data_read(args->trans, dp, map.br_startoff, 0, &bp);
>  	if (error)
>  		goto out;
>  
> -- 
> 2.20.1
> 

[PATCH 9/9] xfs: remove the mappedbno argument to xfs_da_get_buf

[Christoph Hellwig]

Use the xfs_da_get_buf_daddr function directly for the two callers
that pass a mapped disk address, and then remove the mappedbno argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr_leaf.c |  4 ++--
 fs/xfs/libxfs/xfs_da_btree.c  | 18 +++---------------
 fs/xfs/libxfs/xfs_da_btree.h  |  3 +--
 fs/xfs/libxfs/xfs_dir2_data.c |  2 +-
 fs/xfs/libxfs/xfs_dir2_leaf.c |  2 +-
 fs/xfs/libxfs/xfs_dir2_node.c |  2 +-
 fs/xfs/xfs_attr_inactive.c    | 24 +++++++++++++++++++-----
 7 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 450e75cc7c93..32bf3c30238c 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1162,7 +1162,7 @@ xfs_attr3_leaf_to_node(
 	if (error)
 		goto out;
 
-	error = xfs_da_get_buf(args->trans, dp, blkno, -1, &bp2, XFS_ATTR_FORK);
+	error = xfs_da_get_buf(args->trans, dp, blkno, &bp2, XFS_ATTR_FORK);
 	if (error)
 		goto out;
 
@@ -1223,7 +1223,7 @@ xfs_attr3_leaf_create(
 
 	trace_xfs_attr_leaf_create(args);
 
-	error = xfs_da_get_buf(args->trans, args->dp, blkno, -1, &bp,
+	error = xfs_da_get_buf(args->trans, args->dp, blkno, &bp,
 					    XFS_ATTR_FORK);
 	if (error)
 		return error;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 34d0ce93bcc3..dbb2b2f38a7f 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -429,7 +429,7 @@ xfs_da3_node_create(
 	trace_xfs_da_node_create(args);
 	ASSERT(level <= XFS_DA_NODE_MAXDEPTH);
 
-	error = xfs_da_get_buf(tp, dp, blkno, -1, &bp, whichfork);
+	error = xfs_da_get_buf(tp, dp, blkno, &bp, whichfork);
 	if (error)
 		return error;
 	bp->b_ops = &xfs_da3_node_buf_ops;
@@ -656,7 +656,7 @@ xfs_da3_root_split(
 
 	dp = args->dp;
 	tp = args->trans;
-	error = xfs_da_get_buf(tp, dp, blkno, -1, &bp, args->whichfork);
+	error = xfs_da_get_buf(tp, dp, blkno, &bp, args->whichfork);
 	if (error)
 		return error;
 	node = bp->b_addr;
@@ -2665,7 +2665,6 @@ xfs_da_get_buf(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*dp,
 	xfs_dablk_t		bno,
-	xfs_daddr_t		mappedbno,
 	struct xfs_buf		**bpp,
 	int			whichfork)
 {
@@ -2676,17 +2675,7 @@ xfs_da_get_buf(
 	int			error;
 
 	*bpp = NULL;
-
-	if (mappedbno >= 0) {
-		bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, mappedbno,
-				XFS_FSB_TO_BB(mp,
-					xfs_dabuf_nfsb(mp, whichfork)), 0);
-		goto done;
-	}
-
-	error = xfs_dabuf_map(dp, bno,
-			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
-			whichfork, &mapp, &nmap);
+	error = xfs_dabuf_map(dp, bno, 0, whichfork, &mapp, &nmap);
 	if (error) {
 		/* mapping a hole is not an error, but we don't continue */
 		if (error == -ENOENT)
@@ -2695,7 +2684,6 @@ xfs_da_get_buf(
 	}
 
 	bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
-done:
 	error = bp ? bp->b_error : -EIO;
 	if (error) {
 		if (bp)
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 1c8347af8071..1f874e7b0bed 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -213,8 +213,7 @@ int	xfs_da_grow_inode(xfs_da_args_t *args, xfs_dablk_t *new_blkno);
 int	xfs_da_grow_inode_int(struct xfs_da_args *args, xfs_fileoff_t *bno,
 			      int count);
 int	xfs_da_get_buf(struct xfs_trans *trans, struct xfs_inode *dp,
-			      xfs_dablk_t bno, xfs_daddr_t mappedbno,
-			      struct xfs_buf **bp, int whichfork);
+		xfs_dablk_t bno, struct xfs_buf **bp, int whichfork);
 int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
 		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp,
 		int whichfork, const struct xfs_buf_ops *ops);
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 9ac08df96b3f..c616ea1eb0a3 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -680,7 +680,7 @@ xfs_dir3_data_init(
 	 * Get the buffer set up for the block.
 	 */
 	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, blkno),
-			       -1, &bp, XFS_DATA_FORK);
+			       &bp, XFS_DATA_FORK);
 	if (error)
 		return error;
 	bp->b_ops = &xfs_dir3_data_buf_ops;
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 0107a661acd8..4845d4c7055d 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -355,7 +355,7 @@ xfs_dir3_leaf_get_buf(
 	       bno < xfs_dir2_byte_to_db(args->geo, XFS_DIR2_FREE_OFFSET));
 
 	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, bno),
-			       -1, &bp, XFS_DATA_FORK);
+			       &bp, XFS_DATA_FORK);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index cc1a20b69215..c928febb54bf 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -324,7 +324,7 @@ xfs_dir3_free_get_buf(
 	struct xfs_dir3_icfree_hdr hdr;
 
 	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, fbno),
-				   -1, &bp, XFS_DATA_FORK);
+			&bp, XFS_DATA_FORK);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index f1cafd82ec75..5ff49523d8ea 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -196,6 +196,7 @@ xfs_attr3_node_inactive(
 	struct xfs_buf		*bp,
 	int			level)
 {
+	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_da_blkinfo	*info;
 	xfs_dablk_t		child_fsb;
 	xfs_daddr_t		parent_blkno, child_blkno;
@@ -267,10 +268,16 @@ xfs_attr3_node_inactive(
 		/*
 		 * Remove the subsidiary block from the cache and from the log.
 		 */
-		error = xfs_da_get_buf(*trans, dp, 0, child_blkno, &child_bp,
-				       XFS_ATTR_FORK);
-		if (error)
+		child_bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp,
+				child_blkno,
+				XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
+		if (!child_bp)
+			return -EIO;
+		error = bp->b_error;
+		if (error) {
+			xfs_trans_brelse(*trans, child_bp);
 			return error;
+		}
 		xfs_trans_binval(*trans, child_bp);
 
 		/*
@@ -311,6 +318,7 @@ xfs_attr3_root_inactive(
 	struct xfs_trans	**trans,
 	struct xfs_inode	*dp)
 {
+	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_da_blkinfo	*info;
 	struct xfs_buf		*bp;
 	xfs_daddr_t		blkno;
@@ -353,9 +361,15 @@ xfs_attr3_root_inactive(
 	/*
 	 * Invalidate the incore copy of the root block.
 	 */
-	error = xfs_da_get_buf(*trans, dp, 0, blkno, &bp, XFS_ATTR_FORK);
-	if (error)
+	bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp, blkno,
+			XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
+	if (!bp)
+		return -EIO;
+	error = bp->b_error;
+	if (error) {
+		xfs_trans_brelse(*trans, bp);
 		return error;
+	}
 	xfs_trans_binval(*trans, bp);	/* remove from cache */
 	/*
 	 * Commit the invalidate and start the next transaction.
-- 
2.20.1

Re: [PATCH 9/9] xfs: remove the mappedbno argument to xfs_da_get_buf

[Darrick J. Wong]

On Sat, Nov 16, 2019 at 07:22:14PM +0100, Christoph Hellwig wrote:
> Use the xfs_da_get_buf_daddr function directly for the two callers
> that pass a mapped disk address, and then remove the mappedbno argument.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok, though I haven't tested this ... :)

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

--D

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c |  4 ++--
>  fs/xfs/libxfs/xfs_da_btree.c  | 18 +++---------------
>  fs/xfs/libxfs/xfs_da_btree.h  |  3 +--
>  fs/xfs/libxfs/xfs_dir2_data.c |  2 +-
>  fs/xfs/libxfs/xfs_dir2_leaf.c |  2 +-
>  fs/xfs/libxfs/xfs_dir2_node.c |  2 +-
>  fs/xfs/xfs_attr_inactive.c    | 24 +++++++++++++++++++-----
>  7 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 450e75cc7c93..32bf3c30238c 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -1162,7 +1162,7 @@ xfs_attr3_leaf_to_node(
>  	if (error)
>  		goto out;
>  
> -	error = xfs_da_get_buf(args->trans, dp, blkno, -1, &bp2, XFS_ATTR_FORK);
> +	error = xfs_da_get_buf(args->trans, dp, blkno, &bp2, XFS_ATTR_FORK);
>  	if (error)
>  		goto out;
>  
> @@ -1223,7 +1223,7 @@ xfs_attr3_leaf_create(
>  
>  	trace_xfs_attr_leaf_create(args);
>  
> -	error = xfs_da_get_buf(args->trans, args->dp, blkno, -1, &bp,
> +	error = xfs_da_get_buf(args->trans, args->dp, blkno, &bp,
>  					    XFS_ATTR_FORK);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 34d0ce93bcc3..dbb2b2f38a7f 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -429,7 +429,7 @@ xfs_da3_node_create(
>  	trace_xfs_da_node_create(args);
>  	ASSERT(level <= XFS_DA_NODE_MAXDEPTH);
>  
> -	error = xfs_da_get_buf(tp, dp, blkno, -1, &bp, whichfork);
> +	error = xfs_da_get_buf(tp, dp, blkno, &bp, whichfork);
>  	if (error)
>  		return error;
>  	bp->b_ops = &xfs_da3_node_buf_ops;
> @@ -656,7 +656,7 @@ xfs_da3_root_split(
>  
>  	dp = args->dp;
>  	tp = args->trans;
> -	error = xfs_da_get_buf(tp, dp, blkno, -1, &bp, args->whichfork);
> +	error = xfs_da_get_buf(tp, dp, blkno, &bp, args->whichfork);
>  	if (error)
>  		return error;
>  	node = bp->b_addr;
> @@ -2665,7 +2665,6 @@ xfs_da_get_buf(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*dp,
>  	xfs_dablk_t		bno,
> -	xfs_daddr_t		mappedbno,
>  	struct xfs_buf		**bpp,
>  	int			whichfork)
>  {
> @@ -2676,17 +2675,7 @@ xfs_da_get_buf(
>  	int			error;
>  
>  	*bpp = NULL;
> -
> -	if (mappedbno >= 0) {
> -		bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, mappedbno,
> -				XFS_FSB_TO_BB(mp,
> -					xfs_dabuf_nfsb(mp, whichfork)), 0);
> -		goto done;
> -	}
> -
> -	error = xfs_dabuf_map(dp, bno,
> -			mappedbno == -1 ? XFS_DABUF_MAP_HOLE_OK : 0,
> -			whichfork, &mapp, &nmap);
> +	error = xfs_dabuf_map(dp, bno, 0, whichfork, &mapp, &nmap);
>  	if (error) {
>  		/* mapping a hole is not an error, but we don't continue */
>  		if (error == -ENOENT)
> @@ -2695,7 +2684,6 @@ xfs_da_get_buf(
>  	}
>  
>  	bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
> -done:
>  	error = bp ? bp->b_error : -EIO;
>  	if (error) {
>  		if (bp)
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 1c8347af8071..1f874e7b0bed 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -213,8 +213,7 @@ int	xfs_da_grow_inode(xfs_da_args_t *args, xfs_dablk_t *new_blkno);
>  int	xfs_da_grow_inode_int(struct xfs_da_args *args, xfs_fileoff_t *bno,
>  			      int count);
>  int	xfs_da_get_buf(struct xfs_trans *trans, struct xfs_inode *dp,
> -			      xfs_dablk_t bno, xfs_daddr_t mappedbno,
> -			      struct xfs_buf **bp, int whichfork);
> +		xfs_dablk_t bno, struct xfs_buf **bp, int whichfork);
>  int	xfs_da_read_buf(struct xfs_trans *trans, struct xfs_inode *dp,
>  		xfs_dablk_t bno, unsigned int flags, struct xfs_buf **bpp,
>  		int whichfork, const struct xfs_buf_ops *ops);
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 9ac08df96b3f..c616ea1eb0a3 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -680,7 +680,7 @@ xfs_dir3_data_init(
>  	 * Get the buffer set up for the block.
>  	 */
>  	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, blkno),
> -			       -1, &bp, XFS_DATA_FORK);
> +			       &bp, XFS_DATA_FORK);
>  	if (error)
>  		return error;
>  	bp->b_ops = &xfs_dir3_data_buf_ops;
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 0107a661acd8..4845d4c7055d 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -355,7 +355,7 @@ xfs_dir3_leaf_get_buf(
>  	       bno < xfs_dir2_byte_to_db(args->geo, XFS_DIR2_FREE_OFFSET));
>  
>  	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, bno),
> -			       -1, &bp, XFS_DATA_FORK);
> +			       &bp, XFS_DATA_FORK);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index cc1a20b69215..c928febb54bf 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -324,7 +324,7 @@ xfs_dir3_free_get_buf(
>  	struct xfs_dir3_icfree_hdr hdr;
>  
>  	error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, fbno),
> -				   -1, &bp, XFS_DATA_FORK);
> +			&bp, XFS_DATA_FORK);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index f1cafd82ec75..5ff49523d8ea 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -196,6 +196,7 @@ xfs_attr3_node_inactive(
>  	struct xfs_buf		*bp,
>  	int			level)
>  {
> +	struct xfs_mount	*mp = dp->i_mount;
>  	struct xfs_da_blkinfo	*info;
>  	xfs_dablk_t		child_fsb;
>  	xfs_daddr_t		parent_blkno, child_blkno;
> @@ -267,10 +268,16 @@ xfs_attr3_node_inactive(
>  		/*
>  		 * Remove the subsidiary block from the cache and from the log.
>  		 */
> -		error = xfs_da_get_buf(*trans, dp, 0, child_blkno, &child_bp,
> -				       XFS_ATTR_FORK);
> -		if (error)
> +		child_bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp,
> +				child_blkno,
> +				XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
> +		if (!child_bp)
> +			return -EIO;
> +		error = bp->b_error;
> +		if (error) {
> +			xfs_trans_brelse(*trans, child_bp);
>  			return error;
> +		}
>  		xfs_trans_binval(*trans, child_bp);
>  
>  		/*
> @@ -311,6 +318,7 @@ xfs_attr3_root_inactive(
>  	struct xfs_trans	**trans,
>  	struct xfs_inode	*dp)
>  {
> +	struct xfs_mount	*mp = dp->i_mount;
>  	struct xfs_da_blkinfo	*info;
>  	struct xfs_buf		*bp;
>  	xfs_daddr_t		blkno;
> @@ -353,9 +361,15 @@ xfs_attr3_root_inactive(
>  	/*
>  	 * Invalidate the incore copy of the root block.
>  	 */
> -	error = xfs_da_get_buf(*trans, dp, 0, blkno, &bp, XFS_ATTR_FORK);
> -	if (error)
> +	bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp, blkno,
> +			XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
> +	if (!bp)
> +		return -EIO;
> +	error = bp->b_error;
> +	if (error) {
> +		xfs_trans_brelse(*trans, bp);
>  		return error;
> +	}
>  	xfs_trans_binval(*trans, bp);	/* remove from cache */
>  	/*
>  	 * Commit the invalidate and start the next transaction.
> -- 
> 2.20.1
>