From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 05/50] xfs: pass perag to xfs_alloc_read_agf()
Date: Thu, 16 Jun 2022 00:38:23 -0700 [thread overview]
Message-ID: <Yqrd763BrdIO9UvQ@infradead.org> (raw)
In-Reply-To: <20220611012659.3418072-6-david@fromorbit.com>
On Sat, Jun 11, 2022 at 11:26:14AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> xfs_alloc_read_agf() initialises the perag if it hasn't been done
> yet, so it makes sense to pass it the perag rather than pull a
> reference from the buffer. This allows callers to be per-ag centric
> rather than passing mount/agno pairs everywhere.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_ag.c | 19 +++++++--------
> fs/xfs/libxfs/xfs_ag_resv.c | 2 +-
> fs/xfs/libxfs/xfs_alloc.c | 30 ++++++++++-------------
> fs/xfs/libxfs/xfs_alloc.h | 13 ++--------
> fs/xfs/libxfs/xfs_bmap.c | 2 +-
> fs/xfs/libxfs/xfs_ialloc.c | 2 +-
> fs/xfs/libxfs/xfs_refcount.c | 6 ++---
> fs/xfs/libxfs/xfs_refcount_btree.c | 2 +-
> fs/xfs/libxfs/xfs_rmap_btree.c | 2 +-
> fs/xfs/scrub/agheader_repair.c | 6 ++---
> fs/xfs/scrub/bmap.c | 2 +-
> fs/xfs/scrub/common.c | 2 +-
> fs/xfs/scrub/fscounters.c | 2 +-
> fs/xfs/scrub/repair.c | 5 ++--
> fs/xfs/xfs_discard.c | 2 +-
> fs/xfs/xfs_extfree_item.c | 6 ++++-
> fs/xfs/xfs_filestream.c | 2 +-
> fs/xfs/xfs_fsmap.c | 3 +--
> fs/xfs/xfs_reflink.c | 38 +++++++++++++++++-------------
> fs/xfs/xfs_reflink.h | 3 ---
> 20 files changed, 68 insertions(+), 81 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 734ef170936e..c1a1c9f414c3 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -120,16 +120,13 @@ xfs_initialize_perag_data(
>
> for (index = 0; index < agcount; index++) {
> /*
> - * read the agf, then the agi. This gets us
> - * all the information we need and populates the
> - * per-ag structures for us.
> + * Read the AGF and AGI buffers to populate the per-ag
> + * structures for us.
> */
> - error = xfs_alloc_read_agf(mp, NULL, index, 0, NULL);
> - if (error)
> - return error;
> -
> pag = xfs_perag_get(mp, index);
> - error = xfs_ialloc_read_agi(pag, NULL, NULL);
> + error = xfs_alloc_read_agf(pag, NULL, 0, NULL);
> + if (!error)
> + error = xfs_ialloc_read_agi(pag, NULL, NULL);
> if (error) {
> xfs_perag_put(pag);
> return error;
> @@ -792,7 +789,7 @@ xfs_ag_shrink_space(
>
> agi = agibp->b_addr;
>
> - error = xfs_alloc_read_agf(mp, *tpp, pag->pag_agno, 0, &agfbp);
> + error = xfs_alloc_read_agf(pag, *tpp, 0, &agfbp);
> if (error)
> return error;
>
> @@ -909,7 +906,7 @@ xfs_ag_extend_space(
> /*
> * Change agf length.
> */
> - error = xfs_alloc_read_agf(pag->pag_mount, tp, pag->pag_agno, 0, &bp);
> + error = xfs_alloc_read_agf(pag, tp, 0, &bp);
> if (error)
> return error;
>
> @@ -952,7 +949,7 @@ xfs_ag_get_geometry(
> error = xfs_ialloc_read_agi(pag, NULL, &agi_bp);
> if (error)
> return error;
> - error = xfs_alloc_read_agf(pag->pag_mount, NULL, pag->pag_agno, 0, &agf_bp);
> + error = xfs_alloc_read_agf(pag, NULL, 0, &agf_bp);
> if (error)
> goto out_agi;
>
> diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> index ce28bf8f72dc..5af123d13a63 100644
> --- a/fs/xfs/libxfs/xfs_ag_resv.c
> +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> @@ -322,7 +322,7 @@ xfs_ag_resv_init(
> * address.
> */
> if (has_resv) {
> - error2 = xfs_alloc_read_agf(mp, tp, pag->pag_agno, 0, NULL);
> + error2 = xfs_alloc_read_agf(pag, tp, 0, NULL);
> if (error2)
> return error2;
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index f7853ab7b962..5d6ca86c4882 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2609,7 +2609,7 @@ xfs_alloc_fix_freelist(
> ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
>
> if (!pag->pagf_init) {
> - error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
> + error = xfs_alloc_read_agf(pag, tp, flags, &agbp);
> if (error) {
> /* Couldn't lock the AGF so skip this AG. */
> if (error == -EAGAIN)
> @@ -2639,7 +2639,7 @@ xfs_alloc_fix_freelist(
> * Can fail if we're not blocking on locks, and it's held.
> */
> if (!agbp) {
> - error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
> + error = xfs_alloc_read_agf(pag, tp, flags, &agbp);
> if (error) {
> /* Couldn't lock the AGF so skip this AG. */
> if (error == -EAGAIN)
> @@ -3080,34 +3080,30 @@ xfs_read_agf(
> * perag structure if necessary. If the caller provides @agfbpp, then return the
> * locked buffer to the caller, otherwise free it.
> */
> -int /* error */
> +int
> xfs_alloc_read_agf(
> - struct xfs_mount *mp, /* mount point structure */
> - struct xfs_trans *tp, /* transaction pointer */
> - xfs_agnumber_t agno, /* allocation group number */
> - int flags, /* XFS_ALLOC_FLAG_... */
> + struct xfs_perag *pag,
> + struct xfs_trans *tp,
> + int flags,
> struct xfs_buf **agfbpp)
> {
> struct xfs_buf *agfbp;
> - struct xfs_agf *agf; /* ag freelist header */
> - struct xfs_perag *pag; /* per allocation group data */
> + struct xfs_agf *agf;
> int error;
> int allocbt_blks;
>
> - trace_xfs_alloc_read_agf(mp, agno);
> + trace_xfs_alloc_read_agf(pag->pag_mount, pag->pag_agno);
>
> /* We don't support trylock when freeing. */
> ASSERT((flags & (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK)) !=
> (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK));
> - ASSERT(agno != NULLAGNUMBER);
> - error = xfs_read_agf(mp, tp, agno,
> + error = xfs_read_agf(pag->pag_mount, tp, pag->pag_agno,
> (flags & XFS_ALLOC_FLAG_TRYLOCK) ? XBF_TRYLOCK : 0,
> &agfbp);
> if (error)
> return error;
>
> agf = agfbp->b_addr;
> - pag = agfbp->b_pag;
> if (!pag->pagf_init) {
> pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
> pag->pagf_btreeblks = be32_to_cpu(agf->agf_btreeblks);
> @@ -3121,7 +3117,7 @@ xfs_alloc_read_agf(
> be32_to_cpu(agf->agf_levels[XFS_BTNUM_RMAPi]);
> pag->pagf_refcount_level = be32_to_cpu(agf->agf_refcount_level);
> pag->pagf_init = 1;
> - pag->pagf_agflreset = xfs_agfl_needs_reset(mp, agf);
> + pag->pagf_agflreset = xfs_agfl_needs_reset(pag->pag_mount, agf);
>
> /*
> * Update the in-core allocbt counter. Filter out the rmapbt
> @@ -3131,13 +3127,13 @@ xfs_alloc_read_agf(
> * counter only tracks non-root blocks.
> */
> allocbt_blks = pag->pagf_btreeblks;
> - if (xfs_has_rmapbt(mp))
> + if (xfs_has_rmapbt(pag->pag_mount))
> allocbt_blks -= be32_to_cpu(agf->agf_rmap_blocks) - 1;
> if (allocbt_blks > 0)
> - atomic64_add(allocbt_blks, &mp->m_allocbt_blks);
> + atomic64_add(allocbt_blks, &pag->pag_mount->m_allocbt_blks);
Overly long line here. I think in general this function would benefit
from a local xfs_mount *mp variable anyway.
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index bea65f2fe657..65c5dfe17ecf 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -16,9 +16,6 @@ static inline bool xfs_is_cow_inode(struct xfs_inode *ip)
> return xfs_is_reflink_inode(ip) || xfs_is_always_cow_inode(ip);
> }
>
> -extern int xfs_reflink_find_shared(struct xfs_mount *mp, struct xfs_trans *tp,
> - xfs_agnumber_t agno, xfs_agblock_t agbno, xfs_extlen_t aglen,
> - xfs_agblock_t *fbno, xfs_extlen_t *flen, bool find_maximal);
Dropping this extern seems unrelated, and should move into a separate
patch together with actually marking it static.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
next prev parent reply other threads:[~2022-06-16 7:38 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-11 1:26 [RFC] [PATCH 00/50] xfs: per-ag centric allocation alogrithms Dave Chinner
2022-06-11 1:26 ` [PATCH 01/50] xfs: make last AG grow/shrink perag centric Dave Chinner
2022-06-16 7:30 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 02/50] xfs: kill xfs_ialloc_pagi_init() Dave Chinner
2022-06-16 7:32 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 03/50] xfs: pass perag to xfs_ialloc_read_agi() Dave Chinner
2022-06-16 7:34 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 04/50] xfs: kill xfs_alloc_pagf_init() Dave Chinner
2022-06-16 7:35 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 05/50] xfs: pass perag to xfs_alloc_read_agf() Dave Chinner
2022-06-11 2:37 ` kernel test robot
2022-06-11 12:04 ` kernel test robot
2022-06-11 13:46 ` kernel test robot
2022-06-14 12:17 ` kernel test robot
2022-06-16 7:38 ` Christoph Hellwig [this message]
2022-06-11 1:26 ` [PATCH 06/50] xfs: pass perag to xfs_read_agi Dave Chinner
2022-06-16 7:39 ` Christoph Hellwig
2022-06-16 7:39 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 07/50] xfs: pass perag to xfs_read_agf Dave Chinner
2022-06-16 7:40 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 08/50] xfs: pass perag to xfs_alloc_get_freelist Dave Chinner
2022-06-16 7:40 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 09/50] xfs: pass perag to xfs_alloc_put_freelist Dave Chinner
2022-06-16 7:40 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 10/50] xfs: pass perag to xfs_alloc_read_agfl Dave Chinner
2022-06-16 7:41 ` Christoph Hellwig
2022-06-11 1:26 ` [PATCH 11/50] xfs: Pre-calculate per-AG agbno geometry Dave Chinner
2022-06-11 1:26 ` [PATCH 12/50] xfs: Pre-calculate per-AG agino geometry Dave Chinner
2022-06-11 3:08 ` kernel test robot
2022-06-11 1:26 ` [PATCH 13/50] xfs: replace xfs_ag_block_count() with perag accesses Dave Chinner
2022-06-11 1:26 ` [PATCH 14/50] xfs: make is_log_ag() a first class helper Dave Chinner
2022-06-11 1:26 ` [PATCH 15/50] xfs: active perag reference counting Dave Chinner
2022-06-11 1:26 ` [PATCH 16/50] xfs: rework the perag trace points to be perag centric Dave Chinner
2022-06-11 1:26 ` [PATCH 17/50] xfs: convert xfs_imap() to take a perag Dave Chinner
2022-06-11 1:26 ` [PATCH 18/50] xfs: use active perag references for inode allocation Dave Chinner
2022-06-11 1:26 ` [PATCH 19/50] xfs: inobt can use perags in many more places than it does Dave Chinner
2022-06-11 1:26 ` [PATCH 20/50] xfs: convert xfs_ialloc_next_ag() to an atomic Dave Chinner
2022-06-11 1:26 ` [PATCH 21/50] xfs: perags need atomic operational state Dave Chinner
2022-06-11 1:26 ` [PATCH 22/50] xfs: introduce xfs_for_each_perag_wrap() Dave Chinner
2022-06-11 1:26 ` [PATCH 23/50] xfs: rework xfs_alloc_vextent() Dave Chinner
2022-06-11 1:26 ` [PATCH 24/50] xfs: use xfs_alloc_vextent_this_ag() in _iterate_ags() Dave Chinner
2022-06-11 1:26 ` [PATCH 25/50] xfs: combine __xfs_alloc_vextent_this_ag and xfs_alloc_ag_vextent Dave Chinner
2022-06-11 1:26 ` [PATCH 26/50] xfs: use xfs_alloc_vextent_this_ag() where appropriate Dave Chinner
2022-06-11 1:26 ` [PATCH 27/50] xfs: factor xfs_bmap_btalloc() Dave Chinner
2022-06-11 1:26 ` [PATCH 28/50] xfs: use xfs_alloc_vextent_first_ag() where appropriate Dave Chinner
2022-06-11 1:26 ` [PATCH 29/50] xfs: use xfs_alloc_vextent_start_bno() " Dave Chinner
2022-06-11 1:26 ` [PATCH 30/50] xfs: introduce xfs_alloc_vextent_near_bno() Dave Chinner
2022-06-11 1:26 ` [PATCH 31/50] xfs: introduce xfs_alloc_vextent_exact_bno() Dave Chinner
2022-06-11 1:26 ` [PATCH 32/50] xfs: introduce xfs_alloc_vextent_prepare() Dave Chinner
2022-06-11 1:26 ` [PATCH 33/50] xfs: move allocation accounting to xfs_alloc_vextent_set_fsbno() Dave Chinner
2022-06-11 1:26 ` [PATCH 34/50] xfs: fold xfs_alloc_ag_vextent() into callers Dave Chinner
2022-06-11 1:26 ` [PATCH 35/50] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker Dave Chinner
2022-06-11 1:26 ` [PATCH 36/50] xfs: convert trim to use for_each_perag_range Dave Chinner
2022-06-11 1:26 ` [PATCH 37/50] xfs: factor out filestreams from xfs_bmap_btalloc_nullfb Dave Chinner
2022-06-11 1:26 ` [PATCH 38/50] xfs: get rid of notinit from xfs_bmap_longest_free_extent Dave Chinner
2022-06-11 1:26 ` [PATCH 39/50] xfs: use xfs_bmap_longest_free_extent() in filestreams Dave Chinner
2022-06-11 1:26 ` [PATCH 40/50] xfs: move xfs_bmap_btalloc_filestreams() to xfs_filestreams.c Dave Chinner
2022-06-11 1:26 ` [PATCH 41/50] xfs: merge filestream AG lookup into xfs_filestream_select_ag() Dave Chinner
2022-06-11 1:26 ` [PATCH 42/50] xfs: merge new filestream AG selection " Dave Chinner
2022-06-11 1:26 ` [PATCH 43/50] xfs: remove xfs_filestream_select_ag() longest extent check Dave Chinner
2022-06-11 1:26 ` [PATCH 44/50] xfs: factor out MRU hit case in xfs_filestream_select_ag Dave Chinner
2022-06-11 1:26 ` [PATCH 45/50] xfs: track an active perag reference in filestreams Dave Chinner
2022-06-11 1:26 ` [PATCH 46/50] xfs: use for_each_perag_wrap in xfs_filestream_pick_ag Dave Chinner
2022-06-11 1:26 ` [PATCH 47/50] xfs: pass perag to filestreams tracing Dave Chinner
2022-06-11 1:26 ` [PATCH 48/50] xfs: return a referenced perag from filestreams allocator Dave Chinner
2022-06-11 1:26 ` [PATCH 49/50] xfs: refactor the filestreams allocator pick functions Dave Chinner
2022-06-11 1:26 ` [PATCH 50/50] xfs: fix low space alloc deadlock Dave Chinner
2022-06-16 12:01 ` [RFC] [PATCH 00/50] xfs: per-ag centric allocation alogrithms Christoph Hellwig
2022-06-21 2:08 ` Dave Chinner
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=Yqrd763BrdIO9UvQ@infradead.org \
--to=hch@infradead.org \
--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