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

["Darrick J. Wong" <djwong@kernel.org>]

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
> 
>