From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/5] xfs: add a new xfs_sb_version_has_v3inode helper
Date: Wed, 18 Mar 2020 08:33:43 -0700 [thread overview]
Message-ID: <20200318153343.GB256767@magnolia> (raw)
In-Reply-To: <20200317185756.1063268-2-hch@lst.de>
On Tue, Mar 17, 2020 at 07:57:52PM +0100, Christoph Hellwig wrote:
> Add a new wrapper to check if a file system supports the v3 inode format
> with a larger dinode core. Previously we used xfs_sb_version_hascrc for
> that, which is technically correct but a little confusing to read.
>
> Also move xfs_dinode_good_version next to xfs_sb_version_has_v3inode
> so that we have one place that documents the superblock version to
> inode version relationship.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_format.h | 17 +++++++++++++++++
> fs/xfs/libxfs/xfs_ialloc.c | 4 ++--
> fs/xfs/libxfs/xfs_inode_buf.c | 17 +++--------------
> fs/xfs/libxfs/xfs_inode_buf.h | 2 --
> fs/xfs/libxfs/xfs_trans_resv.c | 2 +-
> fs/xfs/xfs_buf_item.c | 2 +-
> fs/xfs/xfs_log_recover.c | 2 +-
> 7 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index cd814f99da28..19899d48517c 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -497,6 +497,23 @@ static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
> return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
> }
>
> +/*
> + * v5 file systems support V3 inodes only, earlier file systems support
> + * v2 and v1 inodes.
> + */
> +static inline bool xfs_sb_version_has_v3inode(struct xfs_sb *sbp)
> +{
> + return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
> +}
> +
> +static inline bool xfs_dinode_good_version(struct xfs_sb *sbp,
> + uint8_t version)
> +{
> + if (xfs_sb_version_has_v3inode(sbp))
> + return version == 3;
> + return version == 1 || version == 2;
> +}
> +
> static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
> {
> return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 21ac3fb52f4e..4de61af3b840 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -304,7 +304,7 @@ xfs_ialloc_inode_init(
> * That means for v3 inode we log the entire buffer rather than just the
> * inode cores.
> */
> - if (xfs_sb_version_hascrc(&mp->m_sb)) {
> + if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> version = 3;
> ino = XFS_AGINO_TO_INO(mp, agno, XFS_AGB_TO_AGINO(mp, agbno));
>
> @@ -2872,7 +2872,7 @@ xfs_ialloc_setup_geometry(
> * cannot change the behavior.
> */
> igeo->inode_cluster_size_raw = XFS_INODE_BIG_CLUSTER_SIZE;
> - if (xfs_sb_version_hascrc(&mp->m_sb)) {
> + if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> int new_size = igeo->inode_cluster_size_raw;
>
> new_size *= mp->m_sb.sb_inodesize / XFS_DINODE_MIN_SIZE;
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 17e88a8c8353..c862c8f1aaa9 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -44,17 +44,6 @@ xfs_inobp_check(
> }
> #endif
>
> -bool
> -xfs_dinode_good_version(
> - struct xfs_mount *mp,
> - __u8 version)
> -{
> - if (xfs_sb_version_hascrc(&mp->m_sb))
> - return version == 3;
> -
> - return version == 1 || version == 2;
> -}
> -
> /*
> * If we are doing readahead on an inode buffer, we might be in log recovery
> * reading an inode allocation buffer that hasn't yet been replayed, and hence
> @@ -93,7 +82,7 @@ xfs_inode_buf_verify(
> dip = xfs_buf_offset(bp, (i << mp->m_sb.sb_inodelog));
> unlinked_ino = be32_to_cpu(dip->di_next_unlinked);
> di_ok = xfs_verify_magic16(bp, dip->di_magic) &&
> - xfs_dinode_good_version(mp, dip->di_version) &&
> + xfs_dinode_good_version(&mp->m_sb, dip->di_version) &&
> xfs_verify_agino_or_null(mp, agno, unlinked_ino);
> if (unlikely(XFS_TEST_ERROR(!di_ok, mp,
> XFS_ERRTAG_ITOBP_INOTOBP))) {
> @@ -454,7 +443,7 @@ xfs_dinode_verify(
>
> /* Verify v3 integrity information first */
> if (dip->di_version >= 3) {
> - if (!xfs_sb_version_hascrc(&mp->m_sb))
> + if (!xfs_sb_version_has_v3inode(&mp->m_sb))
> return __this_address;
> if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
> XFS_DINODE_CRC_OFF))
> @@ -629,7 +618,7 @@ xfs_iread(
>
> /* shortcut IO on inode allocation if possible */
> if ((iget_flags & XFS_IGET_CREATE) &&
> - xfs_sb_version_hascrc(&mp->m_sb) &&
> + xfs_sb_version_has_v3inode(&mp->m_sb) &&
> !(mp->m_flags & XFS_MOUNT_IKEEP)) {
> VFS_I(ip)->i_generation = prandom_u32();
> ip->i_d.di_version = 3;
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 2683e1e2c4a6..66de5964045c 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -59,8 +59,6 @@ void xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
> void xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
> struct xfs_dinode *to);
>
> -bool xfs_dinode_good_version(struct xfs_mount *mp, __u8 version);
> -
> #if defined(DEBUG)
> void xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
> #else
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 7a9c04920505..d1a0848cb52e 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -187,7 +187,7 @@ xfs_calc_inode_chunk_res(
> XFS_FSB_TO_B(mp, 1));
> if (alloc) {
> /* icreate tx uses ordered buffers */
> - if (xfs_sb_version_hascrc(&mp->m_sb))
> + if (xfs_sb_version_has_v3inode(&mp->m_sb))
> return res;
> size = XFS_FSB_TO_B(mp, 1);
> }
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 663810e6cd59..1545657c3ca0 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -345,7 +345,7 @@ xfs_buf_item_format(
> * occurs during recovery.
> */
> if (bip->bli_flags & XFS_BLI_INODE_BUF) {
> - if (xfs_sb_version_hascrc(&lip->li_mountp->m_sb) ||
> + if (xfs_sb_version_has_v3inode(&lip->li_mountp->m_sb) ||
> !((bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF) &&
> xfs_log_item_in_current_chkpt(lip)))
> bip->__bli_format.blf_flags |= XFS_BLF_INODE_BUF;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 6abc0863c9c3..c467488212c2 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2997,7 +2997,7 @@ xlog_recover_inode_pass2(
> * superblock flag to determine whether we need to look at di_flushiter
> * to skip replay when the on disk inode is newer than the log one
> */
> - if (!xfs_sb_version_hascrc(&mp->m_sb) &&
> + if (!xfs_sb_version_has_v3inode(&mp->m_sb) &&
> ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
> /*
> * Deal with the wrap case, DI_MAX_FLUSH is less
> --
> 2.24.1
>
next prev parent reply other threads:[~2020-03-18 15:33 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-17 18:57 remove the di_version icdinode field v3 Christoph Hellwig
2020-03-17 18:57 ` [PATCH 1/5] xfs: add a new xfs_sb_version_has_v3inode helper Christoph Hellwig
2020-03-18 9:59 ` Chandan Rajendra
2020-03-18 15:33 ` Darrick J. Wong [this message]
2020-03-18 16:17 ` Brian Foster
2020-03-17 18:57 ` [PATCH 2/5] xfs: only check the superblock version for dinode size calculation Christoph Hellwig
2020-03-18 15:32 ` Darrick J. Wong
2020-03-17 18:57 ` [PATCH 3/5] xfs: simplify di_flags2 inheritance in xfs_ialloc Christoph Hellwig
2020-03-18 15:31 ` Darrick J. Wong
2020-03-17 18:57 ` [PATCH 4/5] xfs: simplify a check in xfs_ioctl_setattr_check_cowextsize Christoph Hellwig
2020-03-18 15:30 ` Darrick J. Wong
2020-03-17 18:57 ` [PATCH 5/5] xfs: remove the di_version field from struct icdinode Christoph Hellwig
2020-03-18 15:29 ` 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=20200318153343.GB256767@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@lst.de \
--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.