xfs_bmapi_write retval fix v2

xfs_bmapi_write retval fix v2

[Christoph Hellwig]

Hi all,

this series addresses our long standing issue with confusing return
values from xfs_bmapi_write.

The first patch is the actual return value fix for the fuzzer-reported
issue.

The second to last patch cleans up and fixes long-standing issues in
xfs_bmap_add_extent_delay_real in code paths that haven't been used much
or at all.

The last patch changes xfs_bmapi_write to not convert the entire
delalloc extent if it hits one.  This sounds simpler than it is, because
delalloc conversion has been designed to always convert the entire
extent since the initial delalloc commits.

While this has gotten a bit more testing by now, this will still stress
code never used.  I've also looked into the alternative of having all
callers handle short delalloc conversions, but it doesn't look appealing
at all.

Changes since v1:
 - rebased to the latest xfs-for-next tree with a minor conflict
 - spelling fixes

[PATCH 1/9] xfs: fix error returns from xfs_bmapi_write

[Christoph Hellwig]

xfs_bmapi_write can return 0 without actually returning a mapping in
mval in two different cases:

 1) when there is absolutely no space available to do an allocation
 2) when converting delalloc space, and the allocation is so small
    that it only covers parts of the delalloc extent before the
    range requested by the caller

Callers at best can handle one of these cases, but in many cases can't
cope with either one.  Switch xfs_bmapi_write to always return a
mapping or return an error code instead.  For case 1) above ENOSPC is
the obvious choice which is very much what the callers expect anyway.
For case 2) there is no really good error code, so pick a funky one
from the SysV streams portfolio.

This fixes the reproducer here:

    https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/

which uses reserved blocks to create file systems that are gravely
out of space and thus cause at least xfs_file_alloc_space to hang
and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc.

