From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 10/22] xfs: pass perags around in fsmap data dev functions
Date: Tue, 11 May 2021 08:30:04 -0400 [thread overview]
Message-ID: <YJp4zPrmNXLdvIf4@bfoster> (raw)
In-Reply-To: <20210506072054.271157-11-david@fromorbit.com>
On Thu, May 06, 2021 at 05:20:42PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Needs a [from, to] ranged AG walk, and the perag to be stuffed into
> the info structure for callouts to use.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_ag.h | 14 ++++++--
> fs/xfs/xfs_fsmap.c | 75 ++++++++++++++++++++++++++----------------
> 2 files changed, 58 insertions(+), 31 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 3fa88222dacd..bebbe1bfce27 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -116,14 +116,24 @@ void xfs_perag_put(struct xfs_perag *pag);
>
> /*
> * Perag iteration APIs
> + *
> + * XXX: for_each_perag_range() usage really needs an iterator to clean up when
> + * we terminate at end_agno because we may have taken a reference to the perag
> + * beyond end_agno. RIght now callers have to be careful to catch and clean that
> + * up themselves. This is not necessary for the callers of for_each_perag() and
> + * for_each_perag_from() because they terminate at sb_agcount where there are
> + * no perag structures in tree beyond end_agno.
> */
> -#define for_each_perag_from(mp, next_agno, pag) \
> +#define for_each_perag_range(mp, next_agno, end_agno, pag) \
> for ((pag) = xfs_perag_get((mp), (next_agno)); \
> - (pag) != NULL; \
> + (pag) != NULL && (next_agno) <= (end_agno); \
> (next_agno) = (pag)->pag_agno + 1, \
> xfs_perag_put(pag), \
> (pag) = xfs_perag_get((mp), (next_agno)))
>
> +#define for_each_perag_from(mp, next_agno, pag) \
> + for_each_perag_range((mp), (next_agno), (mp)->m_sb.sb_agcount, (pag))
> +
It definitely would be nice to have some kind of abstraction for the
proper setup/teardown of these iterators, as described in the earlier
patch. Otherwise this looks fine to me for now:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> #define for_each_perag(mp, next_agno, pag) \
> (next_agno) = 0; \
> for_each_perag_from((mp), (next_agno), (pag))
> diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> index 34f2b971ce43..835dd6e3819b 100644
> --- a/fs/xfs/xfs_fsmap.c
> +++ b/fs/xfs/xfs_fsmap.c
> @@ -24,6 +24,7 @@
> #include "xfs_refcount_btree.h"
> #include "xfs_alloc_btree.h"
> #include "xfs_rtalloc.h"
> +#include "xfs_ag.h"
>
> /* Convert an xfs_fsmap to an fsmap. */
> static void
> @@ -157,10 +158,10 @@ struct xfs_getfsmap_info {
> struct xfs_fsmap_head *head;
> struct fsmap *fsmap_recs; /* mapping records */
> struct xfs_buf *agf_bp; /* AGF, for refcount queries */
> + struct xfs_perag *pag; /* AG info, if applicable */
> xfs_daddr_t next_daddr; /* next daddr we expect */
> u64 missing_owner; /* owner of holes */
> u32 dev; /* device id */
> - xfs_agnumber_t agno; /* AG number, if applicable */
> struct xfs_rmap_irec low; /* low rmap key */
> struct xfs_rmap_irec high; /* high rmap key */
> bool last; /* last extent? */
> @@ -203,14 +204,14 @@ xfs_getfsmap_is_shared(
> *stat = false;
> if (!xfs_sb_version_hasreflink(&mp->m_sb))
> return 0;
> - /* rt files will have agno set to NULLAGNUMBER */
> - if (info->agno == NULLAGNUMBER)
> + /* rt files will have no perag structure */
> + if (!info->pag)
> return 0;
>
> /* Are there any shared blocks here? */
> flen = 0;
> cur = xfs_refcountbt_init_cursor(mp, tp, info->agf_bp,
> - info->agno);
> + info->pag->pag_agno);
>
> error = xfs_refcount_find_shared(cur, rec->rm_startblock,
> rec->rm_blockcount, &fbno, &flen, false);
> @@ -311,7 +312,8 @@ xfs_getfsmap_helper(
> if (info->head->fmh_entries >= info->head->fmh_count)
> return -ECANCELED;
>
> - trace_xfs_fsmap_mapping(mp, info->dev, info->agno, rec);
> + trace_xfs_fsmap_mapping(mp, info->dev,
> + info->pag ? info->pag->pag_agno : NULLAGNUMBER, rec);
>
> fmr.fmr_device = info->dev;
> fmr.fmr_physical = rec_daddr;
> @@ -429,8 +431,8 @@ xfs_getfsmap_logdev(
> info->high.rm_flags = XFS_RMAP_KEY_FLAGS | XFS_RMAP_REC_FLAGS;
> info->missing_owner = XFS_FMR_OWN_FREE;
>
> - trace_xfs_fsmap_low_key(mp, info->dev, info->agno, &info->low);
> - trace_xfs_fsmap_high_key(mp, info->dev, info->agno, &info->high);
> + trace_xfs_fsmap_low_key(mp, info->dev, NULLAGNUMBER, &info->low);
> + trace_xfs_fsmap_high_key(mp, info->dev, NULLAGNUMBER, &info->high);
>
> if (keys[0].fmr_physical > 0)
> return 0;
> @@ -508,8 +510,8 @@ __xfs_getfsmap_rtdev(
> info->high.rm_blockcount = 0;
> xfs_getfsmap_set_irec_flags(&info->high, &keys[1]);
>
> - trace_xfs_fsmap_low_key(mp, info->dev, info->agno, &info->low);
> - trace_xfs_fsmap_high_key(mp, info->dev, info->agno, &info->high);
> + trace_xfs_fsmap_low_key(mp, info->dev, NULLAGNUMBER, &info->low);
> + trace_xfs_fsmap_high_key(mp, info->dev, NULLAGNUMBER, &info->high);
>
> return query_fn(tp, info);
> }
> @@ -572,6 +574,7 @@ __xfs_getfsmap_datadev(
> void *priv)
> {
> struct xfs_mount *mp = tp->t_mountp;
> + struct xfs_perag *pag;
> struct xfs_btree_cur *bt_cur = NULL;
> xfs_fsblock_t start_fsb;
> xfs_fsblock_t end_fsb;
> @@ -610,20 +613,20 @@ __xfs_getfsmap_datadev(
> start_ag = XFS_FSB_TO_AGNO(mp, start_fsb);
> end_ag = XFS_FSB_TO_AGNO(mp, end_fsb);
>
> - /* Query each AG */
> - for (info->agno = start_ag; info->agno <= end_ag; info->agno++) {
> + for_each_perag_range(mp, start_ag, end_ag, pag) {
> /*
> * Set the AG high key from the fsmap high key if this
> * is the last AG that we're querying.
> */
> - if (info->agno == end_ag) {
> + info->pag = pag;
> + if (pag->pag_agno == end_ag) {
> info->high.rm_startblock = XFS_FSB_TO_AGBNO(mp,
> end_fsb);
> info->high.rm_offset = XFS_BB_TO_FSBT(mp,
> keys[1].fmr_offset);
> error = xfs_fsmap_owner_to_rmap(&info->high, &keys[1]);
> if (error)
> - goto err;
> + break;
> xfs_getfsmap_set_irec_flags(&info->high, &keys[1]);
> }
>
> @@ -634,38 +637,45 @@ __xfs_getfsmap_datadev(
> info->agf_bp = NULL;
> }
>
> - error = xfs_alloc_read_agf(mp, tp, info->agno, 0,
> + error = xfs_alloc_read_agf(mp, tp, pag->pag_agno, 0,
> &info->agf_bp);
> if (error)
> - goto err;
> + break;
>
> - trace_xfs_fsmap_low_key(mp, info->dev, info->agno, &info->low);
> - trace_xfs_fsmap_high_key(mp, info->dev, info->agno,
> + trace_xfs_fsmap_low_key(mp, info->dev, pag->pag_agno,
> + &info->low);
> + trace_xfs_fsmap_high_key(mp, info->dev, pag->pag_agno,
> &info->high);
>
> error = query_fn(tp, info, &bt_cur, priv);
> if (error)
> - goto err;
> + break;
>
> /*
> * Set the AG low key to the start of the AG prior to
> * moving on to the next AG.
> */
> - if (info->agno == start_ag) {
> + if (pag->pag_agno == start_ag) {
> info->low.rm_startblock = 0;
> info->low.rm_owner = 0;
> info->low.rm_offset = 0;
> info->low.rm_flags = 0;
> }
> - }
>
> - /* Report any gap at the end of the AG */
> - info->last = true;
> - error = query_fn(tp, info, &bt_cur, priv);
> - if (error)
> - goto err;
> + /*
> + * If this is the last AG, report any gap at the end of it
> + * before we drop the reference to the perag when the loop
> + * terminates.
> + */
> + if (pag->pag_agno == end_ag) {
> + info->last = true;
> + error = query_fn(tp, info, &bt_cur, priv);
> + if (error)
> + break;
> + }
> + info->pag = NULL;
> + }
>
> -err:
> if (bt_cur)
> xfs_btree_del_cursor(bt_cur, error < 0 ? XFS_BTREE_ERROR :
> XFS_BTREE_NOERROR);
> @@ -673,6 +683,13 @@ __xfs_getfsmap_datadev(
> xfs_trans_brelse(tp, info->agf_bp);
> info->agf_bp = NULL;
> }
> + if (info->pag) {
> + xfs_perag_put(info->pag);
> + info->pag = NULL;
> + } else if (pag) {
> + /* loop termination case */
> + xfs_perag_put(pag);
> + }
>
> return error;
> }
> @@ -691,7 +708,7 @@ xfs_getfsmap_datadev_rmapbt_query(
>
> /* Allocate cursor for this AG and query_range it. */
> *curpp = xfs_rmapbt_init_cursor(tp->t_mountp, tp, info->agf_bp,
> - info->agno);
> + info->pag->pag_agno);
> return xfs_rmap_query_range(*curpp, &info->low, &info->high,
> xfs_getfsmap_datadev_helper, info);
> }
> @@ -724,7 +741,7 @@ xfs_getfsmap_datadev_bnobt_query(
>
> /* Allocate cursor for this AG and query_range it. */
> *curpp = xfs_allocbt_init_cursor(tp->t_mountp, tp, info->agf_bp,
> - info->agno, XFS_BTNUM_BNO);
> + info->pag->pag_agno, XFS_BTNUM_BNO);
> key->ar_startblock = info->low.rm_startblock;
> key[1].ar_startblock = info->high.rm_startblock;
> return xfs_alloc_query_range(*curpp, key, &key[1],
> @@ -937,7 +954,7 @@ xfs_getfsmap(
>
> info.dev = handlers[i].dev;
> info.last = false;
> - info.agno = NULLAGNUMBER;
> + info.pag = NULL;
> error = handlers[i].fn(tp, dkeys, &info);
> if (error)
> break;
> --
> 2.31.1
>
next prev parent reply other threads:[~2021-05-11 12:30 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
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 [this message]
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=YJp4zPrmNXLdvIf4@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