All of lore.kernel.org
 help / color / mirror / Atom feed
* refactor extent manipulation V4
@ 2017-09-22 13:59 Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 01/19] xfs: fix incorrect extent state in xfs_bmap_add_extent_unwritten_real Christoph Hellwig
                   ` (18 more replies)
  0 siblings, 19 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Hi all,

this series refactor the extent manipulation routines so that they are
isolated from the nitty gritty details of the extent list implementation.
It is the first step towards a more scalable extent list data structure.

Changes since V3:
 - fix xfs_bmap_add_extent_delay_real once again
 - trivial indentation fixup

Changes since V2:
 - fix xfs_bmap_add_extent_delay_real again
 - new patch to refactor delalloc accouting in xfs_bmap_add_extent_delay_real

Changes since V1:
 - use better fitting XFS_BMAP_* constant in xfs_bmap_del_extent_*
 - various fixes and cleanups in xfs_bmap_add_extent_delay_real

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 01/19] xfs: fix incorrect extent state in xfs_bmap_add_extent_unwritten_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 02/19] xfs: use xfs_iext_get_extent instead of open coding it Christoph Hellwig
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

There was one spot in xfs_bmap_add_extent_unwritten_real that didn't use the
passed in new extent state but always converted to normal, leading to wrong
behavior when converting from normal to unwritten.

Only found by code inspection, it seems like this code path to move partial
extent from written to unwritten while merging it with the next extent is
rarely exercised.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 459f4b4f08fe..f2f5bdb9e092 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2573,7 +2573,7 @@ xfs_bmap_add_extent_unwritten_real(
 					&i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			cur->bc_rec.b.br_state = XFS_EXT_NORM;
+			cur->bc_rec.b.br_state = new->br_state;
 			if ((error = xfs_btree_insert(cur, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 02/19] xfs: use xfs_iext_get_extent instead of open coding it
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 01/19] xfs: fix incorrect extent state in xfs_bmap_add_extent_unwritten_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 03/19] xfs: don't set XFS_BTCUR_BPRV_WASDEL in xfs_bunmapi Christoph Hellwig
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

This avoids exposure to details of the extent list implementation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 36 ++++++++++++++++--------------------
 fs/xfs/xfs_trace.h       |  2 +-
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index f2f5bdb9e092..555ff198e28c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1467,7 +1467,7 @@ xfs_bmap_last_extent(
 		return 0;
 	}
 
-	xfs_bmbt_get_all(xfs_iext_get_ext(ifp, nextents - 1), rec);
+	xfs_iext_get_extent(ifp, nextents - 1, rec);
 	*is_empty = 0;
 	return 0;
 }
@@ -1553,7 +1553,6 @@ xfs_bmap_one_block(
 	xfs_inode_t	*ip,		/* incore inode */
 	int		whichfork)	/* data or attr fork */
 {
-	xfs_bmbt_rec_host_t *ep;	/* ptr to fork's extent */
 	xfs_ifork_t	*ifp;		/* inode fork pointer */
 	int		rval;		/* return value */
 	xfs_bmbt_irec_t	s;		/* internal version of extent */
@@ -1568,8 +1567,7 @@ xfs_bmap_one_block(
 		return 0;
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
-	ep = xfs_iext_get_ext(ifp, 0);
-	xfs_bmbt_get_all(ep, &s);
+	xfs_iext_get_extent(ifp, 0, &s);
 	rval = s.br_startoff == 0 && s.br_blockcount == 1;
 	if (rval && whichfork == XFS_DATA_FORK)
 		ASSERT(XFS_ISIZE(ip) == ip->i_mount->m_sb.sb_blocksize);
@@ -1655,7 +1653,7 @@ xfs_bmap_add_extent_delay_real(
 	 */
 	if (bma->idx > 0) {
 		state |= BMAP_LEFT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx - 1), &LEFT);
+		xfs_iext_get_extent(ifp, bma->idx - 1, &LEFT);
 
 		if (isnullstartblock(LEFT.br_startblock))
 			state |= BMAP_LEFT_DELAY;
@@ -1675,7 +1673,7 @@ xfs_bmap_add_extent_delay_real(
 	 */
 	if (bma->idx < xfs_iext_count(ifp) - 1) {
 		state |= BMAP_RIGHT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx + 1), &RIGHT);
+		xfs_iext_get_extent(ifp, bma->idx + 1, &RIGHT);
 
 		if (isnullstartblock(RIGHT.br_startblock))
 			state |= BMAP_RIGHT_DELAY;
@@ -2222,7 +2220,7 @@ xfs_bmap_add_extent_unwritten_real(
 	 */
 	if (*idx > 0) {
 		state |= BMAP_LEFT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &LEFT);
+		xfs_iext_get_extent(ifp, *idx - 1, &LEFT);
 
 		if (isnullstartblock(LEFT.br_startblock))
 			state |= BMAP_LEFT_DELAY;
@@ -2242,7 +2240,7 @@ xfs_bmap_add_extent_unwritten_real(
 	 */
 	if (*idx < xfs_iext_count(ifp) - 1) {
 		state |= BMAP_RIGHT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx + 1), &RIGHT);
+		xfs_iext_get_extent(ifp, *idx + 1, &RIGHT);
 		if (isnullstartblock(RIGHT.br_startblock))
 			state |= BMAP_RIGHT_DELAY;
 	}
@@ -2716,7 +2714,7 @@ xfs_bmap_add_extent_hole_delay(
 	 */
 	if (*idx > 0) {
 		state |= BMAP_LEFT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left);
+		xfs_iext_get_extent(ifp, *idx - 1, &left);
 
 		if (isnullstartblock(left.br_startblock))
 			state |= BMAP_LEFT_DELAY;
@@ -2728,7 +2726,7 @@ xfs_bmap_add_extent_hole_delay(
 	 */
 	if (*idx < xfs_iext_count(ifp)) {
 		state |= BMAP_RIGHT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right);
+		xfs_iext_get_extent(ifp, *idx, &right);
 
 		if (isnullstartblock(right.br_startblock))
 			state |= BMAP_RIGHT_DELAY;
@@ -2880,7 +2878,7 @@ xfs_bmap_add_extent_hole_real(
 	 */
 	if (*idx > 0) {
 		state |= BMAP_LEFT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left);
+		xfs_iext_get_extent(ifp, *idx - 1, &left);
 		if (isnullstartblock(left.br_startblock))
 			state |= BMAP_LEFT_DELAY;
 	}
@@ -2891,7 +2889,7 @@ xfs_bmap_add_extent_hole_real(
 	 */
 	if (*idx < xfs_iext_count(ifp)) {
 		state |= BMAP_RIGHT_VALID;
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right);
+		xfs_iext_get_extent(ifp, *idx, &right);
 		if (isnullstartblock(right.br_startblock))
 			state |= BMAP_RIGHT_DELAY;
 	}
@@ -4209,10 +4207,8 @@ xfs_bmapi_allocate(
 	if (bma->wasdel) {
 		bma->length = (xfs_extlen_t)bma->got.br_blockcount;
 		bma->offset = bma->got.br_startoff;
-		if (bma->idx) {
-			xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx - 1),
-					 &bma->prev);
-		}
+		if (bma->idx)
+			xfs_iext_get_extent(ifp, bma->idx - 1, &bma->prev);
 	} else {
 		bma->length = XFS_FILBLKS_MIN(bma->length, MAXEXTLEN);
 		if (!bma->eof)
@@ -4309,7 +4305,7 @@ xfs_bmapi_allocate(
 	 * or xfs_bmap_add_extent_hole_real might have merged it into one of
 	 * the neighbouring ones.
 	 */
-	xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx), &bma->got);
+	xfs_iext_get_extent(ifp, bma->idx, &bma->got);
 
 	ASSERT(bma->got.br_startoff <= bma->offset);
 	ASSERT(bma->got.br_startoff + bma->got.br_blockcount >=
@@ -4390,7 +4386,7 @@ xfs_bmapi_convert_unwritten(
 	 * xfs_bmap_add_extent_unwritten_real might have merged it into one
 	 * of the neighbouring ones.
 	 */
-	xfs_bmbt_get_all(xfs_iext_get_ext(ifp, bma->idx), &bma->got);
+	xfs_iext_get_extent(ifp, bma->idx, &bma->got);
 
 	/*
 	 * We may have combined previously unwritten space with written space,
@@ -5589,8 +5585,8 @@ __xfs_bunmapi(
 					del.br_blockcount : mod;
 				if (bno < got.br_startoff) {
 					if (--lastx >= 0)
-						xfs_bmbt_get_all(xfs_iext_get_ext(
-							ifp, lastx), &got);
+						xfs_iext_get_extent(ifp, lastx,
+								&got);
 				}
 				continue;
 			}
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index bb5514688d47..0a8999a310b9 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -277,7 +277,7 @@ DECLARE_EVENT_CLASS(xfs_bmap_class,
 		struct xfs_bmbt_irec	r;
 
 		ifp = xfs_iext_state_to_fork(ip, state);
-		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &r);
+		xfs_iext_get_extent(ifp, idx, &r);
 		__entry->dev = VFS_I(ip)->i_sb->s_dev;
 		__entry->ino = ip->i_ino;
 		__entry->idx = idx;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 03/19] xfs: don't set XFS_BTCUR_BPRV_WASDEL in xfs_bunmapi
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 01/19] xfs: fix incorrect extent state in xfs_bmap_add_extent_unwritten_real Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 02/19] xfs: use xfs_iext_get_extent instead of open coding it Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 04/19] xfs: rename bno to end in __xfs_bunmapi Christoph Hellwig
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

The XFS_BTCUR_BPRV_WASDEL flag is supposed to indicate that we are
converting a delayed allocation to a real one, which isn't the case
in xfs_bunmapi.  Setting it could theoretically lead to misaccounting
here, but it's unlikely that we ever hit it in practice.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 555ff198e28c..ef30b52c32af 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5725,11 +5725,7 @@ __xfs_bunmapi(
 					XFS_QMOPT_RES_REGBLKS);
 			}
 			ip->i_delayed_blks -= del.br_blockcount;
-			if (cur)
-				cur->bc_private.b.flags |=
-					XFS_BTCUR_BPRV_WASDEL;
-		} else if (cur)
-			cur->bc_private.b.flags &= ~XFS_BTCUR_BPRV_WASDEL;
+		}
 
 		error = xfs_bmap_del_extent(ip, tp, &lastx, dfops, cur, &del,
 				&tmp_logflags, whichfork, flags);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 04/19] xfs: rename bno to end in __xfs_bunmapi
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 03/19] xfs: don't set XFS_BTCUR_BPRV_WASDEL in xfs_bunmapi Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 05/19] xfs: use xfs_bmap_del_extent_delay for the data fork as well Christoph Hellwig
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Rename the bno variable that's used as the end of the range in
__xfs_bunmapi to end, which better describes it.  Additionally change
the start variable which takes the initial value of bno to be the
function parameter itself.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 49 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ef30b52c32af..15650bb7d881 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5410,7 +5410,7 @@ int						/* error */
 __xfs_bunmapi(
 	xfs_trans_t		*tp,		/* transaction pointer */
 	struct xfs_inode	*ip,		/* incore inode */
-	xfs_fileoff_t		bno,		/* starting offset to unmap */
+	xfs_fileoff_t		start,		/* first file offset deleted */
 	xfs_filblks_t		*rlen,		/* i/o: amount remaining */
 	int			flags,		/* misc flags */
 	xfs_extnum_t		nexts,		/* number of extents max */
@@ -5429,7 +5429,6 @@ __xfs_bunmapi(
 	int			logflags;	/* transaction logging flags */
 	xfs_extlen_t		mod;		/* rt extent offset */
 	xfs_mount_t		*mp;		/* mount structure */
-	xfs_fileoff_t		start;		/* first file offset deleted */
 	int			tmp_logflags;	/* partial logging flags */
 	int			wasdel;		/* was a delayed alloc extent */
 	int			whichfork;	/* data or attribute fork */
@@ -5437,8 +5436,9 @@ __xfs_bunmapi(
 	xfs_filblks_t		len = *rlen;	/* length to unmap in file */
 	xfs_fileoff_t		max_len;
 	xfs_agnumber_t		prev_agno = NULLAGNUMBER, agno;
+	xfs_fileoff_t		end;
 
-	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
+	trace_xfs_bunmap(ip, start, len, flags, _RET_IP_);
 
 	whichfork = xfs_bmapi_whichfork(flags);
 	ASSERT(whichfork != XFS_COW_FORK);
@@ -5477,17 +5477,16 @@ __xfs_bunmapi(
 	}
 	XFS_STATS_INC(mp, xs_blk_unmap);
 	isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
-	start = bno;
-	bno = start + len - 1;
+	end = start + len - 1;
 
 	/*
 	 * Check to see if the given block number is past the end of the
 	 * file, back up to the last block if so...
 	 */
-	if (!xfs_iext_lookup_extent(ip, ifp, bno, &lastx, &got)) {
+	if (!xfs_iext_lookup_extent(ip, ifp, end, &lastx, &got)) {
 		ASSERT(lastx > 0);
 		xfs_iext_get_extent(ifp, --lastx, &got);
-		bno = got.br_startoff + got.br_blockcount - 1;
+		end = got.br_startoff + got.br_blockcount - 1;
 	}
 
 	logflags = 0;
@@ -5511,13 +5510,13 @@ __xfs_bunmapi(
 	}
 
 	extno = 0;
-	while (bno != (xfs_fileoff_t)-1 && bno >= start && lastx >= 0 &&
+	while (end != (xfs_fileoff_t)-1 && end >= start && lastx >= 0 &&
 	       (nexts == 0 || extno < nexts) && max_len > 0) {
 		/*
-		 * Is the found extent after a hole in which bno lives?
+		 * Is the found extent after a hole in which end lives?
 		 * Just back up to the previous extent, if so.
 		 */
-		if (got.br_startoff > bno) {
+		if (got.br_startoff > end) {
 			if (--lastx < 0)
 				break;
 			xfs_iext_get_extent(ifp, lastx, &got);
@@ -5526,9 +5525,9 @@ __xfs_bunmapi(
 		 * Is the last block of this extent before the range
 		 * we're supposed to delete?  If so, we're done.
 		 */
-		bno = XFS_FILEOFF_MIN(bno,
+		end = XFS_FILEOFF_MIN(end,
 			got.br_startoff + got.br_blockcount - 1);
-		if (bno < start)
+		if (end < start)
 			break;
 		/*
 		 * Then deal with the (possibly delayed) allocated space
@@ -5553,8 +5552,8 @@ __xfs_bunmapi(
 			if (!wasdel)
 				del.br_startblock += start - got.br_startoff;
 		}
-		if (del.br_startoff + del.br_blockcount > bno + 1)
-			del.br_blockcount = bno + 1 - del.br_startoff;
+		if (del.br_startoff + del.br_blockcount > end + 1)
+			del.br_blockcount = end + 1 - del.br_startoff;
 
 		/* How much can we safely unmap? */
 		if (max_len < del.br_blockcount) {
@@ -5580,10 +5579,10 @@ __xfs_bunmapi(
 				 * This piece is unwritten, or we're not
 				 * using unwritten extents.  Skip over it.
 				 */
-				ASSERT(bno >= mod);
-				bno -= mod > del.br_blockcount ?
+				ASSERT(end >= mod);
+				end -= mod > del.br_blockcount ?
 					del.br_blockcount : mod;
-				if (bno < got.br_startoff) {
+				if (end < got.br_startoff) {
 					if (--lastx >= 0)
 						xfs_iext_get_extent(ifp, lastx,
 								&got);
@@ -5632,9 +5631,9 @@ __xfs_bunmapi(
 				 * Can't make it unwritten.  There isn't
 				 * a full extent here so just skip it.
 				 */
-				ASSERT(bno >= del.br_blockcount);
-				bno -= del.br_blockcount;
-				if (got.br_startoff > bno && --lastx >= 0)
+				ASSERT(end >= del.br_blockcount);
+				end -= del.br_blockcount;
+				if (got.br_startoff > end && --lastx >= 0)
 					xfs_iext_get_extent(ifp, lastx, &got);
 				continue;
 			} else if (del.br_state == XFS_EXT_UNWRITTEN) {
@@ -5737,24 +5736,24 @@ __xfs_bunmapi(
 			xfs_mod_fdblocks(mp, (int64_t)del.br_blockcount, false);
 
 		max_len -= del.br_blockcount;
-		bno = del.br_startoff - 1;
+		end = del.br_startoff - 1;
 nodelete:
 		/*
 		 * If not done go on to the next (previous) record.
 		 */
-		if (bno != (xfs_fileoff_t)-1 && bno >= start) {
+		if (end != (xfs_fileoff_t)-1 && end >= start) {
 			if (lastx >= 0) {
 				xfs_iext_get_extent(ifp, lastx, &got);
-				if (got.br_startoff > bno && --lastx >= 0)
+				if (got.br_startoff > end && --lastx >= 0)
 					xfs_iext_get_extent(ifp, lastx, &got);
 			}
 			extno++;
 		}
 	}
-	if (bno == (xfs_fileoff_t)-1 || bno < start || lastx < 0)
+	if (end == (xfs_fileoff_t)-1 || end < start || lastx < 0)
 		*rlen = 0;
 	else
-		*rlen = bno - start + 1;
+		*rlen = end - start + 1;
 
 	/*
 	 * Convert to a btree if necessary.
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 05/19] xfs: use xfs_bmap_del_extent_delay for the data fork as well
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 04/19] xfs: rename bno to end in __xfs_bunmapi Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 06/19] xfs: move some more code into xfs_bmap_del_extent_real Christoph Hellwig
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

And remove the delalloc code from xfs_bmap_del_extent, which gets renamed
to xfs_bmap_del_extent_real to fit the naming scheme used by the other
xfs_bmap_{add,del}_extent_* routines.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 331 ++++++++++++++++-------------------------------
 1 file changed, 114 insertions(+), 217 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 15650bb7d881..4e6c4cc4168f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5061,10 +5061,10 @@ xfs_bmap_del_extent_cow(
 
 /*
  * Called by xfs_bmapi to update file extent records and the btree
- * after removing space (or undoing a delayed allocation).
+ * after removing space.
  */
 STATIC int				/* error */
-xfs_bmap_del_extent(
+xfs_bmap_del_extent_real(
 	xfs_inode_t		*ip,	/* incore inode pointer */
 	xfs_trans_t		*tp,	/* current transaction pointer */
 	xfs_extnum_t		*idx,	/* extent number to update/delete */
@@ -5075,11 +5075,8 @@ xfs_bmap_del_extent(
 	int			whichfork, /* data or attr fork */
 	int			bflags)	/* bmapi flags */
 {
-	xfs_filblks_t		da_new;	/* new delay-alloc indirect blocks */
-	xfs_filblks_t		da_old;	/* old delay-alloc indirect blocks */
 	xfs_fsblock_t		del_endblock=0;	/* first block past del */
 	xfs_fileoff_t		del_endoff;	/* first offset past del */
-	int			delay;	/* current block is delayed allocated */
 	int			do_fx;	/* free extent at end of routine */
 	xfs_bmbt_rec_host_t	*ep;	/* current extent entry pointer */
 	int			error;	/* error return value */
@@ -5114,63 +5111,40 @@ xfs_bmap_del_extent(
 	del_endoff = del->br_startoff + del->br_blockcount;
 	got_endoff = got.br_startoff + got.br_blockcount;
 	ASSERT(got_endoff >= del_endoff);
-	delay = isnullstartblock(got.br_startblock);
-	ASSERT(isnullstartblock(del->br_startblock) == delay);
-	flags = 0;
+	ASSERT(!isnullstartblock(got.br_startblock));
+	flags = XFS_ILOG_CORE;
 	qfield = 0;
 	error = 0;
-	/*
-	 * If deleting a real allocation, must free up the disk space.
-	 */
-	if (!delay) {
-		flags = XFS_ILOG_CORE;
-		/*
-		 * Realtime allocation.  Free it and record di_nblocks update.
-		 */
-		if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
-			xfs_fsblock_t	bno;
-			xfs_filblks_t	len;
-
-			ASSERT(do_mod(del->br_blockcount,
-				      mp->m_sb.sb_rextsize) == 0);
-			ASSERT(do_mod(del->br_startblock,
-				      mp->m_sb.sb_rextsize) == 0);
-			bno = del->br_startblock;
-			len = del->br_blockcount;
-			do_div(bno, mp->m_sb.sb_rextsize);
-			do_div(len, mp->m_sb.sb_rextsize);
-			error = xfs_rtfree_extent(tp, bno, (xfs_extlen_t)len);
-			if (error)
-				goto done;
-			do_fx = 0;
-			nblks = len * mp->m_sb.sb_rextsize;
-			qfield = XFS_TRANS_DQ_RTBCOUNT;
-		}
-		/*
-		 * Ordinary allocation.
-		 */
-		else {
-			do_fx = 1;
-			nblks = del->br_blockcount;
-			qfield = XFS_TRANS_DQ_BCOUNT;
-		}
-		/*
-		 * Set up del_endblock and cur for later.
-		 */
-		del_endblock = del->br_startblock + del->br_blockcount;
-		if (cur) {
-			if ((error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
-					got.br_startblock, got.br_blockcount,
-					&i)))
-				goto done;
-			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-		}
-		da_old = da_new = 0;
-	} else {
-		da_old = startblockval(got.br_startblock);
-		da_new = 0;
-		nblks = 0;
+
+	if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
+		xfs_fsblock_t	bno;
+		xfs_filblks_t	len;
+
+		ASSERT(do_mod(del->br_blockcount, mp->m_sb.sb_rextsize) == 0);
+		ASSERT(do_mod(del->br_startblock, mp->m_sb.sb_rextsize) == 0);
+		bno = del->br_startblock;
+		len = del->br_blockcount;
+		do_div(bno, mp->m_sb.sb_rextsize);
+		do_div(len, mp->m_sb.sb_rextsize);
+		error = xfs_rtfree_extent(tp, bno, (xfs_extlen_t)len);
+		if (error)
+			goto done;
 		do_fx = 0;
+		nblks = len * mp->m_sb.sb_rextsize;
+		qfield = XFS_TRANS_DQ_RTBCOUNT;
+	} else {
+		do_fx = 1;
+		nblks = del->br_blockcount;
+		qfield = XFS_TRANS_DQ_BCOUNT;
+	}
+
+	del_endblock = del->br_startblock + del->br_blockcount;
+	if (cur) {
+		error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
+				got.br_startblock, got.br_blockcount, &i);
+		if (error)
+			goto done;
+		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 	}
 
 	/*
@@ -5187,8 +5161,6 @@ xfs_bmap_del_extent(
 		xfs_iext_remove(ip, *idx, 1,
 				whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0);
 		--*idx;
-		if (delay)
-			break;
 
 		XFS_IFORK_NEXT_SET(ip, whichfork,
 			XFS_IFORK_NEXTENTS(ip, whichfork) - 1);
@@ -5210,14 +5182,6 @@ xfs_bmap_del_extent(
 		xfs_bmbt_set_startoff(ep, del_endoff);
 		temp = got.br_blockcount - del->br_blockcount;
 		xfs_bmbt_set_blockcount(ep, temp);
-		if (delay) {
-			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
-				da_old);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
-			da_new = temp;
-			break;
-		}
 		xfs_bmbt_set_startblock(ep, del_endblock);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		if (!cur) {
@@ -5237,14 +5201,6 @@ xfs_bmap_del_extent(
 		temp = got.br_blockcount - del->br_blockcount;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
 		xfs_bmbt_set_blockcount(ep, temp);
-		if (delay) {
-			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
-				da_old);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
-			da_new = temp;
-			break;
-		}
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		if (!cur) {
 			flags |= xfs_ilog_fext(whichfork);
@@ -5268,89 +5224,60 @@ xfs_bmap_del_extent(
 		temp2 = got_endoff - del_endoff;
 		new.br_blockcount = temp2;
 		new.br_state = got.br_state;
-		if (!delay) {
-			new.br_startblock = del_endblock;
-			flags |= XFS_ILOG_CORE;
-			if (cur) {
-				if ((error = xfs_bmbt_update(cur,
-						got.br_startoff,
-						got.br_startblock, temp,
-						got.br_state)))
-					goto done;
-				if ((error = xfs_btree_increment(cur, 0, &i)))
-					goto done;
-				cur->bc_rec.b = new;
-				error = xfs_btree_insert(cur, &i);
-				if (error && error != -ENOSPC)
-					goto done;
+		new.br_startblock = del_endblock;
+		flags |= XFS_ILOG_CORE;
+		if (cur) {
+			error = xfs_bmbt_update(cur, got.br_startoff,
+					got.br_startblock, temp,
+					got.br_state);
+			if (error)
+				goto done;
+			error = xfs_btree_increment(cur, 0, &i);
+			if (error)
+				goto done;
+			cur->bc_rec.b = new;
+			error = xfs_btree_insert(cur, &i);
+			if (error && error != -ENOSPC)
+				goto done;
+			/*
+			 * If get no-space back from btree insert, it tried a
+			 * split, and we have a zero block reservation.  Fix up
+			 * our state and return the error.
+			 */
+			if (error == -ENOSPC) {
 				/*
-				 * If get no-space back from btree insert,
-				 * it tried a split, and we have a zero
-				 * block reservation.
-				 * Fix up our state and return the error.
+				 * Reset the cursor, don't trust it after any
+				 * insert operation.
 				 */
-				if (error == -ENOSPC) {
-					/*
-					 * Reset the cursor, don't trust
-					 * it after any insert operation.
-					 */
-					if ((error = xfs_bmbt_lookup_eq(cur,
-							got.br_startoff,
-							got.br_startblock,
-							temp, &i)))
-						goto done;
-					XFS_WANT_CORRUPTED_GOTO(mp,
-								i == 1, done);
-					/*
-					 * Update the btree record back
-					 * to the original value.
-					 */
-					if ((error = xfs_bmbt_update(cur,
-							got.br_startoff,
-							got.br_startblock,
-							got.br_blockcount,
-							got.br_state)))
-						goto done;
-					/*
-					 * Reset the extent record back
-					 * to the original value.
-					 */
-					xfs_bmbt_set_blockcount(ep,
-						got.br_blockcount);
-					flags = 0;
-					error = -ENOSPC;
+				error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
+						got.br_startblock, temp, &i);
+				if (error)
 					goto done;
-				}
 				XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			} else
-				flags |= xfs_ilog_fext(whichfork);
-			XFS_IFORK_NEXT_SET(ip, whichfork,
-				XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
-		} else {
-			xfs_filblks_t	stolen;
-			ASSERT(whichfork == XFS_DATA_FORK);
-
-			/*
-			 * Distribute the original indlen reservation across the
-			 * two new extents. Steal blocks from the deleted extent
-			 * if necessary. Stealing blocks simply fudges the
-			 * fdblocks accounting in xfs_bunmapi().
-			 */
-			temp = xfs_bmap_worst_indlen(ip, got.br_blockcount);
-			temp2 = xfs_bmap_worst_indlen(ip, new.br_blockcount);
-			stolen = xfs_bmap_split_indlen(da_old, &temp, &temp2,
-						       del->br_blockcount);
-			da_new = temp + temp2 - stolen;
-			del->br_blockcount -= stolen;
-
-			/*
-			 * Set the reservation for each extent. Warn if either
-			 * is zero as this can lead to delalloc problems.
-			 */
-			WARN_ON_ONCE(!temp || !temp2);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			new.br_startblock = nullstartblock((int)temp2);
-		}
+				/*
+				 * Update the btree record back
+				 * to the original value.
+				 */
+				error = xfs_bmbt_update(cur, got.br_startoff,
+						got.br_startblock,
+						got.br_blockcount,
+						got.br_state);
+				if (error)
+					goto done;
+				/*
+				 * Reset the extent record back
+				 * to the original value.
+				 */
+				xfs_bmbt_set_blockcount(ep, got.br_blockcount);
+				flags = 0;
+				error = -ENOSPC;
+				goto done;
+			}
+			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
+		} else
+			flags |= xfs_ilog_fext(whichfork);
+		XFS_IFORK_NEXT_SET(ip, whichfork,
+			XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		xfs_iext_insert(ip, *idx + 1, 1, &new, state);
 		++*idx;
@@ -5358,11 +5285,9 @@ xfs_bmap_del_extent(
 	}
 
 	/* remove reverse mapping */
-	if (!delay) {
-		error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, del);
-		if (error)
-			goto done;
-	}
+	error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, del);
+	if (error)
+		goto done;
 
 	/*
 	 * If we need to, add to list of extents to delete.
@@ -5388,13 +5313,6 @@ xfs_bmap_del_extent(
 	if (qfield && !(bflags & XFS_BMAPI_REMAP))
 		xfs_trans_mod_dquot_byino(tp, ip, qfield, (long)-nblks);
 
-	/*
-	 * Account for change in delayed indirect blocks.
-	 * Nothing to do for disk quota accounting here.
-	 */
-	ASSERT(da_old >= da_new);
-	if (da_old > da_new)
-		xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new), false);
 done:
 	*logflagsp = flags;
 	return error;
@@ -5679,62 +5597,41 @@ __xfs_bunmapi(
 			}
 		}
 
-		/*
-		 * If it's the case where the directory code is running
-		 * with no block reservation, and the deleted block is in
-		 * the middle of its extent, and the resulting insert
-		 * of an extent would cause transformation to btree format,
-		 * then reject it.  The calling code will then swap
-		 * blocks around instead.
-		 * We have to do this now, rather than waiting for the
-		 * conversion to btree format, since the transaction
-		 * will be dirty.
-		 */
-		if (!wasdel && tp->t_blk_res == 0 &&
-		    XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS &&
-		    XFS_IFORK_NEXTENTS(ip, whichfork) >= /* Note the >= */
-			XFS_IFORK_MAXEXT(ip, whichfork) &&
-		    del.br_startoff > got.br_startoff &&
-		    del.br_startoff + del.br_blockcount <
-		    got.br_startoff + got.br_blockcount) {
-			error = -ENOSPC;
-			goto error0;
-		}
-
-		/*
-		 * Unreserve quota and update realtime free space, if
-		 * appropriate. If delayed allocation, update the inode delalloc
-		 * counter now and wait to update the sb counters as
-		 * xfs_bmap_del_extent() might need to borrow some blocks.
-		 */
 		if (wasdel) {
-			ASSERT(startblockval(del.br_startblock) > 0);
-			if (isrt) {
-				xfs_filblks_t rtexts;
-
-				rtexts = XFS_FSB_TO_B(mp, del.br_blockcount);
-				do_div(rtexts, mp->m_sb.sb_rextsize);
-				xfs_mod_frextents(mp, (int64_t)rtexts);
-				(void)xfs_trans_reserve_quota_nblks(NULL,
-					ip, -((long)del.br_blockcount), 0,
-					XFS_QMOPT_RES_RTBLKS);
-			} else {
-				(void)xfs_trans_reserve_quota_nblks(NULL,
-					ip, -((long)del.br_blockcount), 0,
-					XFS_QMOPT_RES_REGBLKS);
+			error = xfs_bmap_del_extent_delay(ip, whichfork, &lastx,
+					&got, &del);
+		} else {
+			/*
+			 * If it's the case where the directory code is running
+			 * with no block reservation, and the deleted block is
+			 * in the middle of its extent, and the resulting insert
+			 * of an extent would cause transformation to btree
+			 * format, then reject it.  The calling code will then
+			 * swap blocks around instead.  We have to do this now,
+			 * rather than waiting for the conversion to btree
+			 * format, since the transaction will be dirty.
+			 */
+			if (tp->t_blk_res == 0 &&
+			    XFS_IFORK_FORMAT(ip, whichfork) ==
+					XFS_DINODE_FMT_EXTENTS &&
+			    XFS_IFORK_NEXTENTS(ip, whichfork) >=
+					XFS_IFORK_MAXEXT(ip, whichfork) &&
+			    del.br_startoff > got.br_startoff &&
+			    del.br_startoff + del.br_blockcount <
+			    got.br_startoff + got.br_blockcount) {
+				error = -ENOSPC;
+				goto error0;
 			}
-			ip->i_delayed_blks -= del.br_blockcount;
+
+			error = xfs_bmap_del_extent_real(ip, tp, &lastx, dfops,
+					cur, &del, &tmp_logflags, whichfork,
+					flags);
+			logflags |= tmp_logflags;
 		}
 
-		error = xfs_bmap_del_extent(ip, tp, &lastx, dfops, cur, &del,
-				&tmp_logflags, whichfork, flags);
-		logflags |= tmp_logflags;
 		if (error)
 			goto error0;
 
-		if (!isrt && wasdel)
-			xfs_mod_fdblocks(mp, (int64_t)del.br_blockcount, false);
-
 		max_len -= del.br_blockcount;
 		end = del.br_startoff - 1;
 nodelete:
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 06/19] xfs: move some more code into xfs_bmap_del_extent_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 05/19] xfs: use xfs_bmap_del_extent_delay for the data fork as well Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 07/19] xfs: use correct state defines in xfs_bmap_del_extent_{cow,delay} Christoph Hellwig
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4e6c4cc4168f..06f1bd9e144e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5080,7 +5080,7 @@ xfs_bmap_del_extent_real(
 	int			do_fx;	/* free extent at end of routine */
 	xfs_bmbt_rec_host_t	*ep;	/* current extent entry pointer */
 	int			error;	/* error return value */
-	int			flags;	/* inode logging flags */
+	int			flags = 0;/* inode logging flags */
 	xfs_bmbt_irec_t		got;	/* current extent entry */
 	xfs_fileoff_t		got_endoff;	/* first offset past got */
 	int			i;	/* temp state */
@@ -5112,10 +5112,25 @@ xfs_bmap_del_extent_real(
 	got_endoff = got.br_startoff + got.br_blockcount;
 	ASSERT(got_endoff >= del_endoff);
 	ASSERT(!isnullstartblock(got.br_startblock));
-	flags = XFS_ILOG_CORE;
 	qfield = 0;
 	error = 0;
 
+	/*
+	 * If it's the case where the directory code is running with no block
+	 * reservation, and the deleted block is in the middle of its extent,
+	 * and the resulting insert of an extent would cause transformation to
+	 * btree format, then reject it.  The calling code will then swap blocks
+	 * around instead.  We have to do this now, rather than waiting for the
+	 * conversion to btree format, since the transaction will be dirty then.
+	 */
+	if (tp->t_blk_res == 0 &&
+	    XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS &&
+	    XFS_IFORK_NEXTENTS(ip, whichfork) >=
+			XFS_IFORK_MAXEXT(ip, whichfork) &&
+	    del->br_startoff > got.br_startoff && del_endoff < got_endoff)
+		return -ENOSPC;
+
+	flags = XFS_ILOG_CORE;
 	if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
 		xfs_fsblock_t	bno;
 		xfs_filblks_t	len;
@@ -5601,28 +5616,6 @@ __xfs_bunmapi(
 			error = xfs_bmap_del_extent_delay(ip, whichfork, &lastx,
 					&got, &del);
 		} else {
-			/*
-			 * If it's the case where the directory code is running
-			 * with no block reservation, and the deleted block is
-			 * in the middle of its extent, and the resulting insert
-			 * of an extent would cause transformation to btree
-			 * format, then reject it.  The calling code will then
-			 * swap blocks around instead.  We have to do this now,
-			 * rather than waiting for the conversion to btree
-			 * format, since the transaction will be dirty.
-			 */
-			if (tp->t_blk_res == 0 &&
-			    XFS_IFORK_FORMAT(ip, whichfork) ==
-					XFS_DINODE_FMT_EXTENTS &&
-			    XFS_IFORK_NEXTENTS(ip, whichfork) >=
-					XFS_IFORK_MAXEXT(ip, whichfork) &&
-			    del.br_startoff > got.br_startoff &&
-			    del.br_startoff + del.br_blockcount <
-			    got.br_startoff + got.br_blockcount) {
-				error = -ENOSPC;
-				goto error0;
-			}
-
 			error = xfs_bmap_del_extent_real(ip, tp, &lastx, dfops,
 					cur, &del, &tmp_logflags, whichfork,
 					flags);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 07/19] xfs: use correct state defines in xfs_bmap_del_extent_{cow,delay}
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 06/19] xfs: move some more code into xfs_bmap_del_extent_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real Christoph Hellwig
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use the _FILLING values to match the usage in the xfs_bmap_add_extent_*
helpers.  No change in behavior, just better naming in the code and
tracepoint output.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 06f1bd9e144e..aa4af31750d3 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4897,19 +4897,19 @@ xfs_bmap_del_extent_delay(
 		state |= BMAP_COWFORK;
 
 	if (got->br_startoff == del->br_startoff)
-		state |= BMAP_LEFT_CONTIG;
+		state |= BMAP_LEFT_FILLING;
 	if (got_endoff == del_endoff)
-		state |= BMAP_RIGHT_CONTIG;
+		state |= BMAP_RIGHT_FILLING;
 
-	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
-	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
+	switch (state & (BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING)) {
+	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
 		/*
 		 * Matches the whole extent.  Delete the entry.
 		 */
 		xfs_iext_remove(ip, *idx, 1, state);
 		--*idx;
 		break;
-	case BMAP_LEFT_CONTIG:
+	case BMAP_LEFT_FILLING:
 		/*
 		 * Deleting the first part of the extent.
 		 */
@@ -4922,7 +4922,7 @@ xfs_bmap_del_extent_delay(
 		xfs_iext_update_extent(ifp, *idx, got);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		break;
-	case BMAP_RIGHT_CONTIG:
+	case BMAP_RIGHT_FILLING:
 		/*
 		 * Deleting the last part of the extent.
 		 */
@@ -5007,19 +5007,19 @@ xfs_bmap_del_extent_cow(
 	ASSERT(!isnullstartblock(got->br_startblock));
 
 	if (got->br_startoff == del->br_startoff)
-		state |= BMAP_LEFT_CONTIG;
+		state |= BMAP_LEFT_FILLING;
 	if (got_endoff == del_endoff)
-		state |= BMAP_RIGHT_CONTIG;
+		state |= BMAP_RIGHT_FILLING;
 
-	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
-	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
+	switch (state & (BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING)) {
+	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
 		/*
 		 * Matches the whole extent.  Delete the entry.
 		 */
 		xfs_iext_remove(ip, *idx, 1, state);
 		--*idx;
 		break;
-	case BMAP_LEFT_CONTIG:
+	case BMAP_LEFT_FILLING:
 		/*
 		 * Deleting the first part of the extent.
 		 */
@@ -5030,7 +5030,7 @@ xfs_bmap_del_extent_cow(
 		xfs_iext_update_extent(ifp, *idx, got);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		break;
-	case BMAP_RIGHT_CONTIG:
+	case BMAP_RIGHT_FILLING:
 		/*
 		 * Deleting the last part of the extent.
 		 */
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 07/19] xfs: use correct state defines in xfs_bmap_del_extent_{cow,delay} Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 09/19] xfs: refactor xfs_del_extent_real Christoph Hellwig
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use the same defines as the other extent add and delete helpers, which
both improves code readability and trace point output.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index aa4af31750d3..037efc97499f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5162,13 +5162,13 @@ xfs_bmap_del_extent_real(
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 	}
 
-	/*
-	 * Set flag value to use in switch statement.
-	 * Left-contig is 2, right-contig is 1.
-	 */
-	switch (((got.br_startoff == del->br_startoff) << 1) |
-		(got_endoff == del_endoff)) {
-	case 3:
+	if (got.br_startoff == del->br_startoff)
+		state |= BMAP_LEFT_FILLING;
+	if (got_endoff == del_endoff)
+		state |= BMAP_RIGHT_FILLING;
+
+	switch (state & (BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING)) {
+	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
 		/*
 		 * Matches the whole extent.  Delete the entry.
 		 */
@@ -5188,8 +5188,7 @@ xfs_bmap_del_extent_real(
 			goto done;
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 		break;
-
-	case 2:
+	case BMAP_LEFT_FILLING:
 		/*
 		 * Deleting the first part of the extent.
 		 */
@@ -5208,8 +5207,7 @@ xfs_bmap_del_extent_real(
 				got.br_state)))
 			goto done;
 		break;
-
-	case 1:
+	case BMAP_RIGHT_FILLING:
 		/*
 		 * Deleting the last part of the extent.
 		 */
@@ -5227,7 +5225,6 @@ xfs_bmap_del_extent_real(
 				got.br_state)))
 			goto done;
 		break;
-
 	case 0:
 		/*
 		 * Deleting the middle of the extent.
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 09/19] xfs: refactor xfs_del_extent_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 10/19] xfs: refactor xfs_bmap_add_extent_hole_delay Christoph Hellwig
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use xfs_iext_update_extent to update entries in the in-core extent list.
This isolates the function from the detailed layout of the extent list,
and generally makes the code a lot more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 60 ++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 037efc97499f..bd00f4d135af 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5078,10 +5078,9 @@ xfs_bmap_del_extent_real(
 	xfs_fsblock_t		del_endblock=0;	/* first block past del */
 	xfs_fileoff_t		del_endoff;	/* first offset past del */
 	int			do_fx;	/* free extent at end of routine */
-	xfs_bmbt_rec_host_t	*ep;	/* current extent entry pointer */
 	int			error;	/* error return value */
 	int			flags = 0;/* inode logging flags */
-	xfs_bmbt_irec_t		got;	/* current extent entry */
+	struct xfs_bmbt_irec	got;	/* current extent entry */
 	xfs_fileoff_t		got_endoff;	/* first offset past got */
 	int			i;	/* temp state */
 	xfs_ifork_t		*ifp;	/* inode fork pointer */
@@ -5090,9 +5089,8 @@ xfs_bmap_del_extent_real(
 	xfs_bmbt_irec_t		new;	/* new record to be inserted */
 	/* REFERENCED */
 	uint			qfield;	/* quota field to update */
-	xfs_filblks_t		temp;	/* for indirect length calculations */
-	xfs_filblks_t		temp2;	/* for indirect length calculations */
 	int			state = 0;
+	struct xfs_bmbt_irec	old;
 
 	mp = ip->i_mount;
 	XFS_STATS_INC(mp, xs_del_exlist);
@@ -5105,8 +5103,7 @@ xfs_bmap_del_extent_real(
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 	ASSERT((*idx >= 0) && (*idx < xfs_iext_count(ifp)));
 	ASSERT(del->br_blockcount > 0);
-	ep = xfs_iext_get_ext(ifp, *idx);
-	xfs_bmbt_get_all(ep, &got);
+	xfs_iext_get_extent(ifp, *idx, &got);
 	ASSERT(got.br_startoff <= del->br_startoff);
 	del_endoff = del->br_startoff + del->br_blockcount;
 	got_endoff = got.br_startoff + got.br_blockcount;
@@ -5193,54 +5190,56 @@ xfs_bmap_del_extent_real(
 		 * Deleting the first part of the extent.
 		 */
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_startoff(ep, del_endoff);
-		temp = got.br_blockcount - del->br_blockcount;
-		xfs_bmbt_set_blockcount(ep, temp);
-		xfs_bmbt_set_startblock(ep, del_endblock);
+		got.br_startoff = del_endoff;
+		got.br_startblock = del_endblock;
+		got.br_blockcount -= del->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &got);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		if (!cur) {
 			flags |= xfs_ilog_fext(whichfork);
 			break;
 		}
-		if ((error = xfs_bmbt_update(cur, del_endoff, del_endblock,
-				got.br_blockcount - del->br_blockcount,
-				got.br_state)))
+		error = xfs_bmbt_update(cur, got.br_startoff, got.br_startblock,
+				got.br_blockcount, got.br_state);
+		if (error)
 			goto done;
 		break;
 	case BMAP_RIGHT_FILLING:
 		/*
 		 * Deleting the last part of the extent.
 		 */
-		temp = got.br_blockcount - del->br_blockcount;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
+		got.br_blockcount -= del->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &got);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		if (!cur) {
 			flags |= xfs_ilog_fext(whichfork);
 			break;
 		}
-		if ((error = xfs_bmbt_update(cur, got.br_startoff,
-				got.br_startblock,
-				got.br_blockcount - del->br_blockcount,
-				got.br_state)))
+		error = xfs_bmbt_update(cur, got.br_startoff, got.br_startblock,
+				got.br_blockcount, got.br_state);
+		if (error)
 			goto done;
 		break;
 	case 0:
 		/*
 		 * Deleting the middle of the extent.
 		 */
-		temp = del->br_startoff - got.br_startoff;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
+
+		old = got;
+		got.br_blockcount = del->br_startoff - got.br_startoff;
+		xfs_iext_update_extent(ifp, *idx, &got);
+
 		new.br_startoff = del_endoff;
-		temp2 = got_endoff - del_endoff;
-		new.br_blockcount = temp2;
+		new.br_blockcount = got_endoff - del_endoff;
 		new.br_state = got.br_state;
 		new.br_startblock = del_endblock;
+
 		flags |= XFS_ILOG_CORE;
 		if (cur) {
 			error = xfs_bmbt_update(cur, got.br_startoff,
-					got.br_startblock, temp,
+					got.br_startblock, got.br_blockcount,
 					got.br_state);
 			if (error)
 				goto done;
@@ -5262,7 +5261,8 @@ xfs_bmap_del_extent_real(
 				 * insert operation.
 				 */
 				error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
-						got.br_startblock, temp, &i);
+						got.br_startblock,
+						got.br_blockcount, &i);
 				if (error)
 					goto done;
 				XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -5270,17 +5270,17 @@ xfs_bmap_del_extent_real(
 				 * Update the btree record back
 				 * to the original value.
 				 */
-				error = xfs_bmbt_update(cur, got.br_startoff,
-						got.br_startblock,
-						got.br_blockcount,
-						got.br_state);
+				error = xfs_bmbt_update(cur, old.br_startoff,
+						old.br_startblock,
+						old.br_blockcount,
+						old.br_state);
 				if (error)
 					goto done;
 				/*
 				 * Reset the extent record back
 				 * to the original value.
 				 */
-				xfs_bmbt_set_blockcount(ep, got.br_blockcount);
+				xfs_iext_update_extent(ifp, *idx, &old);
 				flags = 0;
 				error = -ENOSPC;
 				goto done;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 10/19] xfs: refactor xfs_bmap_add_extent_hole_delay
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (8 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 09/19] xfs: refactor xfs_del_extent_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 11/19] xfs: refactor xfs_bmap_add_extent_hole_real Christoph Hellwig
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use xfs_iext_get_extent to find, and xfs_iext_update_extent to update
entries in the in-core extent list.  This isolates the function from
the detailed layout of the extent list, and generally makes the code
a lot more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index bd00f4d135af..833c76084f38 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2701,7 +2701,7 @@ xfs_bmap_add_extent_hole_delay(
 	xfs_filblks_t		oldlen=0;	/* old indirect size */
 	xfs_bmbt_irec_t		right;	/* right neighbor extent entry */
 	int			state;  /* state bits, accessed thru macros */
-	xfs_filblks_t		temp=0;	/* temp for indirect calculations */
+	xfs_filblks_t		temp;	 /* temp for indirect calculations */
 
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 	state = 0;
@@ -2764,14 +2764,14 @@ xfs_bmap_add_extent_hole_delay(
 			right.br_blockcount;
 
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp);
 		oldlen = startblockval(left.br_startblock) +
 			startblockval(new->br_startblock) +
 			startblockval(right.br_startblock);
 		newlen = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
 					 oldlen);
-		xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx),
-			nullstartblock((int)newlen));
+		left.br_startblock = nullstartblock(newlen);
+		left.br_blockcount = temp;
+		xfs_iext_update_extent(ifp, *idx, &left);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		xfs_iext_remove(ip, *idx + 1, 1, state);
@@ -2787,13 +2787,13 @@ xfs_bmap_add_extent_hole_delay(
 		temp = left.br_blockcount + new->br_blockcount;
 
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp);
 		oldlen = startblockval(left.br_startblock) +
 			startblockval(new->br_startblock);
 		newlen = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
 					 oldlen);
-		xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx),
-			nullstartblock((int)newlen));
+		left.br_blockcount = temp;
+		left.br_startblock = nullstartblock(newlen);
+		xfs_iext_update_extent(ifp, *idx, &left);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		break;
 
@@ -2809,9 +2809,10 @@ xfs_bmap_add_extent_hole_delay(
 			startblockval(right.br_startblock);
 		newlen = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
 					 oldlen);
-		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx),
-			new->br_startoff,
-			nullstartblock((int)newlen), temp, right.br_state);
+		right.br_startoff = new->br_startoff;
+		right.br_startblock = nullstartblock(newlen);
+		right.br_blockcount = temp;
+		xfs_iext_update_extent(ifp, *idx, &right);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 		break;
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 11/19] xfs: refactor xfs_bmap_add_extent_hole_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (9 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 10/19] xfs: refactor xfs_bmap_add_extent_hole_delay Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real Christoph Hellwig
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use xfs_iext_update_extent to update entries in the in-core extent list.
This isolates the function from the detailed layout of the extent list,
and generally makes the code a lot more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 48 +++++++++++++++++++++---------------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 833c76084f38..4fb73c0aca05 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2860,6 +2860,7 @@ xfs_bmap_add_extent_hole_real(
 	xfs_bmbt_irec_t		right;	/* right neighbor extent entry */
 	int			rval=0;	/* return value (logging flags) */
 	int			state;	/* state bits, accessed thru macros */
+	struct xfs_bmbt_irec	old;
 
 	ASSERT(*idx >= 0);
 	ASSERT(*idx <= xfs_iext_count(ifp));
@@ -2929,9 +2930,8 @@ xfs_bmap_add_extent_hole_real(
 		 */
 		--*idx;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx),
-			left.br_blockcount + new->br_blockcount +
-			right.br_blockcount);
+		left.br_blockcount += new->br_blockcount + right.br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &left);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		xfs_iext_remove(ip, *idx + 1, 1, state);
@@ -2958,10 +2958,7 @@ xfs_bmap_add_extent_hole_real(
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(cur, left.br_startoff,
 					left.br_startblock,
-					left.br_blockcount +
-						new->br_blockcount +
-						right.br_blockcount,
-					left.br_state);
+					left.br_blockcount, left.br_state);
 			if (error)
 				goto done;
 		}
@@ -2974,26 +2971,25 @@ xfs_bmap_add_extent_hole_real(
 		 * Merge the new allocation with the left neighbor.
 		 */
 		--*idx;
+		old = left;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx),
-			left.br_blockcount + new->br_blockcount);
+		left.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &left);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		if (cur == NULL) {
 			rval = xfs_ilog_fext(whichfork);
 		} else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur, left.br_startoff,
-					left.br_startblock, left.br_blockcount,
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
 					&i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(cur, left.br_startoff,
 					left.br_startblock,
-					left.br_blockcount +
-						new->br_blockcount,
-					left.br_state);
+					left.br_blockcount, left.br_state);
 			if (error)
 				goto done;
 		}
@@ -3005,29 +3001,27 @@ xfs_bmap_add_extent_hole_real(
 		 * on the right.
 		 * Merge the new allocation with the right neighbor.
 		 */
+		old = right;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx),
-			new->br_startoff, new->br_startblock,
-			new->br_blockcount + right.br_blockcount,
-			right.br_state);
+		right.br_startoff = new->br_startoff;
+		right.br_startblock = new->br_startblock;
+		right.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &right);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		if (cur == NULL) {
 			rval = xfs_ilog_fext(whichfork);
 		} else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur,
-					right.br_startoff,
-					right.br_startblock,
-					right.br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, new->br_startoff,
-					new->br_startblock,
-					new->br_blockcount +
-						right.br_blockcount,
-					right.br_state);
+			error = xfs_bmbt_update(cur, right.br_startoff,
+					right.br_startblock,
+					right.br_blockcount, right.br_state);
 			if (error)
 				goto done;
 		}
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (10 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 11/19] xfs: refactor xfs_bmap_add_extent_hole_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 15:31   ` Brian Foster
  2017-09-22 16:12   ` Darrick J. Wong
  2017-09-22 13:59 ` [PATCH 13/19] xfs: refactor delalloc accounting in xfs_bmap_add_extent_delay_real Christoph Hellwig
                   ` (6 subsequent siblings)
  18 siblings, 2 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use xfs_iext_get_extent to find, and xfs_iext_update_extent to update
entries in the in-core extent list.  This isolates the function from
the detailed layout of the extent list, and generally makes the code
a lot more readable.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4fb73c0aca05..e74e2d74cd36 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1588,7 +1588,6 @@ xfs_bmap_add_extent_delay_real(
 {
 	struct xfs_bmbt_irec	*new = &bma->got;
 	int			diff;	/* temp value */
-	xfs_bmbt_rec_host_t	*ep;	/* extent entry for idx */
 	int			error;	/* error return value */
 	int			i;	/* temp state */
 	xfs_ifork_t		*ifp;	/* inode fork pointer */
@@ -1600,10 +1599,10 @@ xfs_bmap_add_extent_delay_real(
 	xfs_filblks_t		da_new; /* new count del alloc blocks used */
 	xfs_filblks_t		da_old; /* old count del alloc blocks used */
 	xfs_filblks_t		temp=0;	/* value for da_new calculations */
-	xfs_filblks_t		temp2=0;/* value for da_new calculations */
 	int			tmp_rval;	/* partial logging flags */
 	struct xfs_mount	*mp;
 	xfs_extnum_t		*nextents;
+	struct xfs_bmbt_irec	old;
 
 	mp = bma->ip->i_mount;
 	ifp = XFS_IFORK_PTR(bma->ip, whichfork);
@@ -1629,9 +1628,9 @@ xfs_bmap_add_extent_delay_real(
 	/*
 	 * Set up a bunch of variables to make the tests simpler.
 	 */
-	ep = xfs_iext_get_ext(ifp, bma->idx);
-	xfs_bmbt_get_all(ep, &PREV);
+	xfs_iext_get_extent(ifp, bma->idx, &PREV);
 	new_endoff = new->br_startoff + new->br_blockcount;
+	ASSERT(isnullstartblock(PREV.br_startblock));
 	ASSERT(PREV.br_startoff <= new->br_startoff);
 	ASSERT(PREV.br_startoff + PREV.br_blockcount >= new_endoff);
 
@@ -1706,9 +1705,8 @@ xfs_bmap_add_extent_delay_real(
 		 */
 		bma->idx--;
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
-			LEFT.br_blockcount + PREV.br_blockcount +
-			RIGHT.br_blockcount);
+		LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
+		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		xfs_iext_remove(bma->ip, bma->idx + 1, 2, state);
@@ -1733,9 +1731,7 @@ xfs_bmap_add_extent_delay_real(
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
 					LEFT.br_startblock,
-					LEFT.br_blockcount +
-					PREV.br_blockcount +
-					RIGHT.br_blockcount, LEFT.br_state);
+					LEFT.br_blockcount, LEFT.br_state);
 			if (error)
 				goto done;
 		}
@@ -1748,9 +1744,10 @@ xfs_bmap_add_extent_delay_real(
 		 */
 		bma->idx--;
 
+		old = LEFT;
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
-			LEFT.br_blockcount + PREV.br_blockcount);
+		LEFT.br_blockcount += PREV.br_blockcount;
+		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
@@ -1758,16 +1755,15 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
+			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
 					&i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
 					LEFT.br_startblock,
-					LEFT.br_blockcount +
-					PREV.br_blockcount, LEFT.br_state);
+					LEFT.br_blockcount, LEFT.br_state);
 			if (error)
 				goto done;
 		}
@@ -1779,9 +1775,9 @@ xfs_bmap_add_extent_delay_real(
 		 * The right neighbor is contiguous, the left is not.
 		 */
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_startblock(ep, new->br_startblock);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount + RIGHT.br_blockcount);
+		PREV.br_startblock = new->br_startblock;
+		PREV.br_blockcount += RIGHT.br_blockcount;
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
@@ -1796,9 +1792,8 @@ xfs_bmap_add_extent_delay_real(
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(bma->cur, PREV.br_startoff,
-					new->br_startblock,
-					PREV.br_blockcount +
-					RIGHT.br_blockcount, PREV.br_state);
+					PREV.br_startblock,
+					PREV.br_blockcount, PREV.br_state);
 			if (error)
 				goto done;
 		}
@@ -1811,8 +1806,9 @@ xfs_bmap_add_extent_delay_real(
 		 * the new one.
 		 */
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_startblock(ep, new->br_startblock);
-		xfs_bmbt_set_state(ep, new->br_state);
+		PREV.br_startblock = new->br_startblock;
+		PREV.br_state = new->br_state;
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		(*nextents)++;
@@ -1839,38 +1835,39 @@ xfs_bmap_add_extent_delay_real(
 		 * Filling in the first part of a previous delayed allocation.
 		 * The left neighbor is contiguous.
 		 */
+		old = LEFT;
+		temp = PREV.br_blockcount - new->br_blockcount;
+		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
+				startblockval(PREV.br_startblock));
+
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx - 1),
-			LEFT.br_blockcount + new->br_blockcount);
-		xfs_bmbt_set_startoff(ep,
-			PREV.br_startoff + new->br_blockcount);
+		LEFT.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, bma->idx - 1, &LEFT);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
 
-		temp = PREV.br_blockcount - new->br_blockcount;
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
+		PREV.br_blockcount = temp = PREV.br_blockcount - new->br_blockcount;
+		PREV.br_startoff += new->br_blockcount;
+		PREV.br_startblock = nullstartblock(da_new);
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
+		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+
 		if (bma->cur == NULL)
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
+			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
 					&i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock,
-					LEFT.br_blockcount +
-					new->br_blockcount,
+					LEFT.br_startblock, LEFT.br_blockcount,
 					LEFT.br_state);
 			if (error)
 				goto done;
 		}
-		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
-			startblockval(PREV.br_startblock));
-		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
-		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		bma->idx--;
 		break;
@@ -1880,10 +1877,6 @@ xfs_bmap_add_extent_delay_real(
 		 * Filling in the first part of a previous delayed allocation.
 		 * The left neighbor is not contiguous.
 		 */
-		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_startoff(ep, new_endoff);
-		temp = PREV.br_blockcount - new->br_blockcount;
-		xfs_bmbt_set_blockcount(ep, temp);
 		xfs_iext_insert(bma->ip, bma->idx, 1, new, state);
 		(*nextents)++;
 		if (bma->cur == NULL)
@@ -1911,12 +1904,19 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+
+		temp = PREV.br_blockcount - new->br_blockcount;
 		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
 			startblockval(PREV.br_startblock) -
 			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
-		ep = xfs_iext_get_ext(ifp, bma->idx + 1);
-		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
+
+		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
+		PREV.br_startoff = new_endoff;
+		PREV.br_blockcount = temp;
+		PREV.br_startblock = nullstartblock(da_new);
+		xfs_iext_update_extent(ifp, bma->idx + 1, &PREV);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
+
 		break;
 
 	case BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
@@ -1924,37 +1924,39 @@ xfs_bmap_add_extent_delay_real(
 		 * Filling in the last part of a previous delayed allocation.
 		 * The right neighbor is contiguous with the new allocation.
 		 */
-		temp = PREV.br_blockcount - new->br_blockcount;
+		old = RIGHT;
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
-		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, bma->idx + 1),
-			new->br_startoff, new->br_startblock,
-			new->br_blockcount + RIGHT.br_blockcount,
-			RIGHT.br_state);
+		RIGHT.br_startoff = new->br_startoff;
+		RIGHT.br_startblock = new->br_startblock;
+		RIGHT.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, bma->idx + 1, &RIGHT);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
+
 		if (bma->cur == NULL)
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
+					old.br_startblock,
+					old.br_blockcount, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, new->br_startoff,
-					new->br_startblock,
-					new->br_blockcount +
-					RIGHT.br_blockcount,
+			error = xfs_bmbt_update(bma->cur, RIGHT.br_startoff,
+					RIGHT.br_startblock, RIGHT.br_blockcount,
 					RIGHT.br_state);
 			if (error)
 				goto done;
 		}
 
+		temp = PREV.br_blockcount - new->br_blockcount;
 		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
 			startblockval(PREV.br_startblock));
+
 		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
+		PREV.br_blockcount = temp;
+		PREV.br_startblock = nullstartblock(da_new);
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		bma->idx++;
@@ -1965,9 +1967,6 @@ xfs_bmap_add_extent_delay_real(
 		 * Filling in the last part of a previous delayed allocation.
 		 * The right neighbor is not contiguous.
 		 */
-		temp = PREV.br_blockcount - new->br_blockcount;
-		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
 		xfs_iext_insert(bma->ip, bma->idx + 1, 1, new, state);
 		(*nextents)++;
 		if (bma->cur == NULL)
@@ -1995,11 +1994,16 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+
+		temp = PREV.br_blockcount - new->br_blockcount;
 		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
 			startblockval(PREV.br_startblock) -
 			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
-		ep = xfs_iext_get_ext(ifp, bma->idx);
-		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
+
+		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
+		PREV.br_startblock = nullstartblock(da_new);
+		PREV.br_blockcount = temp;
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
 		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
 
 		bma->idx++;
@@ -2026,19 +2030,33 @@ xfs_bmap_add_extent_delay_real(
 		 *  PREV @ idx          LEFT              RIGHT
 		 *                      inserted at idx + 1
 		 */
-		temp = new->br_startoff - PREV.br_startoff;
-		temp2 = PREV.br_startoff + PREV.br_blockcount - new_endoff;
-		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);	/* truncate PREV */
+		old = PREV;
+
+		/* LEFT is the new middle */
 		LEFT = *new;
+
+		/* RIGHT is the new right */
 		RIGHT.br_state = PREV.br_state;
-		RIGHT.br_startblock = nullstartblock(
-				(int)xfs_bmap_worst_indlen(bma->ip, temp2));
 		RIGHT.br_startoff = new_endoff;
-		RIGHT.br_blockcount = temp2;
+		RIGHT.br_blockcount =
+			PREV.br_startoff + PREV.br_blockcount - new_endoff;
+		RIGHT.br_startblock =
+			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
+					RIGHT.br_blockcount));
+
+		/* truncate PREV */
+		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
+		PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
+		PREV.br_startblock =
+			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
+					PREV.br_blockcount));
+		xfs_iext_update_extent(ifp, bma->idx, &PREV);
+		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+
 		/* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */
 		xfs_iext_insert(bma->ip, bma->idx + 1, 2, &LEFT, state);
 		(*nextents)++;
+
 		if (bma->cur == NULL)
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
@@ -2064,12 +2082,12 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
-		temp = xfs_bmap_worst_indlen(bma->ip, temp);
-		temp2 = xfs_bmap_worst_indlen(bma->ip, temp2);
-		diff = (int)(temp + temp2 -
-			     (startblockval(PREV.br_startblock) -
-			      (bma->cur ?
-			       bma->cur->bc_private.b.allocated : 0)));
+
+		da_new = startblockval(PREV.br_startblock) +
+			 startblockval(RIGHT.br_startblock);
+		diff = da_new - startblockval(old.br_startblock);
+		if (bma->cur)
+			diff += bma->cur->bc_private.b.allocated;
 		if (diff > 0) {
 			error = xfs_mod_fdblocks(bma->ip->i_mount,
 						 -((int64_t)diff), false);
@@ -2078,16 +2096,7 @@ xfs_bmap_add_extent_delay_real(
 				goto done;
 		}
 
-		ep = xfs_iext_get_ext(ifp, bma->idx);
-		xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
-		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
-		xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, bma->idx + 2),
-			nullstartblock((int)temp2));
-		trace_xfs_bmap_post_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
-
 		bma->idx++;
-		da_new = temp + temp2;
 		break;
 
 	case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 13/19] xfs: refactor delalloc accounting in xfs_bmap_add_extent_delay_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (11 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 14/19] xfs: refactor xfs_bmap_add_extent_unwritten_real Christoph Hellwig
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Account for all changes to the delalloc reservation in da_new, and use a
single call xfs_mod_fdblocks to reserve/free blocks, including always
checking for an error.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e74e2d74cd36..b187d5f28e27 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1587,7 +1587,6 @@ xfs_bmap_add_extent_delay_real(
 	int			whichfork)
 {
 	struct xfs_bmbt_irec	*new = &bma->got;
-	int			diff;	/* temp value */
 	int			error;	/* error return value */
 	int			i;	/* temp state */
 	xfs_ifork_t		*ifp;	/* inode fork pointer */
@@ -2085,17 +2084,6 @@ xfs_bmap_add_extent_delay_real(
 
 		da_new = startblockval(PREV.br_startblock) +
 			 startblockval(RIGHT.br_startblock);
-		diff = da_new - startblockval(old.br_startblock);
-		if (bma->cur)
-			diff += bma->cur->bc_private.b.allocated;
-		if (diff > 0) {
-			error = xfs_mod_fdblocks(bma->ip->i_mount,
-						 -((int64_t)diff), false);
-			ASSERT(!error);
-			if (error)
-				goto done;
-		}
-
 		bma->idx++;
 		break;
 
@@ -2130,19 +2118,17 @@ xfs_bmap_add_extent_delay_real(
 			goto done;
 	}
 
-	/* adjust for changes in reserved delayed indirect blocks */
-	if (da_old || da_new) {
-		temp = da_new;
-		if (bma->cur)
-			temp += bma->cur->bc_private.b.allocated;
-		if (temp < da_old)
-			xfs_mod_fdblocks(bma->ip->i_mount,
-					(int64_t)(da_old - temp), false);
+	if (bma->cur) {
+		da_new += bma->cur->bc_private.b.allocated;
+		bma->cur->bc_private.b.allocated = 0;
 	}
 
-	/* clear out the allocated field, done with it now in any case. */
-	if (bma->cur)
-		bma->cur->bc_private.b.allocated = 0;
+	/* adjust for changes in reserved delayed indirect blocks */
+	if (da_new != da_old) {
+		ASSERT(state == 0 || da_new < da_old);
+		error = xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new),
+				false);
+	}
 
 	xfs_bmap_check_leaf_extents(bma->cur, bma->ip, whichfork);
 done:
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 14/19] xfs: refactor xfs_bmap_add_extent_unwritten_real
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (12 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 13/19] xfs: refactor delalloc accounting in xfs_bmap_add_extent_delay_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 15/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_update Christoph Hellwig
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Use xfs_iext_get_extent to find, and xfs_iext_update_extent to update
entries in the in-core extent list.  This isolates the function from
the detailed layout of the extent list, and generally makes the code
a lot more readable.

Also get rid of the oldext and newext variables as using the extent
records is a lot more descriptive.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 208 +++++++++++++++++++++++------------------------
 1 file changed, 104 insertions(+), 104 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index b187d5f28e27..e42b063f8efe 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -2156,18 +2156,16 @@ xfs_bmap_add_extent_unwritten_real(
 	int			*logflagsp) /* inode logging flags */
 {
 	xfs_btree_cur_t		*cur;	/* btree cursor */
-	xfs_bmbt_rec_host_t	*ep;	/* extent entry for idx */
 	int			error;	/* error return value */
 	int			i;	/* temp state */
 	xfs_ifork_t		*ifp;	/* inode fork pointer */
 	xfs_fileoff_t		new_endoff;	/* end offset of new entry */
-	xfs_exntst_t		newext;	/* new extent state */
-	xfs_exntst_t		oldext;	/* old extent state */
 	xfs_bmbt_irec_t		r[3];	/* neighbor extent entries */
 					/* left is 0, right is 1, prev is 2 */
 	int			rval=0;	/* return value (logging flags) */
 	int			state = 0;/* state bits, accessed thru macros */
 	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_bmbt_irec	old;
 
 	*logflagsp = 0;
 
@@ -2190,12 +2188,8 @@ xfs_bmap_add_extent_unwritten_real(
 	 * Set up a bunch of variables to make the tests simpler.
 	 */
 	error = 0;
-	ep = xfs_iext_get_ext(ifp, *idx);
-	xfs_bmbt_get_all(ep, &PREV);
-	newext = new->br_state;
-	oldext = (newext == XFS_EXT_UNWRITTEN) ?
-		XFS_EXT_NORM : XFS_EXT_UNWRITTEN;
-	ASSERT(PREV.br_state == oldext);
+	xfs_iext_get_extent(ifp, *idx, &PREV);
+	ASSERT(new->br_state != PREV.br_state);
 	new_endoff = new->br_startoff + new->br_blockcount;
 	ASSERT(PREV.br_startoff <= new->br_startoff);
 	ASSERT(PREV.br_startoff + PREV.br_blockcount >= new_endoff);
@@ -2224,7 +2218,7 @@ xfs_bmap_add_extent_unwritten_real(
 	if ((state & BMAP_LEFT_VALID) && !(state & BMAP_LEFT_DELAY) &&
 	    LEFT.br_startoff + LEFT.br_blockcount == new->br_startoff &&
 	    LEFT.br_startblock + LEFT.br_blockcount == new->br_startblock &&
-	    LEFT.br_state == newext &&
+	    LEFT.br_state == new->br_state &&
 	    LEFT.br_blockcount + new->br_blockcount <= MAXEXTLEN)
 		state |= BMAP_LEFT_CONTIG;
 
@@ -2243,7 +2237,7 @@ xfs_bmap_add_extent_unwritten_real(
 	if ((state & BMAP_RIGHT_VALID) && !(state & BMAP_RIGHT_DELAY) &&
 	    new_endoff == RIGHT.br_startoff &&
 	    new->br_startblock + new->br_blockcount == RIGHT.br_startblock &&
-	    newext == RIGHT.br_state &&
+	    new->br_state == RIGHT.br_state &&
 	    new->br_blockcount + RIGHT.br_blockcount <= MAXEXTLEN &&
 	    ((state & (BMAP_LEFT_CONTIG | BMAP_LEFT_FILLING |
 		       BMAP_RIGHT_FILLING)) !=
@@ -2267,9 +2261,8 @@ xfs_bmap_add_extent_unwritten_real(
 		--*idx;
 
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx),
-			LEFT.br_blockcount + PREV.br_blockcount +
-			RIGHT.br_blockcount);
+		LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &LEFT);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		xfs_iext_remove(ip, *idx + 1, 2, state);
@@ -2296,10 +2289,10 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, LEFT.br_startoff,
-				LEFT.br_startblock,
-				LEFT.br_blockcount + PREV.br_blockcount +
-				RIGHT.br_blockcount, LEFT.br_state)))
+			error = xfs_bmbt_update(cur, LEFT.br_startoff,
+					LEFT.br_startblock, LEFT.br_blockcount,
+					LEFT.br_state);
+			if (error)
 				goto done;
 		}
 		break;
@@ -2312,8 +2305,8 @@ xfs_bmap_add_extent_unwritten_real(
 		--*idx;
 
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx),
-			LEFT.br_blockcount + PREV.br_blockcount);
+		LEFT.br_blockcount += PREV.br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &LEFT);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		xfs_iext_remove(ip, *idx + 1, 1, state);
@@ -2334,10 +2327,10 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, LEFT.br_startoff,
-				LEFT.br_startblock,
-				LEFT.br_blockcount + PREV.br_blockcount,
-				LEFT.br_state)))
+			error = xfs_bmbt_update(cur, LEFT.br_startoff,
+					LEFT.br_startblock, LEFT.br_blockcount,
+					LEFT.br_state);
+			if (error)
 				goto done;
 		}
 		break;
@@ -2348,10 +2341,11 @@ xfs_bmap_add_extent_unwritten_real(
 		 * The right neighbor is contiguous, the left is not.
 		 */
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount + RIGHT.br_blockcount);
-		xfs_bmbt_set_state(ep, newext);
+		PREV.br_blockcount += RIGHT.br_blockcount;
+		PREV.br_state = new->br_state;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+
 		xfs_iext_remove(ip, *idx + 1, 1, state);
 		XFS_IFORK_NEXT_SET(ip, whichfork,
 				XFS_IFORK_NEXTENTS(ip, whichfork) - 1);
@@ -2370,10 +2364,10 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, new->br_startoff,
-				new->br_startblock,
-				new->br_blockcount + RIGHT.br_blockcount,
-				newext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
 		}
 		break;
@@ -2385,7 +2379,8 @@ xfs_bmap_add_extent_unwritten_real(
 		 * the new one.
 		 */
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_state(ep, newext);
+		PREV.br_state = new->br_state;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		if (cur == NULL)
@@ -2397,9 +2392,10 @@ xfs_bmap_add_extent_unwritten_real(
 					&i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, new->br_startoff,
-				new->br_startblock, new->br_blockcount,
-				newext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
 		}
 		break;
@@ -2410,17 +2406,16 @@ xfs_bmap_add_extent_unwritten_real(
 		 * The left neighbor is contiguous.
 		 */
 		trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1),
-			LEFT.br_blockcount + new->br_blockcount);
-		xfs_bmbt_set_startoff(ep,
-			PREV.br_startoff + new->br_blockcount);
+		LEFT.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx - 1, &LEFT);
 		trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_);
 
+		old = PREV;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_startblock(ep,
-			new->br_startblock + new->br_blockcount);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount - new->br_blockcount);
+		PREV.br_startoff += new->br_blockcount;
+		PREV.br_startblock += new->br_blockcount;
+		PREV.br_blockcount -= new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		--*idx;
@@ -2429,23 +2424,23 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur,
-				PREV.br_startoff + new->br_blockcount,
-				PREV.br_startblock + new->br_blockcount,
-				PREV.br_blockcount - new->br_blockcount,
-				oldext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
-			if ((error = xfs_btree_decrement(cur, 0, &i)))
+			error = xfs_btree_decrement(cur, 0, &i);
+			if (error)
 				goto done;
 			error = xfs_bmbt_update(cur, LEFT.br_startoff,
-				LEFT.br_startblock,
-				LEFT.br_blockcount + new->br_blockcount,
-				LEFT.br_state);
+					LEFT.br_startblock, LEFT.br_blockcount,
+					LEFT.br_state);
 			if (error)
 				goto done;
 		}
@@ -2456,13 +2451,12 @@ xfs_bmap_add_extent_unwritten_real(
 		 * Setting the first part of a previous oldext extent to newext.
 		 * The left neighbor is not contiguous.
 		 */
+		old = PREV;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		ASSERT(ep && xfs_bmbt_get_state(ep) == oldext);
-		xfs_bmbt_set_startoff(ep, new_endoff);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount - new->br_blockcount);
-		xfs_bmbt_set_startblock(ep,
-			new->br_startblock + new->br_blockcount);
+		PREV.br_startoff += new->br_blockcount;
+		PREV.br_startblock += new->br_blockcount;
+		PREV.br_blockcount -= new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		xfs_iext_insert(ip, *idx, 1, new, state);
@@ -2472,16 +2466,16 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur,
-				PREV.br_startoff + new->br_blockcount,
-				PREV.br_startblock + new->br_blockcount,
-				PREV.br_blockcount - new->br_blockcount,
-				oldext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
 			cur->bc_rec.b = *new;
 			if ((error = xfs_btree_insert(cur, &i)))
@@ -2495,39 +2489,43 @@ xfs_bmap_add_extent_unwritten_real(
 		 * Setting the last part of a previous oldext extent to newext.
 		 * The right neighbor is contiguous with the new allocation.
 		 */
+		old = PREV;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount - new->br_blockcount);
+		PREV.br_blockcount -= new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		++*idx;
 
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx),
-			new->br_startoff, new->br_startblock,
-			new->br_blockcount + RIGHT.br_blockcount, newext);
+		RIGHT.br_startoff = new->br_startoff;
+		RIGHT.br_startblock = new->br_startblock;
+		RIGHT.br_blockcount += new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &RIGHT);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		if (cur == NULL)
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock,
-					PREV.br_blockcount, &i)))
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, PREV.br_startoff,
-				PREV.br_startblock,
-				PREV.br_blockcount - new->br_blockcount,
-				oldext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
-			if ((error = xfs_btree_increment(cur, 0, &i)))
+			error = xfs_btree_increment(cur, 0, &i);
+			if (error)
 				goto done;
-			if ((error = xfs_bmbt_update(cur, new->br_startoff,
-				new->br_startblock,
-				new->br_blockcount + RIGHT.br_blockcount,
-				newext)))
+			error = xfs_bmbt_update(cur, RIGHT.br_startoff,
+					RIGHT.br_startblock,
+					RIGHT.br_blockcount, RIGHT.br_state);
+			if (error)
 				goto done;
 		}
 		break;
@@ -2537,9 +2535,10 @@ xfs_bmap_add_extent_unwritten_real(
 		 * Setting the last part of a previous oldext extent to newext.
 		 * The right neighbor is not contiguous.
 		 */
+		old = PREV;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep,
-			PREV.br_blockcount - new->br_blockcount);
+		PREV.br_blockcount -= new->br_blockcount;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		++*idx;
@@ -2551,15 +2550,16 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			if ((error = xfs_bmbt_update(cur, PREV.br_startoff,
-				PREV.br_startblock,
-				PREV.br_blockcount - new->br_blockcount,
-				oldext)))
+			error = xfs_bmbt_update(cur, PREV.br_startoff,
+					PREV.br_startblock, PREV.br_blockcount,
+					PREV.br_state);
+			if (error)
 				goto done;
 			if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
 					new->br_startblock, new->br_blockcount,
@@ -2579,17 +2579,18 @@ xfs_bmap_add_extent_unwritten_real(
 		 * newext.  Contiguity is impossible here.
 		 * One extent becomes three extents.
 		 */
+		old = PREV;
 		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep,
-			new->br_startoff - PREV.br_startoff);
+		PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
+		xfs_iext_update_extent(ifp, *idx, &PREV);
 		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
 
 		r[0] = *new;
 		r[1].br_startoff = new_endoff;
 		r[1].br_blockcount =
-			PREV.br_startoff + PREV.br_blockcount - new_endoff;
+			old.br_startoff + old.br_blockcount - new_endoff;
 		r[1].br_startblock = new->br_startblock + new->br_blockcount;
-		r[1].br_state = oldext;
+		r[1].br_state = PREV.br_state;
 
 		++*idx;
 		xfs_iext_insert(ip, *idx, 2, &r[0], state);
@@ -2600,9 +2601,10 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
+					old.br_startblock, old.br_blockcount,
+					&i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			/* new right extent - oldext */
@@ -2612,8 +2614,6 @@ xfs_bmap_add_extent_unwritten_real(
 				goto done;
 			/* new left extent - oldext */
 			cur->bc_rec.b = PREV;
-			cur->bc_rec.b.br_blockcount =
-				new->br_startoff - PREV.br_startoff;
 			if ((error = xfs_btree_insert(cur, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 15/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_update
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (13 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 14/19] xfs: refactor xfs_bmap_add_extent_unwritten_real Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 16/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_lookup_eq Christoph Hellwig
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Now that we've massaged the callers into the right form we can always
pass the actual extent record instead of the individual fields.

With that xfs_bmbt_disk_set_allf can go away, and xfs_bmbt_disk_set_all
can be merged into the former implementation of xfs_bmbt_disk_set_allf.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c       | 114 +++++++++++------------------------------
 fs/xfs/libxfs/xfs_bmap_btree.c |  42 +++++----------
 fs/xfs/libxfs/xfs_bmap_btree.h |   4 +-
 3 files changed, 44 insertions(+), 116 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e42b063f8efe..23bea9ed9191 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -161,21 +161,17 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork)
 }
 
 /*
- * Update the record referred to by cur to the value given
- * by [off, bno, len, state].
+ * Update the record referred to by cur to the value given by irec
  * This either works (return 0) or gets an EFSCORRUPTED error.
  */
 STATIC int
 xfs_bmbt_update(
 	struct xfs_btree_cur	*cur,
-	xfs_fileoff_t		off,
-	xfs_fsblock_t		bno,
-	xfs_filblks_t		len,
-	xfs_exntst_t		state)
+	struct xfs_bmbt_irec	*irec)
 {
 	union xfs_btree_rec	rec;
 
-	xfs_bmbt_disk_set_allf(&rec.bmbt, off, bno, len, state);
+	xfs_bmbt_disk_set_all(&rec.bmbt, irec);
 	return xfs_btree_update(cur, &rec);
 }
 
@@ -1728,9 +1724,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock,
-					LEFT.br_blockcount, LEFT.br_state);
+			error = xfs_bmbt_update(bma->cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -1760,9 +1754,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock,
-					LEFT.br_blockcount, LEFT.br_state);
+			error = xfs_bmbt_update(bma->cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -1790,9 +1782,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, PREV.br_startoff,
-					PREV.br_startblock,
-					PREV.br_blockcount, PREV.br_state);
+			error = xfs_bmbt_update(bma->cur, &PREV);
 			if (error)
 				goto done;
 		}
@@ -1861,9 +1851,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
-					LEFT.br_state);
+			error = xfs_bmbt_update(bma->cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -1941,9 +1929,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(bma->cur, RIGHT.br_startoff,
-					RIGHT.br_startblock, RIGHT.br_blockcount,
-					RIGHT.br_state);
+			error = xfs_bmbt_update(bma->cur, &RIGHT);
 			if (error)
 				goto done;
 		}
@@ -2289,9 +2275,7 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
-					LEFT.br_state);
+			error = xfs_bmbt_update(cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -2327,9 +2311,7 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
-					LEFT.br_state);
+			error = xfs_bmbt_update(cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -2364,9 +2346,7 @@ xfs_bmap_add_extent_unwritten_real(
 			if ((error = xfs_btree_decrement(cur, 0, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 		}
@@ -2392,9 +2372,7 @@ xfs_bmap_add_extent_unwritten_real(
 					&i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 		}
@@ -2430,17 +2408,13 @@ xfs_bmap_add_extent_unwritten_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 			error = xfs_btree_decrement(cur, 0, &i);
 			if (error)
 				goto done;
-			error = xfs_bmbt_update(cur, LEFT.br_startoff,
-					LEFT.br_startblock, LEFT.br_blockcount,
-					LEFT.br_state);
+			error = xfs_bmbt_update(cur, &LEFT);
 			if (error)
 				goto done;
 		}
@@ -2472,9 +2446,7 @@ xfs_bmap_add_extent_unwritten_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 			cur->bc_rec.b = *new;
@@ -2514,17 +2486,13 @@ xfs_bmap_add_extent_unwritten_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 			error = xfs_btree_increment(cur, 0, &i);
 			if (error)
 				goto done;
-			error = xfs_bmbt_update(cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, RIGHT.br_state);
+			error = xfs_bmbt_update(cur, &RIGHT);
 			if (error)
 				goto done;
 		}
@@ -2556,9 +2524,7 @@ xfs_bmap_add_extent_unwritten_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					PREV.br_state);
+			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
 			if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
@@ -2608,9 +2574,8 @@ xfs_bmap_add_extent_unwritten_real(
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			/* new right extent - oldext */
-			if ((error = xfs_bmbt_update(cur, r[1].br_startoff,
-				r[1].br_startblock, r[1].br_blockcount,
-				r[1].br_state)))
+			error = xfs_bmbt_update(cur, &r[1]);
+			if (error)
 				goto done;
 			/* new left extent - oldext */
 			cur->bc_rec.b = PREV;
@@ -2951,9 +2916,7 @@ xfs_bmap_add_extent_hole_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, left.br_startoff,
-					left.br_startblock,
-					left.br_blockcount, left.br_state);
+			error = xfs_bmbt_update(cur, &left);
 			if (error)
 				goto done;
 		}
@@ -2982,9 +2945,7 @@ xfs_bmap_add_extent_hole_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, left.br_startoff,
-					left.br_startblock,
-					left.br_blockcount, left.br_state);
+			error = xfs_bmbt_update(cur, &left);
 			if (error)
 				goto done;
 		}
@@ -3014,9 +2975,7 @@ xfs_bmap_add_extent_hole_real(
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
-			error = xfs_bmbt_update(cur, right.br_startoff,
-					right.br_startblock,
-					right.br_blockcount, right.br_state);
+			error = xfs_bmbt_update(cur, &right);
 			if (error)
 				goto done;
 		}
@@ -5189,8 +5148,7 @@ xfs_bmap_del_extent_real(
 			flags |= xfs_ilog_fext(whichfork);
 			break;
 		}
-		error = xfs_bmbt_update(cur, got.br_startoff, got.br_startblock,
-				got.br_blockcount, got.br_state);
+		error = xfs_bmbt_update(cur, &got);
 		if (error)
 			goto done;
 		break;
@@ -5206,8 +5164,7 @@ xfs_bmap_del_extent_real(
 			flags |= xfs_ilog_fext(whichfork);
 			break;
 		}
-		error = xfs_bmbt_update(cur, got.br_startoff, got.br_startblock,
-				got.br_blockcount, got.br_state);
+		error = xfs_bmbt_update(cur, &got);
 		if (error)
 			goto done;
 		break;
@@ -5228,9 +5185,7 @@ xfs_bmap_del_extent_real(
 
 		flags |= XFS_ILOG_CORE;
 		if (cur) {
-			error = xfs_bmbt_update(cur, got.br_startoff,
-					got.br_startblock, got.br_blockcount,
-					got.br_state);
+			error = xfs_bmbt_update(cur, &got);
 			if (error)
 				goto done;
 			error = xfs_btree_increment(cur, 0, &i);
@@ -5260,10 +5215,7 @@ xfs_bmap_del_extent_real(
 				 * Update the btree record back
 				 * to the original value.
 				 */
-				error = xfs_bmbt_update(cur, old.br_startoff,
-						old.br_startblock,
-						old.br_blockcount,
-						old.br_state);
+				error = xfs_bmbt_update(cur, &old);
 				if (error)
 					goto done;
 				/*
@@ -5801,8 +5753,7 @@ xfs_bmse_merge(
 		return error;
 	XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
 
-	error = xfs_bmbt_update(cur, new.br_startoff, new.br_startblock,
-			        new.br_blockcount, new.br_state);
+	error = xfs_bmbt_update(cur, &new);
 	if (error)
 		return error;
 
@@ -5919,9 +5870,7 @@ xfs_bmse_shift_one(
 			return error;
 		XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
 
-		error = xfs_bmbt_update(cur, new.br_startoff,
-				new.br_startblock, new.br_blockcount,
-				new.br_state);
+		error = xfs_bmbt_update(cur, &new);
 		if (error)
 			return error;
 	} else {
@@ -6182,10 +6131,7 @@ xfs_bmap_split_extent_at(
 
 	logflags = XFS_ILOG_CORE;
 	if (cur) {
-		error = xfs_bmbt_update(cur, got.br_startoff,
-				got.br_startblock,
-				got.br_blockcount,
-				got.br_state);
+		error = xfs_bmbt_update(cur, &got);
 		if (error)
 			goto del_cursor;
 	} else
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index a6331ffa51e3..7e2d981626ef 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -228,47 +228,31 @@ xfs_bmbt_set_all(
 			     s->br_blockcount, s->br_state);
 }
 
-
 /*
- * Set all the fields in a disk format bmap extent record from the arguments.
+ * Set all the fields in a bmap extent record from the uncompressed form.
  */
 void
-xfs_bmbt_disk_set_allf(
-	xfs_bmbt_rec_t		*r,
-	xfs_fileoff_t		startoff,
-	xfs_fsblock_t		startblock,
-	xfs_filblks_t		blockcount,
-	xfs_exntst_t		state)
+xfs_bmbt_disk_set_all(
+	struct xfs_bmbt_rec	*r,
+	struct xfs_bmbt_irec	*s)
 {
-	int			extent_flag = (state == XFS_EXT_NORM) ? 0 : 1;
+	int			extent_flag = (s->br_state != XFS_EXT_NORM);
 
-	ASSERT(state == XFS_EXT_NORM || state == XFS_EXT_UNWRITTEN);
-	ASSERT((startoff & xfs_mask64hi(64-BMBT_STARTOFF_BITLEN)) == 0);
-	ASSERT((blockcount & xfs_mask64hi(64-BMBT_BLOCKCOUNT_BITLEN)) == 0);
-	ASSERT((startblock & xfs_mask64hi(64-BMBT_STARTBLOCK_BITLEN)) == 0);
+	ASSERT(s->br_state == XFS_EXT_NORM || s->br_state == XFS_EXT_UNWRITTEN);
+	ASSERT(!(s->br_startoff & xfs_mask64hi(64-BMBT_STARTOFF_BITLEN)));
+	ASSERT(!(s->br_blockcount & xfs_mask64hi(64-BMBT_BLOCKCOUNT_BITLEN)));
+	ASSERT(!(s->br_startblock & xfs_mask64hi(64-BMBT_STARTBLOCK_BITLEN)));
 
 	r->l0 = cpu_to_be64(
 		((xfs_bmbt_rec_base_t)extent_flag << 63) |
-		 ((xfs_bmbt_rec_base_t)startoff << 9) |
-		 ((xfs_bmbt_rec_base_t)startblock >> 43));
+		 ((xfs_bmbt_rec_base_t)s->br_startoff << 9) |
+		 ((xfs_bmbt_rec_base_t)s->br_startblock >> 43));
 	r->l1 = cpu_to_be64(
-		((xfs_bmbt_rec_base_t)startblock << 21) |
-		 ((xfs_bmbt_rec_base_t)blockcount &
+		((xfs_bmbt_rec_base_t)s->br_startblock << 21) |
+		 ((xfs_bmbt_rec_base_t)s->br_blockcount &
 		  (xfs_bmbt_rec_base_t)xfs_mask64lo(21)));
 }
 
-/*
- * Set all the fields in a bmap extent record from the uncompressed form.
- */
-STATIC void
-xfs_bmbt_disk_set_all(
-	xfs_bmbt_rec_t	*r,
-	xfs_bmbt_irec_t *s)
-{
-	xfs_bmbt_disk_set_allf(r, s->br_startoff, s->br_startblock,
-				  s->br_blockcount, s->br_state);
-}
-
 /*
  * Set the blockcount field in a bmap extent record.
  */
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.h b/fs/xfs/libxfs/xfs_bmap_btree.h
index 9da5a8d4f184..bd3c56f1cd03 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.h
+++ b/fs/xfs/libxfs/xfs_bmap_btree.h
@@ -104,6 +104,7 @@ extern xfs_fsblock_t xfs_bmbt_get_startblock(xfs_bmbt_rec_host_t *r);
 extern xfs_fileoff_t xfs_bmbt_get_startoff(xfs_bmbt_rec_host_t *r);
 extern xfs_exntst_t xfs_bmbt_get_state(xfs_bmbt_rec_host_t *r);
 
+void xfs_bmbt_disk_set_all(struct xfs_bmbt_rec *r, struct xfs_bmbt_irec *s);
 extern xfs_filblks_t xfs_bmbt_disk_get_blockcount(xfs_bmbt_rec_t *r);
 extern xfs_fileoff_t xfs_bmbt_disk_get_startoff(xfs_bmbt_rec_t *r);
 
@@ -115,9 +116,6 @@ extern void xfs_bmbt_set_startblock(xfs_bmbt_rec_host_t *r, xfs_fsblock_t v);
 extern void xfs_bmbt_set_startoff(xfs_bmbt_rec_host_t *r, xfs_fileoff_t v);
 extern void xfs_bmbt_set_state(xfs_bmbt_rec_host_t *r, xfs_exntst_t v);
 
-extern void xfs_bmbt_disk_set_allf(xfs_bmbt_rec_t *r, xfs_fileoff_t o,
-			xfs_fsblock_t b, xfs_filblks_t c, xfs_exntst_t v);
-
 extern void xfs_bmbt_to_bmdr(struct xfs_mount *, struct xfs_btree_block *, int,
 			xfs_bmdr_block_t *, int);
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 16/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_lookup_eq
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (14 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 15/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_update Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 17/19] xfs: replace xfs_bmbt_lookup_ge with xfs_bmbt_lookup_first Christoph Hellwig
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Now that we've massaged the callers into the right form we can always
pass the actual extent record instead of the individual fields.

As an additional benefit the btree cursor will now be prepoulated with
the correct extent state instead of having to fix it up later.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 145 +++++++++++++----------------------------------
 1 file changed, 39 insertions(+), 106 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 23bea9ed9191..41da4b63130f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -113,14 +113,10 @@ xfs_bmap_compute_maxlevels(
 STATIC int				/* error */
 xfs_bmbt_lookup_eq(
 	struct xfs_btree_cur	*cur,
-	xfs_fileoff_t		off,
-	xfs_fsblock_t		bno,
-	xfs_filblks_t		len,
+	struct xfs_bmbt_irec	*irec,
 	int			*stat)	/* success/failure */
 {
-	cur->bc_rec.b.br_startoff = off;
-	cur->bc_rec.b.br_startblock = bno;
-	cur->bc_rec.b.br_blockcount = len;
+	cur->bc_rec.b = *irec;
 	return xfs_btree_lookup(cur, XFS_LOOKUP_EQ, stat);
 }
 
@@ -1710,9 +1706,7 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(bma->cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(bma->cur, &RIGHT, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -1748,9 +1742,7 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -1776,9 +1768,7 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(bma->cur, &RIGHT, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -1805,13 +1795,10 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(bma->cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, new, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			bma->cur->bc_rec.b.br_state = XFS_EXT_NORM;
 			error = xfs_btree_insert(bma->cur, &i);
 			if (error)
 				goto done;
@@ -1845,9 +1832,7 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -1870,13 +1855,10 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(bma->cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, new, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			bma->cur->bc_rec.b.br_state = XFS_EXT_NORM;
 			error = xfs_btree_insert(bma->cur, &i);
 			if (error)
 				goto done;
@@ -1923,9 +1905,7 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
-					old.br_startblock,
-					old.br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(bma->cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -1958,13 +1938,10 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(bma->cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, new, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			bma->cur->bc_rec.b.br_state = XFS_EXT_NORM;
 			error = xfs_btree_insert(bma->cur, &i);
 			if (error)
 				goto done;
@@ -2046,13 +2023,10 @@ xfs_bmap_add_extent_delay_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(bma->cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(bma->cur, new, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			bma->cur->bc_rec.b.br_state = XFS_EXT_NORM;
 			error = xfs_btree_insert(bma->cur, &i);
 			if (error)
 				goto done;
@@ -2258,9 +2232,8 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, &i)))
+			error = xfs_bmbt_lookup_eq(cur, &RIGHT, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			if ((error = xfs_btree_delete(cur, &i)))
@@ -2300,9 +2273,8 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, PREV.br_startoff,
-					PREV.br_startblock, PREV.br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, &PREV, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			if ((error = xfs_btree_delete(cur, &i)))
@@ -2335,9 +2307,8 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			if ((error = xfs_bmbt_lookup_eq(cur, RIGHT.br_startoff,
-					RIGHT.br_startblock,
-					RIGHT.br_blockcount, &i)))
+			error = xfs_bmbt_lookup_eq(cur, &RIGHT, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			if ((error = xfs_btree_delete(cur, &i)))
@@ -2367,9 +2338,8 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, new, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(cur, &PREV);
@@ -2402,9 +2372,7 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2440,9 +2408,7 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2480,9 +2446,7 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_DEXT;
 		else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2518,21 +2482,17 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 			error = xfs_bmbt_update(cur, &PREV);
 			if (error)
 				goto done;
-			if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, new, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			cur->bc_rec.b.br_state = new->br_state;
 			if ((error = xfs_btree_insert(cur, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2567,9 +2527,7 @@ xfs_bmap_add_extent_unwritten_real(
 			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
 		else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2587,13 +2545,11 @@ xfs_bmap_add_extent_unwritten_real(
 			 * we are about to insert as we can't trust it after
 			 * the previous insert.
 			 */
-			if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
-					new->br_startblock, new->br_blockcount,
-					&i)))
+			error = xfs_bmbt_lookup_eq(cur, new, &i);
+			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
 			/* new middle extent - newext */
-			cur->bc_rec.b.br_state = new->br_state;
 			if ((error = xfs_btree_insert(cur, &i)))
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2902,9 +2858,7 @@ xfs_bmap_add_extent_hole_real(
 			rval = XFS_ILOG_CORE | xfs_ilog_fext(whichfork);
 		} else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(cur, right.br_startoff,
-					right.br_startblock, right.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &right, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2939,9 +2893,7 @@ xfs_bmap_add_extent_hole_real(
 			rval = xfs_ilog_fext(whichfork);
 		} else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2969,9 +2921,7 @@ xfs_bmap_add_extent_hole_real(
 			rval = xfs_ilog_fext(whichfork);
 		} else {
 			rval = 0;
-			error = xfs_bmbt_lookup_eq(cur, old.br_startoff,
-					old.br_startblock, old.br_blockcount,
-					&i);
+			error = xfs_bmbt_lookup_eq(cur, &old, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -2994,14 +2944,10 @@ xfs_bmap_add_extent_hole_real(
 			rval = XFS_ILOG_CORE | xfs_ilog_fext(whichfork);
 		} else {
 			rval = XFS_ILOG_CORE;
-			error = xfs_bmbt_lookup_eq(cur,
-					new->br_startoff,
-					new->br_startblock,
-					new->br_blockcount, &i);
+			error = xfs_bmbt_lookup_eq(cur, new, &i);
 			if (error)
 				goto done;
 			XFS_WANT_CORRUPTED_GOTO(mp, i == 0, done);
-			cur->bc_rec.b.br_state = new->br_state;
 			error = xfs_btree_insert(cur, &i);
 			if (error)
 				goto done;
@@ -5101,8 +5047,7 @@ xfs_bmap_del_extent_real(
 
 	del_endblock = del->br_startblock + del->br_blockcount;
 	if (cur) {
-		error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
-				got.br_startblock, got.br_blockcount, &i);
+		error = xfs_bmbt_lookup_eq(cur, &got, &i);
 		if (error)
 			goto done;
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -5205,9 +5150,7 @@ xfs_bmap_del_extent_real(
 				 * Reset the cursor, don't trust it after any
 				 * insert operation.
 				 */
-				error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
-						got.br_startblock,
-						got.br_blockcount, &i);
+				error = xfs_bmbt_lookup_eq(cur, &got, &i);
 				if (error)
 					goto done;
 				XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
@@ -5735,8 +5678,7 @@ xfs_bmse_merge(
 	}
 
 	/* lookup and remove the extent to merge */
-	error = xfs_bmbt_lookup_eq(cur, got->br_startoff, got->br_startblock,
-				   got->br_blockcount, &i);
+	error = xfs_bmbt_lookup_eq(cur, got, &i);
 	if (error)
 		return error;
 	XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
@@ -5747,8 +5689,7 @@ xfs_bmse_merge(
 	XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
 
 	/* lookup and update size of the previous extent */
-	error = xfs_bmbt_lookup_eq(cur, left->br_startoff, left->br_startblock,
-				   left->br_blockcount, &i);
+	error = xfs_bmbt_lookup_eq(cur, left, &i);
 	if (error)
 		return error;
 	XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
@@ -5864,8 +5805,7 @@ xfs_bmse_shift_one(
 	new.br_startoff = startoff;
 
 	if (cur) {
-		error = xfs_bmbt_lookup_eq(cur, got->br_startoff,
-				got->br_startblock, got->br_blockcount, &i);
+		error = xfs_bmbt_lookup_eq(cur, got, &i);
 		if (error)
 			return error;
 		XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
@@ -6117,10 +6057,7 @@ xfs_bmap_split_extent_at(
 		cur->bc_private.b.firstblock = *firstfsb;
 		cur->bc_private.b.dfops = dfops;
 		cur->bc_private.b.flags = 0;
-		error = xfs_bmbt_lookup_eq(cur, got.br_startoff,
-				got.br_startblock,
-				got.br_blockcount,
-				&i);
+		error = xfs_bmbt_lookup_eq(cur, &got, &i);
 		if (error)
 			goto del_cursor;
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, del_cursor);
@@ -6144,14 +6081,10 @@ xfs_bmap_split_extent_at(
 			   XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
 
 	if (cur) {
-		error = xfs_bmbt_lookup_eq(cur, new.br_startoff,
-				new.br_startblock, new.br_blockcount,
-				&i);
+		error = xfs_bmbt_lookup_eq(cur, &new, &i);
 		if (error)
 			goto del_cursor;
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 0, del_cursor);
-		cur->bc_rec.b.br_state = new.br_state;
-
 		error = xfs_btree_insert(cur, &i);
 		if (error)
 			goto del_cursor;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 17/19] xfs: replace xfs_bmbt_lookup_ge with xfs_bmbt_lookup_first
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (15 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 16/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_lookup_eq Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 18/19] xfs: remove all xfs_bmbt_set_* helpers except for xfs_bmbt_set_all Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 19/19] xfs: remove xfs_bmbt_get_state Christoph Hellwig
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

We only use xfs_bmbt_lookup_ge to look up the first bmap record in an
inode, so replace xfs_bmbt_lookup_ge with a special purpose helper that
is a bit more descriptive.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.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 41da4b63130f..14cf58e5a3e2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -121,16 +121,13 @@ xfs_bmbt_lookup_eq(
 }
 
 STATIC int				/* error */
-xfs_bmbt_lookup_ge(
+xfs_bmbt_lookup_first(
 	struct xfs_btree_cur	*cur,
-	xfs_fileoff_t		off,
-	xfs_fsblock_t		bno,
-	xfs_filblks_t		len,
 	int			*stat)	/* success/failure */
 {
-	cur->bc_rec.b.br_startoff = off;
-	cur->bc_rec.b.br_startblock = bno;
-	cur->bc_rec.b.br_blockcount = len;
+	cur->bc_rec.b.br_startoff = 0;
+	cur->bc_rec.b.br_startblock = 0;
+	cur->bc_rec.b.br_blockcount = 0;
 	return xfs_btree_lookup(cur, XFS_LOOKUP_GE, stat);
 }
 
@@ -978,7 +975,8 @@ xfs_bmap_add_attrfork_btree(
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, XFS_DATA_FORK);
 		cur->bc_private.b.dfops = dfops;
 		cur->bc_private.b.firstblock = *firstblock;
-		if ((error = xfs_bmbt_lookup_ge(cur, 0, 0, 0, &stat)))
+		error = xfs_bmbt_lookup_first(cur, &stat);
+		if (error)
 			goto error0;
 		/* must be at least one entry */
 		XFS_WANT_CORRUPTED_GOTO(mp, stat == 1, error0);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 18/19] xfs: remove all xfs_bmbt_set_* helpers except for xfs_bmbt_set_all
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (16 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 17/19] xfs: replace xfs_bmbt_lookup_ge with xfs_bmbt_lookup_first Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  2017-09-22 13:59 ` [PATCH 19/19] xfs: remove xfs_bmbt_get_state Christoph Hellwig
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Unused after the big bmap refactor.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap_btree.c | 102 ++++++-----------------------------------
 fs/xfs/libxfs/xfs_bmap_btree.h |   6 ---
 2 files changed, 14 insertions(+), 94 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index 7e2d981626ef..e66ebd982cfb 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -188,44 +188,27 @@ xfs_bmbt_disk_get_startoff(
 		 xfs_mask64lo(64 - BMBT_EXNTFLAG_BITLEN)) >> 9;
 }
 
