[PATCH v3 1/2] xfs: add lock protection when remove perag from radix tree

[PATCH v3 1/2] xfs: add lock protection when remove perag from radix tree

[Long Li]

Take mp->m_perag_lock for deletions from the perag radix tree in
xfs_initialize_perag to prevent racing with tagging operations.
Lookups are fine - they are RCU protected so already deal with the
tree changing shape underneath the lookup - but tagging operations
require the tree to be stable while the tags are propagated back up
to the root.

Right now there's nothing stopping radix tree tagging from operating
while a growfs operation is progress and adding/removing new entries
into the radix tree.

Hence we can have traversals that require a stable tree occurring at
the same time we are removing unused entries from the radix tree which
causes the shape of the tree to change.

Likely this hasn't caused a problem in the past because we are only
doing append addition and removal so the active AG part of the tree
is not changing shape, but that doesn't mean it is safe. Just making
the radix tree modifications serialise against each other is obviously
correct.

Signed-off-by: Long Li <leo.lilong@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index f62ff125a50a..c730976fdfc0 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -424,13 +424,17 @@ xfs_initialize_perag(
 
 out_remove_pag:
 	xfs_defer_drain_free(&pag->pag_intents_drain);
+	spin_lock(&mp->m_perag_lock);
 	radix_tree_delete(&mp->m_perag_tree, index);
+	spin_unlock(&mp->m_perag_lock);
 out_free_pag:
 	kmem_free(pag);
 out_unwind_new_pags:
 	/* unwind any prior newly initialized pags */
 	for (index = first_initialised; index < agcount; index++) {
+		spin_lock(&mp->m_perag_lock);
 		pag = radix_tree_delete(&mp->m_perag_tree, index);
+		spin_unlock(&mp->m_perag_lock);
 		if (!pag)
 			break;
 		xfs_buf_hash_destroy(pag);
-- 
2.31.1

[PATCH v3 2/2] xfs: fix perag leak when growfs fails

[Long Li]

During growfs, if new ag in memory has been initialized, however
sb_agcount has not been updated, if an error occurs at this time it
will cause perag leaks as follows, these new AGs will not been freed
during umount , because of these new AGs are not visible(that is
included in mp->m_sb.sb_agcount).

unreferenced object 0xffff88810be40200 (size 512):
  comm "xfs_growfs", pid 857, jiffies 4294909093
  hex dump (first 32 bytes):
    00 c0 c1 05 81 88 ff ff 04 00 00 00 00 00 00 00  ................
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc 381741e2):
    [<ffffffff8191aef6>] __kmalloc+0x386/0x4f0
    [<ffffffff82553e65>] kmem_alloc+0xb5/0x2f0
    [<ffffffff8238dac5>] xfs_initialize_perag+0xc5/0x810
    [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
    [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
    [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
    [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
    [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
unreferenced object 0xffff88810be40800 (size 512):
  comm "xfs_growfs", pid 857, jiffies 4294909093
  hex dump (first 32 bytes):
    20 00 00 00 00 00 00 00 57 ef be dc 00 00 00 00   .......W.......
    10 08 e4 0b 81 88 ff ff 10 08 e4 0b 81 88 ff ff  ................
  backtrace (crc bde50e2d):
    [<ffffffff8191b43a>] __kmalloc_node+0x3da/0x540
    [<ffffffff81814489>] kvmalloc_node+0x99/0x160
    [<ffffffff8286acff>] bucket_table_alloc.isra.0+0x5f/0x400
    [<ffffffff8286bdc5>] rhashtable_init+0x405/0x760
    [<ffffffff8238dda3>] xfs_initialize_perag+0x3a3/0x810
    [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
    [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
    [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
    [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
    [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a

Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
used for freeing unused perag within a specified range in error handling,
included in the error path of the growfs failure.

Signed-off-by: Long Li <leo.lilong@huawei.com>
---
v3:
- Stop use of typedefs for struct pointers
- Remove unnecessary indentation
- Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
  not xfs_free_perag(). Compared to the v2 version, now the logic for
  freeing perag in xfs_free_perag() and xfs_initialize_perag() error
  handling not quite the same.
 fs/xfs/libxfs/xfs_ag.c | 36 ++++++++++++++++++++++++++----------
 fs/xfs/libxfs/xfs_ag.h |  2 ++
 fs/xfs/xfs_fsops.c     |  5 ++++-
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index c730976fdfc0..39d9525270b7 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -332,6 +332,31 @@ xfs_agino_range(
 	return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last);
 }
 
+/*
+ * Free perag within the specified AG range, it is only used to free unused
+ * perags under the error handling path.
+ */
+void
+xfs_free_unused_perag_range(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agstart,
+	xfs_agnumber_t		agend)
+{
+	struct xfs_perag	*pag;
+	xfs_agnumber_t		index;
+
+	for (index = agstart; index < agend; index++) {
+		spin_lock(&mp->m_perag_lock);
+		pag = radix_tree_delete(&mp->m_perag_tree, index);
+		spin_unlock(&mp->m_perag_lock);
+		if (!pag)
+			break;
+		xfs_buf_hash_destroy(pag);
+		xfs_defer_drain_free(&pag->pag_intents_drain);
+		kmem_free(pag);
+	}
+}
+
 int
 xfs_initialize_perag(
 	struct xfs_mount	*mp,
@@ -431,16 +456,7 @@ xfs_initialize_perag(
 	kmem_free(pag);
 out_unwind_new_pags:
 	/* unwind any prior newly initialized pags */
-	for (index = first_initialised; index < agcount; index++) {
-		spin_lock(&mp->m_perag_lock);
-		pag = radix_tree_delete(&mp->m_perag_tree, index);
-		spin_unlock(&mp->m_perag_lock);
-		if (!pag)
-			break;
-		xfs_buf_hash_destroy(pag);
-		xfs_defer_drain_free(&pag->pag_intents_drain);
-		kmem_free(pag);
-	}
+	xfs_free_unused_perag_range(mp, first_initialised, agcount);
 	return error;
 }
 
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 2e0aef87d633..40d7b6427afb 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -133,6 +133,8 @@ __XFS_AG_OPSTATE(prefers_metadata, PREFERS_METADATA)
 __XFS_AG_OPSTATE(allows_inodes, ALLOWS_INODES)
 __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET)
 
+void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart,
+			xfs_agnumber_t agend);
 int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
 			xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi);
 int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno);
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index e8759b479516..22c3f1e9008e 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -153,7 +153,7 @@ xfs_growfs_data_private(
 		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, -delta, 0,
 				0, &tp);
 	if (error)
-		return error;
+		goto out_free_unused_perag;
 
 	last_pag = xfs_perag_get(mp, oagcount - 1);
 	if (delta > 0) {
@@ -227,6 +227,9 @@ xfs_growfs_data_private(
 
 out_trans_cancel:
 	xfs_trans_cancel(tp);
+out_free_unused_perag:
+	if (nagcount > oagcount)
+		xfs_free_unused_perag_range(mp, oagcount, nagcount);
 	return error;
 }
 
-- 
2.31.1

Re: [PATCH v3 1/2] xfs: add lock protection when remove perag from radix tree

[Darrick J. Wong]

On Wed, Dec 13, 2023 at 11:10:12AM +0800, Long Li wrote:
> Take mp->m_perag_lock for deletions from the perag radix tree in
> xfs_initialize_perag to prevent racing with tagging operations.
> Lookups are fine - they are RCU protected so already deal with the
> tree changing shape underneath the lookup - but tagging operations
> require the tree to be stable while the tags are propagated back up
> to the root.
> 
> Right now there's nothing stopping radix tree tagging from operating
> while a growfs operation is progress and adding/removing new entries
> into the radix tree.
> 
> Hence we can have traversals that require a stable tree occurring at
> the same time we are removing unused entries from the radix tree which
> causes the shape of the tree to change.
> 
> Likely this hasn't caused a problem in the past because we are only
> doing append addition and removal so the active AG part of the tree
> is not changing shape, but that doesn't mean it is safe. Just making
> the radix tree modifications serialise against each other is obviously
> correct.
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks correct to me, and I didn't find any other suspicious accesses of
m_perag_tree so

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

--D

> ---
>  fs/xfs/libxfs/xfs_ag.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index f62ff125a50a..c730976fdfc0 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -424,13 +424,17 @@ xfs_initialize_perag(
>  
>  out_remove_pag:
>  	xfs_defer_drain_free(&pag->pag_intents_drain);
> +	spin_lock(&mp->m_perag_lock);
>  	radix_tree_delete(&mp->m_perag_tree, index);
> +	spin_unlock(&mp->m_perag_lock);
>  out_free_pag:
>  	kmem_free(pag);
>  out_unwind_new_pags:
>  	/* unwind any prior newly initialized pags */
>  	for (index = first_initialised; index < agcount; index++) {
> +		spin_lock(&mp->m_perag_lock);
>  		pag = radix_tree_delete(&mp->m_perag_tree, index);
> +		spin_unlock(&mp->m_perag_lock);
>  		if (!pag)
>  			break;
>  		xfs_buf_hash_destroy(pag);
> -- 
> 2.31.1
> 
> 

Re: [PATCH v3 2/2] xfs: fix perag leak when growfs fails

[Darrick J. Wong]

On Wed, Dec 13, 2023 at 11:10:13AM +0800, Long Li wrote:
> During growfs, if new ag in memory has been initialized, however
> sb_agcount has not been updated, if an error occurs at this time it
> will cause perag leaks as follows, these new AGs will not been freed
> during umount , because of these new AGs are not visible(that is
> included in mp->m_sb.sb_agcount).
> 
> unreferenced object 0xffff88810be40200 (size 512):
>   comm "xfs_growfs", pid 857, jiffies 4294909093
>   hex dump (first 32 bytes):
>     00 c0 c1 05 81 88 ff ff 04 00 00 00 00 00 00 00  ................
>     01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace (crc 381741e2):
>     [<ffffffff8191aef6>] __kmalloc+0x386/0x4f0
>     [<ffffffff82553e65>] kmem_alloc+0xb5/0x2f0
>     [<ffffffff8238dac5>] xfs_initialize_perag+0xc5/0x810
>     [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
>     [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
>     [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
>     [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
>     [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> unreferenced object 0xffff88810be40800 (size 512):
>   comm "xfs_growfs", pid 857, jiffies 4294909093
>   hex dump (first 32 bytes):
>     20 00 00 00 00 00 00 00 57 ef be dc 00 00 00 00   .......W.......
>     10 08 e4 0b 81 88 ff ff 10 08 e4 0b 81 88 ff ff  ................
>   backtrace (crc bde50e2d):
>     [<ffffffff8191b43a>] __kmalloc_node+0x3da/0x540
>     [<ffffffff81814489>] kvmalloc_node+0x99/0x160
>     [<ffffffff8286acff>] bucket_table_alloc.isra.0+0x5f/0x400
>     [<ffffffff8286bdc5>] rhashtable_init+0x405/0x760
>     [<ffffffff8238dda3>] xfs_initialize_perag+0x3a3/0x810
>     [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
>     [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
>     [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
>     [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
>     [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> 
> Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
> used for freeing unused perag within a specified range in error handling,
> included in the error path of the growfs failure.
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>

This looks like a fix for a bug; is there a reason you omitted a Fixes:
tag?  Has this bug been here so long that blame points at the
granddaddy commit?

Code looks ok to me though.

--D

> ---
> v3:
> - Stop use of typedefs for struct pointers
> - Remove unnecessary indentation
> - Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
>   not xfs_free_perag(). Compared to the v2 version, now the logic for
>   freeing perag in xfs_free_perag() and xfs_initialize_perag() error
>   handling not quite the same.
>  fs/xfs/libxfs/xfs_ag.c | 36 ++++++++++++++++++++++++++----------
>  fs/xfs/libxfs/xfs_ag.h |  2 ++
>  fs/xfs/xfs_fsops.c     |  5 ++++-
>  3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index c730976fdfc0..39d9525270b7 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -332,6 +332,31 @@ xfs_agino_range(
>  	return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last);
>  }
>  
> +/*
> + * Free perag within the specified AG range, it is only used to free unused
> + * perags under the error handling path.
> + */
> +void
> +xfs_free_unused_perag_range(
> +	struct xfs_mount	*mp,
> +	xfs_agnumber_t		agstart,
> +	xfs_agnumber_t		agend)
> +{
> +	struct xfs_perag	*pag;
> +	xfs_agnumber_t		index;
> +
> +	for (index = agstart; index < agend; index++) {
> +		spin_lock(&mp->m_perag_lock);
> +		pag = radix_tree_delete(&mp->m_perag_tree, index);
> +		spin_unlock(&mp->m_perag_lock);
> +		if (!pag)
> +			break;
> +		xfs_buf_hash_destroy(pag);
> +		xfs_defer_drain_free(&pag->pag_intents_drain);
> +		kmem_free(pag);
> +	}
> +}
> +
>  int
>  xfs_initialize_perag(
>  	struct xfs_mount	*mp,
> @@ -431,16 +456,7 @@ xfs_initialize_perag(
>  	kmem_free(pag);
>  out_unwind_new_pags:
>  	/* unwind any prior newly initialized pags */
> -	for (index = first_initialised; index < agcount; index++) {
> -		spin_lock(&mp->m_perag_lock);
> -		pag = radix_tree_delete(&mp->m_perag_tree, index);
> -		spin_unlock(&mp->m_perag_lock);
> -		if (!pag)
> -			break;
> -		xfs_buf_hash_destroy(pag);
> -		xfs_defer_drain_free(&pag->pag_intents_drain);
> -		kmem_free(pag);
> -	}
> +	xfs_free_unused_perag_range(mp, first_initialised, agcount);
>  	return error;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 2e0aef87d633..40d7b6427afb 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -133,6 +133,8 @@ __XFS_AG_OPSTATE(prefers_metadata, PREFERS_METADATA)
>  __XFS_AG_OPSTATE(allows_inodes, ALLOWS_INODES)
>  __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET)
>  
> +void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart,
> +			xfs_agnumber_t agend);
>  int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
>  			xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi);
>  int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno);
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index e8759b479516..22c3f1e9008e 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -153,7 +153,7 @@ xfs_growfs_data_private(
>  		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, -delta, 0,
>  				0, &tp);
>  	if (error)
> -		return error;
> +		goto out_free_unused_perag;
>  
>  	last_pag = xfs_perag_get(mp, oagcount - 1);
>  	if (delta > 0) {
> @@ -227,6 +227,9 @@ xfs_growfs_data_private(
>  
>  out_trans_cancel:
>  	xfs_trans_cancel(tp);
> +out_free_unused_perag:
> +	if (nagcount > oagcount)
> +		xfs_free_unused_perag_range(mp, oagcount, nagcount);
>  	return error;
>  }
>  
> -- 
> 2.31.1
> 
> 

Re: [PATCH v3 2/2] xfs: fix perag leak when growfs fails

[Long Li]

On Wed, Dec 13, 2023 at 10:21:12AM -0800, Darrick J. Wong wrote:
> On Wed, Dec 13, 2023 at 11:10:13AM +0800, Long Li wrote:
> > During growfs, if new ag in memory has been initialized, however
> > sb_agcount has not been updated, if an error occurs at this time it
> > will cause perag leaks as follows, these new AGs will not been freed
> > during umount , because of these new AGs are not visible(that is
> > included in mp->m_sb.sb_agcount).
> > 
> > unreferenced object 0xffff88810be40200 (size 512):
> >   comm "xfs_growfs", pid 857, jiffies 4294909093
> >   hex dump (first 32 bytes):
> >     00 c0 c1 05 81 88 ff ff 04 00 00 00 00 00 00 00  ................
> >     01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace (crc 381741e2):
> >     [<ffffffff8191aef6>] __kmalloc+0x386/0x4f0
> >     [<ffffffff82553e65>] kmem_alloc+0xb5/0x2f0
> >     [<ffffffff8238dac5>] xfs_initialize_perag+0xc5/0x810
> >     [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
> >     [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
> >     [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
> >     [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
> >     [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> > unreferenced object 0xffff88810be40800 (size 512):
> >   comm "xfs_growfs", pid 857, jiffies 4294909093
> >   hex dump (first 32 bytes):
> >     20 00 00 00 00 00 00 00 57 ef be dc 00 00 00 00   .......W.......
> >     10 08 e4 0b 81 88 ff ff 10 08 e4 0b 81 88 ff ff  ................
> >   backtrace (crc bde50e2d):
> >     [<ffffffff8191b43a>] __kmalloc_node+0x3da/0x540
> >     [<ffffffff81814489>] kvmalloc_node+0x99/0x160
> >     [<ffffffff8286acff>] bucket_table_alloc.isra.0+0x5f/0x400
> >     [<ffffffff8286bdc5>] rhashtable_init+0x405/0x760
> >     [<ffffffff8238dda3>] xfs_initialize_perag+0x3a3/0x810
> >     [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
> >     [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
> >     [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
> >     [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
> >     [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> > 
> > Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
> > used for freeing unused perag within a specified range in error handling,
> > included in the error path of the growfs failure.
> > 
> > Signed-off-by: Long Li <leo.lilong@huawei.com>
> 
> This looks like a fix for a bug; is there a reason you omitted a Fixes:
> tag?  Has this bug been here so long that blame points at the
> granddaddy commit?

Yes, this bug has been around for a long time, and I've looked up the
submission history again.

Long times ago, it used an array for the per-ag structures, per-ag
array can be freed in xfs_free_perag(). After commit 1c1c6ebcf528 ("xfs:
Replace per-ag array with a radix tree") was added, only visible AGs
will be freed, so it need to add the following fix tag.

Fixes: 1c1c6ebcf528 ("xfs: Replace per-ag array with a radix tree")

Thanks.

> 
> Code looks ok to me though.
> 
> --D
> 
> > ---
> > v3:
> > - Stop use of typedefs for struct pointers
> > - Remove unnecessary indentation
> > - Factor out xfs_free_unused_perag_range() from xfs_initialize_perag(),
> >   not xfs_free_perag(). Compared to the v2 version, now the logic for
> >   freeing perag in xfs_free_perag() and xfs_initialize_perag() error
> >   handling not quite the same.
> >  fs/xfs/libxfs/xfs_ag.c | 36 ++++++++++++++++++++++++++----------
> >  fs/xfs/libxfs/xfs_ag.h |  2 ++
> >  fs/xfs/xfs_fsops.c     |  5 ++++-
> >  3 files changed, 32 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> > index c730976fdfc0..39d9525270b7 100644
> > --- a/fs/xfs/libxfs/xfs_ag.c
> > +++ b/fs/xfs/libxfs/xfs_ag.c
> > @@ -332,6 +332,31 @@ xfs_agino_range(
> >  	return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last);
> >  }
> >  
> > +/*
> > + * Free perag within the specified AG range, it is only used to free unused
> > + * perags under the error handling path.
> > + */
> > +void
> > +xfs_free_unused_perag_range(
> > +	struct xfs_mount	*mp,
> > +	xfs_agnumber_t		agstart,
> > +	xfs_agnumber_t		agend)
> > +{
> > +	struct xfs_perag	*pag;
> > +	xfs_agnumber_t		index;
> > +
> > +	for (index = agstart; index < agend; index++) {
> > +		spin_lock(&mp->m_perag_lock);
> > +		pag = radix_tree_delete(&mp->m_perag_tree, index);
> > +		spin_unlock(&mp->m_perag_lock);
> > +		if (!pag)
> > +			break;
> > +		xfs_buf_hash_destroy(pag);
> > +		xfs_defer_drain_free(&pag->pag_intents_drain);
> > +		kmem_free(pag);
> > +	}
> > +}
> > +
> >  int
> >  xfs_initialize_perag(
> >  	struct xfs_mount	*mp,
> > @@ -431,16 +456,7 @@ xfs_initialize_perag(
> >  	kmem_free(pag);
> >  out_unwind_new_pags:
> >  	/* unwind any prior newly initialized pags */
> > -	for (index = first_initialised; index < agcount; index++) {
> > -		spin_lock(&mp->m_perag_lock);
> > -		pag = radix_tree_delete(&mp->m_perag_tree, index);
> > -		spin_unlock(&mp->m_perag_lock);
> > -		if (!pag)
> > -			break;
> > -		xfs_buf_hash_destroy(pag);
> > -		xfs_defer_drain_free(&pag->pag_intents_drain);
> > -		kmem_free(pag);
> > -	}
> > +	xfs_free_unused_perag_range(mp, first_initialised, agcount);
> >  	return error;
> >  }
> >  
> > diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> > index 2e0aef87d633..40d7b6427afb 100644
> > --- a/fs/xfs/libxfs/xfs_ag.h
> > +++ b/fs/xfs/libxfs/xfs_ag.h
> > @@ -133,6 +133,8 @@ __XFS_AG_OPSTATE(prefers_metadata, PREFERS_METADATA)
> >  __XFS_AG_OPSTATE(allows_inodes, ALLOWS_INODES)
> >  __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET)
> >  
> > +void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart,
> > +			xfs_agnumber_t agend);
> >  int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
> >  			xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi);
> >  int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno);
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index e8759b479516..22c3f1e9008e 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -153,7 +153,7 @@ xfs_growfs_data_private(
> >  		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, -delta, 0,
> >  				0, &tp);
> >  	if (error)
> > -		return error;
> > +		goto out_free_unused_perag;
> >  
> >  	last_pag = xfs_perag_get(mp, oagcount - 1);
> >  	if (delta > 0) {
> > @@ -227,6 +227,9 @@ xfs_growfs_data_private(
> >  
> >  out_trans_cancel:
> >  	xfs_trans_cancel(tp);
> > +out_free_unused_perag:
> > +	if (nagcount > oagcount)
> > +		xfs_free_unused_perag_range(mp, oagcount, nagcount);
> >  	return error;
> >  }
> >  
> > -- 
> > 2.31.1
> > 
> > 
>