new helpers to clean up extent tree lookups

new helpers to clean up extent tree lookups

[Christoph Hellwig]

Now that Eric send out xfs_iext_count before I could get to it I need
to get my other iext helpers out ASAP before running into another
rebase :)

This series adds two new helpers that make iterating the extent list
a lot cleaner.  They are the first step towards better abstracting out
access to the extent list and thus allowing us to experiment with
better data structures for it.  The next step is the manipulations
in xfs_bmap_{add,del}_extent*, which will be more work than these
helpers, but I have some of that in progress.

[PATCH 01/14] xfs: new inode extent list lookup helpers

[Christoph Hellwig]

xfs_iext_lookup_extent looks up a single extent at the passed in offset,
and returns the extent covering the area, or the one behind it in case
of a hole, as well as the index of the returned extent in arguments,
as well as a simple bool as return value that is set to false if no
extent could be found because the offset is behind EOF.  It is a simpler
replacement for xfs_bmap_search_extent that leaves looking up the rarely
needed previous extent to the caller and has a nicer calling convention.

xfs_iext_get_extent is a helper for iterating over the extent list,
it takes an extent index as input, and returns the extent at that index
in it's expanded form in an argument if it exists.  The actual return
value is a bool whether the index is valid or not.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_inode_fork.c | 46 ++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_inode_fork.h |  6 ++++++
 2 files changed, 52 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 5fbe24c..749fbfb 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -2003,3 +2003,49 @@ xfs_ifork_init_cow(
 	ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
 	ip->i_cnextents = 0;
 }
+
+/*
+ * Lookup the extent covering bno.
+ *
+ * If there is an extent covering bno return the extent index, and store the
+ * expanded extent structure in *gotp, and the extent indes in *idx.
+ * If there is no extent covering bno, but there is an extent after it (e.g.
+ * it lies in a hole) return that extent in *gotp and its index in *idx
+ * instead.
+ * If bno is beyond the last extent return false, and return the index after
+ * the last valid index in *idxp.
+ */
+bool
+xfs_iext_lookup_extent(
+	struct xfs_inode	*ip,
+	struct xfs_ifork	*ifp,
+	xfs_fileoff_t		bno,
+	xfs_extnum_t		*idxp,
+	struct xfs_bmbt_irec	*gotp)
+{
+	struct xfs_bmbt_rec_host *ep;
+
+	XFS_STATS_INC(ip->i_mount, xs_look_exlist);
+
+	ep = xfs_iext_bno_to_ext(ifp, bno, idxp);
+	if (!ep)
+		return false;
+	xfs_bmbt_get_all(ep, gotp);
+	return true;
+}
+
+/*
+ * Return true if there is an extent at index idx, and return the expanded
+ * extent structure at idx in that case.  Else return false.
+ */
+bool
+xfs_iext_get_extent(
+	struct xfs_ifork	*ifp,
+	xfs_extnum_t		idx,
+	struct xfs_bmbt_irec	*gotp)
+{
+	if (idx < 0 || idx >= xfs_iext_count(ifp))
+		return false;
+	xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), gotp);
+	return true;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 8bf112e..7fb8365 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -182,6 +182,12 @@ void		xfs_iext_irec_compact_pages(struct xfs_ifork *);
 void		xfs_iext_irec_compact_full(struct xfs_ifork *);
 void		xfs_iext_irec_update_extoffs(struct xfs_ifork *, int, int);
 
+bool		xfs_iext_lookup_extent(struct xfs_inode *ip,
+			struct xfs_ifork *ifp, xfs_fileoff_t bno,
+			xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
+bool		xfs_iext_get_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
+			struct xfs_bmbt_irec *gotp);
+
 extern struct kmem_zone	*xfs_ifork_zone;
 
 extern void xfs_ifork_init_cow(struct xfs_inode *ip);
-- 
2.1.4

Re: [PATCH 01/14] xfs: new inode extent list lookup helpers

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:32PM +0100, Christoph Hellwig wrote:
> xfs_iext_lookup_extent looks up a single extent at the passed in offset,
> and returns the extent covering the area, or the one behind it in case
> of a hole, as well as the index of the returned extent in arguments,
> as well as a simple bool as return value that is set to false if no
> extent could be found because the offset is behind EOF.  It is a simpler
> replacement for xfs_bmap_search_extent that leaves looking up the rarely
> needed previous extent to the caller and has a nicer calling convention.
> 
> xfs_iext_get_extent is a helper for iterating over the extent list,
> it takes an extent index as input, and returns the extent at that index
> in it's expanded form in an argument if it exists.  The actual return
> value is a bool whether the index is valid or not.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_inode_fork.c | 46 ++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_inode_fork.h |  6 ++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 5fbe24c..749fbfb 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -2003,3 +2003,49 @@ xfs_ifork_init_cow(
>  	ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
>  	ip->i_cnextents = 0;
>  }
> +
> +/*
> + * Lookup the extent covering bno.
> + *
> + * If there is an extent covering bno return the extent index, and store the
> + * expanded extent structure in *gotp, and the extent indes in *idx.

Typo:							 index

Otherwise looks good:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> + * If there is no extent covering bno, but there is an extent after it (e.g.
> + * it lies in a hole) return that extent in *gotp and its index in *idx
> + * instead.
> + * If bno is beyond the last extent return false, and return the index after
> + * the last valid index in *idxp.
> + */
> +bool
> +xfs_iext_lookup_extent(
> +	struct xfs_inode	*ip,
> +	struct xfs_ifork	*ifp,
> +	xfs_fileoff_t		bno,
> +	xfs_extnum_t		*idxp,
> +	struct xfs_bmbt_irec	*gotp)
> +{
> +	struct xfs_bmbt_rec_host *ep;
> +
> +	XFS_STATS_INC(ip->i_mount, xs_look_exlist);
> +
> +	ep = xfs_iext_bno_to_ext(ifp, bno, idxp);
> +	if (!ep)
> +		return false;
> +	xfs_bmbt_get_all(ep, gotp);
> +	return true;
> +}
> +
> +/*
> + * Return true if there is an extent at index idx, and return the expanded
> + * extent structure at idx in that case.  Else return false.
> + */
> +bool
> +xfs_iext_get_extent(
> +	struct xfs_ifork	*ifp,
> +	xfs_extnum_t		idx,
> +	struct xfs_bmbt_irec	*gotp)
> +{
> +	if (idx < 0 || idx >= xfs_iext_count(ifp))
> +		return false;
> +	xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), gotp);
> +	return true;
> +}
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 8bf112e..7fb8365 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -182,6 +182,12 @@ void		xfs_iext_irec_compact_pages(struct xfs_ifork *);
>  void		xfs_iext_irec_compact_full(struct xfs_ifork *);
>  void		xfs_iext_irec_update_extoffs(struct xfs_ifork *, int, int);
>  
> +bool		xfs_iext_lookup_extent(struct xfs_inode *ip,
> +			struct xfs_ifork *ifp, xfs_fileoff_t bno,
> +			xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
> +bool		xfs_iext_get_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
> +			struct xfs_bmbt_irec *gotp);
> +
>  extern struct kmem_zone	*xfs_ifork_zone;
>  
>  extern void xfs_ifork_init_cow(struct xfs_inode *ip);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/14] xfs: cleanup xfs_bmap_last_before

[Christoph Hellwig]

Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
and massage the flow into something easily understandable.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 5c3c4dd..98f490b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
  */
 int						/* error */
 xfs_bmap_last_before(
-	xfs_trans_t	*tp,			/* transaction pointer */
-	xfs_inode_t	*ip,			/* incore inode */
-	xfs_fileoff_t	*last_block,		/* last block */
-	int		whichfork)		/* data or attr fork */
+	struct xfs_trans	*tp,		/* transaction pointer */
+	struct xfs_inode	*ip,		/* incore inode */
+	xfs_fileoff_t		*last_block,	/* last block */
+	int			whichfork)	/* data or attr fork */
 {
-	xfs_fileoff_t	bno;			/* input file offset */
-	int		eof;			/* hit end of file */
-	xfs_bmbt_rec_host_t *ep;		/* pointer to last extent */
-	int		error;			/* error return value */
-	xfs_bmbt_irec_t	got;			/* current extent value */
-	xfs_ifork_t	*ifp;			/* inode fork pointer */
-	xfs_extnum_t	lastx;			/* last extent used */
-	xfs_bmbt_irec_t	prev;			/* previous extent value */
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
+	struct xfs_bmbt_irec	got;
+	xfs_extnum_t		idx;
+	int			error;
 
-	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
-	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
-	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
-	       return -EIO;
-	if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
+	switch (XFS_IFORK_FORMAT(ip, whichfork)) {
+	case XFS_DINODE_FMT_LOCAL:
 		*last_block = 0;
 		return 0;
+	case XFS_DINODE_FMT_BTREE:
+	case XFS_DINODE_FMT_EXTENTS:
+		break;
+	default:
+		return -EIO;
 	}
-	ifp = XFS_IFORK_PTR(ip, whichfork);
-	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
-	    (error = xfs_iread_extents(tp, ip, whichfork)))
-		return error;
-	bno = *last_block - 1;
-	ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
-		&prev);
-	if (eof || xfs_bmbt_get_startoff(ep) > bno) {
-		if (prev.br_startoff == NULLFILEOFF)
-			*last_block = 0;
-		else
-			*last_block = prev.br_startoff + prev.br_blockcount;
+
+	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
+		error = xfs_iread_extents(tp, ip, whichfork);
+		if (error)
+			return error;
 	}