-
 /*
- * Set all the fields in a bmap extent record from the arguments.
+ * Set all the fields in a bmap extent record from the uncompressed form.
  */
 void
-xfs_bmbt_set_allf(
-	xfs_bmbt_rec_host_t	*r,
-	xfs_fileoff_t		startoff,
-	xfs_fsblock_t		startblock,
-	xfs_filblks_t		blockcount,
-	xfs_exntst_t		state)
+xfs_bmbt_set_all(
+	struct xfs_bmbt_rec_host *r,
+	struct xfs_bmbt_irec	*s)
 {
-	int		extent_flag = (state == XFS_EXT_NORM) ? 0 : 1;
-
-	ASSERT(state == XFS_EXT_NORM || state == XFS_EXT_UNWRITTEN);
-	ASSERT((startoff & xfs_mask64hi(64-BMBT_STARTOFF_BITLEN)) == 0);
-	ASSERT((blockcount & xfs_mask64hi(64-BMBT_BLOCKCOUNT_BITLEN)) == 0);
+	int			extent_flag = (s->br_state != XFS_EXT_NORM);
 
-	ASSERT((startblock & xfs_mask64hi(64-BMBT_STARTBLOCK_BITLEN)) == 0);
+	ASSERT(s->br_state == XFS_EXT_NORM || s->br_state == XFS_EXT_UNWRITTEN);
+	ASSERT(!(s->br_startoff & xfs_mask64hi(64-BMBT_STARTOFF_BITLEN)));
+	ASSERT(!(s->br_blockcount & xfs_mask64hi(64-BMBT_BLOCKCOUNT_BITLEN)));
+	ASSERT(!(s->br_startblock & xfs_mask64hi(64-BMBT_STARTBLOCK_BITLEN)));
 
 	r->l0 = ((xfs_bmbt_rec_base_t)extent_flag << 63) |
-		((xfs_bmbt_rec_base_t)startoff << 9) |
-		((xfs_bmbt_rec_base_t)startblock >> 43);
-	r->l1 = ((xfs_bmbt_rec_base_t)startblock << 21) |
-		((xfs_bmbt_rec_base_t)blockcount &
-		(xfs_bmbt_rec_base_t)xfs_mask64lo(21));
-}
-
-/*
- * Set all the fields in a bmap extent record from the uncompressed form.
- */
-void
-xfs_bmbt_set_all(
-	xfs_bmbt_rec_host_t *r,
-	xfs_bmbt_irec_t	*s)
-{
-	xfs_bmbt_set_allf(r, s->br_startoff, s->br_startblock,
-			     s->br_blockcount, s->br_state);
+		 ((xfs_bmbt_rec_base_t)s->br_startoff << 9) |
+		 ((xfs_bmbt_rec_base_t)s->br_startblock >> 43);
+	r->l1 = ((xfs_bmbt_rec_base_t)s->br_startblock << 21) |
+		 ((xfs_bmbt_rec_base_t)s->br_blockcount &
+		  (xfs_bmbt_rec_base_t)xfs_mask64lo(21));
 }
 
 /*
@@ -253,63 +236,6 @@ xfs_bmbt_disk_set_all(
 		  (xfs_bmbt_rec_base_t)xfs_mask64lo(21)));
 }
 
-/*
- * Set the blockcount field in a bmap extent record.
- */
-void
-xfs_bmbt_set_blockcount(
-	xfs_bmbt_rec_host_t *r,
-	xfs_filblks_t	v)
-{
-	ASSERT((v & xfs_mask64hi(43)) == 0);
-	r->l1 = (r->l1 & (xfs_bmbt_rec_base_t)xfs_mask64hi(43)) |
-		  (xfs_bmbt_rec_base_t)(v & xfs_mask64lo(21));
-}
-
-/*
- * Set the startblock field in a bmap extent record.
- */
-void
-xfs_bmbt_set_startblock(
-	xfs_bmbt_rec_host_t *r,
-	xfs_fsblock_t	v)
-{
-	ASSERT((v & xfs_mask64hi(12)) == 0);
-	r->l0 = (r->l0 & (xfs_bmbt_rec_base_t)xfs_mask64hi(55)) |
-		  (xfs_bmbt_rec_base_t)(v >> 43);
-	r->l1 = (r->l1 & (xfs_bmbt_rec_base_t)xfs_mask64lo(21)) |
-		  (xfs_bmbt_rec_base_t)(v << 21);
-}
-
-/*
- * Set the startoff field in a bmap extent record.
- */
-void
-xfs_bmbt_set_startoff(
-	xfs_bmbt_rec_host_t *r,
-	xfs_fileoff_t	v)
-{
-	ASSERT((v & xfs_mask64hi(9)) == 0);
-	r->l0 = (r->l0 & (xfs_bmbt_rec_base_t) xfs_mask64hi(1)) |
-		((xfs_bmbt_rec_base_t)v << 9) |
-		  (r->l0 & (xfs_bmbt_rec_base_t)xfs_mask64lo(9));
-}
-
-/*
- * Set the extent state field in a bmap extent record.
- */
-void
-xfs_bmbt_set_state(
-	xfs_bmbt_rec_host_t *r,
-	xfs_exntst_t	v)
-{
-	ASSERT(v == XFS_EXT_NORM || v == XFS_EXT_UNWRITTEN);
-	if (v == XFS_EXT_NORM)
-		r->l0 &= xfs_mask64lo(64 - BMBT_EXNTFLAG_BITLEN);
-	else
-		r->l0 |= xfs_mask64hi(BMBT_EXNTFLAG_BITLEN);
-}
-
 /*
  * Convert in-memory form of btree root to on-disk form.
  */
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.h b/fs/xfs/libxfs/xfs_bmap_btree.h
index bd3c56f1cd03..93f95bcee915 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.h
+++ b/fs/xfs/libxfs/xfs_bmap_btree.h
@@ -109,12 +109,6 @@ extern xfs_filblks_t xfs_bmbt_disk_get_blockcount(xfs_bmbt_rec_t *r);
 extern xfs_fileoff_t xfs_bmbt_disk_get_startoff(xfs_bmbt_rec_t *r);
 
 extern void xfs_bmbt_set_all(xfs_bmbt_rec_host_t *r, xfs_bmbt_irec_t *s);