Note that this patch does not actually make any caller but
xfs_alloc_file_space deal intelligently with case 2) above.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: 刘通 <lyutoon@gmail.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_attr_remote.c |  1 -
 fs/xfs/libxfs/xfs_bmap.c        | 46 ++++++++++++++++++++++++++-------
 fs/xfs/libxfs/xfs_da_btree.c    | 20 ++++----------
 fs/xfs/scrub/quota_repair.c     |  6 -----
 fs/xfs/scrub/rtbitmap_repair.c  |  2 --
 fs/xfs/xfs_bmap_util.c          | 31 +++++++++++-----------
 fs/xfs/xfs_dquot.c              |  1 -
 fs/xfs/xfs_iomap.c              |  8 ------
 fs/xfs/xfs_reflink.c            | 14 ----------
 fs/xfs/xfs_rtalloc.c            |  2 --
 10 files changed, 57 insertions(+), 74 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index a8de9dc1e998a3..beb0efdd8f6b83 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -625,7 +625,6 @@ xfs_attr_rmtval_set_blk(
 	if (error)
 		return error;
 
-	ASSERT(nmap == 1);
 	ASSERT((map->br_startblock != DELAYSTARTBLOCK) &&
 	       (map->br_startblock != HOLESTARTBLOCK));
 
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6053f5e5c71eec..f19191d6eade7e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4217,8 +4217,10 @@ xfs_bmapi_allocate(
 	} else {
 		error = xfs_bmap_alloc_userdata(bma);
 	}
-	if (error || bma->blkno == NULLFSBLOCK)
+	if (error)
 		return error;
+	if (bma->blkno == NULLFSBLOCK)
+		return -ENOSPC;
 
 	if (bma->flags & XFS_BMAPI_ZERO) {
 		error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
@@ -4397,6 +4399,15 @@ xfs_bmapi_finish(
  * extent state if necessary.  Details behaviour is controlled by the flags
  * parameter.  Only allocates blocks from a single allocation group, to avoid
  * locking problems.
+ *
+ * Returns 0 on success and places the extent mappings in mval.  nmaps is used
+ * as an input/output parameter where the caller specifies the maximum number
+ * of mappings that may be returned and xfs_bmapi_write passes back the number
+ * of mappings (including existing mappings) it found.
+ *
+ * Returns a negative error code on failure, including -ENOSPC when it could not
+ * allocate any blocks and -ENOSR when it did allocate blocks to convert a
+ * delalloc range, but those blocks were before the passed in range.
  */
 int
 xfs_bmapi_write(
@@ -4525,10 +4536,16 @@ xfs_bmapi_write(
 			ASSERT(len > 0);
 			ASSERT(bma.length > 0);
 			error = xfs_bmapi_allocate(&bma);
-			if (error)
+			if (error) {
+				/*
+				 * If we already allocated space in a previous
+				 * iteration return what we go so far when
+				 * running out of space.
+				 */
+				if (error == -ENOSPC && bma.nallocs)
+					break;
 				goto error0;
-			if (bma.blkno == NULLFSBLOCK)
-				break;
+			}
 
 			/*
 			 * If this is a CoW allocation, record the data in
@@ -4566,7 +4583,6 @@ xfs_bmapi_write(
 		if (!xfs_iext_next_extent(ifp, &bma.icur, &bma.got))
 			eof = true;
 	}
-	*nmap = n;
 
 	error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags,
 			whichfork);
@@ -4577,7 +4593,22 @@ xfs_bmapi_write(
 	       ifp->if_nextents > XFS_IFORK_MAXEXT(ip, whichfork));
 	xfs_bmapi_finish(&bma, whichfork, 0);
 	xfs_bmap_validate_ret(orig_bno, orig_len, orig_flags, orig_mval,
-		orig_nmap, *nmap);
+		orig_nmap, n);
+
+	/*
+	 * When converting delayed allocations, xfs_bmapi_allocate ignores
+	 * the passed in bno and always converts from the start of the found
+	 * delalloc extent.
+	 *
+	 * To avoid a successful return with *nmap set to 0, return the magic
+	 * -ENOSR error code for this particular case so that the caller can
+	 * handle it.
+	 */
+	if (!n) {
+		ASSERT(bma.nallocs >= *nmap);
+		return -ENOSR;
+	}
+	*nmap = n;
 	return 0;
 error0:
 	xfs_bmapi_finish(&bma, whichfork, error);
@@ -4684,9 +4715,6 @@ xfs_bmapi_convert_delalloc(
 	if (error)
 		goto out_finish;
 
-	error = -ENOSPC;
-	if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
-		goto out_finish;
 	if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock))) {
 		xfs_bmap_mark_sick(ip, whichfork);
 		error = -EFSCORRUPTED;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index b13796629e2213..16a529a8878083 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2297,8 +2297,8 @@ xfs_da_grow_inode_int(
 	struct xfs_inode	*dp = args->dp;
 	int			w = args->whichfork;
 	xfs_rfsblock_t		nblks = dp->i_nblocks;
-	struct xfs_bmbt_irec	map, *mapp;
-	int			nmap, error, got, i, mapi;
+	struct xfs_bmbt_irec	map, *mapp = &map;
+	int			nmap, error, got, i, mapi = 1;
 
 	/*
 	 * Find a spot in the file space to put the new block.
@@ -2314,14 +2314,7 @@ xfs_da_grow_inode_int(
 	error = xfs_bmapi_write(tp, dp, *bno, count,
 			xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
 			args->total, &map, &nmap);
-	if (error)
-		return error;
-
-	ASSERT(nmap <= 1);
-	if (nmap == 1) {
-		mapp = &map;
-		mapi = 1;
-	} else if (nmap == 0 && count > 1) {
+	if (error == -ENOSPC && count > 1) {
 		xfs_fileoff_t		b;
 		int			c;
 
@@ -2339,16 +2332,13 @@ xfs_da_grow_inode_int(
 					args->total, &mapp[mapi], &nmap);
 			if (error)
 				goto out_free_map;
-			if (nmap < 1)
-				break;
 			mapi += nmap;
 			b = mapp[mapi - 1].br_startoff +
 			    mapp[mapi - 1].br_blockcount;
 		}
-	} else {
-		mapi = 0;
-		mapp = NULL;
 	}
+	if (error)
+		goto out_free_map;
 
 	/*
 	 * Count the blocks we got, make sure it matches the total.
diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index 0bab4c30cb85ab..90cd1512bba961 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -77,8 +77,6 @@ xrep_quota_item_fill_bmap_hole(
 			irec, &nmaps);
 	if (error)
 		return error;
-	if (nmaps != 1)
-		return -ENOSPC;
 
 	dq->q_blkno = XFS_FSB_TO_DADDR(mp, irec->br_startblock);
 
@@ -444,10 +442,6 @@ xrep_quota_data_fork(
 					XFS_BMAPI_CONVERT, 0, &nrec, &nmap);
 			if (error)
 				goto out;
-			if (nmap != 1) {
-				error = -ENOSPC;
-				goto out;
-			}
 			ASSERT(nrec.br_startoff == irec.br_startoff);
 			ASSERT(nrec.br_blockcount == irec.br_blockcount);
 
diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index 46f5d5f605c915..0fef98e9f83409 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -108,8 +108,6 @@ xrep_rtbitmap_data_mappings(
 				0, &map, &nmaps);
 		if (error)
 			return error;
-		if (nmaps != 1)
-			return -EFSCORRUPTED;
 
 		/* Commit new extent and all deferred work. */
 		error = xrep_defer_finish(sc);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 53aa90a0ee3a85..2e6f08198c0719 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -721,33 +721,32 @@ xfs_alloc_file_space(
 		if (error)
 			goto error;
 
-		error = xfs_bmapi_write(tp, ip, startoffset_fsb,
-				allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
-				&nimaps);
-		if (error)
-			goto error;
-
-		ip->i_diflags |= XFS_DIFLAG_PREALLOC;
-		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-
-		error = xfs_trans_commit(tp);
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-		if (error)
-			break;
-
 		/*
 		 * If the allocator cannot find a single free extent large
 		 * enough to cover the start block of the requested range,
-		 * xfs_bmapi_write will return 0 but leave *nimaps set to 0.
+		 * xfs_bmapi_write will return -ENOSR.
 		 *
 		 * In that case we simply need to keep looping with the same
 		 * startoffset_fsb so that one of the following allocations
 		 * will eventually reach the requested range.
 		 */
-		if (nimaps) {
+		error = xfs_bmapi_write(tp, ip, startoffset_fsb,
+				allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
+				&nimaps);
+		if (error) {
+			if (error != -ENOSR)
+				goto error;
+			error = 0;
+		} else {
 			startoffset_fsb += imapp->br_blockcount;
 			allocatesize_fsb -= imapp->br_blockcount;
 		}
+
+		ip->i_diflags |= XFS_DIFLAG_PREALLOC;
+		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+		error = xfs_trans_commit(tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	}
 
 	return error;
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 13aba84bd64afb..43acb4f0d17433 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -357,7 +357,6 @@ xfs_dquot_disk_alloc(
 		goto err_cancel;
 
 	ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
-	ASSERT(nmaps == 1);
 	ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
 	       (map.br_startblock != HOLESTARTBLOCK));
 
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 9ce0f6b9df93e6..60463160820b62 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -322,14 +322,6 @@ xfs_iomap_write_direct(
 	if (error)
 		goto out_unlock;
 
-	/*
-	 * Copy any maps to caller's array and return any error.
-	 */
-	if (nimaps == 0) {
-		error = -ENOSPC;
-		goto out_unlock;
-	}
-
 	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
 		xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
 		error = xfs_alert_fsblock_zero(ip, imap);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 7da0e8f961d351..5ecb52a234becc 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -430,13 +430,6 @@ xfs_reflink_fill_cow_hole(
 	if (error)
 		return error;
 
-	/*
-	 * Allocation succeeded but the requested range was not even partially
-	 * satisfied?  Bail out!
-	 */
-	if (nimaps == 0)
-		return -ENOSPC;
-
 convert:
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
 
@@ -499,13 +492,6 @@ xfs_reflink_fill_delalloc(
 		error = xfs_trans_commit(tp);
 		if (error)
 			return error;
-
-		/*
-		 * Allocation succeeded but the requested range was not even
-		 * partially satisfied?  Bail out!
-		 */
-		if (nimaps == 0)
-			return -ENOSPC;
 	} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
 
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index b476a876478d93..150f544445ca82 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -709,8 +709,6 @@ xfs_growfs_rt_alloc(
 		nmap = 1;
 		error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks,
 					XFS_BMAPI_METADATA, 0, &map, &nmap);
-		if (!error && nmap < 1)
-			error = -ENOSPC;
 		if (error)
 			goto out_trans_cancel;
 		/*
-- 
2.39.2

[PATCH 2/9] xfs: remove the unusued tmp_logflags variable in xfs_bmapi_allocate

[Christoph Hellwig]

tmp_logflags is initialized to 0 and then ORed into bma->logflags, which
isn't actually doing anything.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index f19191d6eade7e..1ea1b78ad5a560 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4182,7 +4182,6 @@ xfs_bmapi_allocate(
 	struct xfs_mount	*mp = bma->ip->i_mount;
 	int			whichfork = xfs_bmapi_whichfork(bma->flags);
 	struct xfs_ifork	*ifp = xfs_ifork_ptr(bma->ip, whichfork);
-	int			tmp_logflags = 0;
 	int			error;
 
 	ASSERT(bma->length > 0);
@@ -4253,8 +4252,6 @@ xfs_bmapi_allocate(
 		error = xfs_bmap_add_extent_hole_real(bma->tp, bma->ip,
 				whichfork, &bma->icur, &bma->cur, &bma->got,
 				&bma->logflags, bma->flags);
-
-	bma->logflags |= tmp_logflags;
 	if (error)
 		return error;
 
-- 
2.39.2

[PATCH 3/9] xfs: lift a xfs_valid_startblock into xfs_bmapi_allocate

[Christoph Hellwig]

xfs_bmapi_convert_delalloc has a xfs_valid_startblock check on the block
allocated by xfs_bmapi_allocate.  Lift it into xfs_bmapi_allocate as
we should assert the same for xfs_bmapi_write.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1ea1b78ad5a560..ee8f86c03185fc 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4221,6 +4221,11 @@ xfs_bmapi_allocate(
 	if (bma->blkno == NULLFSBLOCK)
 		return -ENOSPC;
 
+	if (WARN_ON_ONCE(!xfs_valid_startblock(bma->ip, bma->blkno))) {
+		xfs_bmap_mark_sick(bma->ip, whichfork);
+		return -EFSCORRUPTED;
+	}
+
 	if (bma->flags & XFS_BMAPI_ZERO) {
 		error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
 		if (error)
@@ -4712,12 +4717,6 @@ xfs_bmapi_convert_delalloc(
 	if (error)
 		goto out_finish;
 
-	if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock))) {
-		xfs_bmap_mark_sick(ip, whichfork);
-		error = -EFSCORRUPTED;
-		goto out_finish;
-	}
-
 	XFS_STATS_ADD(mp, xs_xstrat_bytes, XFS_FSB_TO_B(mp, bma.length));
 	XFS_STATS_INC(mp, xs_xstrat_quick);
 
-- 
2.39.2

[PATCH 4/9] xfs: don't open code XFS_FILBLKS_MIN in xfs_bmapi_write

[Christoph Hellwig]

XFS_FILBLKS_MIN uses min_t and thus does the comparison using the correct
xfs_filblks_t type.  Use it in xfs_bmapi_write and slightly adjust the
comment document th potential pitfall to take account of this

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ee8f86c03185fc..f7b263d0b0cf1c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4528,14 +4528,11 @@ xfs_bmapi_write(
 			 * allocation length request (which can be 64 bits in
 			 * length) and the bma length request, which is
 			 * xfs_extlen_t and therefore 32 bits. Hence we have to
-			 * check for 32-bit overflows and handle them here.
+			 * be careful and do the min() using the larger type to
+			 * avoid overflows.
 			 */
-			if (len > (xfs_filblks_t)XFS_MAX_BMBT_EXTLEN)
-				bma.length = XFS_MAX_BMBT_EXTLEN;
-			else
-				bma.length = len;
+			bma.length = XFS_FILBLKS_MIN(len, XFS_MAX_BMBT_EXTLEN);
 
-			ASSERT(len > 0);
 			ASSERT(bma.length > 0);
 			error = xfs_bmapi_allocate(&bma);
 			if (error) {
-- 
2.39.2

[PATCH 5/9] xfs: pass the actual offset and len to allocate to xfs_bmapi_allocate

[Christoph Hellwig]

xfs_bmapi_allocate currently overwrites offset and len when converting
delayed allocations, and duplicates the length cap done for non-delalloc
allocations.  Move all that logic into the callers to avoid duplication
and to make the calling conventions more obvious.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index f7b263d0b0cf1c..fe1ccc083eb3c4 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4185,21 +4185,11 @@ xfs_bmapi_allocate(
 	int			error;
 
 	ASSERT(bma->length > 0);
+	ASSERT(bma->length <= XFS_MAX_BMBT_EXTLEN);
 
-	/*
-	 * For the wasdelay case, we could also just allocate the stuff asked
-	 * for in this bmap call but that wouldn't be as good.
-	 */
 	if (bma->wasdel) {
-		bma->length = (xfs_extlen_t)bma->got.br_blockcount;
-		bma->offset = bma->got.br_startoff;
 		if (!xfs_iext_peek_prev_extent(ifp, &bma->icur, &bma->prev))
 			bma->prev.br_startoff = NULLFILEOFF;
-	} else {
-		bma->length = XFS_FILBLKS_MIN(bma->length, XFS_MAX_BMBT_EXTLEN);
-		if (!bma->eof)
-			bma->length = XFS_FILBLKS_MIN(bma->length,
-					bma->got.br_startoff - bma->offset);
 	}
 
 	if (bma->flags & XFS_BMAPI_CONTIG)
@@ -4533,6 +4523,15 @@ xfs_bmapi_write(
 			 */
 			bma.length = XFS_FILBLKS_MIN(len, XFS_MAX_BMBT_EXTLEN);
 
+			if (wasdelay) {
+				bma.offset = bma.got.br_startoff;
+				bma.length = bma.got.br_blockcount;
+			} else {
+				if (!eof)
+					bma.length = XFS_FILBLKS_MIN(bma.length,
+						bma.got.br_startoff - bno);
+			}
+
 			ASSERT(bma.length > 0);
 			error = xfs_bmapi_allocate(&bma);
 			if (error) {
@@ -4685,11 +4684,16 @@ xfs_bmapi_convert_delalloc(
 	bma.tp = tp;
 	bma.ip = ip;
 	bma.wasdel = true;
-	bma.offset = bma.got.br_startoff;
-	bma.length = max_t(xfs_filblks_t, bma.got.br_blockcount,
-			XFS_MAX_BMBT_EXTLEN);
 	bma.minleft = xfs_bmapi_minleft(tp, ip, whichfork);
 
+	/*
+	 * Always allocate convert from the start of the delalloc extent even if
+	 * that is outside the passed in range to create large contiguous
+	 * extents on disk.
+	 */
+	bma.offset = bma.got.br_startoff;
+	bma.length = bma.got.br_blockcount;
+
 	/*
 	 * When we're converting the delalloc reservations backing dirty pages
 	 * in the page cache, we must be careful about how we create the new
-- 
2.39.2

[PATCH 6/9] xfs: remove the xfs_iext_peek_prev_extent call in xfs_bmapi_allocate

[Christoph Hellwig]

Both callers of xfs_bmapi_allocate already initialize bma->prev, don't
redo that in xfs_bmapi_allocate.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index fe1ccc083eb3c4..472c795beb8add 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4187,11 +4187,6 @@ xfs_bmapi_allocate(
 	ASSERT(bma->length > 0);
 	ASSERT(bma->length <= XFS_MAX_BMBT_EXTLEN);
 
-	if (bma->wasdel) {
-		if (!xfs_iext_peek_prev_extent(ifp, &bma->icur, &bma->prev))
-			bma->prev.br_startoff = NULLFILEOFF;
-	}
-
 	if (bma->flags & XFS_BMAPI_CONTIG)
 		bma->minlen = bma->length;
 	else
-- 
2.39.2

[PATCH 7/9] xfs: fix xfs_bmap_add_extent_delay_real for partial conversions

[Christoph Hellwig]

xfs_bmap_add_extent_delay_real takes parts or all of a delalloc extent
and converts them to a real extent.  It is written to deal with any
potential overlap of the to be converted range with the delalloc extent,
but it turns out that currently only converting the entire extents, or a
part starting at the beginning is actually exercised, as the only caller
always tries to convert the entire delalloc extent, and either succeeds
or at least progresses partially from the start.

If it only converts a tiny part of a delalloc extent, the indirect block
calculation for the new delalloc extent (da_new) might be equivalent to that
of the existing delalloc extent (da_old).  If this extent conversion now
requires allocating an indirect block that gets accounted into da_new,
leading to the assert that da_new must be smaller or equal to da_new
unless we split the extent to trigger.

Except for the assert that case is actually handled by just trying to
allocate more space, as that already handled for the split case (which
currently can't be reached at all), so just reusing it should be fine.
Except that without dipping into the reserved block pool that would make
it a bit too easy to trigger a fs shutdown due to ENOSPC.  So in addition
to adjusting the assert, also dip into the reserved block pool.

Note that I could only reproduce the assert with a change to only convert
the actually asked range instead of the full delalloc extent from
xfs_bmapi_write.

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

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 472c795beb8add..42c5a2efa656a5 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1570,6 +1570,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_LEFT_CONTIG:
@@ -1600,6 +1601,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
@@ -1634,6 +1636,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
@@ -1668,6 +1671,7 @@ xfs_bmap_add_extent_delay_real(
 				goto done;
 			}
 		}
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG:
@@ -1706,6 +1710,7 @@ xfs_bmap_add_extent_delay_real(
 			if (error)
 				goto done;
 		}
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_LEFT_FILLING:
@@ -1796,6 +1801,7 @@ xfs_bmap_add_extent_delay_real(
 		xfs_iext_update_extent(bma->ip, state, &bma->icur, &PREV);
 		xfs_iext_next(ifp, &bma->icur);
 		xfs_iext_update_extent(bma->ip, state, &bma->icur, &RIGHT);
+		ASSERT(da_new <= da_old);
 		break;
 
 	case BMAP_RIGHT_FILLING:
@@ -1845,6 +1851,7 @@ xfs_bmap_add_extent_delay_real(
 		PREV.br_blockcount = temp;
 		xfs_iext_insert(bma->ip, &bma->icur, &PREV, state);
 		xfs_iext_next(ifp, &bma->icur);
+		ASSERT(da_new <= da_old);
 		break;
 
 	case 0:
@@ -1967,12 +1974,10 @@ xfs_bmap_add_extent_delay_real(
 	}
 
 	/* adjust for changes in reserved delayed indirect blocks */
-	if (da_new < da_old) {
+	if (da_new < da_old)
 		xfs_add_fdblocks(mp, da_old - da_new);
-	} else if (da_new > da_old) {
-		ASSERT(state == 0);
-		error = xfs_dec_fdblocks(mp, da_new - da_old, false);
-	}
+	else if (da_new > da_old)
+		error = xfs_dec_fdblocks(mp, da_new - da_old, true);
 
 	xfs_bmap_check_leaf_extents(bma->cur, bma->ip, whichfork);
 done:
-- 
2.39.2

[PATCH 8/9] xfs: do not allocate the entire delalloc extent in xfs_bmapi_write

[Christoph Hellwig]

While trying to convert the entire delalloc extent is a good decision
for regular writeback as it leads to larger contigous on-disk extents,
but for other callers of xfs_bmapi_write is is rather questionable as
it forced them to loop creating new transactions just in case there
is no large enough contiguous extent to cover the whole delalloc
reservation.

Change xfs_bmapi_write to only allocate the passed in range instead,
whіle the writeback path through xfs_bmapi_convert_delalloc and
xfs_bmapi_allocate still always converts the full extents.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 42c5a2efa656a5..f5488cc975342b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4524,8 +4524,9 @@ xfs_bmapi_write(
 			bma.length = XFS_FILBLKS_MIN(len, XFS_MAX_BMBT_EXTLEN);
 
 			if (wasdelay) {
-				bma.offset = bma.got.br_startoff;
-				bma.length = bma.got.br_blockcount;
+				bma.length = XFS_FILBLKS_MIN(bma.length,
+					bma.got.br_blockcount -
+					(bno - bma.got.br_startoff));
 			} else {
 				if (!eof)
 					bma.length = XFS_FILBLKS_MIN(bma.length,
-- 
2.39.2

[PATCH 9/9] mm,page_owner: don't remove GFP flags in add_stack_record_to_list

[Christoph Hellwig]

This loses flags like GFP_NOFS and GFP_NOIO that are important to avoid
deadlocks as well as GFP_NOLOCKDEP that otherwise generates lockdep false
positives.

Fixes: 217b2119b9e2 ("mm,page_owner: implement the tracking of the stacks count")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 mm/page_owner.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index d17d1351ec84af..d214488846fa92 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -168,9 +168,7 @@ static void add_stack_record_to_list(struct stack_record *stack_record,
 	unsigned long flags;
 	struct stack *stack;
 
-	/* Filter gfp_mask the same way stackdepot does, for consistency */
 	gfp_mask &= ~GFP_ZONEMASK;
-	gfp_mask &= (GFP_ATOMIC | GFP_KERNEL);
 	gfp_mask |= __GFP_NOWARN;
 
 	set_current_in_page_owner();
-- 
2.39.2

Re: [PATCH 9/9] mm,page_owner: don't remove GFP flags in add_stack_record_to_list

[Christoph Hellwig]

Please discard one, I developed this on the currently checked out tree,
but it has been sent separately.

Re: [PATCH 5/9] xfs: pass the actual offset and len to allocate to xfs_bmapi_allocate

[Darrick J. Wong]

On Mon, Apr 29, 2024 at 08:15:25AM +0200, Christoph Hellwig wrote:
> xfs_bmapi_allocate currently overwrites offset and len when converting
> delayed allocations, and duplicates the length cap done for non-delalloc
> allocations.  Move all that logic into the callers to avoid duplication
> and to make the calling conventions more obvious.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index f7b263d0b0cf1c..fe1ccc083eb3c4 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4185,21 +4185,11 @@ xfs_bmapi_allocate(
>  	int			error;
>  
>  	ASSERT(bma->length > 0);
> +	ASSERT(bma->length <= XFS_MAX_BMBT_EXTLEN);
>  
> -	/*
> -	 * For the wasdelay case, we could also just allocate the stuff asked
> -	 * for in this bmap call but that wouldn't be as good.
> -	 */
>  	if (bma->wasdel) {
> -		bma->length = (xfs_extlen_t)bma->got.br_blockcount;
> -		bma->offset = bma->got.br_startoff;
>  		if (!xfs_iext_peek_prev_extent(ifp, &bma->icur, &bma->prev))
>  			bma->prev.br_startoff = NULLFILEOFF;
> -	} else {
> -		bma->length = XFS_FILBLKS_MIN(bma->length, XFS_MAX_BMBT_EXTLEN);
> -		if (!bma->eof)
> -			bma->length = XFS_FILBLKS_MIN(bma->length,
> -					bma->got.br_startoff - bma->offset);
>  	}
>  
>  	if (bma->flags & XFS_BMAPI_CONTIG)
> @@ -4533,6 +4523,15 @@ xfs_bmapi_write(
>  			 */
>  			bma.length = XFS_FILBLKS_MIN(len, XFS_MAX_BMBT_EXTLEN);
>  
> +			if (wasdelay) {
> +				bma.offset = bma.got.br_startoff;
> +				bma.length = bma.got.br_blockcount;
> +			} else {
> +				if (!eof)
> +					bma.length = XFS_FILBLKS_MIN(bma.length,
> +						bma.got.br_startoff - bno);
> +			}

Referencing
https://lore.kernel.org/linux-xfs/20240410040353.GC1883@lst.de/

Did you decide against "[moving] the assignments into the other
branch here to make things more obvious"?

--D

> +
>  			ASSERT(bma.length > 0);
>  			error = xfs_bmapi_allocate(&bma);
>  			if (error) {
> @@ -4685,11 +4684,16 @@ xfs_bmapi_convert_delalloc(
>  	bma.tp = tp;
>  	bma.ip = ip;
>  	bma.wasdel = true;
> -	bma.offset = bma.got.br_startoff;
> -	bma.length = max_t(xfs_filblks_t, bma.got.br_blockcount,
> -			XFS_MAX_BMBT_EXTLEN);
>  	bma.minleft = xfs_bmapi_minleft(tp, ip, whichfork);
>  
> +	/*
> +	 * Always allocate convert from the start of the delalloc extent even if
> +	 * that is outside the passed in range to create large contiguous
> +	 * extents on disk.
> +	 */
> +	bma.offset = bma.got.br_startoff;
> +	bma.length = bma.got.br_blockcount;
> +
>  	/*
>  	 * When we're converting the delalloc reservations backing dirty pages
>  	 * in the page cache, we must be careful about how we create the new
> -- 
> 2.39.2
> 
> 

Re: [PATCH 7/9] xfs: fix xfs_bmap_add_extent_delay_real for partial conversions

[Darrick J. Wong]

On Mon, Apr 29, 2024 at 08:15:27AM +0200, Christoph Hellwig wrote:
> xfs_bmap_add_extent_delay_real takes parts or all of a delalloc extent
> and converts them to a real extent.  It is written to deal with any
> potential overlap of the to be converted range with the delalloc extent,
> but it turns out that currently only converting the entire extents, or a
> part starting at the beginning is actually exercised, as the only caller
> always tries to convert the entire delalloc extent, and either succeeds
> or at least progresses partially from the start.
> 
> If it only converts a tiny part of a delalloc extent, the indirect block
> calculation for the new delalloc extent (da_new) might be equivalent to that
> of the existing delalloc extent (da_old).  If this extent conversion now
> requires allocating an indirect block that gets accounted into da_new,
> leading to the assert that da_new must be smaller or equal to da_new
> unless we split the extent to trigger.
> 
> Except for the assert that case is actually handled by just trying to
> allocate more space, as that already handled for the split case (which
> currently can't be reached at all), so just reusing it should be fine.
> Except that without dipping into the reserved block pool that would make
> it a bit too easy to trigger a fs shutdown due to ENOSPC.  So in addition
> to adjusting the assert, also dip into the reserved block pool.
> 
> Note that I could only reproduce the assert with a change to only convert
> the actually asked range instead of the full delalloc extent from
> xfs_bmapi_write.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good now,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 472c795beb8add..42c5a2efa656a5 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1570,6 +1570,7 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_LEFT_CONTIG:
> @@ -1600,6 +1601,7 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
> @@ -1634,6 +1636,7 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING:
> @@ -1668,6 +1671,7 @@ xfs_bmap_add_extent_delay_real(
>  				goto done;
>  			}
>  		}
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG:
> @@ -1706,6 +1710,7 @@ xfs_bmap_add_extent_delay_real(
>  			if (error)
>  				goto done;
>  		}
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_LEFT_FILLING:
> @@ -1796,6 +1801,7 @@ xfs_bmap_add_extent_delay_real(
>  		xfs_iext_update_extent(bma->ip, state, &bma->icur, &PREV);
>  		xfs_iext_next(ifp, &bma->icur);
>  		xfs_iext_update_extent(bma->ip, state, &bma->icur, &RIGHT);
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case BMAP_RIGHT_FILLING:
> @@ -1845,6 +1851,7 @@ xfs_bmap_add_extent_delay_real(
>  		PREV.br_blockcount = temp;
>  		xfs_iext_insert(bma->ip, &bma->icur, &PREV, state);
>  		xfs_iext_next(ifp, &bma->icur);
> +		ASSERT(da_new <= da_old);
>  		break;
>  
>  	case 0:
> @@ -1967,12 +1974,10 @@ xfs_bmap_add_extent_delay_real(
>  	}
>  
>  	/* adjust for changes in reserved delayed indirect blocks */
> -	if (da_new < da_old) {
> +	if (da_new < da_old)
>  		xfs_add_fdblocks(mp, da_old - da_new);
> -	} else if (da_new > da_old) {
> -		ASSERT(state == 0);
> -		error = xfs_dec_fdblocks(mp, da_new - da_old, false);
> -	}
> +	else if (da_new > da_old)
> +		error = xfs_dec_fdblocks(mp, da_new - da_old, true);
>  
>  	xfs_bmap_check_leaf_extents(bma->cur, bma->ip, whichfork);
>  done:
> -- 
> 2.39.2
> 
> 

Re: [PATCH 5/9] xfs: pass the actual offset and len to allocate to xfs_bmapi_allocate

[Christoph Hellwig]

On Mon, Apr 29, 2024 at 08:48:17AM -0700, Darrick J. Wong wrote:
> Referencing
> https://lore.kernel.org/linux-xfs/20240410040353.GC1883@lst.de/
> 
> Did you decide against "[moving] the assignments into the other
> branch here to make things more obvious"?

Yes.  I did the move and then noticed that I'd just have to move it
back in the last patch, which feels a bit silly.

Re: [PATCH 5/9] xfs: pass the actual offset and len to allocate to xfs_bmapi_allocate

[Darrick J. Wong]

On Mon, Apr 29, 2024 at 07:18:40PM +0200, Christoph Hellwig wrote:
> On Mon, Apr 29, 2024 at 08:48:17AM -0700, Darrick J. Wong wrote:
> > Referencing
> > https://lore.kernel.org/linux-xfs/20240410040353.GC1883@lst.de/
> > 
> > Did you decide against "[moving] the assignments into the other
> > branch here to make things more obvious"?
> 
> Yes.  I did the move and then noticed that I'd just have to move it
> back in the last patch, which feels a bit silly.

Ok then
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

Re: [PATCH 1/9] xfs: fix error returns from xfs_bmapi_write

[刘通]

Hello!

Thank you for promptly fixing this issue. May I ask if it's possible
to assign a CVE to this vulnerability?
Once again, thank you for your efforts in fixing this vulnerability!

Wish you all the best.

Tong


刘通 <lyutoon@gmail.com> 于2024年5月6日周一 20:24写道：
>
> Hello!
>
> Thank you for promptly fixing this issue. May I ask if it's possible to assign a CVE to this vulnerability?
> Once again, thank you for your efforts in fixing this vulnerability!
>
> Wish you all the best.
>
> Tong
>
> Christoph Hellwig <hch@lst.de> 于2024年4月29日周一 14:15写道：
>>
>> xfs_bmapi_write can return 0 without actually returning a mapping in
>> mval in two different cases:
>>
>>  1) when there is absolutely no space available to do an allocation
>>  2) when converting delalloc space, and the allocation is so small
>>     that it only covers parts of the delalloc extent before the
>>     range requested by the caller
>>
>> Callers at best can handle one of these cases, but in many cases can't
>> cope with either one.  Switch xfs_bmapi_write to always return a
>> mapping or return an error code instead.  For case 1) above ENOSPC is
>> the obvious choice which is very much what the callers expect anyway.
>> For case 2) there is no really good error code, so pick a funky one
>> from the SysV streams portfolio.
>>
>> This fixes the reproducer here:
>>
>>     https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/
>>
>> which uses reserved blocks to create file systems that are gravely
>> out of space and thus cause at least xfs_file_alloc_space to hang
>> and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc.
>>
>> Note that this patch does not actually make any caller but
>> xfs_alloc_file_space deal intelligently with case 2) above.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reported-by: 刘通 <lyutoon@gmail.com>
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> ---
>>  fs/xfs/libxfs/xfs_attr_remote.c |  1 -
>>  fs/xfs/libxfs/xfs_bmap.c        | 46 ++++++++++++++++++++++++++-------
>>  fs/xfs/libxfs/xfs_da_btree.c    | 20 ++++----------
>>  fs/xfs/scrub/quota_repair.c     |  6 -----
>>  fs/xfs/scrub/rtbitmap_repair.c  |  2 --
>>  fs/xfs/xfs_bmap_util.c          | 31 +++++++++++-----------
>>  fs/xfs/xfs_dquot.c              |  1 -
>>  fs/xfs/xfs_iomap.c              |  8 ------
>>  fs/xfs/xfs_reflink.c            | 14 ----------
>>  fs/xfs/xfs_rtalloc.c            |  2 --
>>  10 files changed, 57 insertions(+), 74 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index a8de9dc1e998a3..beb0efdd8f6b83 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -625,7 +625,6 @@ xfs_attr_rmtval_set_blk(
>>         if (error)
>>                 return error;
>>
>> -       ASSERT(nmap == 1);
>>         ASSERT((map->br_startblock != DELAYSTARTBLOCK) &&
>>                (map->br_startblock != HOLESTARTBLOCK));
>>
>> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> index 6053f5e5c71eec..f19191d6eade7e 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -4217,8 +4217,10 @@ xfs_bmapi_allocate(
>>         } else {
>>                 error = xfs_bmap_alloc_userdata(bma);
>>         }
>> -       if (error || bma->blkno == NULLFSBLOCK)
>> +       if (error)
>>                 return error;
>> +       if (bma->blkno == NULLFSBLOCK)
>> +               return -ENOSPC;
>>
>>         if (bma->flags & XFS_BMAPI_ZERO) {
>>                 error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
>> @@ -4397,6 +4399,15 @@ xfs_bmapi_finish(
>>   * extent state if necessary.  Details behaviour is controlled by the flags
>>   * parameter.  Only allocates blocks from a single allocation group, to avoid
>>   * locking problems.
>> + *
>> + * Returns 0 on success and places the extent mappings in mval.  nmaps is used
>> + * as an input/output parameter where the caller specifies the maximum number
>> + * of mappings that may be returned and xfs_bmapi_write passes back the number
>> + * of mappings (including existing mappings) it found.
>> + *
>> + * Returns a negative error code on failure, including -ENOSPC when it could not
>> + * allocate any blocks and -ENOSR when it did allocate blocks to convert a
>> + * delalloc range, but those blocks were before the passed in range.
>>   */
>>  int
>>  xfs_bmapi_write(
>> @@ -4525,10 +4536,16 @@ xfs_bmapi_write(
>>                         ASSERT(len > 0);
>>                         ASSERT(bma.length > 0);
>>                         error = xfs_bmapi_allocate(&bma);
>> -                       if (error)
>> +                       if (error) {
>> +                               /*
>> +                                * If we already allocated space in a previous
>> +                                * iteration return what we go so far when
>> +                                * running out of space.
>> +                                */
>> +                               if (error == -ENOSPC && bma.nallocs)
>> +                                       break;
>>                                 goto error0;
>> -                       if (bma.blkno == NULLFSBLOCK)
>> -                               break;
>> +                       }
>>
>>                         /*
>>                          * If this is a CoW allocation, record the data in
>> @@ -4566,7 +4583,6 @@ xfs_bmapi_write(
>>                 if (!xfs_iext_next_extent(ifp, &bma.icur, &bma.got))
>>                         eof = true;
>>         }
>> -       *nmap = n;
>>
>>         error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags,
>>                         whichfork);
>> @@ -4577,7 +4593,22 @@ xfs_bmapi_write(
>>                ifp->if_nextents > XFS_IFORK_MAXEXT(ip, whichfork));
>>         xfs_bmapi_finish(&bma, whichfork, 0);
>>         xfs_bmap_validate_ret(orig_bno, orig_len, orig_flags, orig_mval,
>> -               orig_nmap, *nmap);
>> +               orig_nmap, n);
>> +
>> +       /*
>> +        * When converting delayed allocations, xfs_bmapi_allocate ignores
>> +        * the passed in bno and always converts from the start of the found
>> +        * delalloc extent.
>> +        *
>> +        * To avoid a successful return with *nmap set to 0, return the magic
>> +        * -ENOSR error code for this particular case so that the caller can
>> +        * handle it.
>> +        */
>> +       if (!n) {
>> +               ASSERT(bma.nallocs >= *nmap);
>> +               return -ENOSR;
>> +       }
>> +       *nmap = n;
>>         return 0;
>>  error0:
>>         xfs_bmapi_finish(&bma, whichfork, error);
>> @@ -4684,9 +4715,6 @@ xfs_bmapi_convert_delalloc(
>>         if (error)
>>                 goto out_finish;
>>
>> -       error = -ENOSPC;
>> -       if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
>> -               goto out_finish;
>>         if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock))) {
>>                 xfs_bmap_mark_sick(ip, whichfork);
>>                 error = -EFSCORRUPTED;
>> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
>> index b13796629e2213..16a529a8878083 100644
>> --- a/fs/xfs/libxfs/xfs_da_btree.c
>> +++ b/fs/xfs/libxfs/xfs_da_btree.c
>> @@ -2297,8 +2297,8 @@ xfs_da_grow_inode_int(
>>         struct xfs_inode        *dp = args->dp;
>>         int                     w = args->whichfork;
>>         xfs_rfsblock_t          nblks = dp->i_nblocks;
>> -       struct xfs_bmbt_irec    map, *mapp;
>> -       int                     nmap, error, got, i, mapi;
>> +       struct xfs_bmbt_irec    map, *mapp = &map;
>> +       int                     nmap, error, got, i, mapi = 1;
>>
>>         /*
>>          * Find a spot in the file space to put the new block.
>> @@ -2314,14 +2314,7 @@ xfs_da_grow_inode_int(
>>         error = xfs_bmapi_write(tp, dp, *bno, count,
>>                         xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
>>                         args->total, &map, &nmap);
>> -       if (error)
>> -               return error;
>> -
>> -       ASSERT(nmap <= 1);
>> -       if (nmap == 1) {
>> -               mapp = &map;
>> -               mapi = 1;
>> -       } else if (nmap == 0 && count > 1) {
>> +       if (error == -ENOSPC && count > 1) {
>>                 xfs_fileoff_t           b;
>>                 int                     c;
>>
>> @@ -2339,16 +2332,13 @@ xfs_da_grow_inode_int(
>>                                         args->total, &mapp[mapi], &nmap);
>>                         if (error)
>>                                 goto out_free_map;
>> -                       if (nmap < 1)
>> -                               break;
>>                         mapi += nmap;
>>                         b = mapp[mapi - 1].br_startoff +
>>                             mapp[mapi - 1].br_blockcount;
>>                 }
>> -       } else {
>> -               mapi = 0;
>> -               mapp = NULL;
>>         }
>> +       if (error)
>> +               goto out_free_map;
>>
>>         /*
>>          * Count the blocks we got, make sure it matches the total.
>> diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
>> index 0bab4c30cb85ab..90cd1512bba961 100644
>> --- a/fs/xfs/scrub/quota_repair.c
>> +++ b/fs/xfs/scrub/quota_repair.c
>> @@ -77,8 +77,6 @@ xrep_quota_item_fill_bmap_hole(
>>                         irec, &nmaps);
>>         if (error)
>>                 return error;
>> -       if (nmaps != 1)
>> -               return -ENOSPC;
>>
>>         dq->q_blkno = XFS_FSB_TO_DADDR(mp, irec->br_startblock);
>>
>> @@ -444,10 +442,6 @@ xrep_quota_data_fork(
>>                                         XFS_BMAPI_CONVERT, 0, &nrec, &nmap);
>>                         if (error)
>>                                 goto out;
>> -                       if (nmap != 1) {
>> -                               error = -ENOSPC;
>> -                               goto out;
>> -                       }
>>                         ASSERT(nrec.br_startoff == irec.br_startoff);
>>                         ASSERT(nrec.br_blockcount == irec.br_blockcount);
>>
>> diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
>> index 46f5d5f605c915..0fef98e9f83409 100644
>> --- a/fs/xfs/scrub/rtbitmap_repair.c
>> +++ b/fs/xfs/scrub/rtbitmap_repair.c
>> @@ -108,8 +108,6 @@ xrep_rtbitmap_data_mappings(
>>                                 0, &map, &nmaps);
>>                 if (error)
>>                         return error;
>> -               if (nmaps != 1)
>> -                       return -EFSCORRUPTED;
>>
>>                 /* Commit new extent and all deferred work. */
>>                 error = xrep_defer_finish(sc);
>> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
>> index 53aa90a0ee3a85..2e6f08198c0719 100644
>> --- a/fs/xfs/xfs_bmap_util.c
>> +++ b/fs/xfs/xfs_bmap_util.c
>> @@ -721,33 +721,32 @@ xfs_alloc_file_space(
>>                 if (error)
>>                         goto error;
>>
>> -               error = xfs_bmapi_write(tp, ip, startoffset_fsb,
>> -                               allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
>> -                               &nimaps);
>> -               if (error)
>> -                       goto error;
>> -
>> -               ip->i_diflags |= XFS_DIFLAG_PREALLOC;
>> -               xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>> -
>> -               error = xfs_trans_commit(tp);
>> -               xfs_iunlock(ip, XFS_ILOCK_EXCL);
>> -               if (error)
>> -                       break;
>> -
>>                 /*
>>                  * If the allocator cannot find a single free extent large
>>                  * enough to cover the start block of the requested range,
>> -                * xfs_bmapi_write will return 0 but leave *nimaps set to 0.
>> +                * xfs_bmapi_write will return -ENOSR.
>>                  *
>>                  * In that case we simply need to keep looping with the same
>>                  * startoffset_fsb so that one of the following allocations
>>                  * will eventually reach the requested range.
>>                  */
>> -               if (nimaps) {
>> +               error = xfs_bmapi_write(tp, ip, startoffset_fsb,
>> +                               allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
>> +                               &nimaps);
>> +               if (error) {
>> +                       if (error != -ENOSR)
>> +                               goto error;
>> +                       error = 0;
>> +               } else {
>>                         startoffset_fsb += imapp->br_blockcount;
>>                         allocatesize_fsb -= imapp->br_blockcount;
>>                 }
>> +
>> +               ip->i_diflags |= XFS_DIFLAG_PREALLOC;
>> +               xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>> +
>> +               error = xfs_trans_commit(tp);
>> +               xfs_iunlock(ip, XFS_ILOCK_EXCL);
>>         }
>>
>>         return error;
>> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
>> index 13aba84bd64afb..43acb4f0d17433 100644
>> --- a/fs/xfs/xfs_dquot.c
>> +++ b/fs/xfs/xfs_dquot.c
>> @@ -357,7 +357,6 @@ xfs_dquot_disk_alloc(
>>                 goto err_cancel;
>>
>>         ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
>> -       ASSERT(nmaps == 1);
>>         ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
>>                (map.br_startblock != HOLESTARTBLOCK));
>>
>> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
>> index 9ce0f6b9df93e6..60463160820b62 100644
>> --- a/fs/xfs/xfs_iomap.c
>> +++ b/fs/xfs/xfs_iomap.c
>> @@ -322,14 +322,6 @@ xfs_iomap_write_direct(
>>         if (error)
>>                 goto out_unlock;
>>
>> -       /*
>> -        * Copy any maps to caller's array and return any error.
>> -        */
>> -       if (nimaps == 0) {
>> -               error = -ENOSPC;
>> -               goto out_unlock;
>> -       }
>> -
>>         if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
>>                 xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
>>                 error = xfs_alert_fsblock_zero(ip, imap);
>> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
>> index 7da0e8f961d351..5ecb52a234becc 100644
>> --- a/fs/xfs/xfs_reflink.c
>> +++ b/fs/xfs/xfs_reflink.c
>> @@ -430,13 +430,6 @@ xfs_reflink_fill_cow_hole(
>>         if (error)
>>                 return error;
>>
>> -       /*
>> -        * Allocation succeeded but the requested range was not even partially
>> -        * satisfied?  Bail out!
>> -        */
>> -       if (nimaps == 0)
>> -               return -ENOSPC;
>> -
>>  convert:
>>         return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
>>
>> @@ -499,13 +492,6 @@ xfs_reflink_fill_delalloc(
>>                 error = xfs_trans_commit(tp);
>>                 if (error)
>>                         return error;
>> -
>> -               /*
>> -                * Allocation succeeded but the requested range was not even
>> -                * partially satisfied?  Bail out!
>> -                */
>> -               if (nimaps == 0)
>> -                       return -ENOSPC;
>>         } while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
>>
>>         return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
>> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
>> index b476a876478d93..150f544445ca82 100644
>> --- a/fs/xfs/xfs_rtalloc.c
>> +++ b/fs/xfs/xfs_rtalloc.c
>> @@ -709,8 +709,6 @@ xfs_growfs_rt_alloc(
>>                 nmap = 1;
>>                 error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks,
>>                                         XFS_BMAPI_METADATA, 0, &map, &nmap);
>> -               if (!error && nmap < 1)
>> -                       error = -ENOSPC;
>>                 if (error)
>>                         goto out_trans_cancel;
>>                 /*
>> --
>> 2.39.2
>>