public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/22] xfs: convert xfs_iwalk to use perag references
Date: Mon, 10 May 2021 09:41:52 -0400	[thread overview]
Message-ID: <YJk4IA1sNrgnJNj1@bfoster> (raw)
In-Reply-To: <20210506072054.271157-7-david@fromorbit.com>

On Thu, May 06, 2021 at 05:20:38PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Rather than manually walking the ags and passing agnunbers around,
> pass the perag for the AG we are currently working on around in the
> iwalk structure.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

LGTM:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_ag.h |  8 +++-
>  fs/xfs/xfs_iwalk.c     | 86 ++++++++++++++++++++++++++----------------
>  2 files changed, 60 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 33783120263c..3fa88222dacd 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -117,13 +117,17 @@ void	xfs_perag_put(struct xfs_perag *pag);
>  /*
>   * Perag iteration APIs
>   */
> -#define for_each_perag(mp, next_agno, pag) \
> -	for ((next_agno) = 0, (pag) = xfs_perag_get((mp), 0); \
> +#define for_each_perag_from(mp, next_agno, pag) \
> +	for ((pag) = xfs_perag_get((mp), (next_agno)); \
>  		(pag) != NULL; \
>  		(next_agno) = (pag)->pag_agno + 1, \
>  		xfs_perag_put(pag), \
>  		(pag) = xfs_perag_get((mp), (next_agno)))
>  
> +#define for_each_perag(mp, next_agno, pag) \
> +	(next_agno) = 0; \
> +	for_each_perag_from((mp), (next_agno), (pag))
> +
>  #define for_each_perag_tag(mp, next_agno, pag, tag) \
>  	for ((next_agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \
>  		(pag) != NULL; \
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index c4a340f1f1e1..c7e8f48a3ec4 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -21,6 +21,7 @@
>  #include "xfs_health.h"
>  #include "xfs_trans.h"
>  #include "xfs_pwork.h"
> +#include "xfs_ag.h"
>  
>  /*
>   * Walking Inodes in the Filesystem
> @@ -51,6 +52,7 @@ struct xfs_iwalk_ag {
>  
>  	struct xfs_mount		*mp;
>  	struct xfs_trans		*tp;
> +	struct xfs_perag		*pag;
>  
>  	/* Where do we start the traversal? */
>  	xfs_ino_t			startino;
> @@ -90,7 +92,7 @@ struct xfs_iwalk_ag {
>  STATIC void
>  xfs_iwalk_ichunk_ra(
>  	struct xfs_mount		*mp,
> -	xfs_agnumber_t			agno,
> +	struct xfs_perag		*pag,
>  	struct xfs_inobt_rec_incore	*irec)
>  {
>  	struct xfs_ino_geometry		*igeo = M_IGEO(mp);
> @@ -106,7 +108,7 @@ xfs_iwalk_ichunk_ra(
>  
>  		imask = xfs_inobt_maskn(i, igeo->inodes_per_cluster);
>  		if (imask & ~irec->ir_free) {
> -			xfs_btree_reada_bufs(mp, agno, agbno,
> +			xfs_btree_reada_bufs(mp, pag->pag_agno, agbno,
>  					igeo->blocks_per_cluster,
>  					&xfs_inode_buf_ops);
>  		}
> @@ -174,26 +176,25 @@ xfs_iwalk_free(
>  /* For each inuse inode in each cached inobt record, call our function. */
>  STATIC int
>  xfs_iwalk_ag_recs(
> -	struct xfs_iwalk_ag		*iwag)
> +	struct xfs_iwalk_ag	*iwag)
>  {
> -	struct xfs_mount		*mp = iwag->mp;
> -	struct xfs_trans		*tp = iwag->tp;
> -	xfs_ino_t			ino;
> -	unsigned int			i, j;
> -	xfs_agnumber_t			agno;
> -	int				error;
> +	struct xfs_mount	*mp = iwag->mp;
> +	struct xfs_trans	*tp = iwag->tp;
> +	struct xfs_perag	*pag = iwag->pag;
> +	xfs_ino_t		ino;
> +	unsigned int		i, j;
> +	int			error;
>  
> -	agno = XFS_INO_TO_AGNO(mp, iwag->startino);
>  	for (i = 0; i < iwag->nr_recs; i++) {
>  		struct xfs_inobt_rec_incore	*irec = &iwag->recs[i];
>  
> -		trace_xfs_iwalk_ag_rec(mp, agno, irec);
> +		trace_xfs_iwalk_ag_rec(mp, pag->pag_agno, irec);
>  
>  		if (xfs_pwork_want_abort(&iwag->pwork))
>  			return 0;
>  
>  		if (iwag->inobt_walk_fn) {
> -			error = iwag->inobt_walk_fn(mp, tp, agno, irec,
> +			error = iwag->inobt_walk_fn(mp, tp, pag->pag_agno, irec,
>  					iwag->data);
>  			if (error)
>  				return error;
> @@ -211,7 +212,8 @@ xfs_iwalk_ag_recs(
>  				continue;
>  
>  			/* Otherwise call our function. */
> -			ino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino + j);
> +			ino = XFS_AGINO_TO_INO(mp, pag->pag_agno,
> +						irec->ir_startino + j);
>  			error = iwag->iwalk_fn(mp, tp, ino, iwag->data);
>  			if (error)
>  				return error;
> @@ -257,7 +259,6 @@ xfs_iwalk_del_inobt(
>  STATIC int
>  xfs_iwalk_ag_start(
>  	struct xfs_iwalk_ag	*iwag,
> -	xfs_agnumber_t		agno,
>  	xfs_agino_t		agino,
>  	struct xfs_btree_cur	**curpp,
>  	struct xfs_buf		**agi_bpp,
> @@ -265,12 +266,14 @@ xfs_iwalk_ag_start(
>  {
>  	struct xfs_mount	*mp = iwag->mp;
>  	struct xfs_trans	*tp = iwag->tp;
> +	struct xfs_perag	*pag = iwag->pag;
>  	struct xfs_inobt_rec_incore *irec;
>  	int			error;
>  
>  	/* Set up a fresh cursor and empty the inobt cache. */
>  	iwag->nr_recs = 0;
> -	error = xfs_inobt_cur(mp, tp, agno, XFS_BTNUM_INO, curpp, agi_bpp);
> +	error = xfs_inobt_cur(mp, tp, pag->pag_agno, XFS_BTNUM_INO,
> +				curpp, agi_bpp);
>  	if (error)
>  		return error;
>  
> @@ -304,7 +307,7 @@ xfs_iwalk_ag_start(
>  	if (XFS_IS_CORRUPT(mp, *has_more != 1))
>  		return -EFSCORRUPTED;
>  
> -	iwag->lastino = XFS_AGINO_TO_INO(mp, agno,
> +	iwag->lastino = XFS_AGINO_TO_INO(mp, pag->pag_agno,
>  				irec->ir_startino + XFS_INODES_PER_CHUNK - 1);
>  
>  	/*
> @@ -345,7 +348,6 @@ xfs_iwalk_ag_start(
>  STATIC int
>  xfs_iwalk_run_callbacks(
>  	struct xfs_iwalk_ag		*iwag,
> -	xfs_agnumber_t			agno,
>  	struct xfs_btree_cur		**curpp,
>  	struct xfs_buf			**agi_bpp,
>  	int				*has_more)
> @@ -376,7 +378,8 @@ xfs_iwalk_run_callbacks(
>  		return 0;
>  
>  	/* ...and recreate the cursor just past where we left off. */
> -	error = xfs_inobt_cur(mp, tp, agno, XFS_BTNUM_INO, curpp, agi_bpp);
> +	error = xfs_inobt_cur(mp, tp, iwag->pag->pag_agno, XFS_BTNUM_INO,
> +				curpp, agi_bpp);
>  	if (error)
>  		return error;
>  
> @@ -390,17 +393,17 @@ xfs_iwalk_ag(
>  {
>  	struct xfs_mount		*mp = iwag->mp;
>  	struct xfs_trans		*tp = iwag->tp;
> +	struct xfs_perag		*pag = iwag->pag;
>  	struct xfs_buf			*agi_bp = NULL;
>  	struct xfs_btree_cur		*cur = NULL;
> -	xfs_agnumber_t			agno;
>  	xfs_agino_t			agino;
>  	int				has_more;
>  	int				error = 0;
>  
>  	/* Set up our cursor at the right place in the inode btree. */
> -	agno = XFS_INO_TO_AGNO(mp, iwag->startino);
> +	ASSERT(pag->pag_agno == XFS_INO_TO_AGNO(mp, iwag->startino));
>  	agino = XFS_INO_TO_AGINO(mp, iwag->startino);
> -	error = xfs_iwalk_ag_start(iwag, agno, agino, &cur, &agi_bp, &has_more);
> +	error = xfs_iwalk_ag_start(iwag, agino, &cur, &agi_bp, &has_more);
>  
>  	while (!error && has_more) {
>  		struct xfs_inobt_rec_incore	*irec;
> @@ -417,7 +420,7 @@ xfs_iwalk_ag(
>  			break;
>  
>  		/* Make sure that we always move forward. */
> -		rec_fsino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino);
> +		rec_fsino = XFS_AGINO_TO_INO(mp, pag->pag_agno, irec->ir_startino);
>  		if (iwag->lastino != NULLFSINO &&
>  		    XFS_IS_CORRUPT(mp, iwag->lastino >= rec_fsino)) {
>  			error = -EFSCORRUPTED;
> @@ -438,7 +441,7 @@ xfs_iwalk_ag(
>  		 * walking the inodes.
>  		 */
>  		if (iwag->iwalk_fn)
> -			xfs_iwalk_ichunk_ra(mp, agno, irec);
> +			xfs_iwalk_ichunk_ra(mp, pag, irec);
>  
>  		/*
>  		 * If there's space in the buffer for more records, increment
> @@ -458,15 +461,14 @@ xfs_iwalk_ag(
>  		 * we would be if we had been able to increment like above.
>  		 */
>  		ASSERT(has_more);
> -		error = xfs_iwalk_run_callbacks(iwag, agno, &cur, &agi_bp,
> -				&has_more);
> +		error = xfs_iwalk_run_callbacks(iwag, &cur, &agi_bp, &has_more);
>  	}
>  
>  	if (iwag->nr_recs == 0 || error)
>  		goto out;
>  
>  	/* Walk the unprocessed records in the cache. */
> -	error = xfs_iwalk_run_callbacks(iwag, agno, &cur, &agi_bp, &has_more);
> +	error = xfs_iwalk_run_callbacks(iwag, &cur, &agi_bp, &has_more);
>  
>  out:
>  	xfs_iwalk_del_inobt(tp, &cur, &agi_bp, error);
> @@ -555,6 +557,7 @@ xfs_iwalk(
>  		.pwork		= XFS_PWORK_SINGLE_THREADED,
>  		.lastino	= NULLFSINO,
>  	};
> +	struct xfs_perag	*pag;
>  	xfs_agnumber_t		agno = XFS_INO_TO_AGNO(mp, startino);
>  	int			error;
>  
> @@ -565,15 +568,19 @@ xfs_iwalk(
>  	if (error)
>  		return error;
>  
> -	for (; agno < mp->m_sb.sb_agcount; agno++) {
> +	for_each_perag_from(mp, agno, pag) {
> +		iwag.pag = pag;
>  		error = xfs_iwalk_ag(&iwag);
>  		if (error)
>  			break;
>  		iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
>  		if (flags & XFS_INOBT_WALK_SAME_AG)
>  			break;
> +		iwag.pag = NULL;
>  	}
>  
> +	if (iwag.pag)
> +		xfs_perag_put(pag);
>  	xfs_iwalk_free(&iwag);
>  	return error;
>  }
> @@ -598,6 +605,7 @@ xfs_iwalk_ag_work(
>  	error = xfs_iwalk_ag(iwag);
>  	xfs_iwalk_free(iwag);
>  out:
> +	xfs_perag_put(iwag->pag);
>  	kmem_free(iwag);
>  	return error;
>  }
> @@ -617,6 +625,7 @@ xfs_iwalk_threaded(
>  	void			*data)
>  {
>  	struct xfs_pwork_ctl	pctl;
> +	struct xfs_perag	*pag;
>  	xfs_agnumber_t		agno = XFS_INO_TO_AGNO(mp, startino);
>  	int			error;
>  
> @@ -627,7 +636,7 @@ xfs_iwalk_threaded(
>  	if (error)
>  		return error;
>  
> -	for (; agno < mp->m_sb.sb_agcount; agno++) {
> +	for_each_perag_from(mp, agno, pag) {
>  		struct xfs_iwalk_ag	*iwag;
>  
>  		if (xfs_pwork_ctl_want_abort(&pctl))
> @@ -635,17 +644,25 @@ xfs_iwalk_threaded(
>  
>  		iwag = kmem_zalloc(sizeof(struct xfs_iwalk_ag), 0);
>  		iwag->mp = mp;
> +
> +		/*
> +		 * perag is being handed off to async work, so take another
> +		 * reference for the async work to release.
> +		 */
> +		atomic_inc(&pag->pag_ref);
> +		iwag->pag = pag;
>  		iwag->iwalk_fn = iwalk_fn;
>  		iwag->data = data;
>  		iwag->startino = startino;
>  		iwag->sz_recs = xfs_iwalk_prefetch(inode_records);
>  		iwag->lastino = NULLFSINO;
>  		xfs_pwork_queue(&pctl, &iwag->pwork);
> -		startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
> +		startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0);
>  		if (flags & XFS_INOBT_WALK_SAME_AG)
>  			break;
>  	}
> -
> +	if (pag)
> +		xfs_perag_put(pag);
>  	if (polled)
>  		xfs_pwork_poll(&pctl);
>  	return xfs_pwork_destroy(&pctl);
> @@ -715,6 +732,7 @@ xfs_inobt_walk(
>  		.pwork		= XFS_PWORK_SINGLE_THREADED,
>  		.lastino	= NULLFSINO,
>  	};
> +	struct xfs_perag	*pag;
>  	xfs_agnumber_t		agno = XFS_INO_TO_AGNO(mp, startino);
>  	int			error;
>  
> @@ -725,15 +743,19 @@ xfs_inobt_walk(
>  	if (error)
>  		return error;
>  
> -	for (; agno < mp->m_sb.sb_agcount; agno++) {
> +	for_each_perag_from(mp, agno, pag) {
> +		iwag.pag = pag;
>  		error = xfs_iwalk_ag(&iwag);
>  		if (error)
>  			break;
> -		iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
> +		iwag.startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0);
>  		if (flags & XFS_INOBT_WALK_SAME_AG)
>  			break;
> +		iwag.pag = NULL;
>  	}
>  
> +	if (iwag.pag)
> +		xfs_perag_put(pag);
>  	xfs_iwalk_free(&iwag);
>  	return error;
>  }
> -- 
> 2.31.1
> 


  reply	other threads:[~2021-05-10 13:51 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06  7:20 [RFC 00/22] xfs: initial agnumber -> perag conversions for shrink Dave Chinner
2021-05-06  7:20 ` [PATCH 01/22] xfs: move xfs_perag_get/put to xfs_ag.[ch] Dave Chinner
2021-05-10 12:52   ` Brian Foster
2021-05-11  7:18     ` Dave Chinner
2021-05-10 22:28   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 02/22] xfs: prepare for moving perag definitions and support to libxfs Dave Chinner
2021-05-10 12:53   ` Brian Foster
2021-05-11  7:19     ` Dave Chinner
2021-05-06  7:20 ` [PATCH 03/22] xfs: move perag structure and setup to libxfs/xfs_ag.[ch] Dave Chinner
2021-05-10 22:26   ` Darrick J. Wong
2021-05-10 23:38     ` Dave Chinner
2021-05-06  7:20 ` [PATCH 04/22] xfs: make for_each_perag... a first class citizen Dave Chinner
2021-05-10 12:53   ` Brian Foster
2021-05-11  7:35     ` Dave Chinner
2021-05-11 12:29       ` Brian Foster
2021-05-11 21:33         ` Dave Chinner
2021-05-12 21:58           ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 05/22] xfs: convert raw ag walks to use for_each_perag Dave Chinner
2021-05-10 12:54   ` Brian Foster
2021-05-06  7:20 ` [PATCH 06/22] xfs: convert xfs_iwalk to use perag references Dave Chinner
2021-05-10 13:41   ` Brian Foster [this message]
2021-05-12 22:08   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 07/22] xfs: convert secondary superblock walk to use perags Dave Chinner
2021-05-10 13:41   ` Brian Foster
2021-05-12 22:09   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 08/22] xfs: pass perags through to the busy extent code Dave Chinner
2021-05-11 12:29   ` Brian Foster
2021-05-12 22:13   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 09/22] xfs: push perags through the ag reservation callouts Dave Chinner
2021-05-11 12:29   ` Brian Foster
2021-05-13  0:29     ` Dave Chinner
2021-05-12 22:16   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 10/22] xfs: pass perags around in fsmap data dev functions Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-12 22:23   ` Darrick J. Wong
2021-05-18  1:00     ` Dave Chinner
2021-05-06  7:20 ` [PATCH 11/22] xfs: add a perag to the btree cursor Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-11 20:51     ` Darrick J. Wong
2021-05-11 21:52       ` Dave Chinner
2021-05-12 12:49         ` Brian Foster
2021-05-12 22:41           ` Darrick J. Wong
2021-05-12 22:40   ` Darrick J. Wong
2021-05-13  0:12     ` Dave Chinner
2021-05-13  0:55       ` Darrick J. Wong
2021-05-13  1:07         ` Dave Chinner
2021-05-13  3:49           ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 12/22] xfs: convert rmap btree cursor to using a perag Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-12 22:45   ` Darrick J. Wong
2021-05-13  3:54     ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 13/22] xfs: convert refcount btree cursor to use perags Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-12 22:47   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 14/22] xfs: convert allocbt cursors " Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-13  3:55   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 15/22] xfs: use perag for ialloc btree cursors Dave Chinner
2021-05-11 12:30   ` Brian Foster
2021-05-13  3:55   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 16/22] xfs: remove agno from btree cursor Dave Chinner
2021-05-11 12:34   ` Brian Foster
2021-05-11 22:02     ` Dave Chinner
2021-05-12 22:52   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 17/22] xfs: simplify xfs_dialloc_select_ag() return values Dave Chinner
2021-05-12 12:49   ` Brian Foster
2021-05-12 22:55   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 18/22] xfs: collapse AG selection for inode allocation Dave Chinner
2021-05-12 12:52   ` Brian Foster
2021-05-18  1:21     ` Dave Chinner
2021-05-12 23:11   ` Darrick J. Wong
2021-05-18  1:29     ` Dave Chinner
2021-05-06  7:20 ` [PATCH 19/22] xfs: get rid of xfs_dir_ialloc() Dave Chinner
2021-05-06 11:26   ` kernel test robot
2021-05-06 11:26   ` [RFC PATCH] xfs: xfs_dialloc_ag can be static kernel test robot
2021-05-12 12:52   ` [PATCH 19/22] xfs: get rid of xfs_dir_ialloc() Brian Foster
2021-05-12 23:19   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 20/22] xfs: inode allocation can use a single perag instance Dave Chinner
2021-05-12 12:52   ` Brian Foster
2021-05-12 23:19   ` Darrick J. Wong
2021-05-06  7:20 ` [PATCH 21/22] xfs: clean up and simplify xfs_dialloc() Dave Chinner
2021-05-12 21:49   ` Darrick J. Wong
2021-05-18  1:46     ` Dave Chinner
2021-05-06  7:20 ` [PATCH 22/22] xfs: use perag through unlink processing Dave Chinner
2021-05-12 21:37   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJk4IA1sNrgnJNj1@bfoster \
    --to=bfoster@redhat.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox