From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/5] xfs: store inode btree block counts in AGI header
Date: Wed, 2 Sep 2020 09:23:36 -0400 [thread overview]
Message-ID: <20200902132336.GA289426@bfoster> (raw)
In-Reply-To: <159901535858.547164.11928856896363415325.stgit@magnolia>
On Tue, Sep 01, 2020 at 07:55:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Add a btree block usage counters for both inode btrees to the AGI header
> so that we don't have to walk the entire finobt at mount time to create
> the per-AG reservations.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
A couple nits..
> fs/xfs/libxfs/xfs_ag.c | 5 +++++
> fs/xfs/libxfs/xfs_format.h | 18 +++++++++++++++++-
> fs/xfs/libxfs/xfs_ialloc.c | 1 +
> fs/xfs/libxfs/xfs_ialloc_btree.c | 24 ++++++++++++++++++++++++
> fs/xfs/xfs_ondisk.h | 2 +-
> fs/xfs/xfs_super.c | 4 ++++
> 6 files changed, 52 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 8cf73fe4338e..9331f3516afa 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -333,6 +333,11 @@ xfs_agiblock_init(
> }
> for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++)
> agi->agi_unlinked[bucket] = cpu_to_be32(NULLAGINO);
> + if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) {
> + agi->agi_iblocks = cpu_to_be32(1);
> + if (xfs_sb_version_hasfinobt(&mp->m_sb))
> + agi->agi_fblocks = cpu_to_be32(1);
> + }
> }
>
> typedef void (*aghdr_init_work_f)(struct xfs_mount *mp, struct xfs_buf *bp,
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 31b7ece985bb..03cbedb7eafc 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -449,6 +449,7 @@ xfs_sb_has_compat_feature(
> #define XFS_SB_FEAT_RO_COMPAT_FINOBT (1 << 0) /* free inode btree */
> #define XFS_SB_FEAT_RO_COMPAT_RMAPBT (1 << 1) /* reverse map btree */
> #define XFS_SB_FEAT_RO_COMPAT_REFLINK (1 << 2) /* reflinked files */
> +#define XFS_SB_FEAT_RO_COMPAT_INOBTCNT (1 << 3) /* inobt block counts */
> #define XFS_SB_FEAT_RO_COMPAT_ALL \
> (XFS_SB_FEAT_RO_COMPAT_FINOBT | \
> XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
> @@ -563,6 +564,17 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
> (sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
> }
>
> +/*
> + * Inode btree block counter. We record the number of inobt and finobt blocks
> + * in the AGI header so that we can skip the finobt walk at mount time when
> + * setting up per-AG reservations.
> + */
> +static inline bool xfs_sb_version_hasinobtcounts(struct xfs_sb *sbp)
> +{
> + return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
> + (sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_INOBTCNT);
> +}
> +
> /*
> * end of superblock version macros
> */
> @@ -765,6 +777,9 @@ typedef struct xfs_agi {
> __be32 agi_free_root; /* root of the free inode btree */
> __be32 agi_free_level;/* levels in free inode btree */
>
> + __be32 agi_iblocks; /* inobt blocks used */
> + __be32 agi_fblocks; /* finobt blocks used */
> +
> /* structure must be padded to 64 bit alignment */
> } xfs_agi_t;
>
> @@ -785,7 +800,8 @@ typedef struct xfs_agi {
> #define XFS_AGI_ALL_BITS_R1 ((1 << XFS_AGI_NUM_BITS_R1) - 1)
> #define XFS_AGI_FREE_ROOT (1 << 11)
> #define XFS_AGI_FREE_LEVEL (1 << 12)
> -#define XFS_AGI_NUM_BITS_R2 13
> +#define XFS_AGI_IBLOCKS (1 << 13) /* both inobt/finobt block counters */
> +#define XFS_AGI_NUM_BITS_R2 14
I still find it a little odd that we'd log both fields if only one might
be supported/modified, as opposed to just tracking them both
independently with a couple extra lines of code. That said, I don't see
it as a functional problem that couldn't be fixed later.
>
> /* disk block (xfs_daddr_t) in the AG */
> #define XFS_AGI_DADDR(mp) ((xfs_daddr_t)(2 << (mp)->m_sectbb_log))
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index f742a96a2fe1..fef1d94c60a4 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2473,6 +2473,7 @@ xfs_ialloc_log_agi(
> offsetof(xfs_agi_t, agi_unlinked),
> offsetof(xfs_agi_t, agi_free_root),
> offsetof(xfs_agi_t, agi_free_level),
> + offsetof(xfs_agi_t, agi_iblocks),
> sizeof(xfs_agi_t)
> };
> #ifdef DEBUG
> diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
> index 3c8aebc36e64..cf51b342b6ef 100644
> --- a/fs/xfs/libxfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
> @@ -67,6 +67,28 @@ xfs_finobt_set_root(
> XFS_AGI_FREE_ROOT | XFS_AGI_FREE_LEVEL);
> }
>
> +/* Update the inode btree block counter for this btree. */
> +static inline void
> +xfs_inobt_mod_blockcount(
> + struct xfs_btree_cur *cur,
> + int howmuch)
> +{
> + struct xfs_buf *agbp = cur->bc_ag.agbp;
> + struct xfs_agi *agi = agbp->b_addr;
> +
> + if (!xfs_sb_version_hasinobtcounts(&cur->bc_mp->m_sb))
> + return;
> +
> + if (cur->bc_btnum == XFS_BTNUM_FINO &&
> + xfs_sb_version_hasfinobt(&cur->bc_mp->m_sb)) {
This check might be spurious because I suspect you wouldn't get a finobt
cursor without the feature enabled. Those nits aside:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> + be32_add_cpu(&agi->agi_fblocks, howmuch);
> + xfs_ialloc_log_agi(cur->bc_tp, agbp, XFS_AGI_IBLOCKS);
> + } else if (cur->bc_btnum == XFS_BTNUM_INO) {
> + be32_add_cpu(&agi->agi_iblocks, howmuch);
> + xfs_ialloc_log_agi(cur->bc_tp, agbp, XFS_AGI_IBLOCKS);
> + }
> +}
> +
> STATIC int
> __xfs_inobt_alloc_block(
> struct xfs_btree_cur *cur,
> @@ -102,6 +124,7 @@ __xfs_inobt_alloc_block(
>
> new->s = cpu_to_be32(XFS_FSB_TO_AGBNO(args.mp, args.fsbno));
> *stat = 1;
> + xfs_inobt_mod_blockcount(cur, 1);
> return 0;
> }
>
> @@ -134,6 +157,7 @@ __xfs_inobt_free_block(
> struct xfs_buf *bp,
> enum xfs_ag_resv_type resv)
> {
> + xfs_inobt_mod_blockcount(cur, -1);
> return xfs_free_extent(cur->bc_tp,
> XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1,
> &XFS_RMAP_OINFO_INOBT, resv);
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 5f04d8a5ab2a..acb9b737fe6b 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -23,7 +23,7 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry, 12);
> XFS_CHECK_STRUCT_SIZE(struct xfs_agf, 224);
> XFS_CHECK_STRUCT_SIZE(struct xfs_agfl, 36);
> - XFS_CHECK_STRUCT_SIZE(struct xfs_agi, 336);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_agi, 344);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_key, 8);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_rec, 16);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmdr_block, 4);
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 71ac6c1cdc36..c7ffcb57b586 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1549,6 +1549,10 @@ xfs_fc_fill_super(
> goto out_filestream_unmount;
> }
>
> + if (xfs_sb_version_hasinobtcounts(&mp->m_sb))
> + xfs_warn(mp,
> + "EXPERIMENTAL inode btree counters feature in use. Use at your own risk!");
> +
> error = xfs_mountfs(mp);
> if (error)
> goto out_filestream_unmount;
>
next prev parent reply other threads:[~2020-09-02 15:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-02 2:55 [PATCH v4 0/5] xfs: add inode btree blocks counters to the AGI header Darrick J. Wong
2020-09-02 2:55 ` [PATCH 1/5] xfs: store inode btree block counts in " Darrick J. Wong
2020-09-02 13:23 ` Brian Foster [this message]
2020-09-02 17:13 ` Darrick J. Wong
2020-09-02 2:56 ` [PATCH 2/5] xfs: use the finobt block counts to speed up mount times Darrick J. Wong
2020-09-02 2:56 ` [PATCH 3/5] xfs: support inode btree blockcounts in online scrub Darrick J. Wong
2020-09-02 2:56 ` [PATCH 4/5] xfs: support inode btree blockcounts in online repair Darrick J. Wong
2020-09-02 13:23 ` Brian Foster
2020-09-02 2:56 ` [PATCH 5/5] xfs: enable new inode btree counters feature Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2020-08-28 2:36 [PATCH v3 0/5] xfs: add inode btree blocks counters to the AGI header Darrick J. Wong
2020-08-28 2:36 ` [PATCH 1/5] xfs: store inode btree block counts in " Darrick J. Wong
2020-08-31 19:06 ` Brian Foster
2020-08-31 19:40 ` 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=20200902132336.GA289426@bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.