-	/*
-	 * Otherwise *last_block is already the right answer.
-	 */
+
+	if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
+		if (got.br_startoff <= *last_block - 1)
+			return 0;
+	}
+
+	if (xfs_iext_get_extent(ifp, idx - 1, &got)) {
+		*last_block = got.br_startoff + got.br_blockcount;
+		return 0;
+	}
+
+	*last_block = 0;
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH 02/14] xfs: cleanup xfs_bmap_last_before

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:33PM +0100, Christoph Hellwig wrote:
> Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
> and massage the flow into something easily understandable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 64 ++++++++++++++++++++++++------------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5c3c4dd..98f490b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
>   */
>  int						/* error */
>  xfs_bmap_last_before(
> -	xfs_trans_t	*tp,			/* transaction pointer */
> -	xfs_inode_t	*ip,			/* incore inode */
> -	xfs_fileoff_t	*last_block,		/* last block */
> -	int		whichfork)		/* data or attr fork */
> +	struct xfs_trans	*tp,		/* transaction pointer */
> +	struct xfs_inode	*ip,		/* incore inode */
> +	xfs_fileoff_t		*last_block,	/* last block */
> +	int			whichfork)	/* data or attr fork */
>  {
> -	xfs_fileoff_t	bno;			/* input file offset */
> -	int		eof;			/* hit end of file */
> -	xfs_bmbt_rec_host_t *ep;		/* pointer to last extent */
> -	int		error;			/* error return value */
> -	xfs_bmbt_irec_t	got;			/* current extent value */
> -	xfs_ifork_t	*ifp;			/* inode fork pointer */
> -	xfs_extnum_t	lastx;			/* last extent used */
> -	xfs_bmbt_irec_t	prev;			/* previous extent value */
> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
> +	struct xfs_bmbt_irec	got;
> +	xfs_extnum_t		idx;
> +	int			error;
>  
> -	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> -	       return -EIO;
> -	if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> +	switch (XFS_IFORK_FORMAT(ip, whichfork)) {
> +	case XFS_DINODE_FMT_LOCAL:
>  		*last_block = 0;
>  		return 0;
> +	case XFS_DINODE_FMT_BTREE:
> +	case XFS_DINODE_FMT_EXTENTS:
> +		break;
> +	default:
> +		return -EIO;
>  	}
> -	ifp = XFS_IFORK_PTR(ip, whichfork);
> -	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> -	    (error = xfs_iread_extents(tp, ip, whichfork)))
> -		return error;
> -	bno = *last_block - 1;
> -	ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
> -		&prev);
> -	if (eof || xfs_bmbt_get_startoff(ep) > bno) {
> -		if (prev.br_startoff == NULLFILEOFF)
> -			*last_block = 0;
> -		else
> -			*last_block = prev.br_startoff + prev.br_blockcount;
> +
> +	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> +		error = xfs_iread_extents(tp, ip, whichfork);
> +		if (error)
> +			return error;
>  	}
> -	/*
> -	 * Otherwise *last_block is already the right answer.
> -	 */
> +
> +	if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
> +		if (got.br_startoff <= *last_block - 1)
> +			return 0;
> +	}
> +
> +	if (xfs_iext_get_extent(ifp, idx - 1, &got)) {

It may not be an issue in current usage, but I think we should check for
idx here (i.e., 'if (idx && xfs_iext_get_extent(..))').

Brian

> +		*last_block = got.br_startoff + got.br_blockcount;
> +		return 0;
> +	}
> +
> +	*last_block = 0;
>  	return 0;
>  }
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/14] xfs: cleanup xfs_bmap_last_before

[Christoph Hellwig]

On Thu, Nov 17, 2016 at 01:11:54PM -0500, Brian Foster wrote:
> On Mon, Nov 14, 2016 at 06:12:33PM +0100, Christoph Hellwig wrote:
> > Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
> > and massage the flow into something easily understandable.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/libxfs/xfs_bmap.c | 64 ++++++++++++++++++++++++------------------------
> >  1 file changed, 32 insertions(+), 32 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index 5c3c4dd..98f490b 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
> >   */
> >  int						/* error */
> >  xfs_bmap_last_before(
> > -	xfs_trans_t	*tp,			/* transaction pointer */
> > -	xfs_inode_t	*ip,			/* incore inode */
> > -	xfs_fileoff_t	*last_block,		/* last block */
> > -	int		whichfork)		/* data or attr fork */
> > +	struct xfs_trans	*tp,		/* transaction pointer */
> > +	struct xfs_inode	*ip,		/* incore inode */
> > +	xfs_fileoff_t		*last_block,	/* last block */
> > +	int			whichfork)	/* data or attr fork */
> >  {
> > -	xfs_fileoff_t	bno;			/* input file offset */
> > -	int		eof;			/* hit end of file */
> > -	xfs_bmbt_rec_host_t *ep;		/* pointer to last extent */
> > -	int		error;			/* error return value */
> > -	xfs_bmbt_irec_t	got;			/* current extent value */
> > -	xfs_ifork_t	*ifp;			/* inode fork pointer */
> > -	xfs_extnum_t	lastx;			/* last extent used */
> > -	xfs_bmbt_irec_t	prev;			/* previous extent value */
> > +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
> > +	struct xfs_bmbt_irec	got;
> > +	xfs_extnum_t		idx;
> > +	int			error;
> >  
> > -	if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> > -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> > -	    XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> > -	       return -EIO;
> > -	if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> > +	switch (XFS_IFORK_FORMAT(ip, whichfork)) {
> > +	case XFS_DINODE_FMT_LOCAL:
> >  		*last_block = 0;
> >  		return 0;
> > +	case XFS_DINODE_FMT_BTREE:
> > +	case XFS_DINODE_FMT_EXTENTS:
> > +		break;
> > +	default:
> > +		return -EIO;
> >  	}
> > -	ifp = XFS_IFORK_PTR(ip, whichfork);
> > -	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> > -	    (error = xfs_iread_extents(tp, ip, whichfork)))
> > -		return error;
> > -	bno = *last_block - 1;
> > -	ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
> > -		&prev);
> > -	if (eof || xfs_bmbt_get_startoff(ep) > bno) {
> > -		if (prev.br_startoff == NULLFILEOFF)
> > -			*last_block = 0;
> > -		else
> > -			*last_block = prev.br_startoff + prev.br_blockcount;
> > +
> > +	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> > +		error = xfs_iread_extents(tp, ip, whichfork);
> > +		if (error)
> > +			return error;
> >  	}
> > -	/*
> > -	 * Otherwise *last_block is already the right answer.
> > -	 */
> > +
> > +	if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
> > +		if (got.br_startoff <= *last_block - 1)
> > +			return 0;
> > +	}
> > +
> > +	if (xfs_iext_get_extent(ifp, idx - 1, &got)) {
> 
> It may not be an issue in current usage, but I think we should check for
> idx here (i.e., 'if (idx && xfs_iext_get_extent(..))').

I've designed xfs_iext_get_extent to gracefully handle invalid indices
(including negative ones) and return NULL.
case.

[PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 98f490b..1a0fee4 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4145,12 +4145,11 @@ xfs_bmapi_read(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_ifork	*ifp;
 	struct xfs_bmbt_irec	got;
-	struct xfs_bmbt_irec	prev;
 	xfs_fileoff_t		obno;
 	xfs_fileoff_t		end;
-	xfs_extnum_t		lastx;
+	xfs_extnum_t		idx;
 	int			error;
-	int			eof;
+	bool			eof = false;
 	int			n = 0;
 	int			whichfork = xfs_bmapi_whichfork(flags);
 
@@ -4190,7 +4189,8 @@ xfs_bmapi_read(
 			return error;
 	}
 
-	xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got, &prev);
+	if (!xfs_iext_lookup_extent(ip, ifp, bno, &idx, &got))
+		eof = true;
 	end = bno + len;
 	obno = bno;
 
@@ -4221,10 +4221,8 @@ xfs_bmapi_read(
 			break;
 
 		/* Else go on to the next record. */
-		if (++lastx < xfs_iext_count(ifp))
-			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx), &got);
-		else
-			eof = 1;
+		if (!xfs_iext_get_extent(ifp, ++idx, &got))
+			eof = true;
 	}
 	*nmap = n;
 	return 0;
-- 
2.1.4

Re: [PATCH 03/14] xfs: use new extent lookup helpers in xfs_bmapi_read

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:34PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 98f490b..1a0fee4 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4145,12 +4145,11 @@ xfs_bmapi_read(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_ifork	*ifp;
>  	struct xfs_bmbt_irec	got;
> -	struct xfs_bmbt_irec	prev;
>  	xfs_fileoff_t		obno;
>  	xfs_fileoff_t		end;
> -	xfs_extnum_t		lastx;
> +	xfs_extnum_t		idx;
>  	int			error;
> -	int			eof;
> +	bool			eof = false;
>  	int			n = 0;
>  	int			whichfork = xfs_bmapi_whichfork(flags);
>  
> @@ -4190,7 +4189,8 @@ xfs_bmapi_read(
>  			return error;
>  	}
>  
> -	xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got, &prev);
> +	if (!xfs_iext_lookup_extent(ip, ifp, bno, &idx, &got))
> +		eof = true;
>  	end = bno + len;
>  	obno = bno;
>  
> @@ -4221,10 +4221,8 @@ xfs_bmapi_read(
>  			break;
>  
>  		/* Else go on to the next record. */
> -		if (++lastx < xfs_iext_count(ifp))
> -			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx), &got);
> -		else
> -			eof = 1;
> +		if (!xfs_iext_get_extent(ifp, ++idx, &got))
> +			eof = true;
>  	}
>  	*nmap = n;
>  	return 0;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1a0fee4..9a8621d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4561,7 +4561,7 @@ xfs_bmapi_write(
 	struct xfs_ifork	*ifp;
 	struct xfs_bmalloca	bma = { NULL };	/* args for xfs_bmap_alloc */
 	xfs_fileoff_t		end;		/* end of mapped file region */
-	int			eof;		/* after the end of extents */
+	bool			eof = false;	/* after the end of extents */
 	int			error;		/* error return */
 	int			n;		/* current extent index */
 	xfs_fileoff_t		obno;		/* old block number (offset) */
@@ -4639,12 +4639,14 @@ xfs_bmapi_write(
 			goto error0;
 	}
 
-	xfs_bmap_search_extents(ip, bno, whichfork, &eof, &bma.idx, &bma.got,
-				&bma.prev);
 	n = 0;
 	end = bno + len;
 	obno = bno;
 
+	if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.idx, &bma.got))
+		eof = true;
+	if (!xfs_iext_get_extent(ifp, bma.idx - 1, &bma.prev))
+		bma.prev.br_startoff = NULLFILEOFF;
 	bma.tp = tp;
 	bma.ip = ip;
 	bma.total = total;
@@ -4731,11 +4733,8 @@ xfs_bmapi_write(
 
 		/* Else go on to the next record. */
 		bma.prev = bma.got;
-		if (++bma.idx < xfs_iext_count(ifp)) {
-			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma.idx),
-					 &bma.got);
-		} else
-			eof = 1;
+		if (!xfs_iext_get_extent(ifp, ++bma.idx, &bma.got))
+			eof = true;
 	}
 	*nmap = n;
 
-- 
2.1.4

