Re: [PATCH 1/5] xfs: use kfree_rcu_mightsleep to free the perag structures

Darrick J. Wong
2024-08-21T16:19:39Z

On Wed, Aug 21, 2024 at 08:38:28AM +0200, Christoph Hellwig wrote:
> Using the kfree_rcu_mightsleep is simpler and removes the need for a
> rcu_head in the perag structure.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_ag.c | 12 +-----------
> fs/xfs/libxfs/xfs_ag.h | 3 ---
> 2 files changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 7e80732cb54708..4b5a39a83f7aed 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -235,16 +235,6 @@ xfs_initialize_perag_data(
> return error;
> }
>
> -STATIC void
> -__xfs_free_perag(
> - struct rcu_head *head)
> -{
> - struct xfs_perag *pag = container_of(head, struct xfs_perag, rcu_head);
> -
> - ASSERT(!delayed_work_pending(&pag->pag_blockgc_work));
> - kfree(pag);
> -}
> -
> /*
> * Free up the per-ag resources associated with the mount structure.
> */
> @@ -270,7 +260,7 @@ xfs_free_perag(
> xfs_perag_rele(pag);
> XFS_IS_CORRUPT(pag->pag_mount,
> atomic_read(&pag->pag_active_ref) != 0);
> - call_rcu(&pag->rcu_head, __xfs_free_perag);
> + kfree_rcu_mightsleep(pag);

I started wondering, have you seen any complaints from might_sleep when
freeing pags after a failed growfs? Then I wondered if growfs_data
could actually take any locks that would prevent sleeping, which led me
to another question: why do growfs_{data,log} hold m_growlock but
growfs_rt doesn't? Is that actually safe?

I think the kfree_rcu_mightsleep conversion is ok, but I want to see if
anything blows up with the rtgroups variant.

--D

> }
> }
>
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 35de09a2516c70..d62c266c0b44d5 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -63,9 +63,6 @@ struct xfs_perag {
> /* Blocks reserved for the reverse mapping btree. */
> struct xfs_ag_resv pag_rmapbt_resv;
>
> - /* for rcu-safe freeing */
> - struct rcu_head rcu_head;
> -
> /* Precalculated geometry info */
> xfs_agblock_t block_count;
> xfs_agblock_t min_block;
> --
> 2.43.0
>
>