-extern void xfs_bmbt_set_allf(xfs_bmbt_rec_host_t *r, xfs_fileoff_t o,
-			xfs_fsblock_t b, xfs_filblks_t c, xfs_exntst_t v);
-extern void xfs_bmbt_set_blockcount(xfs_bmbt_rec_host_t *r, xfs_filblks_t v);
-extern void xfs_bmbt_set_startblock(xfs_bmbt_rec_host_t *r, xfs_fsblock_t v);
-extern void xfs_bmbt_set_startoff(xfs_bmbt_rec_host_t *r, xfs_fileoff_t v);
-extern void xfs_bmbt_set_state(xfs_bmbt_rec_host_t *r, xfs_exntst_t v);
 
 extern void xfs_bmbt_to_bmdr(struct xfs_mount *, struct xfs_btree_block *, int,
 			xfs_bmdr_block_t *, int);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 19/19] xfs: remove xfs_bmbt_get_state
  2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
                   ` (17 preceding siblings ...)
  2017-09-22 13:59 ` [PATCH 18/19] xfs: remove all xfs_bmbt_set_* helpers except for xfs_bmbt_set_all Christoph Hellwig
@ 2017-09-22 13:59 ` Christoph Hellwig
  18 siblings, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-22 13:59 UTC (permalink / raw)
  To: linux-xfs

Unused after the big bmap refactor.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap_btree.c | 29 +----------------------------
 fs/xfs/libxfs/xfs_bmap_btree.h |  1 -
 2 files changed, 1 insertion(+), 29 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index e66ebd982cfb..086e6fc8e4fc 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -37,22 +37,6 @@
 #include "xfs_cksum.h"
 #include "xfs_rmap.h"
 
-/*
- * Determine the extent state.
- */
-/* ARGSUSED */
-STATIC xfs_exntst_t
-xfs_extent_state(
-	xfs_filblks_t		blks,
-	int			extent_flag)
-{
-	if (extent_flag) {
-		ASSERT(blks != 0);	/* saved for DMIG */
-		return XFS_EXT_UNWRITTEN;
-	}
-	return XFS_EXT_NORM;
-}
-
 /*
  * Convert on-disk form of btree root to in-memory form.
  */
@@ -90,7 +74,7 @@ xfs_bmdr_to_bmbt(
 /*
  * Convert a compressed bmap extent record to an uncompressed form.
  * This code must be in sync with the routines xfs_bmbt_get_startoff,
- * xfs_bmbt_get_startblock, xfs_bmbt_get_blockcount and xfs_bmbt_get_state.
+ * xfs_bmbt_get_startblock and xfs_bmbt_get_blockcount.
  */
 STATIC void
 __xfs_bmbt_get_all(
@@ -156,17 +140,6 @@ xfs_bmbt_get_startoff(
 		 xfs_mask64lo(64 - BMBT_EXNTFLAG_BITLEN)) >> 9;
 }
 
-xfs_exntst_t
-xfs_bmbt_get_state(
-	xfs_bmbt_rec_host_t	*r)
-{
-	int	ext_flag;
-
-	ext_flag = (int)((r->l0) >> (64 - BMBT_EXNTFLAG_BITLEN));
-	return xfs_extent_state(xfs_bmbt_get_blockcount(r),
-				ext_flag);
-}
-
 /*
  * Extract the blockcount field from an on disk bmap extent record.
  */
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.h b/fs/xfs/libxfs/xfs_bmap_btree.h
index 93f95bcee915..6f891eeb88f6 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.h
+++ b/fs/xfs/libxfs/xfs_bmap_btree.h
@@ -102,7 +102,6 @@ extern void xfs_bmbt_get_all(xfs_bmbt_rec_host_t *r, xfs_bmbt_irec_t *s);
 extern xfs_filblks_t xfs_bmbt_get_blockcount(xfs_bmbt_rec_host_t *r);
 extern xfs_fsblock_t xfs_bmbt_get_startblock(xfs_bmbt_rec_host_t *r);
 extern xfs_fileoff_t xfs_bmbt_get_startoff(xfs_bmbt_rec_host_t *r);
-extern xfs_exntst_t xfs_bmbt_get_state(xfs_bmbt_rec_host_t *r);
 
 void xfs_bmbt_disk_set_all(struct xfs_bmbt_rec *r, struct xfs_bmbt_irec *s);
 extern xfs_filblks_t xfs_bmbt_disk_get_blockcount(xfs_bmbt_rec_t *r);
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real
  2017-09-22 13:59 ` [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real Christoph Hellwig
@ 2017-09-22 15:31   ` Brian Foster
  2017-09-22 16:12   ` Darrick J. Wong
  1 sibling, 0 replies; 24+ messages in thread
From: Brian Foster @ 2017-09-22 15:31 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Sep 22, 2017 at 06:59:38AM -0700, Christoph Hellwig wrote:
> Use xfs_iext_get_extent to find, and xfs_iext_update_extent to update
> entries in the in-core extent list.  This isolates the function from
> the detailed layout of the extent list, and generally makes the code
> a lot more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_bmap.c | 181 +++++++++++++++++++++++++----------------------
>  1 file changed, 95 insertions(+), 86 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4fb73c0aca05..e74e2d74cd36 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1588,7 +1588,6 @@ xfs_bmap_add_extent_delay_real(
>  {
>  	struct xfs_bmbt_irec	*new = &bma->got;
>  	int			diff;	/* temp value */
> -	xfs_bmbt_rec_host_t	*ep;	/* extent entry for idx */
>  	int			error;	/* error return value */
>  	int			i;	/* temp state */
>  	xfs_ifork_t		*ifp;	/* inode fork pointer */
> @@ -1600,10 +1599,10 @@ xfs_bmap_add_extent_delay_real(
>  	xfs_filblks_t		da_new; /* new count del alloc blocks used */
>  	xfs_filblks_t		da_old; /* old count del alloc blocks used */
>  	xfs_filblks_t		temp=0;	/* value for da_new calculations */
> -	xfs_filblks_t		temp2=0;/* value for da_new calculations */
>  	int			tmp_rval;	/* partial logging flags */
>  	struct xfs_mount	*mp;
>  	xfs_extnum_t		*nextents;
> +	struct xfs_bmbt_irec	old;
>  
>  	mp = bma->ip->i_mount;
>  	ifp = XFS_IFORK_PTR(bma->ip, whichfork);
> @@ -1629,9 +1628,9 @@ xfs_bmap_add_extent_delay_real(
>  	/*
>  	 * Set up a bunch of variables to make the tests simpler.
>  	 */
> -	ep = xfs_iext_get_ext(ifp, bma->idx);
> -	xfs_bmbt_get_all(ep, &PREV);
> +	xfs_iext_get_extent(ifp, bma->idx, &PREV);
>  	new_endoff = new->br_startoff + new->br_blockcount;
> +	ASSERT(isnullstartblock(PREV.br_startblock));
>  	ASSERT(PREV.br_startoff <= new->br_startoff);
>  	ASSERT(PREV.br_startoff + PREV.br_blockcount >= new_endoff);
>  
> @@ -1706,9 +1705,8 @@ xfs_bmap_add_extent_delay_real(
>  		 */
>  		bma->idx--;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
> -			LEFT.br_blockcount + PREV.br_blockcount +
> -			RIGHT.br_blockcount);
> +		LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 2, state);
> @@ -1733,9 +1731,7 @@ xfs_bmap_add_extent_delay_real(
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
>  					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					PREV.br_blockcount +
> -					RIGHT.br_blockcount, LEFT.br_state);
> +					LEFT.br_blockcount, LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1748,9 +1744,10 @@ xfs_bmap_add_extent_delay_real(
>  		 */
>  		bma->idx--;
>  
> +		old = LEFT;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
> -			LEFT.br_blockcount + PREV.br_blockcount);
> +		LEFT.br_blockcount += PREV.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> @@ -1758,16 +1755,15 @@ xfs_bmap_add_extent_delay_real(
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock, LEFT.br_blockcount,
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock, old.br_blockcount,
>  					&i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
>  					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					PREV.br_blockcount, LEFT.br_state);
> +					LEFT.br_blockcount, LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1779,9 +1775,9 @@ xfs_bmap_add_extent_delay_real(
>  		 * The right neighbor is contiguous, the left is not.
>  		 */
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, new->br_startblock);
> -		xfs_bmbt_set_blockcount(ep,
> -			PREV.br_blockcount + RIGHT.br_blockcount);
> +		PREV.br_startblock = new->br_startblock;
> +		PREV.br_blockcount += RIGHT.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> @@ -1796,9 +1792,8 @@ xfs_bmap_add_extent_delay_real(
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, PREV.br_startoff,
> -					new->br_startblock,
> -					PREV.br_blockcount +
> -					RIGHT.br_blockcount, PREV.br_state);
> +					PREV.br_startblock,
> +					PREV.br_blockcount, PREV.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1811,8 +1806,9 @@ xfs_bmap_add_extent_delay_real(
>  		 * the new one.
>  		 */
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, new->br_startblock);
> -		xfs_bmbt_set_state(ep, new->br_state);
> +		PREV.br_startblock = new->br_startblock;
> +		PREV.br_state = new->br_state;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		(*nextents)++;
> @@ -1839,38 +1835,39 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the first part of a previous delayed allocation.
>  		 * The left neighbor is contiguous.
>  		 */
> +		old = LEFT;
> +		temp = PREV.br_blockcount - new->br_blockcount;
> +		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> +				startblockval(PREV.br_startblock));
> +
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx - 1),
> -			LEFT.br_blockcount + new->br_blockcount);
> -		xfs_bmbt_set_startoff(ep,
> -			PREV.br_startoff + new->br_blockcount);
> +		LEFT.br_blockcount += new->br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx - 1, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
>  
> -		temp = PREV.br_blockcount - new->br_blockcount;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> +		PREV.br_blockcount = temp = PREV.br_blockcount - new->br_blockcount;
> +		PREV.br_startoff += new->br_blockcount;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
> +		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock, LEFT.br_blockcount,
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock, old.br_blockcount,
>  					&i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					new->br_blockcount,
> +					LEFT.br_startblock, LEFT.br_blockcount,
>  					LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> -		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> -			startblockval(PREV.br_startblock));
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx--;
>  		break;
> @@ -1880,10 +1877,6 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the first part of a previous delayed allocation.
>  		 * The left neighbor is not contiguous.
>  		 */
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startoff(ep, new_endoff);
> -		temp = PREV.br_blockcount - new->br_blockcount;
> -		xfs_bmbt_set_blockcount(ep, temp);
>  		xfs_iext_insert(bma->ip, bma->idx, 1, new, state);
>  		(*nextents)++;
>  		if (bma->cur == NULL)
> @@ -1911,12 +1904,19 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock) -
>  			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
> -		ep = xfs_iext_get_ext(ifp, bma->idx + 1);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +		PREV.br_startoff = new_endoff;
> +		PREV.br_blockcount = temp;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx + 1, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +
>  		break;
>  
>  	case BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
> @@ -1924,37 +1924,39 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the last part of a previous delayed allocation.
>  		 * The right neighbor is contiguous with the new allocation.
>  		 */
> -		temp = PREV.br_blockcount - new->br_blockcount;
> +		old = RIGHT;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> -		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, bma->idx + 1),
> -			new->br_startoff, new->br_startblock,
> -			new->br_blockcount + RIGHT.br_blockcount,
> -			RIGHT.br_state);
> +		RIGHT.br_startoff = new->br_startoff;
> +		RIGHT.br_startblock = new->br_startblock;
> +		RIGHT.br_blockcount += new->br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx + 1, &RIGHT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, RIGHT.br_startoff,
> -					RIGHT.br_startblock,
> -					RIGHT.br_blockcount, &i);
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock,
> +					old.br_blockcount, &i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> -			error = xfs_bmbt_update(bma->cur, new->br_startoff,
> -					new->br_startblock,
> -					new->br_blockcount +
> -					RIGHT.br_blockcount,
> +			error = xfs_bmbt_update(bma->cur, RIGHT.br_startoff,
> +					RIGHT.br_startblock, RIGHT.br_blockcount,
>  					RIGHT.br_state);
>  			if (error)
>  				goto done;
>  		}
>  
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock));
> +
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +		PREV.br_blockcount = temp;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx++;
> @@ -1965,9 +1967,6 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the last part of a previous delayed allocation.
>  		 * The right neighbor is not contiguous.
>  		 */
> -		temp = PREV.br_blockcount - new->br_blockcount;
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
>  		xfs_iext_insert(bma->ip, bma->idx + 1, 1, new, state);
>  		(*nextents)++;
>  		if (bma->cur == NULL)
> @@ -1995,11 +1994,16 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock) -
>  			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
> -		ep = xfs_iext_get_ext(ifp, bma->idx);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> +		PREV.br_startblock = nullstartblock(da_new);
> +		PREV.br_blockcount = temp;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx++;
> @@ -2026,19 +2030,33 @@ xfs_bmap_add_extent_delay_real(
>  		 *  PREV @ idx          LEFT              RIGHT
>  		 *                      inserted at idx + 1
>  		 */
> -		temp = new->br_startoff - PREV.br_startoff;
> -		temp2 = PREV.br_startoff + PREV.br_blockcount - new_endoff;
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);	/* truncate PREV */
> +		old = PREV;
> +
> +		/* LEFT is the new middle */
>  		LEFT = *new;
> +
> +		/* RIGHT is the new right */
>  		RIGHT.br_state = PREV.br_state;
> -		RIGHT.br_startblock = nullstartblock(
> -				(int)xfs_bmap_worst_indlen(bma->ip, temp2));
>  		RIGHT.br_startoff = new_endoff;
> -		RIGHT.br_blockcount = temp2;
> +		RIGHT.br_blockcount =
> +			PREV.br_startoff + PREV.br_blockcount - new_endoff;
> +		RIGHT.br_startblock =
> +			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
> +					RIGHT.br_blockcount));
> +
> +		/* truncate PREV */
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
> +		PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
> +		PREV.br_startblock =
> +			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
> +					PREV.br_blockcount));
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
> +		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> +
>  		/* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */
>  		xfs_iext_insert(bma->ip, bma->idx + 1, 2, &LEFT, state);
>  		(*nextents)++;
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
>  		else {
> @@ -2064,12 +2082,12 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> -		temp = xfs_bmap_worst_indlen(bma->ip, temp);
> -		temp2 = xfs_bmap_worst_indlen(bma->ip, temp2);
> -		diff = (int)(temp + temp2 -
> -			     (startblockval(PREV.br_startblock) -
> -			      (bma->cur ?
> -			       bma->cur->bc_private.b.allocated : 0)));
> +
> +		da_new = startblockval(PREV.br_startblock) +
> +			 startblockval(RIGHT.br_startblock);
> +		diff = da_new - startblockval(old.br_startblock);
> +		if (bma->cur)
> +			diff += bma->cur->bc_private.b.allocated;
>  		if (diff > 0) {
>  			error = xfs_mod_fdblocks(bma->ip->i_mount,
>  						 -((int64_t)diff), false);
> @@ -2078,16 +2096,7 @@ xfs_bmap_add_extent_delay_real(
>  				goto done;
>  		}
>  
> -		ep = xfs_iext_get_ext(ifp, bma->idx);
> -		xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, bma->idx + 2),
> -			nullstartblock((int)temp2));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
> -
>  		bma->idx++;
> -		da_new = temp + temp2;
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
> -- 
> 2.14.1
> 
> --
> 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real
  2017-09-22 13:59 ` [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real Christoph Hellwig
  2017-09-22 15:31   ` Brian Foster