Re: [PATCH 04/14] xfs: use new extent lookup helpers in xfs_bmapi_write

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:35PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1a0fee4..9a8621d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4561,7 +4561,7 @@ xfs_bmapi_write(
>  	struct xfs_ifork	*ifp;
>  	struct xfs_bmalloca	bma = { NULL };	/* args for xfs_bmap_alloc */
>  	xfs_fileoff_t		end;		/* end of mapped file region */
> -	int			eof;		/* after the end of extents */
> +	bool			eof = false;	/* after the end of extents */
>  	int			error;		/* error return */
>  	int			n;		/* current extent index */
>  	xfs_fileoff_t		obno;		/* old block number (offset) */
> @@ -4639,12 +4639,14 @@ xfs_bmapi_write(
>  			goto error0;
>  	}
>  
> -	xfs_bmap_search_extents(ip, bno, whichfork, &eof, &bma.idx, &bma.got,
> -				&bma.prev);
>  	n = 0;
>  	end = bno + len;
>  	obno = bno;
>  
> +	if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.idx, &bma.got))
> +		eof = true;
> +	if (!xfs_iext_get_extent(ifp, bma.idx - 1, &bma.prev))

Similar issue here:

	if (!bma.idx || !xfs_iext_get_extent(..))
		...

Brian

