fix dblocks adjustment in growfs

fix dblocks adjustment in growfs

[Christoph Hellwig]

Hi all,

the first patch fixes a possible incorrect sb_dblocks when growfs gits
the maximum agcount or a too small AG.  The second then cleans up the
surrounding code to be better readable.

Btw, is there any reason to have this code in xfs_ag.c?  It is only used
by growfs and not at all in userspace, so having it in xfs_fsops.c would
seem more natural.

Diffstat:
 libxfs/xfs_ag.c |   44 +++++++++++++++++++++-----------------------
 libxfs/xfs_ag.h |    5 ++---
 xfs_fsops.c     |    4 +++-
 3 files changed, 26 insertions(+), 27 deletions(-)

[PATCH 1/2] xfs: pass back updated nb from xfs_growfs_compute_deltas

[Christoph Hellwig]

xfs_growfs_compute_deltas can update nb for corner cases like a number
of blocks that would create a less the minimal sized AG, or running
past the max AG limit.  Pass back the calculated value to the caller,
as it relies on that for a few things.

Fixes: a49b7ff63f98 ("xfs: Refactoring the nagcount and delta calculation")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c | 10 +++++-----
 fs/xfs/libxfs/xfs_ag.h |  2 +-
 fs/xfs/xfs_fsops.c     |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index dcd2f93b6a6c..0c5f0548021f 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -866,7 +866,7 @@ xfs_ag_shrink_space(
 void
 xfs_growfs_compute_deltas(
 	struct xfs_mount	*mp,
-	xfs_rfsblock_t		nb,
+	xfs_rfsblock_t		*nb,
 	int64_t			*deltap,
 	xfs_agnumber_t		*nagcountp)
 {
@@ -874,19 +874,19 @@ xfs_growfs_compute_deltas(
 	int64_t		delta;
 	xfs_agnumber_t	nagcount;
 
-	nb_div = nb;
+	nb_div = *nb;
 	nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
 	if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS)
 		nb_div++;
 	else if (nb_mod)
-		nb = nb_div * mp->m_sb.sb_agblocks;
+		*nb = nb_div * mp->m_sb.sb_agblocks;
 
 	if (nb_div > XFS_MAX_AGNUMBER + 1) {
 		nb_div = XFS_MAX_AGNUMBER + 1;
-		nb = nb_div * mp->m_sb.sb_agblocks;
+		*nb = nb_div * mp->m_sb.sb_agblocks;
 	}
 	nagcount = nb_div;
-	delta = nb - mp->m_sb.sb_dblocks;
+	delta = *nb - mp->m_sb.sb_dblocks;
 	*deltap = delta;
 	*nagcountp = nagcount;
 }
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 16a9b43a3c27..8aa4266c5571 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -330,7 +330,7 @@ int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
 int xfs_ag_shrink_space(struct xfs_perag *pag, struct xfs_trans **tpp,
 			xfs_extlen_t delta);
 void
-xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t nb,
+xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t *nb,
 			int64_t *deltap, xfs_agnumber_t *nagcountp);
 int xfs_ag_extend_space(struct xfs_perag *pag, struct xfs_trans *tp,
 			xfs_extlen_t len);
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 8d64d904d73c..436857356a0a 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -124,7 +124,7 @@ xfs_growfs_data_private(
 			mp->m_sb.sb_rextsize);
 	if (error)
 		return error;
