Re: [PATCH 2/5] xfs: move the tagged perag lookup helpers to xfs_icache.c

Darrick J. Wong
2024-08-21T16:34:07Z

On Wed, Aug 21, 2024 at 08:38:29AM +0200, Christoph Hellwig wrote:
> The tagged perag helpers are only used in xfs_icache.c in the kernel code
> and not at all in xfsprogs. Move them to xfs_icache.c in preparation for
> switching to an xarray, for which I have no plan to implement the tagged
> lookup functions for userspace.

I don't particularly like moving these functions to another file, but I
suppose the icache is the only user of these tags. How hard is it to
make userspace stubs that assert if anyone ever tries to use it?

--D

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_ag.c | 51 -------------------------------------
> fs/xfs/libxfs/xfs_ag.h | 11 --------
> fs/xfs/xfs_icache.c | 58 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 58 insertions(+), 62 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 4b5a39a83f7aed..87f00f0180846f 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -56,31 +56,6 @@ xfs_perag_get(
> return pag;
> }
>
> -/*
> - * search from @first to find the next perag with the given tag set.
> - */
> -struct xfs_perag *
> -xfs_perag_get_tag(
> - struct xfs_mount *mp,
> - xfs_agnumber_t first,
> - unsigned int tag)
> -{
> - struct xfs_perag *pag;
> - int found;
> -
> - rcu_read_lock();
> - found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
> - (void **)&pag, first, 1, tag);
> - if (found <= 0) {
> - rcu_read_unlock();
> - return NULL;
> - }
> - trace_xfs_perag_get_tag(pag, _RET_IP_);
> - atomic_inc(&pag->pag_ref);
> - rcu_read_unlock();
> - return pag;
> -}
> -
> /* Get a passive reference to the given perag. */
> struct xfs_perag *
> xfs_perag_hold(
> @@ -127,32 +102,6 @@ xfs_perag_grab(
> return pag;
> }
>
> -/*
> - * search from @first to find the next perag with the given tag set.
> - */
> -struct xfs_perag *
> -xfs_perag_grab_tag(
> - struct xfs_mount *mp,
> - xfs_agnumber_t first,
> - int tag)
> -{
> - struct xfs_perag *pag;
> - int found;
> -
> - rcu_read_lock();
> - found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
> - (void **)&pag, first, 1, tag);
> - if (found <= 0) {
> - rcu_read_unlock();
> - return NULL;
> - }
> - trace_xfs_perag_grab_tag(pag, _RET_IP_);
> - if (!atomic_inc_not_zero(&pag->pag_active_ref))
> - pag = NULL;
> - rcu_read_unlock();
> - return pag;
> -}
> -
> void
> xfs_perag_rele(
> struct xfs_perag *pag)
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index d62c266c0b44d5..d9cccd093b60e0 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -153,15 +153,11 @@ void xfs_free_perag(struct xfs_mount *mp);
>
> /* Passive AG references */
> struct xfs_perag *xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno);
> -struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *mp, xfs_agnumber_t agno,
> - unsigned int tag);
> struct xfs_perag *xfs_perag_hold(struct xfs_perag *pag);
> void xfs_perag_put(struct xfs_perag *pag);
>
> /* Active AG references */
> struct xfs_perag *xfs_perag_grab(struct xfs_mount *, xfs_agnumber_t);
> -struct xfs_perag *xfs_perag_grab_tag(struct xfs_mount *, xfs_agnumber_t,
> - int tag);
> void xfs_perag_rele(struct xfs_perag *pag);
>
> /*
> @@ -263,13 +259,6 @@ xfs_perag_next(
> (agno) = 0; \
> for_each_perag_from((mp), (agno), (pag))
>
> -#define for_each_perag_tag(mp, agno, pag, tag) \
> - for ((agno) = 0, (pag) = xfs_perag_grab_tag((mp), 0, (tag)); \
> - (pag) != NULL; \
> - (agno) = (pag)->pag_agno + 1, \
> - xfs_perag_rele(pag), \
> - (pag) = xfs_perag_grab_tag((mp), (agno), (tag)))
> -
> static inline struct xfs_perag *
> xfs_perag_next_wrap(
> struct xfs_perag *pag,
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index cf629302d48e74..ac604640d36229 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -292,6 +292,64 @@ 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_tag(
> + struct xfs_mount *mp,
> + xfs_agnumber_t first,
> + unsigned int tag)
> +{
> + struct xfs_perag *pag;
> + int found;
> +
> + rcu_read_lock();
> + found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
> + (void **)&pag, first, 1, tag);
> + if (found <= 0) {
> + rcu_read_unlock();
> + return NULL;
> + }
> + trace_xfs_perag_get_tag(pag, _RET_IP_);
> + atomic_inc(&pag->pag_ref);
> + rcu_read_unlock();
> + return pag;
> +}
> +
> +/*
> + * Search from @first to find the next perag with the given tag set.
> + */
> +static struct xfs_perag *
> +xfs_perag_grab_tag(
> + struct xfs_mount *mp,
> + xfs_agnumber_t first,
> + int tag)
> +{
> + struct xfs_perag *pag;
> + int found;
> +
> + rcu_read_lock();
> + found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
> + (void **)&pag, first, 1, tag);
> + if (found <= 0) {
> + rcu_read_unlock();
> + return NULL;
> + }
> + trace_xfs_perag_grab_tag(pag, _RET_IP_);
> + if (!atomic_inc_not_zero(&pag->pag_active_ref))
> + pag = NULL;
> + rcu_read_unlock();
> + return pag;
> +}
> +
> +#define for_each_perag_tag(mp, agno, pag, tag) \
> + for ((agno) = 0, (pag) = xfs_perag_grab_tag((mp), 0, (tag)); \
> + (pag) != NULL; \
> + (agno) = (pag)->pag_agno + 1, \
> + xfs_perag_rele(pag), \
> + (pag) = xfs_perag_grab_tag((mp), (agno), (tag)))
> +
> /*
> * When we recycle a reclaimable inode, we need to re-initialise the VFS inode
> * part of the structure. This is made more complex by the fact we store
> --
> 2.43.0
>
>