> +		bma.prev.br_startoff = NULLFILEOFF;
>  	bma.tp = tp;
>  	bma.ip = ip;
>  	bma.total = total;
> @@ -4731,11 +4733,8 @@ xfs_bmapi_write(
>  
>  		/* Else go on to the next record. */
>  		bma.prev = bma.got;
> -		if (++bma.idx < xfs_iext_count(ifp)) {
> -			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma.idx),
> -					 &bma.got);
> -		} else
> -			eof = 1;
> +		if (!xfs_iext_get_extent(ifp, ++bma.idx, &bma.got))
> +			eof = true;
>  	}
>  	*nmap = n;
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9a8621d..18de89c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5433,8 +5433,6 @@ __xfs_bunmapi(
 {
 	xfs_btree_cur_t		*cur;		/* bmap btree cursor */
 	xfs_bmbt_irec_t		del;		/* extent being deleted */
-	int			eof;		/* is deleting at eof */
-	xfs_bmbt_rec_host_t	*ep;		/* extent record pointer */
 	int			error;		/* error return value */
 	xfs_extnum_t		extno;		/* extent number in list */
 	xfs_bmbt_irec_t		got;		/* current extent record */
@@ -5444,7 +5442,6 @@ __xfs_bunmapi(
 	int			logflags;	/* transaction logging flags */
 	xfs_extlen_t		mod;		/* rt extent offset */
 	xfs_mount_t		*mp;		/* mount structure */
-	xfs_bmbt_irec_t		prev;		/* previous extent record */
 	xfs_fileoff_t		start;		/* first file offset deleted */
 	int			tmp_logflags;	/* partial logging flags */
 	int			wasdel;		/* was a delayed alloc extent */
@@ -5483,18 +5480,17 @@ __xfs_bunmapi(
 	isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
 	start = bno;
 	bno = start + len - 1;
-	ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
-		&prev);
 
 	/*
 	 * Check to see if the given block number is past the end of the
 	 * file, back up to the last block if so...
 	 */
-	if (eof) {
-		ep = xfs_iext_get_ext(ifp, --lastx);
-		xfs_bmbt_get_all(ep, &got);
+	if (!xfs_iext_lookup_extent(ip, ifp, bno, &lastx, &got)) {
+		ASSERT(lastx > 0);
+		xfs_iext_get_extent(ifp, --lastx, &got);
 		bno = got.br_startoff + got.br_blockcount - 1;
 	}
+
 	logflags = 0;
 	if (ifp->if_flags & XFS_IFBROOT) {
 		ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE);
@@ -5525,8 +5521,7 @@ __xfs_bunmapi(
 		if (got.br_startoff > bno) {
 			if (--lastx < 0)
 				break;
-			ep = xfs_iext_get_ext(ifp, lastx);
-			xfs_bmbt_get_all(ep, &got);
+			xfs_iext_get_extent(ifp, lastx, &got);
 		}
 		/*
 		 * Is the last block of this extent before the range
@@ -5540,7 +5535,6 @@ __xfs_bunmapi(
 		 * Then deal with the (possibly delayed) allocated space
 		 * we found.
 		 */
-		ASSERT(ep != NULL);
 		del = got;
 		wasdel = isnullstartblock(del.br_startblock);
 		if (got.br_startoff < start) {
@@ -5621,15 +5615,11 @@ __xfs_bunmapi(
 				 */
 				ASSERT(bno >= del.br_blockcount);
 				bno -= del.br_blockcount;
-				if (got.br_startoff > bno) {
-					if (--lastx >= 0) {
-						ep = xfs_iext_get_ext(ifp,
-								      lastx);
-						xfs_bmbt_get_all(ep, &got);
-					}
-				}
+				if (got.br_startoff > bno && --lastx >= 0)
+					xfs_iext_get_extent(ifp, lastx, &got);
 				continue;
 			} else if (del.br_state == XFS_EXT_UNWRITTEN) {
+				xfs_bmbt_irec_t		prev;
 				/*
 				 * This one is already unwritten.
 				 * It must have a written left neighbor.
@@ -5637,8 +5627,7 @@ __xfs_bunmapi(
 				 * try again.
 				 */
 				ASSERT(lastx > 0);
-				xfs_bmbt_get_all(xfs_iext_get_ext(ifp,
-						lastx - 1), &prev);
+				xfs_iext_get_extent(ifp, lastx - 1, &prev);
 				ASSERT(prev.br_state == XFS_EXT_NORM);
 				ASSERT(!isnullstartblock(prev.br_startblock));
 				ASSERT(del.br_startblock ==
@@ -5736,13 +5725,9 @@ __xfs_bunmapi(
 		 */
 		if (bno != (xfs_fileoff_t)-1 && bno >= start) {
 			if (lastx >= 0) {
-				ep = xfs_iext_get_ext(ifp, lastx);
-				if (xfs_bmbt_get_startoff(ep) > bno) {
-					if (--lastx >= 0)
-						ep = xfs_iext_get_ext(ifp,
-								      lastx);
-				}
-				xfs_bmbt_get_all(ep, &got);
+				xfs_iext_get_extent(ifp, lastx, &got);
+				if (got.br_startoff > bno && --lastx >= 0)
+					xfs_iext_get_extent(ifp, lastx, &got);
 			}
 			extno++;
 		}
-- 
2.1.4

Re: [PATCH 05/14] xfs: use new extent lookup helpers in __xfs_bunmapi

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:36PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 39 ++++++++++++---------------------------
>  1 file changed, 12 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 9a8621d..18de89c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5433,8 +5433,6 @@ __xfs_bunmapi(
>  {
>  	xfs_btree_cur_t		*cur;		/* bmap btree cursor */
>  	xfs_bmbt_irec_t		del;		/* extent being deleted */
> -	int			eof;		/* is deleting at eof */
> -	xfs_bmbt_rec_host_t	*ep;		/* extent record pointer */
>  	int			error;		/* error return value */
>  	xfs_extnum_t		extno;		/* extent number in list */
>  	xfs_bmbt_irec_t		got;		/* current extent record */
> @@ -5444,7 +5442,6 @@ __xfs_bunmapi(
>  	int			logflags;	/* transaction logging flags */
>  	xfs_extlen_t		mod;		/* rt extent offset */
>  	xfs_mount_t		*mp;		/* mount structure */
> -	xfs_bmbt_irec_t		prev;		/* previous extent record */
>  	xfs_fileoff_t		start;		/* first file offset deleted */
>  	int			tmp_logflags;	/* partial logging flags */
>  	int			wasdel;		/* was a delayed alloc extent */
> @@ -5483,18 +5480,17 @@ __xfs_bunmapi(
>  	isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
>  	start = bno;
>  	bno = start + len - 1;
> -	ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
> -		&prev);
>  
>  	/*
>  	 * Check to see if the given block number is past the end of the
>  	 * file, back up to the last block if so...
>  	 */
> -	if (eof) {
> -		ep = xfs_iext_get_ext(ifp, --lastx);
> -		xfs_bmbt_get_all(ep, &got);
> +	if (!xfs_iext_lookup_extent(ip, ifp, bno, &lastx, &got)) {
> +		ASSERT(lastx > 0);
> +		xfs_iext_get_extent(ifp, --lastx, &got);
>  		bno = got.br_startoff + got.br_blockcount - 1;
>  	}
> +
>  	logflags = 0;
>  	if (ifp->if_flags & XFS_IFBROOT) {
>  		ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE);
> @@ -5525,8 +5521,7 @@ __xfs_bunmapi(
>  		if (got.br_startoff > bno) {
>  			if (--lastx < 0)
>  				break;
> -			ep = xfs_iext_get_ext(ifp, lastx);
> -			xfs_bmbt_get_all(ep, &got);
> +			xfs_iext_get_extent(ifp, lastx, &got);
>  		}
>  		/*
>  		 * Is the last block of this extent before the range
> @@ -5540,7 +5535,6 @@ __xfs_bunmapi(
>  		 * Then deal with the (possibly delayed) allocated space
>  		 * we found.
>  		 */
> -		ASSERT(ep != NULL);
>  		del = got;
>  		wasdel = isnullstartblock(del.br_startblock);
>  		if (got.br_startoff < start) {
> @@ -5621,15 +5615,11 @@ __xfs_bunmapi(
>  				 */
>  				ASSERT(bno >= del.br_blockcount);
>  				bno -= del.br_blockcount;
> -				if (got.br_startoff > bno) {
> -					if (--lastx >= 0) {
> -						ep = xfs_iext_get_ext(ifp,
> -								      lastx);
> -						xfs_bmbt_get_all(ep, &got);
> -					}
> -				}
> +				if (got.br_startoff > bno && --lastx >= 0)
> +					xfs_iext_get_extent(ifp, lastx, &got);

Slightly tricky, but Ok.

>  				continue;
>  			} else if (del.br_state == XFS_EXT_UNWRITTEN) {
> +				xfs_bmbt_irec_t		prev;

struct xfs_bmbt_irec

Otherwise looks good:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  				/*
>  				 * This one is already unwritten.
>  				 * It must have a written left neighbor.
> @@ -5637,8 +5627,7 @@ __xfs_bunmapi(
>  				 * try again.
>  				 */
>  				ASSERT(lastx > 0);
> -				xfs_bmbt_get_all(xfs_iext_get_ext(ifp,
> -						lastx - 1), &prev);
> +				xfs_iext_get_extent(ifp, lastx - 1, &prev);
>  				ASSERT(prev.br_state == XFS_EXT_NORM);
>  				ASSERT(!isnullstartblock(prev.br_startblock));
>  				ASSERT(del.br_startblock ==
> @@ -5736,13 +5725,9 @@ __xfs_bunmapi(
>  		 */
>  		if (bno != (xfs_fileoff_t)-1 && bno >= start) {
>  			if (lastx >= 0) {
> -				ep = xfs_iext_get_ext(ifp, lastx);
> -				if (xfs_bmbt_get_startoff(ep) > bno) {
> -					if (--lastx >= 0)
> -						ep = xfs_iext_get_ext(ifp,
> -								      lastx);
> -				}
> -				xfs_bmbt_get_all(ep, &got);
> +				xfs_iext_get_extent(ifp, lastx, &got);
> +				if (got.br_startoff > bno && --lastx >= 0)
> +					xfs_iext_get_extent(ifp, lastx, &got);
>  			}
>  			extno++;
>  		}
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Christoph Hellwig]

We can easily lookup the previous extent for the cases where we need it,
which saves the callers from looking it up for us later in the series.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 18de89c..4aa9c07 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
 	xfs_fileoff_t		aoff,
 	xfs_filblks_t		len,
 	struct xfs_bmbt_irec	*got,
-	struct xfs_bmbt_irec	*prev,
 	xfs_extnum_t		*lastx,
 	int			eof)
 {
@@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
 	else
 		extsz = xfs_get_extsz_hint(ip);
 	if (extsz) {
-		error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
+		struct xfs_bmbt_irec	prev;
+
+		if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
+			prev.br_startoff = NULLFILEOFF;
+
+		error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
 					       1, 0, &aoff, &alen);
 		ASSERT(!error);
 	}
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 7cae6ec..e3c2b5a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
 		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
 int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
 		xfs_fileoff_t aoff, xfs_filblks_t len,
-		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
-		xfs_extnum_t *lastx, int eof);
+		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
 
 enum xfs_bmap_intent_type {
 	XFS_BMAP_MAP = 1,
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 436e109..59ffcac 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
 
 retry:
 	error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
-			end_fsb - offset_fsb, &got,
-			&prev, &idx, eof);
+			end_fsb - offset_fsb, &got, &idx, eof);
 	switch (error) {
 	case 0:
 		break;
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 0edf835..52cdfba 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
 
 retry:
 	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
-			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
+			end_fsb - imap->br_startoff, &got, &idx, eof);
 	switch (error) {
 	case 0:
 		break;
-- 
2.1.4

Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:37PM +0100, Christoph Hellwig wrote:
> We can easily lookup the previous extent for the cases where we need it,
> which saves the callers from looking it up for us later in the series.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
>  fs/xfs/libxfs/xfs_bmap.h | 3 +--
>  fs/xfs/xfs_iomap.c       | 3 +--
>  fs/xfs/xfs_reflink.c     | 2 +-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 18de89c..4aa9c07 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
>  	xfs_fileoff_t		aoff,
>  	xfs_filblks_t		len,
>  	struct xfs_bmbt_irec	*got,
> -	struct xfs_bmbt_irec	*prev,
>  	xfs_extnum_t		*lastx,
>  	int			eof)
>  {
> @@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
>  	else
>  		extsz = xfs_get_extsz_hint(ip);
>  	if (extsz) {
> -		error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
> +		struct xfs_bmbt_irec	prev;
> +
> +		if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
> +			prev.br_startoff = NULLFILEOFF;

It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
for < 0, so that covers here and my similar previous few comments. I
still think we should probably check it in context rather than bury the
check in the caller (I'd prefer an assert). Just my .02.

Brian

> +
> +		error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
>  					       1, 0, &aoff, &alen);
>  		ASSERT(!error);
>  	}
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 7cae6ec..e3c2b5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
>  		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
>  int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
>  		xfs_fileoff_t aoff, xfs_filblks_t len,
> -		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
> -		xfs_extnum_t *lastx, int eof);
> +		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
>  
>  enum xfs_bmap_intent_type {
>  	XFS_BMAP_MAP = 1,
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 436e109..59ffcac 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
>  
>  retry:
>  	error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
> -			end_fsb - offset_fsb, &got,
> -			&prev, &idx, eof);
> +			end_fsb - offset_fsb, &got, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 0edf835..52cdfba 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
>  
>  retry:
>  	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> -			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
> +			end_fsb - imap->br_startoff, &got, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Christoph Hellwig]

On Thu, Nov 17, 2016 at 01:27:07PM -0500, Brian Foster wrote:
> It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
> for < 0, so that covers here and my similar previous few comments. I
> still think we should probably check it in context rather than bury the
> check in the caller (I'd prefer an assert). Just my .02.

There are several callers that rely on xfs_iext_get_extent handling
negative extents with a NULL return - in fact one reason for the
exact prototype of the function is to cover out of bound indices
that happen during normal operation based on how we iterate over
the extent list.

Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Brian Foster]

On Fri, Nov 18, 2016 at 09:19:44AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 17, 2016 at 01:27:07PM -0500, Brian Foster wrote:
> > It just hit me that extnum_t is signed and xfs_iext_get_extent() checks
> > for < 0, so that covers here and my similar previous few comments. I
> > still think we should probably check it in context rather than bury the
> > check in the caller (I'd prefer an assert). Just my .02.
> 
> There are several callers that rely on xfs_iext_get_extent handling
> negative extents with a NULL return - in fact one reason for the
> exact prototype of the function is to cover out of bound indices
> that happen during normal operation based on how we iterate over
> the extent list.

Fair enough. I'm not a fan of the approach in principle, but I'm less
worried about it given that it's not an actual bug.

Brian

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay

[Christoph Hellwig]

And only lookup the previous extent inside xfs_iomap_prealloc_size
if we actually need it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iomap.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 59ffcac..2272190 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -395,11 +395,12 @@ xfs_iomap_prealloc_size(
 	struct xfs_inode	*ip,
 	loff_t			offset,
 	loff_t			count,
-	xfs_extnum_t		idx,
-	struct xfs_bmbt_irec	*prev)
+	xfs_extnum_t		idx)
 {
 	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	struct xfs_bmbt_irec	prev;
 	int			shift = 0;
 	int64_t			freesp;
 	xfs_fsblock_t		qblocks;
@@ -419,8 +420,8 @@ xfs_iomap_prealloc_size(
 	 */
 	if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ||
 	    XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
-	    idx == 0 ||
-	    prev->br_startoff + prev->br_blockcount < offset_fsb)
+	    !xfs_iext_get_extent(ifp, idx - 1, &prev) ||
+	    prev.br_startoff + prev.br_blockcount < offset_fsb)
 		return mp->m_writeio_blocks;
 
 	/*
@@ -439,8 +440,8 @@ xfs_iomap_prealloc_size(
 	 * always extends to MAXEXTLEN rather than falling short due to things
 	 * like stripe unit/width alignment of real extents.
 	 */
-	if (prev->br_blockcount <= (MAXEXTLEN >> 1))
-		alloc_blocks = prev->br_blockcount << 1;
+	if (prev.br_blockcount <= (MAXEXTLEN >> 1))
+		alloc_blocks = prev.br_blockcount << 1;
 	else
 		alloc_blocks = XFS_B_TO_FSB(mp, offset);
 	if (!alloc_blocks)
@@ -538,7 +539,6 @@ xfs_file_iomap_begin_delay(
 	xfs_fileoff_t		end_fsb, orig_end_fsb;
 	int			error = 0, eof = 0;
 	struct xfs_bmbt_irec	got;
-	struct xfs_bmbt_irec	prev;
 	xfs_extnum_t		idx;
 
 	ASSERT(!XFS_IS_REALTIME_INODE(ip));
@@ -563,8 +563,7 @@ xfs_file_iomap_begin_delay(
 			goto out_unlock;
 	}
 
-	xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
-			&got, &prev);
+	eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
 	if (!eof && got.br_startoff <= offset_fsb) {
 		if (xfs_is_reflink_inode(ip)) {
 			bool		shared;
@@ -601,8 +600,7 @@ xfs_file_iomap_begin_delay(
 	if (eof) {
 		xfs_fsblock_t	prealloc_blocks;
 
-		prealloc_blocks =
-			xfs_iomap_prealloc_size(ip, offset, count, idx, &prev);
+		prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count, idx);
 		if (prealloc_blocks) {
 			xfs_extlen_t	align;
 			xfs_off_t	end_offset;
-- 
2.1.4

Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:38PM +0100, Christoph Hellwig wrote:
> And only lookup the previous extent inside xfs_iomap_prealloc_size
> if we actually need it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_iomap.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 59ffcac..2272190 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -395,11 +395,12 @@ xfs_iomap_prealloc_size(
>  	struct xfs_inode	*ip,
>  	loff_t			offset,
>  	loff_t			count,
> -	xfs_extnum_t		idx,
> -	struct xfs_bmbt_irec	*prev)
> +	xfs_extnum_t		idx)

My cow prealloc series is going to end up adding this right back, fwiw.
Though at that point it's not really "prev" as used throughout the
extent manipulation code, but rather just an extent on which to base the
initial prealloc size.

That aside, looks good to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  {
>  	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
>  	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
> +	struct xfs_bmbt_irec	prev;
>  	int			shift = 0;
>  	int64_t			freesp;
>  	xfs_fsblock_t		qblocks;
> @@ -419,8 +420,8 @@ xfs_iomap_prealloc_size(
>  	 */
>  	if ((mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) ||
>  	    XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, mp->m_dalign) ||
> -	    idx == 0 ||
> -	    prev->br_startoff + prev->br_blockcount < offset_fsb)
> +	    !xfs_iext_get_extent(ifp, idx - 1, &prev) ||
> +	    prev.br_startoff + prev.br_blockcount < offset_fsb)
>  		return mp->m_writeio_blocks;
>  
>  	/*
> @@ -439,8 +440,8 @@ xfs_iomap_prealloc_size(
>  	 * always extends to MAXEXTLEN rather than falling short due to things
>  	 * like stripe unit/width alignment of real extents.
>  	 */
> -	if (prev->br_blockcount <= (MAXEXTLEN >> 1))
> -		alloc_blocks = prev->br_blockcount << 1;
> +	if (prev.br_blockcount <= (MAXEXTLEN >> 1))
> +		alloc_blocks = prev.br_blockcount << 1;
>  	else
>  		alloc_blocks = XFS_B_TO_FSB(mp, offset);
>  	if (!alloc_blocks)
> @@ -538,7 +539,6 @@ xfs_file_iomap_begin_delay(
>  	xfs_fileoff_t		end_fsb, orig_end_fsb;
>  	int			error = 0, eof = 0;
>  	struct xfs_bmbt_irec	got;
> -	struct xfs_bmbt_irec	prev;
>  	xfs_extnum_t		idx;
>  
>  	ASSERT(!XFS_IS_REALTIME_INODE(ip));
> @@ -563,8 +563,7 @@ xfs_file_iomap_begin_delay(
>  			goto out_unlock;
>  	}
>  
> -	xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
> -			&got, &prev);
> +	eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
>  	if (!eof && got.br_startoff <= offset_fsb) {
>  		if (xfs_is_reflink_inode(ip)) {
>  			bool		shared;
> @@ -601,8 +600,7 @@ xfs_file_iomap_begin_delay(
>  	if (eof) {
>  		xfs_fsblock_t	prealloc_blocks;
>  
> -		prealloc_blocks =
> -			xfs_iomap_prealloc_size(ip, offset, count, idx, &prev);
> +		prealloc_blocks = xfs_iomap_prealloc_size(ip, offset, count, idx);
>  		if (prealloc_blocks) {
>  			xfs_extlen_t	align;
>  			xfs_off_t	end_offset;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay

[Christoph Hellwig]

On Thu, Nov 17, 2016 at 01:33:30PM -0500, Brian Foster wrote:
> My cow prealloc series is going to end up adding this right back, fwiw.
> Though at that point it's not really "prev" as used throughout the
> extent manipulation code, but rather just an extent on which to base the
> initial prealloc size.

Oh, well.  I could keep the argument for now, but doing the lookups
in the caller just for the prev value that's not even always used
seems a little silly.

Re: [PATCH 07/14] xfs: use new extent lookup helpers xfs_file_iomap_begin_delay

[Brian Foster]

On Fri, Nov 18, 2016 at 09:20:47AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 17, 2016 at 01:33:30PM -0500, Brian Foster wrote:
> > My cow prealloc series is going to end up adding this right back, fwiw.
> > Though at that point it's not really "prev" as used throughout the
> > extent manipulation code, but rather just an extent on which to base the
> > initial prealloc size.
> 
> Oh, well.  I could keep the argument for now, but doing the lookups
> in the caller just for the prev value that's not even always used
> seems a little silly.

It's probably not worth it. This looks like a good clean up to me as it
is... I just wanted to call that out since I was using/changing 'prev'.
Looking again, the cow prealloc stuff is still going to use the data
extent record. I may be able to convert 'prev' to 'base' within
*_prealloc_size() just the same without changing the function signature
back.

Brian

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 52cdfba..35e02ce 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -243,10 +243,11 @@ xfs_reflink_reserve_cow(
 	struct xfs_bmbt_irec	*imap,
 	bool			*shared)
 {
-	struct xfs_bmbt_irec	got, prev;
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	struct xfs_bmbt_irec	got;
 	xfs_fileoff_t		end_fsb, orig_end_fsb;
-	int			eof = 0, error = 0;
-	bool			trimmed;
+	int			error = 0;
+	bool			eof = false, trimmed;
 	xfs_extnum_t		idx;
 	xfs_extlen_t		align;
 
@@ -258,8 +259,9 @@ xfs_reflink_reserve_cow(
 	 * extent list is generally faster than going out to the shared extent
 	 * tree.
 	 */
-	xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
-			&got, &prev);
+
+	if (!xfs_iext_lookup_extent(ip, ifp, imap->br_startoff, &idx, &got))
+		eof = true;
 	if (!eof && got.br_startoff <= imap->br_startoff) {
 		trace_xfs_reflink_cow_found(ip, imap);
 		xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
-- 
2.1.4

Re: [PATCH 08/14] xfs: use new extent lookup helpers in __xfs_reflink_reserve_cow

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:39PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 52cdfba..35e02ce 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -243,10 +243,11 @@ xfs_reflink_reserve_cow(
>  	struct xfs_bmbt_irec	*imap,
>  	bool			*shared)
>  {
> -	struct xfs_bmbt_irec	got, prev;
> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec	got;
>  	xfs_fileoff_t		end_fsb, orig_end_fsb;
> -	int			eof = 0, error = 0;
> -	bool			trimmed;
> +	int			error = 0;
> +	bool			eof = false, trimmed;
>  	xfs_extnum_t		idx;
>  	xfs_extlen_t		align;
>  
> @@ -258,8 +259,9 @@ xfs_reflink_reserve_cow(
>  	 * extent list is generally faster than going out to the shared extent
>  	 * tree.
>  	 */
> -	xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
> -			&got, &prev);
> +
> +	if (!xfs_iext_lookup_extent(ip, ifp, imap->br_startoff, &idx, &got))
> +		eof = true;
>  	if (!eof && got.br_startoff <= imap->br_startoff) {
>  		trace_xfs_reflink_cow_found(ip, imap);
>  		xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping

[Christoph Hellwig]

Use xfs_iext_lookup_extent to look up the extent, drop a useless check,
drop a unneeded return value and clean up the general style a little bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_aops.c    | 16 ++++++++--------
 fs/xfs/xfs_reflink.c | 30 +++++++++---------------------
 fs/xfs/xfs_reflink.h |  2 +-
 3 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index dd6cacf..ab266d6 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -777,7 +777,7 @@ xfs_map_cow(
 {
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_bmbt_irec	imap;
-	bool			is_cow = false, need_alloc = false;
+	bool			is_cow = false;
 	int			error;
 
 	/*
@@ -795,7 +795,7 @@ xfs_map_cow(
 	 * Else we need to check if there is a COW mapping at this offset.
 	 */
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
-	is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap, &need_alloc);
+	is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap);
 	xfs_iunlock(ip, XFS_ILOCK_SHARED);
 
 	if (!is_cow)
@@ -805,7 +805,7 @@ xfs_map_cow(
 	 * And if the COW mapping has a delayed extent here we need to
 	 * allocate real space for it now.
 	 */
-	if (need_alloc) {
+	if (isnullstartblock(imap.br_startblock)) {
 		error = xfs_iomap_write_allocate(ip, XFS_COW_FORK, offset,
 				&imap);
 		if (error)
@@ -1311,7 +1311,6 @@ __xfs_get_blocks(
 	ssize_t			size;
 	int			new = 0;
 	bool			is_cow = false;
-	bool			need_alloc = false;
 
 	BUG_ON(create && !direct);
 
@@ -1337,9 +1336,11 @@ __xfs_get_blocks(
 	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
-	if (create && direct && xfs_is_reflink_inode(ip))
-		is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap,
-					&need_alloc);
+	if (create && direct && xfs_is_reflink_inode(ip)) {
+		is_cow = xfs_reflink_find_cow_mapping(ip, offset, &imap);
+		ASSERT(!is_cow || !isnullstartblock(imap.br_startblock));
+	}
+
 	if (!is_cow) {
 		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
 					&imap, &nimaps, XFS_BMAPI_ENTIRE);
@@ -1356,7 +1357,6 @@ __xfs_get_blocks(
 			xfs_reflink_trim_irec_to_next_cow(ip, offset_fsb,
 					&imap);
 	}
-	ASSERT(!need_alloc);
 	if (error)
 		goto out_unlock;
 
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 35e02ce..6056fd1 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -427,37 +427,25 @@ bool
 xfs_reflink_find_cow_mapping(
 	struct xfs_inode		*ip,
 	xfs_off_t			offset,
-	struct xfs_bmbt_irec		*imap,
-	bool				*need_alloc)
+	struct xfs_bmbt_irec		*imap)
 {
-	struct xfs_bmbt_irec		irec;
-	struct xfs_ifork		*ifp;
-	struct xfs_bmbt_rec_host	*gotp;
-	xfs_fileoff_t			bno;
+	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	xfs_fileoff_t			offset_fsb;
+	struct xfs_bmbt_irec		got;
 	xfs_extnum_t			idx;
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
 	ASSERT(xfs_is_reflink_inode(ip));
 
-	/* Find the extent in the CoW fork. */
-	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
-	bno = XFS_B_TO_FSBT(ip->i_mount, offset);
-	gotp = xfs_iext_bno_to_ext(ifp, bno, &idx);
-	if (!gotp)
+	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
+	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
 		return false;
-
-	xfs_bmbt_get_all(gotp, &irec);
-	if (bno >= irec.br_startoff + irec.br_blockcount ||
-	    bno < irec.br_startoff)
+	if (got.br_startoff > offset_fsb)
 		return false;
 
 	trace_xfs_reflink_find_cow_mapping(ip, offset, 1, XFS_IO_OVERWRITE,
-			&irec);
-
-	/* If it's still delalloc, we must allocate later. */
-	*imap = irec;
-	*need_alloc = !!(isnullstartblock(irec.br_startblock));
-
+			&got);
+	*imap = got;
 	return true;
 }
 
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 97ea9b4..cff5fc3 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -31,7 +31,7 @@ extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
 extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
 		xfs_off_t offset, xfs_off_t count);
 extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
-		struct xfs_bmbt_irec *imap, bool *need_alloc);
+		struct xfs_bmbt_irec *imap);
 extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
 		xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
 
-- 
2.1.4

Re: [PATCH 09/14] xfs: cleanup xfs_reflink_find_cow_mapping

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:40PM +0100, Christoph Hellwig wrote:
> Use xfs_iext_lookup_extent to look up the extent, drop a useless check,
> drop a unneeded return value and clean up the general style a little bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_aops.c    | 16 ++++++++--------
>  fs/xfs/xfs_reflink.c | 30 +++++++++---------------------
>  fs/xfs/xfs_reflink.h |  2 +-
>  3 files changed, 18 insertions(+), 30 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 35e02ce..6056fd1 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -427,37 +427,25 @@ bool
>  xfs_reflink_find_cow_mapping(
>  	struct xfs_inode		*ip,
>  	xfs_off_t			offset,
> -	struct xfs_bmbt_irec		*imap,
> -	bool				*need_alloc)
> +	struct xfs_bmbt_irec		*imap)

The comment above the function needs fixing as well. Otherwise:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  {
> -	struct xfs_bmbt_irec		irec;
> -	struct xfs_ifork		*ifp;
> -	struct xfs_bmbt_rec_host	*gotp;
> -	xfs_fileoff_t			bno;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	xfs_fileoff_t			offset_fsb;
> +	struct xfs_bmbt_irec		got;
>  	xfs_extnum_t			idx;
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
>  	ASSERT(xfs_is_reflink_inode(ip));
>  
> -	/* Find the extent in the CoW fork. */
> -	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> -	bno = XFS_B_TO_FSBT(ip->i_mount, offset);
> -	gotp = xfs_iext_bno_to_ext(ifp, bno, &idx);
> -	if (!gotp)
> +	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
> +	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
>  		return false;
> -
> -	xfs_bmbt_get_all(gotp, &irec);
> -	if (bno >= irec.br_startoff + irec.br_blockcount ||
> -	    bno < irec.br_startoff)
> +	if (got.br_startoff > offset_fsb)
>  		return false;
>  
>  	trace_xfs_reflink_find_cow_mapping(ip, offset, 1, XFS_IO_OVERWRITE,
> -			&irec);
> -
> -	/* If it's still delalloc, we must allocate later. */
> -	*imap = irec;
> -	*need_alloc = !!(isnullstartblock(irec.br_startblock));
> -
> +			&got);
> +	*imap = got;
>  	return true;
>  }
>  
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index 97ea9b4..cff5fc3 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -31,7 +31,7 @@ extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
>  extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
>  		xfs_off_t offset, xfs_off_t count);
>  extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
> -		struct xfs_bmbt_irec *imap, bool *need_alloc);
> +		struct xfs_bmbt_irec *imap);
>  extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
>  		xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow

[Christoph Hellwig]

And remove the unused return value.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 33 ++++++++++++---------------------
 fs/xfs/xfs_reflink.h |  2 +-
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 6056fd1..0668490 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -452,43 +452,34 @@ xfs_reflink_find_cow_mapping(
 /*
  * Trim an extent to end at the next CoW reservation past offset_fsb.
  */
-int
+void
 xfs_reflink_trim_irec_to_next_cow(
 	struct xfs_inode		*ip,
 	xfs_fileoff_t			offset_fsb,
 	struct xfs_bmbt_irec		*imap)
 {
-	struct xfs_bmbt_irec		irec;
-	struct xfs_ifork		*ifp;
-	struct xfs_bmbt_rec_host	*gotp;
+	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	struct xfs_bmbt_irec		got;
 	xfs_extnum_t			idx;
 
 	if (!xfs_is_reflink_inode(ip))
-		return 0;
+		return;
 
 	/* Find the extent in the CoW fork. */
-	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
-	gotp = xfs_iext_bno_to_ext(ifp, offset_fsb, &idx);
-	if (!gotp)
-		return 0;
-	xfs_bmbt_get_all(gotp, &irec);
+	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
+		return;
 
 	/* This is the extent before; try sliding up one. */
-	if (irec.br_startoff < offset_fsb) {
-		idx++;
-		if (idx >= xfs_iext_count(ifp))
-			return 0;
-		gotp = xfs_iext_get_ext(ifp, idx);
-		xfs_bmbt_get_all(gotp, &irec);
+	if (got.br_startoff < offset_fsb) {
+		if (!xfs_iext_get_extent(ifp, idx + 1, &got))
+			return;
 	}
 
-	if (irec.br_startoff >= imap->br_startoff + imap->br_blockcount)
-		return 0;
+	if (got.br_startoff >= imap->br_startoff + imap->br_blockcount)
+		return;
 
-	imap->br_blockcount = irec.br_startoff - imap->br_startoff;
+	imap->br_blockcount = got.br_startoff - imap->br_startoff;
 	trace_xfs_reflink_trim_irec(ip, imap);
-
-	return 0;
 }
 
 /*
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index cff5fc3..aa6a4d6 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -32,7 +32,7 @@ extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
 		xfs_off_t offset, xfs_off_t count);
 extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
 		struct xfs_bmbt_irec *imap);
-extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
+extern void xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
 		xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
 
 extern int xfs_reflink_cancel_cow_blocks(struct xfs_inode *ip,
-- 
2.1.4

Re: [PATCH 10/14] xfs: use new extent lookup helpers in xfs_reflink_trim_irec_to_next_cow

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:41PM +0100, Christoph Hellwig wrote:
> And remove the unused return value.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c | 33 ++++++++++++---------------------
>  fs/xfs/xfs_reflink.h |  2 +-
>  2 files changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 6056fd1..0668490 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -452,43 +452,34 @@ xfs_reflink_find_cow_mapping(
>  /*
>   * Trim an extent to end at the next CoW reservation past offset_fsb.
>   */
> -int
> +void
>  xfs_reflink_trim_irec_to_next_cow(
>  	struct xfs_inode		*ip,
>  	xfs_fileoff_t			offset_fsb,
>  	struct xfs_bmbt_irec		*imap)
>  {
> -	struct xfs_bmbt_irec		irec;
> -	struct xfs_ifork		*ifp;
> -	struct xfs_bmbt_rec_host	*gotp;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec		got;
>  	xfs_extnum_t			idx;
>  
>  	if (!xfs_is_reflink_inode(ip))
> -		return 0;
> +		return;
>  
>  	/* Find the extent in the CoW fork. */
> -	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> -	gotp = xfs_iext_bno_to_ext(ifp, offset_fsb, &idx);
> -	if (!gotp)
> -		return 0;
> -	xfs_bmbt_get_all(gotp, &irec);
> +	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
> +		return;
>  
>  	/* This is the extent before; try sliding up one. */
> -	if (irec.br_startoff < offset_fsb) {
> -		idx++;
> -		if (idx >= xfs_iext_count(ifp))
> -			return 0;
> -		gotp = xfs_iext_get_ext(ifp, idx);
> -		xfs_bmbt_get_all(gotp, &irec);
> +	if (got.br_startoff < offset_fsb) {
> +		if (!xfs_iext_get_extent(ifp, idx + 1, &got))
> +			return;
>  	}
>  
> -	if (irec.br_startoff >= imap->br_startoff + imap->br_blockcount)
> -		return 0;
> +	if (got.br_startoff >= imap->br_startoff + imap->br_blockcount)
> +		return;
>  
> -	imap->br_blockcount = irec.br_startoff - imap->br_startoff;
> +	imap->br_blockcount = got.br_startoff - imap->br_startoff;
>  	trace_xfs_reflink_trim_irec(ip, imap);
> -
> -	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index cff5fc3..aa6a4d6 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -32,7 +32,7 @@ extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
>  		xfs_off_t offset, xfs_off_t count);
>  extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
>  		struct xfs_bmbt_irec *imap);
> -extern int xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
> +extern void xfs_reflink_trim_irec_to_next_cow(struct xfs_inode *ip,
>  		xfs_fileoff_t offset_fsb, struct xfs_bmbt_irec *imap);
>  
>  extern int xfs_reflink_cancel_cow_blocks(struct xfs_inode *ip,
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 0668490..a878d42 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -493,18 +493,15 @@ xfs_reflink_cancel_cow_blocks(
 	xfs_fileoff_t			end_fsb)
 {
 	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
-	struct xfs_bmbt_irec		got, prev, del;
+	struct xfs_bmbt_irec		got, del;
 	xfs_extnum_t			idx;
 	xfs_fsblock_t			firstfsb;
 	struct xfs_defer_ops		dfops;
-	int				error = 0, eof = 0;
+	int				error = 0;
 
 	if (!xfs_is_reflink_inode(ip))
 		return 0;
-
-	xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
-			&got, &prev);
-	if (eof)
+	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
 		return 0;
 
 	while (got.br_startoff < end_fsb) {
@@ -547,9 +544,8 @@ xfs_reflink_cancel_cow_blocks(
 			xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
 		}
 
-		if (++idx >= xfs_iext_count(ifp))
+		if (!xfs_iext_get_extent(ifp, ++idx, &got))
 			break;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
 	}
 
 	/* clear tag if cow fork is emptied */
-- 
2.1.4

Re: [PATCH 11/14] xfs: use new extent lookup helpers in xfs_reflink_cancel_cow_blocks

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:42PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 0668490..a878d42 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -493,18 +493,15 @@ xfs_reflink_cancel_cow_blocks(
>  	xfs_fileoff_t			end_fsb)
>  {
>  	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> -	struct xfs_bmbt_irec		got, prev, del;
> +	struct xfs_bmbt_irec		got, del;
>  	xfs_extnum_t			idx;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error = 0, eof = 0;
> +	int				error = 0;
>  
>  	if (!xfs_is_reflink_inode(ip))
>  		return 0;
> -
> -	xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
> -			&got, &prev);
> -	if (eof)
> +	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got))
>  		return 0;
>  
>  	while (got.br_startoff < end_fsb) {
> @@ -547,9 +544,8 @@ xfs_reflink_cancel_cow_blocks(
>  			xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
>  		}
>  
> -		if (++idx >= xfs_iext_count(ifp))
> +		if (!xfs_iext_get_extent(ifp, ++idx, &got))
>  			break;
> -		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	/* clear tag if cow fork is emptied */
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index a878d42..4b024e7 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -615,13 +615,13 @@ xfs_reflink_end_cow(
 	xfs_off_t			count)
 {
 	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
-	struct xfs_bmbt_irec		got, prev, del;
+	struct xfs_bmbt_irec		got, del;
 	struct xfs_trans		*tp;
 	xfs_fileoff_t			offset_fsb;
 	xfs_fileoff_t			end_fsb;
 	xfs_fsblock_t			firstfsb;
 	struct xfs_defer_ops		dfops;
-	int				error, eof = 0;
+	int				error;
 	unsigned int			resblks;
 	xfs_filblks_t			rlen;
 	xfs_extnum_t			idx;
@@ -645,13 +645,11 @@ xfs_reflink_end_cow(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
-	xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
-			&got, &prev);
-
 	/* If there is a hole at end_fsb - 1 go to the previous extent */
-	if (eof || got.br_startoff > end_fsb) {
+	if (!xfs_iext_lookup_extent(ip, ifp, end_fsb - 1, &idx, &got) ||
+	    got.br_startoff > end_fsb) {
 		ASSERT(idx > 0);
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
+		xfs_iext_get_extent(ifp, --idx, &got);
 	}
 
 	/* Walk backwards until we're out of the I/O range... */
@@ -699,11 +697,9 @@ xfs_reflink_end_cow(
 		error = xfs_defer_finish(&tp, &dfops, ip);
 		if (error)
 			goto out_defer;
-
 next_extent:
-		if (idx < 0)
+		if (!xfs_iext_get_extent(ifp, idx, &got))
 			break;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
 	}
 
 	error = xfs_trans_commit(tp);
-- 
2.1.4

Re: [PATCH 12/14] xfs: use new extent lookup helpers in xfs_reflink_end_cow

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:43PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index a878d42..4b024e7 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -615,13 +615,13 @@ xfs_reflink_end_cow(
>  	xfs_off_t			count)
>  {
>  	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> -	struct xfs_bmbt_irec		got, prev, del;
> +	struct xfs_bmbt_irec		got, del;
>  	struct xfs_trans		*tp;
>  	xfs_fileoff_t			offset_fsb;
>  	xfs_fileoff_t			end_fsb;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error, eof = 0;
> +	int				error;
>  	unsigned int			resblks;
>  	xfs_filblks_t			rlen;
>  	xfs_extnum_t			idx;
> @@ -645,13 +645,11 @@ xfs_reflink_end_cow(
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ijoin(tp, ip, 0);
>  
> -	xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
> -			&got, &prev);
> -
>  	/* If there is a hole at end_fsb - 1 go to the previous extent */
> -	if (eof || got.br_startoff > end_fsb) {
> +	if (!xfs_iext_lookup_extent(ip, ifp, end_fsb - 1, &idx, &got) ||
> +	    got.br_startoff > end_fsb) {
>  		ASSERT(idx > 0);
> -		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
> +		xfs_iext_get_extent(ifp, --idx, &got);
>  	}
>  
>  	/* Walk backwards until we're out of the I/O range... */
> @@ -699,11 +697,9 @@ xfs_reflink_end_cow(
>  		error = xfs_defer_finish(&tp, &dfops, ip);
>  		if (error)
>  			goto out_defer;
> -
>  next_extent:
> -		if (idx < 0)
> +		if (!xfs_iext_get_extent(ifp, idx, &got))
>  			break;
> -		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	error = xfs_trans_commit(tp);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/14] xfs: remove xfs_bmap_search_extents

[Christoph Hellwig]

Now that all users are gone.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 91 ------------------------------------------------
 fs/xfs/libxfs/xfs_bmap.h |  4 ---
 2 files changed, 95 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4aa9c07..856d98d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1370,97 +1370,6 @@ xfs_bmap_read_extents(
 	return -EFSCORRUPTED;
 }
 
-
-/*
- * Search the extent records for the entry containing block bno.
- * If bno lies in a hole, point to the next entry.  If bno lies
- * past eof, *eofp will be set, and *prevp will contain the last
- * entry (null if none).  Else, *lastxp will be set to the index
- * of the found entry; *gotp will contain the entry.
- */
-STATIC xfs_bmbt_rec_host_t *		/* pointer to found extent entry */
-xfs_bmap_search_multi_extents(
-	xfs_ifork_t	*ifp,		/* inode fork pointer */
-	xfs_fileoff_t	bno,		/* block number searched for */
-	int		*eofp,		/* out: end of file found */
-	xfs_extnum_t	*lastxp,	/* out: last extent index */
-	xfs_bmbt_irec_t	*gotp,		/* out: extent entry found */
-	xfs_bmbt_irec_t	*prevp)		/* out: previous extent entry found */
-{
-	xfs_bmbt_rec_host_t *ep;		/* extent record pointer */
-	xfs_extnum_t	lastx;		/* last extent index */
-
-	/*
-	 * Initialize the extent entry structure to catch access to
-	 * uninitialized br_startblock field.
-	 */
-	gotp->br_startoff = 0xffa5a5a5a5a5a5a5LL;
-	gotp->br_blockcount = 0xa55a5a5a5a5a5a5aLL;
-	gotp->br_state = XFS_EXT_INVALID;
-	gotp->br_startblock = 0xffffa5a5a5a5a5a5LL;
-	prevp->br_startoff = NULLFILEOFF;
-
-	ep = xfs_iext_bno_to_ext(ifp, bno, &lastx);
-	if (lastx > 0) {
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx - 1), prevp);
-	}
-	if (lastx < xfs_iext_count(ifp)) {
-		xfs_bmbt_get_all(ep, gotp);
-		*eofp = 0;
-	} else {
-		if (lastx > 0) {
-			*gotp = *prevp;
-		}
-		*eofp = 1;
-		ep = NULL;
-	}
-	*lastxp = lastx;
-	return ep;
-}
-
-/*
- * Search the extents list for the inode, for the extent containing bno.
- * If bno lies in a hole, point to the next entry.  If bno lies past eof,
- * *eofp will be set, and *prevp will contain the last entry (null if none).
- * Else, *lastxp will be set to the index of the found
- * entry; *gotp will contain the entry.
- */
-xfs_bmbt_rec_host_t *                 /* pointer to found extent entry */
-xfs_bmap_search_extents(
-	xfs_inode_t     *ip,            /* incore inode pointer */
-	xfs_fileoff_t   bno,            /* block number searched for */
-	int             fork,      	/* data or attr fork */
-	int             *eofp,          /* out: end of file found */
-	xfs_extnum_t    *lastxp,        /* out: last extent index */
-	xfs_bmbt_irec_t *gotp,          /* out: extent entry found */
-	xfs_bmbt_irec_t *prevp)         /* out: previous extent entry found */
-{
-	xfs_ifork_t	*ifp;		/* inode fork pointer */
-	xfs_bmbt_rec_host_t  *ep;            /* extent record pointer */
-
-	XFS_STATS_INC(ip->i_mount, xs_look_exlist);
-	ifp = XFS_IFORK_PTR(ip, fork);
-
-	ep = xfs_bmap_search_multi_extents(ifp, bno, eofp, lastxp, gotp, prevp);
-
-	if (unlikely(!(gotp->br_startblock) && (*lastxp != NULLEXTNUM) &&
-		     !(XFS_IS_REALTIME_INODE(ip) && fork == XFS_DATA_FORK))) {
-		xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO,
-				"Access to block zero in inode %llu "
-				"start_block: %llx start_off: %llx "
-				"blkcnt: %llx extent-state: %x lastx: %x",
-			(unsigned long long)ip->i_ino,
-			(unsigned long long)gotp->br_startblock,
-			(unsigned long long)gotp->br_startoff,
-			(unsigned long long)gotp->br_blockcount,
-			gotp->br_state, *lastxp);
-		*lastxp = NULLEXTNUM;
-		*eofp = 1;
-		return NULL;
-	}
-	return ep;
-}
-
 /*
  * Returns the file-relative block number of the first unused block(s)
  * in the file with at least "len" logically contiguous blocks free.
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index e3c2b5a..ffed1f9 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -237,10 +237,6 @@ int	xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
 		struct xfs_defer_ops *dfops, enum shift_direction direction,
 		int num_exts);
 int	xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
-struct xfs_bmbt_rec_host *
-	xfs_bmap_search_extents(struct xfs_inode *ip, xfs_fileoff_t bno,
-		int fork, int *eofp, xfs_extnum_t *lastxp,
-		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
 int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
 		xfs_fileoff_t aoff, xfs_filblks_t len,
 		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
-- 
2.1.4

Re: [PATCH 13/14] xfs: remove xfs_bmap_search_extents

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:44PM +0100, Christoph Hellwig wrote:
> Now that all users are gone.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c | 91 ------------------------------------------------
>  fs/xfs/libxfs/xfs_bmap.h |  4 ---
>  2 files changed, 95 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4aa9c07..856d98d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1370,97 +1370,6 @@ xfs_bmap_read_extents(
>  	return -EFSCORRUPTED;
>  }
>  
> -
> -/*
> - * Search the extent records for the entry containing block bno.
> - * If bno lies in a hole, point to the next entry.  If bno lies
> - * past eof, *eofp will be set, and *prevp will contain the last
> - * entry (null if none).  Else, *lastxp will be set to the index
> - * of the found entry; *gotp will contain the entry.
> - */
> -STATIC xfs_bmbt_rec_host_t *		/* pointer to found extent entry */
> -xfs_bmap_search_multi_extents(
> -	xfs_ifork_t	*ifp,		/* inode fork pointer */
> -	xfs_fileoff_t	bno,		/* block number searched for */
> -	int		*eofp,		/* out: end of file found */
> -	xfs_extnum_t	*lastxp,	/* out: last extent index */
> -	xfs_bmbt_irec_t	*gotp,		/* out: extent entry found */
> -	xfs_bmbt_irec_t	*prevp)		/* out: previous extent entry found */
> -{
> -	xfs_bmbt_rec_host_t *ep;		/* extent record pointer */
> -	xfs_extnum_t	lastx;		/* last extent index */
> -
> -	/*
> -	 * Initialize the extent entry structure to catch access to
> -	 * uninitialized br_startblock field.
> -	 */
> -	gotp->br_startoff = 0xffa5a5a5a5a5a5a5LL;
> -	gotp->br_blockcount = 0xa55a5a5a5a5a5a5aLL;
> -	gotp->br_state = XFS_EXT_INVALID;
> -	gotp->br_startblock = 0xffffa5a5a5a5a5a5LL;
> -	prevp->br_startoff = NULLFILEOFF;
> -
> -	ep = xfs_iext_bno_to_ext(ifp, bno, &lastx);
> -	if (lastx > 0) {
> -		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx - 1), prevp);
> -	}
> -	if (lastx < xfs_iext_count(ifp)) {
> -		xfs_bmbt_get_all(ep, gotp);
> -		*eofp = 0;
> -	} else {
> -		if (lastx > 0) {
> -			*gotp = *prevp;
> -		}
> -		*eofp = 1;
> -		ep = NULL;
> -	}
> -	*lastxp = lastx;
> -	return ep;
> -}
> -
> -/*
> - * Search the extents list for the inode, for the extent containing bno.
> - * If bno lies in a hole, point to the next entry.  If bno lies past eof,
> - * *eofp will be set, and *prevp will contain the last entry (null if none).
> - * Else, *lastxp will be set to the index of the found
> - * entry; *gotp will contain the entry.
> - */
> -xfs_bmbt_rec_host_t *                 /* pointer to found extent entry */
> -xfs_bmap_search_extents(
> -	xfs_inode_t     *ip,            /* incore inode pointer */
> -	xfs_fileoff_t   bno,            /* block number searched for */
> -	int             fork,      	/* data or attr fork */
> -	int             *eofp,          /* out: end of file found */
> -	xfs_extnum_t    *lastxp,        /* out: last extent index */
> -	xfs_bmbt_irec_t *gotp,          /* out: extent entry found */
> -	xfs_bmbt_irec_t *prevp)         /* out: previous extent entry found */
> -{
> -	xfs_ifork_t	*ifp;		/* inode fork pointer */
> -	xfs_bmbt_rec_host_t  *ep;            /* extent record pointer */
> -
> -	XFS_STATS_INC(ip->i_mount, xs_look_exlist);
> -	ifp = XFS_IFORK_PTR(ip, fork);
> -
> -	ep = xfs_bmap_search_multi_extents(ifp, bno, eofp, lastxp, gotp, prevp);
> -
> -	if (unlikely(!(gotp->br_startblock) && (*lastxp != NULLEXTNUM) &&
> -		     !(XFS_IS_REALTIME_INODE(ip) && fork == XFS_DATA_FORK))) {
> -		xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO,
> -				"Access to block zero in inode %llu "
> -				"start_block: %llx start_off: %llx "
> -				"blkcnt: %llx extent-state: %x lastx: %x",
> -			(unsigned long long)ip->i_ino,
> -			(unsigned long long)gotp->br_startblock,
> -			(unsigned long long)gotp->br_startoff,
> -			(unsigned long long)gotp->br_blockcount,
> -			gotp->br_state, *lastxp);
> -		*lastxp = NULLEXTNUM;
> -		*eofp = 1;
> -		return NULL;
> -	}
> -	return ep;
> -}
> -
>  /*
>   * Returns the file-relative block number of the first unused block(s)
>   * in the file with at least "len" logically contiguous blocks free.
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index e3c2b5a..ffed1f9 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -237,10 +237,6 @@ int	xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
>  		struct xfs_defer_ops *dfops, enum shift_direction direction,
>  		int num_exts);
>  int	xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
> -struct xfs_bmbt_rec_host *
> -	xfs_bmap_search_extents(struct xfs_inode *ip, xfs_fileoff_t bno,
> -		int fork, int *eofp, xfs_extnum_t *lastxp,
> -		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
>  int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
>  		xfs_fileoff_t aoff, xfs_filblks_t len,
>  		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/14] xfs: remove NULLEXTNUM

[Christoph Hellwig]

We only ever set a field to this constant for an impossible to reach
error case in xfs_bmap_search_extents.  That functions has been removed,
so we can remove the constant as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c  | 2 +-
 fs/xfs/libxfs/xfs_types.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 856d98d..3ab68db 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4260,7 +4260,7 @@ xfs_bmapi_allocate(
 	if (bma->wasdel) {
 		bma->length = (xfs_extlen_t)bma->got.br_blockcount;
 		bma->offset = bma->got.br_startoff;
-		if (bma->idx != NULLEXTNUM && bma->idx) {
+		if (bma->idx) {
 			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx - 1),
 					 &bma->prev);
 		}
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index cf044c0..717909f 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -57,7 +57,6 @@ typedef __int64_t	xfs_sfiloff_t;	/* signed block number in a file */
 
 #define	NULLAGBLOCK	((xfs_agblock_t)-1)
 #define	NULLAGNUMBER	((xfs_agnumber_t)-1)
-#define	NULLEXTNUM	((xfs_extnum_t)-1)
 
 #define NULLCOMMITLSN	((xfs_lsn_t)-1)
 
-- 
2.1.4

Re: [PATCH 14/14] xfs: remove NULLEXTNUM

[Brian Foster]

On Mon, Nov 14, 2016 at 06:12:45PM +0100, Christoph Hellwig wrote:
> We only ever set a field to this constant for an impossible to reach
> error case in xfs_bmap_search_extents.  That functions has been removed,
> so we can remove the constant as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c  | 2 +-
>  fs/xfs/libxfs/xfs_types.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 856d98d..3ab68db 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4260,7 +4260,7 @@ xfs_bmapi_allocate(
>  	if (bma->wasdel) {
>  		bma->length = (xfs_extlen_t)bma->got.br_blockcount;
>  		bma->offset = bma->got.br_startoff;
> -		if (bma->idx != NULLEXTNUM && bma->idx) {
> +		if (bma->idx) {
>  			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx - 1),
>  					 &bma->prev);
>  		}
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index cf044c0..717909f 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -57,7 +57,6 @@ typedef __int64_t	xfs_sfiloff_t;	/* signed block number in a file */
>  
>  #define	NULLAGBLOCK	((xfs_agblock_t)-1)
>  #define	NULLAGNUMBER	((xfs_agnumber_t)-1)
> -#define	NULLEXTNUM	((xfs_extnum_t)-1)
>  
>  #define NULLCOMMITLSN	((xfs_lsn_t)-1)
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: new helpers to clean up extent tree lookups

[Eric Sandeen]

On 11/14/16 11:12 AM, Christoph Hellwig wrote:
> Now that Eric send out xfs_iext_count before I could get to it I need
> to get my other iext helpers out ASAP before running into another
> rebase :)

I could have deferred if you had spoken up ;)

(I did think that iext_count probably wasn't the end of it)

-Eric

> This series adds two new helpers that make iterating the extent list
> a lot cleaner.  They are the first step towards better abstracting out
> access to the extent list and thus allowing us to experiment with
> better data structures for it.  The next step is the manipulations
> in xfs_bmap_{add,del}_extent*, which will be more work than these
> helpers, but I have some of that in progress.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: new helpers to clean up extent tree lookups

[Christoph Hellwig]

On Fri, Nov 18, 2016 at 06:21:32PM -0600, Eric Sandeen wrote:
> On 11/14/16 11:12 AM, Christoph Hellwig wrote:
> > Now that Eric send out xfs_iext_count before I could get to it I need
> > to get my other iext helpers out ASAP before running into another
> > rebase :)
> 
> I could have deferred if you had spoken up ;)
> 
> (I did think that iext_count probably wasn't the end of it)

Np - your patch was a good reminder that I should finally send this
series out.

new helpers to clean up extent tree lookups V2

[Christoph Hellwig]

Now that Eric send out xfs_iext_count before I could get to it I need
to get my other iext helpers out ASAP before running into another
rebase :)

This series adds two new helpers that make iterating the extent list
a lot cleaner.  They are the first step towards better abstracting out
access to the extent list and thus allowing us to experiment with
better data structures for it.  The next step is the manipulations
in xfs_bmap_{add,del}_extent*, which will be more work than these
helpers, but I have some of that in progress.

Changes since V1:
 - incorporate review feedback from Brian (various minor items)

[PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Christoph Hellwig]

We can easily lookup the previous extent for the cases where we need it,
which saves the callers from looking it up for us later in the series.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3ed2157..f5de137 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
 	xfs_fileoff_t		aoff,
 	xfs_filblks_t		len,
 	struct xfs_bmbt_irec	*got,
-	struct xfs_bmbt_irec	*prev,
 	xfs_extnum_t		*lastx,
 	int			eof)
 {
@@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
 	else
 		extsz = xfs_get_extsz_hint(ip);
 	if (extsz) {
-		error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
+		struct xfs_bmbt_irec	prev;
+
+		if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
+			prev.br_startoff = NULLFILEOFF;
+
+		error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
 					       1, 0, &aoff, &alen);
 		ASSERT(!error);
 	}
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 7cae6ec..e3c2b5a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
 		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
 int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
 		xfs_fileoff_t aoff, xfs_filblks_t len,
-		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
-		xfs_extnum_t *lastx, int eof);
+		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
 
 enum xfs_bmap_intent_type {
 	XFS_BMAP_MAP = 1,
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 436e109..59ffcac 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
 
 retry:
 	error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
-			end_fsb - offset_fsb, &got,
-			&prev, &idx, eof);
+			end_fsb - offset_fsb, &got, &idx, eof);
 	switch (error) {
 	case 0:
 		break;
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 0edf835..52cdfba 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
 
 retry:
 	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
-			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
+			end_fsb - imap->br_startoff, &got, &idx, eof);
 	switch (error) {
 	case 0:
 		break;
-- 
2.1.4

Re: [PATCH 06/14] xfs: remove prev argument to xfs_bmapi_reserve_delalloc

[Brian Foster]

On Mon, Nov 21, 2016 at 05:38:48PM +0100, Christoph Hellwig wrote:
> We can easily lookup the previous extent for the cases where we need it,
> which saves the callers from looking it up for us later in the series.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c | 8 ++++++--
>  fs/xfs/libxfs/xfs_bmap.h | 3 +--
>  fs/xfs/xfs_iomap.c       | 3 +--
>  fs/xfs/xfs_reflink.c     | 2 +-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 3ed2157..f5de137 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4235,7 +4235,6 @@ xfs_bmapi_reserve_delalloc(
>  	xfs_fileoff_t		aoff,
>  	xfs_filblks_t		len,
>  	struct xfs_bmbt_irec	*got,
> -	struct xfs_bmbt_irec	*prev,
>  	xfs_extnum_t		*lastx,
>  	int			eof)
>  {
> @@ -4257,7 +4256,12 @@ xfs_bmapi_reserve_delalloc(
>  	else
>  		extsz = xfs_get_extsz_hint(ip);
>  	if (extsz) {
> -		error = xfs_bmap_extsize_align(mp, got, prev, extsz, rt, eof,
> +		struct xfs_bmbt_irec	prev;
> +
> +		if (!xfs_iext_get_extent(ifp, *lastx - 1, &prev))
> +			prev.br_startoff = NULLFILEOFF;
> +
> +		error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
>  					       1, 0, &aoff, &alen);
>  		ASSERT(!error);
>  	}
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 7cae6ec..e3c2b5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -243,8 +243,7 @@ struct xfs_bmbt_rec_host *
>  		struct xfs_bmbt_irec *gotp, struct xfs_bmbt_irec *prevp);
>  int	xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
>  		xfs_fileoff_t aoff, xfs_filblks_t len,
> -		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *prev,
> -		xfs_extnum_t *lastx, int eof);
> +		struct xfs_bmbt_irec *got, xfs_extnum_t *lastx, int eof);
>  
>  enum xfs_bmap_intent_type {
>  	XFS_BMAP_MAP = 1,
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 436e109..59ffcac 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -622,8 +622,7 @@ xfs_file_iomap_begin_delay(
>  
>  retry:
>  	error = xfs_bmapi_reserve_delalloc(ip, XFS_DATA_FORK, offset_fsb,
> -			end_fsb - offset_fsb, &got,
> -			&prev, &idx, eof);
> +			end_fsb - offset_fsb, &got, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 0edf835..52cdfba 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -293,7 +293,7 @@ xfs_reflink_reserve_cow(
>  
>  retry:
>  	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> -			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
> +			end_fsb - imap->br_startoff, &got, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html