@ 2017-09-22 16:12   ` Darrick J. Wong
  1 sibling, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2017-09-22 16:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Sep 22, 2017 at 06:59:38AM -0700, Christoph Hellwig wrote:
> Use xfs_iext_get_extent to find, and xfs_iext_update_extent to update
> entries in the in-core extent list.  This isolates the function from
> the detailed layout of the extent list, and generally makes the code
> a lot more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 181 +++++++++++++++++++++++++----------------------
>  1 file changed, 95 insertions(+), 86 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4fb73c0aca05..e74e2d74cd36 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1588,7 +1588,6 @@ xfs_bmap_add_extent_delay_real(
>  {
>  	struct xfs_bmbt_irec	*new = &bma->got;
>  	int			diff;	/* temp value */
> -	xfs_bmbt_rec_host_t	*ep;	/* extent entry for idx */
>  	int			error;	/* error return value */
>  	int			i;	/* temp state */
>  	xfs_ifork_t		*ifp;	/* inode fork pointer */
> @@ -1600,10 +1599,10 @@ xfs_bmap_add_extent_delay_real(
>  	xfs_filblks_t		da_new; /* new count del alloc blocks used */
>  	xfs_filblks_t		da_old; /* old count del alloc blocks used */
>  	xfs_filblks_t		temp=0;	/* value for da_new calculations */
> -	xfs_filblks_t		temp2=0;/* value for da_new calculations */
>  	int			tmp_rval;	/* partial logging flags */
>  	struct xfs_mount	*mp;
>  	xfs_extnum_t		*nextents;
> +	struct xfs_bmbt_irec	old;
>  
>  	mp = bma->ip->i_mount;
>  	ifp = XFS_IFORK_PTR(bma->ip, whichfork);
> @@ -1629,9 +1628,9 @@ xfs_bmap_add_extent_delay_real(
>  	/*
>  	 * Set up a bunch of variables to make the tests simpler.
>  	 */
> -	ep = xfs_iext_get_ext(ifp, bma->idx);
> -	xfs_bmbt_get_all(ep, &PREV);
> +	xfs_iext_get_extent(ifp, bma->idx, &PREV);
>  	new_endoff = new->br_startoff + new->br_blockcount;
> +	ASSERT(isnullstartblock(PREV.br_startblock));
>  	ASSERT(PREV.br_startoff <= new->br_startoff);
>  	ASSERT(PREV.br_startoff + PREV.br_blockcount >= new_endoff);
>  
> @@ -1706,9 +1705,8 @@ xfs_bmap_add_extent_delay_real(
>  		 */
>  		bma->idx--;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
> -			LEFT.br_blockcount + PREV.br_blockcount +
> -			RIGHT.br_blockcount);
> +		LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 2, state);
> @@ -1733,9 +1731,7 @@ xfs_bmap_add_extent_delay_real(
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
>  					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					PREV.br_blockcount +
> -					RIGHT.br_blockcount, LEFT.br_state);
> +					LEFT.br_blockcount, LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1748,9 +1744,10 @@ xfs_bmap_add_extent_delay_real(
>  		 */
>  		bma->idx--;
>  
> +		old = LEFT;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx),
> -			LEFT.br_blockcount + PREV.br_blockcount);
> +		LEFT.br_blockcount += PREV.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> @@ -1758,16 +1755,15 @@ xfs_bmap_add_extent_delay_real(
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock, LEFT.br_blockcount,
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock, old.br_blockcount,
>  					&i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
>  					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					PREV.br_blockcount, LEFT.br_state);
> +					LEFT.br_blockcount, LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1779,9 +1775,9 @@ xfs_bmap_add_extent_delay_real(
>  		 * The right neighbor is contiguous, the left is not.
>  		 */
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, new->br_startblock);
> -		xfs_bmbt_set_blockcount(ep,
> -			PREV.br_blockcount + RIGHT.br_blockcount);
> +		PREV.br_startblock = new->br_startblock;
> +		PREV.br_blockcount += RIGHT.br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> @@ -1796,9 +1792,8 @@ xfs_bmap_add_extent_delay_real(
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, PREV.br_startoff,
> -					new->br_startblock,
> -					PREV.br_blockcount +
> -					RIGHT.br_blockcount, PREV.br_state);
> +					PREV.br_startblock,
> +					PREV.br_blockcount, PREV.br_state);
>  			if (error)
>  				goto done;
>  		}
> @@ -1811,8 +1806,9 @@ xfs_bmap_add_extent_delay_real(
>  		 * the new one.
>  		 */
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, new->br_startblock);
> -		xfs_bmbt_set_state(ep, new->br_state);
> +		PREV.br_startblock = new->br_startblock;
> +		PREV.br_state = new->br_state;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		(*nextents)++;
> @@ -1839,38 +1835,39 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the first part of a previous delayed allocation.
>  		 * The left neighbor is contiguous.
>  		 */
> +		old = LEFT;
> +		temp = PREV.br_blockcount - new->br_blockcount;
> +		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> +				startblockval(PREV.br_startblock));
> +
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, bma->idx - 1),
> -			LEFT.br_blockcount + new->br_blockcount);
> -		xfs_bmbt_set_startoff(ep,
> -			PREV.br_startoff + new->br_blockcount);
> +		LEFT.br_blockcount += new->br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx - 1, &LEFT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
>  
> -		temp = PREV.br_blockcount - new->br_blockcount;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> +		PREV.br_blockcount = temp = PREV.br_blockcount - new->br_blockcount;
> +		PREV.br_startoff += new->br_blockcount;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
> +		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock, LEFT.br_blockcount,
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock, old.br_blockcount,
>  					&i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  			error = xfs_bmbt_update(bma->cur, LEFT.br_startoff,
> -					LEFT.br_startblock,
> -					LEFT.br_blockcount +
> -					new->br_blockcount,
> +					LEFT.br_startblock, LEFT.br_blockcount,
>  					LEFT.br_state);
>  			if (error)
>  				goto done;
>  		}
> -		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> -			startblockval(PREV.br_startblock));
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx--;
>  		break;
> @@ -1880,10 +1877,6 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the first part of a previous delayed allocation.
>  		 * The left neighbor is not contiguous.
>  		 */
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startoff(ep, new_endoff);
> -		temp = PREV.br_blockcount - new->br_blockcount;
> -		xfs_bmbt_set_blockcount(ep, temp);
>  		xfs_iext_insert(bma->ip, bma->idx, 1, new, state);
>  		(*nextents)++;
>  		if (bma->cur == NULL)
> @@ -1911,12 +1904,19 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock) -
>  			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
> -		ep = xfs_iext_get_ext(ifp, bma->idx + 1);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +		PREV.br_startoff = new_endoff;
> +		PREV.br_blockcount = temp;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx + 1, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +
>  		break;
>  
>  	case BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
> @@ -1924,37 +1924,39 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the last part of a previous delayed allocation.
>  		 * The right neighbor is contiguous with the new allocation.
>  		 */
> -		temp = PREV.br_blockcount - new->br_blockcount;
> +		old = RIGHT;
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> -		xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, bma->idx + 1),
> -			new->br_startoff, new->br_startblock,
> -			new->br_blockcount + RIGHT.br_blockcount,
> -			RIGHT.br_state);
> +		RIGHT.br_startoff = new->br_startoff;
> +		RIGHT.br_startblock = new->br_startblock;
> +		RIGHT.br_blockcount += new->br_blockcount;
> +		xfs_iext_update_extent(ifp, bma->idx + 1, &RIGHT);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_DEXT;
>  		else {
>  			rval = 0;
> -			error = xfs_bmbt_lookup_eq(bma->cur, RIGHT.br_startoff,
> -					RIGHT.br_startblock,
> -					RIGHT.br_blockcount, &i);
> +			error = xfs_bmbt_lookup_eq(bma->cur, old.br_startoff,
> +					old.br_startblock,
> +					old.br_blockcount, &i);
>  			if (error)
>  				goto done;
>  			XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> -			error = xfs_bmbt_update(bma->cur, new->br_startoff,
> -					new->br_startblock,
> -					new->br_blockcount +
> -					RIGHT.br_blockcount,
> +			error = xfs_bmbt_update(bma->cur, RIGHT.br_startoff,
> +					RIGHT.br_startblock, RIGHT.br_blockcount,
>  					RIGHT.br_state);
>  			if (error)
>  				goto done;
>  		}
>  
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock));
> +
>  		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +		PREV.br_blockcount = temp;
> +		PREV.br_startblock = nullstartblock(da_new);
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx++;
> @@ -1965,9 +1967,6 @@ xfs_bmap_add_extent_delay_real(
>  		 * Filling in the last part of a previous delayed allocation.
>  		 * The right neighbor is not contiguous.
>  		 */
> -		temp = PREV.br_blockcount - new->br_blockcount;
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
>  		xfs_iext_insert(bma->ip, bma->idx + 1, 1, new, state);
>  		(*nextents)++;
>  		if (bma->cur == NULL)
> @@ -1995,11 +1994,16 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +
> +		temp = PREV.br_blockcount - new->br_blockcount;
>  		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
>  			startblockval(PREV.br_startblock) -
>  			(bma->cur ? bma->cur->bc_private.b.allocated : 0));
> -		ep = xfs_iext_get_ext(ifp, bma->idx);
> -		xfs_bmbt_set_startblock(ep, nullstartblock(da_new));
> +
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> +		PREV.br_startblock = nullstartblock(da_new);
> +		PREV.br_blockcount = temp;
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
>  		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
>  
>  		bma->idx++;
> @@ -2026,19 +2030,33 @@ xfs_bmap_add_extent_delay_real(
>  		 *  PREV @ idx          LEFT              RIGHT
>  		 *                      inserted at idx + 1
>  		 */
> -		temp = new->br_startoff - PREV.br_startoff;
> -		temp2 = PREV.br_startoff + PREV.br_blockcount - new_endoff;
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);	/* truncate PREV */
> +		old = PREV;
> +
> +		/* LEFT is the new middle */
>  		LEFT = *new;
> +
> +		/* RIGHT is the new right */
>  		RIGHT.br_state = PREV.br_state;
> -		RIGHT.br_startblock = nullstartblock(
> -				(int)xfs_bmap_worst_indlen(bma->ip, temp2));
>  		RIGHT.br_startoff = new_endoff;
> -		RIGHT.br_blockcount = temp2;
> +		RIGHT.br_blockcount =
> +			PREV.br_startoff + PREV.br_blockcount - new_endoff;
> +		RIGHT.br_startblock =
> +			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
> +					RIGHT.br_blockcount));
> +
> +		/* truncate PREV */
> +		trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
> +		PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
> +		PREV.br_startblock =
> +			nullstartblock(xfs_bmap_worst_indlen(bma->ip,
> +					PREV.br_blockcount));
> +		xfs_iext_update_extent(ifp, bma->idx, &PREV);
> +		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> +
>  		/* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */
>  		xfs_iext_insert(bma->ip, bma->idx + 1, 2, &LEFT, state);
>  		(*nextents)++;
> +
>  		if (bma->cur == NULL)
>  			rval = XFS_ILOG_CORE | XFS_ILOG_DEXT;
>  		else {
> @@ -2064,12 +2082,12 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> -		temp = xfs_bmap_worst_indlen(bma->ip, temp);
> -		temp2 = xfs_bmap_worst_indlen(bma->ip, temp2);
> -		diff = (int)(temp + temp2 -
> -			     (startblockval(PREV.br_startblock) -
> -			      (bma->cur ?
> -			       bma->cur->bc_private.b.allocated : 0)));
> +
> +		da_new = startblockval(PREV.br_startblock) +
> +			 startblockval(RIGHT.br_startblock);
> +		diff = da_new - startblockval(old.br_startblock);
> +		if (bma->cur)
> +			diff += bma->cur->bc_private.b.allocated;
>  		if (diff > 0) {
>  			error = xfs_mod_fdblocks(bma->ip->i_mount,
>  						 -((int64_t)diff), false);
> @@ -2078,16 +2096,7 @@ xfs_bmap_add_extent_delay_real(
>  				goto done;
>  		}
>  
> -		ep = xfs_iext_get_ext(ifp, bma->idx);
> -		xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> -		trace_xfs_bmap_pre_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
> -		xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, bma->idx + 2),
> -			nullstartblock((int)temp2));
> -		trace_xfs_bmap_post_update(bma->ip, bma->idx + 2, state, _THIS_IP_);
> -
>  		bma->idx++;
> -		da_new = temp + temp2;
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
> -- 
> 2.14.1
> 
> --
> 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real
  2017-09-18 15:24 ` [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real Christoph Hellwig
@ 2017-09-20 21:41   ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2017-09-20 21:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Mon, Sep 18, 2017 at 08:24:11AM -0700, Christoph Hellwig wrote:
> Use the same defines as the other extent add and delete helpers, which
> both improves code readability and trace point output.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

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

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index aa4af31750d3..037efc97499f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5162,13 +5162,13 @@ xfs_bmap_del_extent_real(
>  		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  	}
>  
> -	/*
> -	 * Set flag value to use in switch statement.
> -	 * Left-contig is 2, right-contig is 1.
> -	 */
> -	switch (((got.br_startoff == del->br_startoff) << 1) |
> -		(got_endoff == del_endoff)) {
> -	case 3:
> +	if (got.br_startoff == del->br_startoff)
> +		state |= BMAP_LEFT_FILLING;
> +	if (got_endoff == del_endoff)
> +		state |= BMAP_RIGHT_FILLING;
> +
> +	switch (state & (BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING)) {
> +	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
>  		/*
>  		 * Matches the whole extent.  Delete the entry.
>  		 */
> @@ -5188,8 +5188,7 @@ xfs_bmap_del_extent_real(
>  			goto done;
>  		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
>  		break;
> -
> -	case 2:
> +	case BMAP_LEFT_FILLING:
>  		/*
>  		 * Deleting the first part of the extent.
>  		 */
> @@ -5208,8 +5207,7 @@ xfs_bmap_del_extent_real(
>  				got.br_state)))
>  			goto done;
>  		break;
> -
> -	case 1:
> +	case BMAP_RIGHT_FILLING:
>  		/*
>  		 * Deleting the last part of the extent.
>  		 */
> @@ -5227,7 +5225,6 @@ xfs_bmap_del_extent_real(
>  				got.br_state)))
>  			goto done;
>  		break;
> -
>  	case 0:
>  		/*
>  		 * Deleting the middle of the extent.
> -- 
> 2.14.1
> 
> --
> 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real
  2017-09-18 15:24 refactor extent manipulation V3 Christoph Hellwig
@ 2017-09-18 15:24 ` Christoph Hellwig
  2017-09-20 21:41   ` Darrick J. Wong
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2017-09-18 15:24 UTC (permalink / raw)
  To: linux-xfs

Use the same defines as the other extent add and delete helpers, which
both improves code readability and trace point output.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index aa4af31750d3..037efc97499f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5162,13 +5162,13 @@ xfs_bmap_del_extent_real(
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 	}
 
-	/*
-	 * Set flag value to use in switch statement.
-	 * Left-contig is 2, right-contig is 1.
-	 */
-	switch (((got.br_startoff == del->br_startoff) << 1) |
-		(got_endoff == del_endoff)) {
-	case 3:
+	if (got.br_startoff == del->br_startoff)
+		state |= BMAP_LEFT_FILLING;
+	if (got_endoff == del_endoff)
+		state |= BMAP_RIGHT_FILLING;
+
+	switch (state & (BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING)) {
+	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
 		/*
 		 * Matches the whole extent.  Delete the entry.
 		 */
@@ -5188,8 +5188,7 @@ xfs_bmap_del_extent_real(
 			goto done;
 		XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
 		break;
-
-	case 2:
+	case BMAP_LEFT_FILLING:
 		/*
 		 * Deleting the first part of the extent.
 		 */
@@ -5208,8 +5207,7 @@ xfs_bmap_del_extent_real(
 				got.br_state)))
 			goto done;
 		break;
-
-	case 1:
+	case BMAP_RIGHT_FILLING:
 		/*
 		 * Deleting the last part of the extent.
 		 */
@@ -5227,7 +5225,6 @@ xfs_bmap_del_extent_real(
 				got.br_state)))
 			goto done;
 		break;
