Re: [PATCH 5/5] xfs: use xas_for_each_marked in xfs_reclaim_inodes_count

Darrick J. Wong
2024-08-21T16:22:05Z

On Wed, Aug 21, 2024 at 08:38:32AM +0200, Christoph Hellwig wrote:
> xfs_reclaim_inodes_count iterates over all AGs to sum up the reclaimable
> inodes counts. There is no point in grabbing a reference to the them or
> unlock the RCU critical section for each iteration, so switch to the
> more efficient xas_for_each_marked iterator.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
> fs/xfs/xfs_icache.c | 36 ++++++++----------------------------
> fs/xfs/xfs_trace.h | 2 +-
> 2 files changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 5bca845e702f1d..d36dbaba660013 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -300,32 +300,6 @@ xfs_perag_clear_inode_tag(
> trace_xfs_perag_clear_inode_tag(pag, _RET_IP_);
> }
>
> -/*
> - * Search from @first to find the next perag with the given tag set.
> - */
> -static struct xfs_perag *
> -xfs_perag_get_next_tag(
> - struct xfs_mount *mp,
> - struct xfs_perag *pag,
> - unsigned int tag)
> -{
> - unsigned long index = 0;
> -
> - if (pag) {
> - index = pag->pag_agno + 1;
> - xfs_perag_rele(pag);
> - }
> -
> - rcu_read_lock();
> - pag = xa_find(&mp->m_perags, &index, ULONG_MAX, ici_tag_to_mark(tag));
> - if (pag) {
> - trace_xfs_perag_get_next_tag(pag, _RET_IP_);
> - atomic_inc(&pag->pag_ref);
> - }
> - rcu_read_unlock();
> - return pag;
> -}
> -
> /*
> * Find the next AG after @pag, or the first AG if @pag is NULL.
> */
> @@ -1080,11 +1054,17 @@ long
> xfs_reclaim_inodes_count(
> struct xfs_mount *mp)
> {
> - struct xfs_perag *pag = NULL;
> + XA_STATE (xas, &mp->m_perags, 0);
> long reclaimable = 0;
> + struct xfs_perag *pag;
>
> - while ((pag = xfs_perag_get_next_tag(mp, pag, XFS_ICI_RECLAIM_TAG)))
> + rcu_read_lock();
> + xas_for_each_marked(&xas, pag, ULONG_MAX, XFS_PERAG_RECLAIM_MARK) {
> + trace_xfs_reclaim_inodes_count(pag, _THIS_IP_);
> reclaimable += pag->pag_ici_reclaimable;
> + }
> + rcu_read_unlock();
> +
> return reclaimable;
> }
>
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 002d012ebd83cb..d73c0a49d9dc29 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -210,7 +210,6 @@ DEFINE_EVENT(xfs_perag_class, name, \
> TP_PROTO(struct xfs_perag *pag, unsigned long caller_ip), \
> TP_ARGS(pag, caller_ip))
> DEFINE_PERAG_REF_EVENT(xfs_perag_get);
> -DEFINE_PERAG_REF_EVENT(xfs_perag_get_next_tag);
> DEFINE_PERAG_REF_EVENT(xfs_perag_hold);
> DEFINE_PERAG_REF_EVENT(xfs_perag_put);
> DEFINE_PERAG_REF_EVENT(xfs_perag_grab);
> @@ -218,6 +217,7 @@ DEFINE_PERAG_REF_EVENT(xfs_perag_grab_next_tag);
> DEFINE_PERAG_REF_EVENT(xfs_perag_rele);
> DEFINE_PERAG_REF_EVENT(xfs_perag_set_inode_tag);
> DEFINE_PERAG_REF_EVENT(xfs_perag_clear_inode_tag);
> +DEFINE_PERAG_REF_EVENT(xfs_reclaim_inodes_count);
>
> TRACE_EVENT(xfs_inodegc_worker,
> TP_PROTO(struct xfs_mount *mp, unsigned int shrinker_hits),
> --
> 2.43.0
>
>