From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 04/23] xfs: make for_each_perag... a first class citizen
Date: Wed, 26 May 2021 08:33:51 -0400 [thread overview]
Message-ID: <YK5AL/Zd3dKgzgT6@bfoster> (raw)
In-Reply-To: <20210519012102.450926-5-david@fromorbit.com>
On Wed, May 19, 2021 at 11:20:43AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> for_each_perag_tag() is defined in xfs_icache.c for local use.
> Promote this to xfs_ag.h and define equivalent iteration functions
> so that we can use them to iterate AGs instead to replace open coded
> perag walks and perag lookups.
>
> We also convert as many of the straight forward open coded AG walks
> to use these iterators as possible. Anything that is not a direct
> conversion to an iterator is ignored and will be updated in future
> commits.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_ag.h | 17 +++++++++++++++++
> fs/xfs/scrub/fscounters.c | 40 +++++++++++++++------------------------
> fs/xfs/xfs_extent_busy.c | 7 ++-----
> fs/xfs/xfs_fsops.c | 8 ++------
> fs/xfs/xfs_health.c | 4 +---
> fs/xfs/xfs_icache.c | 15 ++-------------
> 6 files changed, 39 insertions(+), 52 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index ec37f9d9f310..33783120263c 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -114,6 +114,23 @@ struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *, xfs_agnumber_t,
> int tag);
> 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); \
> + (pag) != NULL; \
> + (next_agno) = (pag)->pag_agno + 1, \
> + xfs_perag_put(pag), \
> + (pag) = xfs_perag_get((mp), (next_agno)))
> +
> +#define for_each_perag_tag(mp, next_agno, pag, tag) \
> + for ((next_agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \
> + (pag) != NULL; \
> + (next_agno) = (pag)->pag_agno + 1, \
> + xfs_perag_put(pag), \
> + (pag) = xfs_perag_get_tag((mp), (next_agno), (tag)))
> +
> struct aghdr_init_data {
> /* per ag data */
> xfs_agblock_t agno; /* ag to init */
> diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
> index 453ae9adf94c..fd7941e04ae1 100644
> --- a/fs/xfs/scrub/fscounters.c
> +++ b/fs/xfs/scrub/fscounters.c
> @@ -71,11 +71,11 @@ xchk_fscount_warmup(
> xfs_agnumber_t agno;
> int error = 0;
>
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - pag = xfs_perag_get(mp, agno);
> -
> + for_each_perag(mp, agno, pag) {
> + if (xchk_should_terminate(sc, &error))
> + break;
> if (pag->pagi_init && pag->pagf_init)
> - goto next_loop_perag;
> + continue;
>
> /* Lock both AG headers. */
> error = xfs_ialloc_read_agi(mp, sc->tp, agno, &agi_bp);
> @@ -89,21 +89,15 @@ xchk_fscount_warmup(
> * These are supposed to be initialized by the header read
> * function.
> */
> - error = -EFSCORRUPTED;
> - if (!pag->pagi_init || !pag->pagf_init)
> + if (!pag->pagi_init || !pag->pagf_init) {
> + error = -EFSCORRUPTED;
> break;
> + }
>
> xfs_buf_relse(agf_bp);
> agf_bp = NULL;
> xfs_buf_relse(agi_bp);
> agi_bp = NULL;
> -next_loop_perag:
> - xfs_perag_put(pag);
> - pag = NULL;
> - error = 0;
> -
> - if (xchk_should_terminate(sc, &error))
> - break;
> }
>
> if (agf_bp)
> @@ -196,13 +190,14 @@ xchk_fscount_aggregate_agcounts(
> fsc->ifree = 0;
> fsc->fdblocks = 0;
>
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - pag = xfs_perag_get(mp, agno);
> + for_each_perag(mp, agno, pag) {
> + if (xchk_should_terminate(sc, &error))
> + break;
>
> /* This somehow got unset since the warmup? */
> if (!pag->pagi_init || !pag->pagf_init) {
> - xfs_perag_put(pag);
> - return -EFSCORRUPTED;
> + error = -EFSCORRUPTED;
> + break;
> }
>
> /* Count all the inodes */
> @@ -216,10 +211,8 @@ xchk_fscount_aggregate_agcounts(
> fsc->fdblocks += pag->pagf_btreeblks;
> } else {
> error = xchk_fscount_btreeblks(sc, fsc, agno);
> - if (error) {
> - xfs_perag_put(pag);
> + if (error)
> break;
> - }
> }
>
> /*
> @@ -229,12 +222,9 @@ xchk_fscount_aggregate_agcounts(
> fsc->fdblocks -= pag->pag_meta_resv.ar_reserved;
> fsc->fdblocks -= pag->pag_rmapbt_resv.ar_orig_reserved;
>
> - xfs_perag_put(pag);
> -
> - if (xchk_should_terminate(sc, &error))
> - break;
> }
> -
> + if (pag)
> + xfs_perag_put(pag);
> if (error)
> return error;
>
> diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
> index cb037d7c72b2..422667e0668b 100644
> --- a/fs/xfs/xfs_extent_busy.c
> +++ b/fs/xfs/xfs_extent_busy.c
> @@ -605,12 +605,11 @@ void
> xfs_extent_busy_wait_all(
> struct xfs_mount *mp)
> {
> + struct xfs_perag *pag;
> DEFINE_WAIT (wait);
> xfs_agnumber_t agno;
>
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - struct xfs_perag *pag = xfs_perag_get(mp, agno);
> -
> + for_each_perag(mp, agno, pag) {
> do {
> prepare_to_wait(&pag->pagb_wait, &wait, TASK_KILLABLE);
> if (RB_EMPTY_ROOT(&pag->pagb_tree))
> @@ -618,8 +617,6 @@ xfs_extent_busy_wait_all(
> schedule();
> } while (1);
> finish_wait(&pag->pagb_wait, &wait);
> -
> - xfs_perag_put(pag);
> }
> }
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index be9cf88d2ad7..07c745cd483e 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -576,10 +576,8 @@ xfs_fs_reserve_ag_blocks(
> int err2;
>
> mp->m_finobt_nores = false;
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - pag = xfs_perag_get(mp, agno);
> + for_each_perag(mp, agno, pag) {
> err2 = xfs_ag_resv_init(pag, NULL);
> - xfs_perag_put(pag);
> if (err2 && !error)
> error = err2;
> }
> @@ -605,10 +603,8 @@ xfs_fs_unreserve_ag_blocks(
> int error = 0;
> int err2;
>
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - pag = xfs_perag_get(mp, agno);
> + for_each_perag(mp, agno, pag) {
> err2 = xfs_ag_resv_free(pag);
> - xfs_perag_put(pag);
> if (err2 && !error)
> error = err2;
> }
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index b79475ea3dbd..5de3195f6cb2 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -34,14 +34,12 @@ xfs_health_unmount(
> return;
>
> /* Measure AG corruption levels. */
> - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> - pag = xfs_perag_get(mp, agno);
> + for_each_perag(mp, agno, pag) {
> xfs_ag_measure_sickness(pag, &sick, &checked);
> if (sick) {
> trace_xfs_ag_unfixed_corruption(mp, agno, sick);
> warn = true;
> }
> - xfs_perag_put(pag);
> }
>
> /* Measure realtime volume corruption levels. */
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 588ea2bf88bb..7dad83a6f586 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1061,15 +1061,13 @@ xfs_reclaim_inodes_ag(
> int *nr_to_scan)
> {
> struct xfs_perag *pag;
> - xfs_agnumber_t ag = 0;
> + xfs_agnumber_t agno;
>
> - while ((pag = xfs_perag_get_tag(mp, ag, XFS_ICI_RECLAIM_TAG))) {
> + for_each_perag_tag(mp, agno, pag, XFS_ICI_RECLAIM_TAG) {
> unsigned long first_index = 0;
> int done = 0;
> int nr_found = 0;
>
> - ag = pag->pag_agno + 1;
> -
> first_index = READ_ONCE(pag->pag_ici_reclaim_cursor);
> do {
> struct xfs_inode *batch[XFS_LOOKUP_BATCH];
> @@ -1134,7 +1132,6 @@ xfs_reclaim_inodes_ag(
> if (done)
> first_index = 0;
> WRITE_ONCE(pag->pag_ici_reclaim_cursor, first_index);
> - xfs_perag_put(pag);
> }
> }
>
> @@ -1554,14 +1551,6 @@ xfs_inode_clear_cowblocks_tag(
> return xfs_blockgc_clear_iflag(ip, XFS_ICOWBLOCKS);
> }
>
> -#define for_each_perag_tag(mp, next_agno, pag, tag) \
> - for ((next_agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \
> - (pag) != NULL; \
> - (next_agno) = (pag)->pag_agno + 1, \
> - xfs_perag_put(pag), \
> - (pag) = xfs_perag_get_tag((mp), (next_agno), (tag)))
> -
> -
> /* Disable post-EOF and CoW block auto-reclamation. */
> void
> xfs_blockgc_stop(
> --
> 2.31.1
>
next prev parent reply other threads:[~2021-05-26 12:33 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-19 1:20 [PATCH 00/23] xfs: initial agnumber -> perag conversions for shrink Dave Chinner
2021-05-19 1:20 ` [PATCH 01/23] xfs: move xfs_perag_get/put to xfs_ag.[ch] Dave Chinner
2021-05-19 1:20 ` [PATCH 02/23] xfs: prepare for moving perag definitions and support to libxfs Dave Chinner
2021-05-26 12:33 ` Brian Foster
2021-05-27 22:02 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 03/23] xfs: move perag structure and setup to libxfs/xfs_ag.[ch] Dave Chinner
2021-05-26 12:33 ` Brian Foster
2021-05-27 22:07 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 04/23] xfs: make for_each_perag... a first class citizen Dave Chinner
2021-05-26 12:33 ` Brian Foster [this message]
2021-05-27 22:10 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 05/23] xfs: convert raw ag walks to use for_each_perag Dave Chinner
2021-05-26 12:33 ` Brian Foster
2021-05-27 22:12 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 06/23] xfs: convert xfs_iwalk to use perag references Dave Chinner
2021-05-27 22:16 ` Darrick J. Wong
2021-06-01 22:00 ` [PATCH 06/23 V2] " Dave Chinner
2021-06-01 23:32 ` Darrick J. Wong
2021-06-02 0:34 ` Dave Chinner
2021-05-19 1:20 ` [PATCH 07/23] xfs: convert secondary superblock walk to use perags Dave Chinner
2021-05-19 1:20 ` [PATCH 08/23] xfs: pass perags through to the busy extent code Dave Chinner
2021-05-19 1:20 ` [PATCH 09/23] xfs: push perags through the ag reservation callouts Dave Chinner
2021-05-26 12:34 ` Brian Foster
2021-05-19 1:20 ` [PATCH 10/23] xfs: pass perags around in fsmap data dev functions Dave Chinner
2021-05-27 22:19 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 11/23] xfs: add a perag to the btree cursor Dave Chinner
2021-05-26 12:34 ` Brian Foster
2021-05-19 1:20 ` [PATCH 12/23] xfs: convert rmap btree cursor to using a perag Dave Chinner
2021-05-19 1:20 ` [PATCH 13/23] xfs: convert refcount btree cursor to use perags Dave Chinner
2021-05-27 22:22 ` Darrick J. Wong
2021-05-19 1:20 ` [PATCH 14/23] xfs: convert allocbt cursors " Dave Chinner
2021-05-19 1:20 ` [PATCH 15/23] xfs: use perag for ialloc btree cursors Dave Chinner
2021-05-19 1:20 ` [PATCH 16/23] xfs: remove agno from btree cursor Dave Chinner
2021-05-19 1:20 ` [PATCH 17/23] xfs: simplify xfs_dialloc_select_ag() return values Dave Chinner
2021-05-19 1:20 ` [PATCH 18/23] xfs: collapse AG selection for inode allocation Dave Chinner
2021-05-27 11:19 ` Brian Foster
2021-06-01 22:40 ` [PATCH 18/23 V2] " Dave Chinner
2021-06-01 23:27 ` Darrick J. Wong
2021-05-27 22:33 ` [PATCH 18/23] " Darrick J. Wong
2021-05-19 1:20 ` [PATCH 19/23] xfs: get rid of xfs_dir_ialloc() Dave Chinner
2021-05-19 1:20 ` [PATCH 20/23] xfs: inode allocation can use a single perag instance Dave Chinner
2021-05-19 1:21 ` [PATCH 21/23] xfs: clean up and simplify xfs_dialloc() Dave Chinner
2021-05-27 11:20 ` Brian Foster
2021-05-27 22:39 ` Darrick J. Wong
2021-05-19 1:21 ` [PATCH 22/23] xfs: use perag through unlink processing Dave Chinner
2021-05-27 11:20 ` Brian Foster
2021-05-19 1:21 ` [PATCH 23/23] xfs: remove xfs_perag_t Dave Chinner
2021-05-27 11:20 ` Brian Foster
2021-05-27 22:06 ` 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=YK5AL/Zd3dKgzgT6@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