-
 	case 0:
 		/*
 		 * Deleting the middle of the extent.
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-09-22 16:12 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 13:59 refactor extent manipulation V4 Christoph Hellwig
2017-09-22 13:59 ` [PATCH 01/19] xfs: fix incorrect extent state in xfs_bmap_add_extent_unwritten_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 02/19] xfs: use xfs_iext_get_extent instead of open coding it Christoph Hellwig
2017-09-22 13:59 ` [PATCH 03/19] xfs: don't set XFS_BTCUR_BPRV_WASDEL in xfs_bunmapi Christoph Hellwig
2017-09-22 13:59 ` [PATCH 04/19] xfs: rename bno to end in __xfs_bunmapi Christoph Hellwig
2017-09-22 13:59 ` [PATCH 05/19] xfs: use xfs_bmap_del_extent_delay for the data fork as well Christoph Hellwig
2017-09-22 13:59 ` [PATCH 06/19] xfs: move some more code into xfs_bmap_del_extent_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 07/19] xfs: use correct state defines in xfs_bmap_del_extent_{cow,delay} Christoph Hellwig
2017-09-22 13:59 ` [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 09/19] xfs: refactor xfs_del_extent_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 10/19] xfs: refactor xfs_bmap_add_extent_hole_delay Christoph Hellwig
2017-09-22 13:59 ` [PATCH 11/19] xfs: refactor xfs_bmap_add_extent_hole_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 12/19] xfs: refactor xfs_bmap_add_extent_delay_real Christoph Hellwig
2017-09-22 15:31   ` Brian Foster
2017-09-22 16:12   ` Darrick J. Wong
2017-09-22 13:59 ` [PATCH 13/19] xfs: refactor delalloc accounting in xfs_bmap_add_extent_delay_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 14/19] xfs: refactor xfs_bmap_add_extent_unwritten_real Christoph Hellwig
2017-09-22 13:59 ` [PATCH 15/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_update Christoph Hellwig
2017-09-22 13:59 ` [PATCH 16/19] xfs: pass a struct xfs_bmbt_irec to xfs_bmbt_lookup_eq Christoph Hellwig
2017-09-22 13:59 ` [PATCH 17/19] xfs: replace xfs_bmbt_lookup_ge with xfs_bmbt_lookup_first Christoph Hellwig
2017-09-22 13:59 ` [PATCH 18/19] xfs: remove all xfs_bmbt_set_* helpers except for xfs_bmbt_set_all Christoph Hellwig
2017-09-22 13:59 ` [PATCH 19/19] xfs: remove xfs_bmbt_get_state Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2017-09-18 15:24 refactor extent manipulation V3 Christoph Hellwig
2017-09-18 15:24 ` [PATCH 08/19] xfs: use the state defines in xfs_bmap_del_extent_real Christoph Hellwig
2017-09-20 21:41   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.