-	xfs_growfs_compute_deltas(mp, nb, &delta, &nagcount);
+	xfs_growfs_compute_deltas(mp, &nb, &delta, &nagcount);
 
 	/*
 	 * Reject filesystems with a single AG because they are not
-- 
2.53.0

[PATCH 2/2] xfs: cleanup xfs_growfs_compute_deltas

[Christoph Hellwig]

xfs_growfs_compute_deltas has an odd calling conventions, and looks
very convoluted due to the use of do_div and strangely named and typed
variables.

Rename it, make it return the agcount and let the caller calculate the
delta.  The internally use the better div_u64_rem helper and descriptive
variable names and types.  Also add a comment describing what the
function is used for.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c | 44 ++++++++++++++++++++----------------------
 fs/xfs/libxfs/xfs_ag.h |  5 ++---
 fs/xfs/xfs_fsops.c     |  4 +++-
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 0c5f0548021f..095af128d2fb 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -863,32 +863,30 @@ xfs_ag_shrink_space(
 	return err2;
 }
 
-void
-xfs_growfs_compute_deltas(
+/*
+ * Return the agcount for the new file system size passed in *nb and adjust *nb
+ * when it has to be reduced because of maximum AG count or because it would
+ * create a below minimum size AG.
+ */
+xfs_agnumber_t
+xfs_growfs_compute_agcount(
 	struct xfs_mount	*mp,
-	xfs_rfsblock_t		*nb,
-	int64_t			*deltap,
-	xfs_agnumber_t		*nagcountp)
+	xfs_rfsblock_t		*nb)
 {
-	xfs_rfsblock_t	nb_div, nb_mod;
-	int64_t		delta;
-	xfs_agnumber_t	nagcount;
-
-	nb_div = *nb;
-	nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
-	if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS)
-		nb_div++;
-	else if (nb_mod)
-		*nb = nb_div * mp->m_sb.sb_agblocks;
-
-	if (nb_div > XFS_MAX_AGNUMBER + 1) {
-		nb_div = XFS_MAX_AGNUMBER + 1;
-		*nb = nb_div * mp->m_sb.sb_agblocks;
+	xfs_agnumber_t		agcount;
+	xfs_extlen_t		remainder;
+
+	agcount = div_u64_rem(*nb, mp->m_sb.sb_agblocks, &remainder);
+	if (agcount > XFS_MAX_AGNUMBER + 1) {
+		agcount = XFS_MAX_AGNUMBER + 1;
+		remainder = 0;
+	}
+	*nb = (xfs_rfsblock_t)agcount * mp->m_sb.sb_agblocks;
+	if (remainder >= XFS_MIN_AG_BLOCKS) {
+		*nb += remainder;
+		agcount++;
 	}
-	nagcount = nb_div;
-	delta = *nb - mp->m_sb.sb_dblocks;
-	*deltap = delta;
-	*nagcountp = nagcount;
+	return agcount;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 8aa4266c5571..fd22fe598931 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -329,12 +329,11 @@ struct aghdr_init_data {
 int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
 int xfs_ag_shrink_space(struct xfs_perag *pag, struct xfs_trans **tpp,
 			xfs_extlen_t delta);
-void
-xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t *nb,
-			int64_t *deltap, xfs_agnumber_t *nagcountp);
 int xfs_ag_extend_space(struct xfs_perag *pag, struct xfs_trans *tp,
 			xfs_extlen_t len);
 int xfs_ag_get_geometry(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
+xfs_agnumber_t xfs_growfs_compute_agcount(struct xfs_mount *mp,
+		xfs_rfsblock_t *nb);
 
 static inline xfs_fsblock_t
 xfs_agbno_to_fsb(
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 436857356a0a..67624a804a7f 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -124,7 +124,9 @@ xfs_growfs_data_private(
 			mp->m_sb.sb_rextsize);
 	if (error)
 		return error;
-	xfs_growfs_compute_deltas(mp, &nb, &delta, &nagcount);
+
+	nagcount = xfs_growfs_compute_agcount(mp, &nb);
+	delta = nb - mp->m_sb.sb_dblocks;
 
 	/*
 	 * Reject filesystems with a single AG because they are not
-- 
2.53.0

Re: [PATCH 1/2] xfs: pass back updated nb from xfs_growfs_compute_deltas

[Darrick J. Wong]

On Mon, Jun 01, 2026 at 01:07:52PM +0200, Christoph Hellwig wrote:
> xfs_growfs_compute_deltas can update nb for corner cases like a number
> of blocks that would create a less the minimal sized AG, or running
> past the max AG limit.  Pass back the calculated value to the caller,
> as it relies on that for a few things.

A few things, like getting the fs size right after the grow?

Cc: <stable@vger.kernel.org> # v7.0

With that added,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Fixes: a49b7ff63f98 ("xfs: Refactoring the nagcount and delta calculation")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_ag.c | 10 +++++-----
>  fs/xfs/libxfs/xfs_ag.h |  2 +-
>  fs/xfs/xfs_fsops.c     |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index dcd2f93b6a6c..0c5f0548021f 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -866,7 +866,7 @@ xfs_ag_shrink_space(
>  void
>  xfs_growfs_compute_deltas(
>  	struct xfs_mount	*mp,
> -	xfs_rfsblock_t		nb,
> +	xfs_rfsblock_t		*nb,
>  	int64_t			*deltap,
>  	xfs_agnumber_t		*nagcountp)
>  {
> @@ -874,19 +874,19 @@ xfs_growfs_compute_deltas(
>  	int64_t		delta;
>  	xfs_agnumber_t	nagcount;
>  
> -	nb_div = nb;
> +	nb_div = *nb;
>  	nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
>  	if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS)
>  		nb_div++;
>  	else if (nb_mod)
> -		nb = nb_div * mp->m_sb.sb_agblocks;
> +		*nb = nb_div * mp->m_sb.sb_agblocks;
>  
>  	if (nb_div > XFS_MAX_AGNUMBER + 1) {
>  		nb_div = XFS_MAX_AGNUMBER + 1;
> -		nb = nb_div * mp->m_sb.sb_agblocks;
> +		*nb = nb_div * mp->m_sb.sb_agblocks;
>  	}
>  	nagcount = nb_div;
> -	delta = nb - mp->m_sb.sb_dblocks;
> +	delta = *nb - mp->m_sb.sb_dblocks;
>  	*deltap = delta;
>  	*nagcountp = nagcount;
>  }
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 16a9b43a3c27..8aa4266c5571 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -330,7 +330,7 @@ int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
>  int xfs_ag_shrink_space(struct xfs_perag *pag, struct xfs_trans **tpp,
>  			xfs_extlen_t delta);
>  void
> -xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t nb,
> +xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t *nb,
>  			int64_t *deltap, xfs_agnumber_t *nagcountp);
>  int xfs_ag_extend_space(struct xfs_perag *pag, struct xfs_trans *tp,
>  			xfs_extlen_t len);
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 8d64d904d73c..436857356a0a 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -124,7 +124,7 @@ xfs_growfs_data_private(
>  			mp->m_sb.sb_rextsize);
>  	if (error)
>  		return error;
> -	xfs_growfs_compute_deltas(mp, nb, &delta, &nagcount);
> +	xfs_growfs_compute_deltas(mp, &nb, &delta, &nagcount);
>  
>  	/*
>  	 * Reject filesystems with a single AG because they are not
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/2] xfs: cleanup xfs_growfs_compute_deltas

[Darrick J. Wong]

On Mon, Jun 01, 2026 at 01:07:53PM +0200, Christoph Hellwig wrote:
> xfs_growfs_compute_deltas has an odd calling conventions, and looks
> very convoluted due to the use of do_div and strangely named and typed
> variables.
> 
> Rename it, make it return the agcount and let the caller calculate the
> delta.  The internally use the better div_u64_rem helper and descriptive
> variable names and types.  Also add a comment describing what the
> function is used for.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

AFAICT the logic is the same before and after, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Though I half wonder if that XFS_MAX_AGNUMBER+1 comparison is really
correct.  MAX_AGNUMBER is defined to be ((xfs_agnumber_t)-2) so we're
checking if an unsigned 32-bit number is greater than 0xFFFFFFFF?
Which is also NULLAGNUMBER?

--D

> ---
>  fs/xfs/libxfs/xfs_ag.c | 44 ++++++++++++++++++++----------------------
>  fs/xfs/libxfs/xfs_ag.h |  5 ++---
>  fs/xfs/xfs_fsops.c     |  4 +++-
>  3 files changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 0c5f0548021f..095af128d2fb 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -863,32 +863,30 @@ xfs_ag_shrink_space(
>  	return err2;
>  }
>  
> -void
> -xfs_growfs_compute_deltas(
> +/*
> + * Return the agcount for the new file system size passed in *nb and adjust *nb
> + * when it has to be reduced because of maximum AG count or because it would
> + * create a below minimum size AG.
> + */
> +xfs_agnumber_t
> +xfs_growfs_compute_agcount(
>  	struct xfs_mount	*mp,
> -	xfs_rfsblock_t		*nb,
> -	int64_t			*deltap,
> -	xfs_agnumber_t		*nagcountp)
> +	xfs_rfsblock_t		*nb)
>  {
> -	xfs_rfsblock_t	nb_div, nb_mod;
> -	int64_t		delta;
> -	xfs_agnumber_t	nagcount;
> -
> -	nb_div = *nb;
> -	nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
> -	if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS)
> -		nb_div++;
> -	else if (nb_mod)
> -		*nb = nb_div * mp->m_sb.sb_agblocks;
> -
> -	if (nb_div > XFS_MAX_AGNUMBER + 1) {
> -		nb_div = XFS_MAX_AGNUMBER + 1;
> -		*nb = nb_div * mp->m_sb.sb_agblocks;
> +	xfs_agnumber_t		agcount;
> +	xfs_extlen_t		remainder;
> +
> +	agcount = div_u64_rem(*nb, mp->m_sb.sb_agblocks, &remainder);
> +	if (agcount > XFS_MAX_AGNUMBER + 1) {
> +		agcount = XFS_MAX_AGNUMBER + 1;
> +		remainder = 0;
> +	}
> +	*nb = (xfs_rfsblock_t)agcount * mp->m_sb.sb_agblocks;
> +	if (remainder >= XFS_MIN_AG_BLOCKS) {
> +		*nb += remainder;
> +		agcount++;
>  	}
> -	nagcount = nb_div;
> -	delta = *nb - mp->m_sb.sb_dblocks;
> -	*deltap = delta;
> -	*nagcountp = nagcount;
> +	return agcount;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 8aa4266c5571..fd22fe598931 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -329,12 +329,11 @@ struct aghdr_init_data {
>  int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
>  int xfs_ag_shrink_space(struct xfs_perag *pag, struct xfs_trans **tpp,
>  			xfs_extlen_t delta);
> -void
> -xfs_growfs_compute_deltas(struct xfs_mount *mp, xfs_rfsblock_t *nb,
> -			int64_t *deltap, xfs_agnumber_t *nagcountp);
>  int xfs_ag_extend_space(struct xfs_perag *pag, struct xfs_trans *tp,
>  			xfs_extlen_t len);
>  int xfs_ag_get_geometry(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
> +xfs_agnumber_t xfs_growfs_compute_agcount(struct xfs_mount *mp,
> +		xfs_rfsblock_t *nb);
>  
>  static inline xfs_fsblock_t
>  xfs_agbno_to_fsb(
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 436857356a0a..67624a804a7f 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -124,7 +124,9 @@ xfs_growfs_data_private(
>  			mp->m_sb.sb_rextsize);
>  	if (error)
>  		return error;
> -	xfs_growfs_compute_deltas(mp, &nb, &delta, &nagcount);
> +
> +	nagcount = xfs_growfs_compute_agcount(mp, &nb);
> +	delta = nb - mp->m_sb.sb_dblocks;
>  
>  	/*
>  	 * Reject filesystems with a single AG because they are not
> -- 
> 2.53.0
> 
> 

Re: [PATCH 1/2] xfs: pass back updated nb from xfs_growfs_compute_deltas

[Christoph Hellwig]

On Mon, Jun 01, 2026 at 09:36:40PM -0700, Darrick J. Wong wrote:
> On Mon, Jun 01, 2026 at 01:07:52PM +0200, Christoph Hellwig wrote:
> > xfs_growfs_compute_deltas can update nb for corner cases like a number
> > of blocks that would create a less the minimal sized AG, or running
> > past the max AG limit.  Pass back the calculated value to the caller,
> > as it relies on that for a few things.
> 
> A few things, like getting the fs size right after the grow?

Heh, yes.  I gues I'll have to reword this a bit to not sound silly.

Re: [PATCH 2/2] xfs: cleanup xfs_growfs_compute_deltas

[Christoph Hellwig]

On Mon, Jun 01, 2026 at 09:42:03PM -0700, Darrick J. Wong wrote:
> AFAICT the logic is the same before and after, so
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> Though I half wonder if that XFS_MAX_AGNUMBER+1 comparison is really
> correct.  MAX_AGNUMBER is defined to be ((xfs_agnumber_t)-2) so we're
> checking if an unsigned 32-bit number is greater than 0xFFFFFFFF?
> Which is also NULLAGNUMBER?

Not this this is the count and the number.  This also is the only
use of MAX_AGNUMBER in the kernel and xfsprogs has a copy of it
defined in XFS_MAX_AGNUMBER.  I'll see if there is a cleaner way
to express this, and the fact that agnumber is now a 32-bit value
might also mess things up a bit.