On Fri, Feb 16, 2018 at 09:23:58AM +1100, Dave Chinner wrote:

On Fri, Feb 09, 2018 at 11:11:54AM -0500, Brian Foster wrote:

quoted

On Thu, Feb 01, 2018 at 05:42:01PM +1100, Dave Chinner wrote:

quoted

From: Dave Chinner <redacted>

This happens after all the transactions to update the superblock
occur, and errors need to be handled slightly differently. Seperate

Separate

quoted

out the code into it's own function, and clean up the error goto
stack in the core growfs code as it is now much simpler.

Signed-Off-By: Dave Chinner <redacted>
---
 fs/xfs/xfs_fsops.c | 154 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 87 insertions(+), 67 deletions(-)

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 5c844e540320..113be7dbdc81 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c

...

quoted

@@ -572,16 +572,79 @@ xfs_growfs_data_private(
 		error = xfs_ag_resv_free(pag);
 		xfs_perag_put(pag);
 		if (error)
-			goto out;
+			return error;
 	}
 
 	/* Reserve AG metadata blocks. */
-	error = xfs_fs_reserve_ag_blocks(mp);
-	if (error && error != -ENOSPC)
-		goto out;
+	return xfs_fs_reserve_ag_blocks(mp);

It looks like we change the semantics of -ENOSPC during perag
reservation init. No mention of whether this is intentional and/or
why..?

Not sure what I changed here - it just returns the error to the
caller because it's no longer going to jump over code after
xfs_fs_reserve_ag_blocks(mp) has already shut down the filesystem
(which it does on any error other than ENOSPC).

It the semantics of -ENOSPC (i.e., how that error is/was specially
handled) that looked different..

Perhaps....

quoted

quoted

@@ -694,6 +707,7 @@ xfs_growfs_data(
 	struct xfs_mount	*mp,
 	struct xfs_growfs_data	*in)
 {
+	xfs_agnumber_t		oagcount;
 	int			error = 0;
 
 	if (!capable(CAP_SYS_ADMIN))

@@ -708,6 +722,7 @@ xfs_growfs_data(
 			goto out_error;
 	}
 
+	oagcount = mp->m_sb.sb_agcount;
 	error = xfs_growfs_data_private(mp, in);
 	if (error)
 		goto out_error;

.... you are commenting on this code here, were ENOSPC is not
specially handled to all the superblocks to be updated even if we
got an ENOSPC on data-grow?

Not sure I parse that...

quoted

quoted

@@ -722,6 +737,11 @@ xfs_growfs_data(
 	} else
 		mp->m_maxicount = 0;
 
+	/*
+	 * Update secondary superblocks now the physical grow has completed
+	 */
+	error = xfs_growfs_update_superblocks(mp, oagcount);
+

i.e. it doesn't run this at ENOSPC now?

... but yeah, this I think, taking a quick look back.

Essentially it looked like -ENOSPC from the perag res init currently
does not result in a growfs operation error. We'd simply move on to the
next step and the growfs may very well return success. Here, it looks
like we've changed behavior to return -ENOSPC to userspace (without any
explanation).

Brian

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com