From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/3] xfs: move various type verifiers to common file
Date: Thu, 7 Jun 2018 07:41:47 -0400 [thread overview]
Message-ID: <20180607114146.GC7798@bfoster> (raw)
In-Reply-To: <20180607052751.6541-2-david@fromorbit.com>
On Thu, Jun 07, 2018 at 03:27:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> New verification functions like xfs_verify_fsbno() and
> xfs_verify_agino() are spread across multiple files and different
> header files. They really don't fit cleanly into the places they've
> been put, and have wider scope than the current header includes.
>
> Move the type verifiers to a new file in libxfs (xfs-types.c) and
> the prototypes to xfs_types.h where they will be visible to all the
> code that uses the types.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/Makefile | 1 +
> fs/xfs/libxfs/xfs_alloc.c | 49 ----------
> fs/xfs/libxfs/xfs_alloc.h | 4 -
> fs/xfs/libxfs/xfs_ialloc.c | 90 ------------------
> fs/xfs/libxfs/xfs_ialloc.h | 7 --
> fs/xfs/libxfs/xfs_rtbitmap.c | 12 ---
> fs/xfs/libxfs/xfs_types.c | 173 +++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_types.h | 19 ++++
> fs/xfs/scrub/agheader.c | 2 +-
> 9 files changed, 194 insertions(+), 163 deletions(-)
> create mode 100644 fs/xfs/libxfs/xfs_types.c
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 19a1f8064d8a..5333f7cbc95a 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -50,6 +50,7 @@ xfs-y += $(addprefix libxfs/, \
> xfs_sb.o \
> xfs_symlink_remote.o \
> xfs_trans_resv.o \
> + xfs_types.o \
> )
> # xfs_rtbitmap is shared with libxfs
> xfs-$(CONFIG_XFS_RT) += $(addprefix libxfs/, \
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 1db50cfc0212..eef466260d43 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3123,55 +3123,6 @@ xfs_alloc_query_all(
> return xfs_btree_query_all(cur, xfs_alloc_query_range_helper, &query);
> }
>
> -/* Find the size of the AG, in blocks. */
> -xfs_agblock_t
> -xfs_ag_block_count(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno)
> -{
> - ASSERT(agno < mp->m_sb.sb_agcount);
> -
> - if (agno < mp->m_sb.sb_agcount - 1)
> - return mp->m_sb.sb_agblocks;
> - return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> -}
> -
> -/*
> - * Verify that an AG block number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agbno(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agblock_t agbno)
> -{
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> - if (agbno >= eoag)
> - return false;
> - if (agbno <= XFS_AGFL_BLOCK(mp))
> - return false;
> - return true;
> -}
> -
> -/*
> - * Verify that an FS block number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_fsbno(
> - struct xfs_mount *mp,
> - xfs_fsblock_t fsbno)
> -{
> - xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> -}
> -
> /* Is there a record covering a given extent? */
> int
> xfs_alloc_has_record(
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1651d924aaf1..e716c993ac4c 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -242,10 +242,6 @@ int xfs_alloc_query_range(struct xfs_btree_cur *cur,
> xfs_alloc_query_range_fn fn, void *priv);
> int xfs_alloc_query_all(struct xfs_btree_cur *cur, xfs_alloc_query_range_fn fn,
> void *priv);
> -xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> -bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agblock_t agbno);
> -bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
>
> int xfs_alloc_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
> xfs_extlen_t len, bool *exist);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 3f551eb29157..8ec39dad62d7 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2674,96 +2674,6 @@ xfs_ialloc_pagi_init(
> return 0;
> }
>
> -/* Calculate the first and last possible inode number in an AG. */
> -void
> -xfs_ialloc_agino_range(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t *first,
> - xfs_agino_t *last)
> -{
> - xfs_agblock_t bno;
> - xfs_agblock_t eoag;
> -
> - eoag = xfs_ag_block_count(mp, agno);
> -
> - /*
> - * Calculate the first inode, which will be in the first
> - * cluster-aligned block after the AGFL.
> - */
> - bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> - xfs_ialloc_cluster_alignment(mp));
> - *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> -
> - /*
> - * Calculate the last inode, which will be at the end of the
> - * last (aligned) cluster that can be allocated in the AG.
> - */
> - bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> - *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> -}
> -
> -/*
> - * Verify that an AG inode number pointer neither points outside the AG
> - * nor points at static metadata.
> - */
> -bool
> -xfs_verify_agino(
> - struct xfs_mount *mp,
> - xfs_agnumber_t agno,
> - xfs_agino_t agino)
> -{
> - xfs_agino_t first;
> - xfs_agino_t last;
> -
> - xfs_ialloc_agino_range(mp, agno, &first, &last);
> - return agino >= first && agino <= last;
> -}
> -
> -/*
> - * Verify that an FS inode number pointer neither points outside the
> - * filesystem nor points at static AG metadata.
> - */
> -bool
> -xfs_verify_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> - xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> -
> - if (agno >= mp->m_sb.sb_agcount)
> - return false;
> - if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> - return false;
> - return xfs_verify_agino(mp, agno, agino);
> -}
> -
> -/* Is this an internal inode number? */
> -bool
> -xfs_internal_inum(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> - (xfs_sb_version_hasquota(&mp->m_sb) &&
> - xfs_is_quota_inode(&mp->m_sb, ino));
> -}
> -
> -/*
> - * Verify that a directory entry's inode number doesn't point at an internal
> - * inode, empty space, or static AG metadata.
> - */
> -bool
> -xfs_verify_dir_ino(
> - struct xfs_mount *mp,
> - xfs_ino_t ino)
> -{
> - if (xfs_internal_inum(mp, ino))
> - return false;
> - return xfs_verify_ino(mp, ino);
> -}
> -
> /* Is there an inode record covering a given range of inode numbers? */
> int
> xfs_ialloc_has_inode_record(
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 144f3eac9b93..90b09c5f163b 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -169,12 +169,5 @@ int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
> int *stat);
>
> int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
> -void xfs_ialloc_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t *first, xfs_agino_t *last);
> -bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> - xfs_agino_t agino);
> -bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> -bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
>
> #endif /* __XFS_IALLOC_H__ */
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index ffc72075a44e..65fc4ed2e9a1 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1080,18 +1080,6 @@ xfs_rtalloc_query_all(
> return xfs_rtalloc_query_range(tp, &keys[0], &keys[1], fn, priv);
> }
>
> -/*
> - * Verify that an realtime block number pointer doesn't point off the
> - * end of the realtime device.
> - */
> -bool
> -xfs_verify_rtbno(
> - struct xfs_mount *mp,
> - xfs_rtblock_t rtbno)
> -{
> - return rtbno < mp->m_sb.sb_rblocks;
> -}
> -
> /* Is the given extent all free? */
> int
> xfs_rtalloc_extent_is_free(
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> new file mode 100644
> index 000000000000..2e2a243cef2e
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.
> + * Copyright (C) 2017 Oracle.
> + * All Rights Reserved.
> + */
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_shared.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_bit.h"
> +#include "xfs_sb.h"
> +#include "xfs_mount.h"
> +#include "xfs_defer.h"
> +#include "xfs_inode.h"
> +#include "xfs_btree.h"
> +#include "xfs_rmap.h"
> +#include "xfs_alloc_btree.h"
> +#include "xfs_alloc.h"
> +#include "xfs_ialloc.h"
> +
> +/* Find the size of the AG, in blocks. */
> +xfs_agblock_t
> +xfs_ag_block_count(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno)
> +{
> + ASSERT(agno < mp->m_sb.sb_agcount);
> +
> + if (agno < mp->m_sb.sb_agcount - 1)
> + return mp->m_sb.sb_agblocks;
> + return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks);
> +}
> +
> +/*
> + * Verify that an AG block number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agbno(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agblock_t agbno)
> +{
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> + if (agbno >= eoag)
> + return false;
> + if (agbno <= XFS_AGFL_BLOCK(mp))
> + return false;
> + return true;
> +}
> +
> +/*
> + * Verify that an FS block number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_fsbno(
> + struct xfs_mount *mp,
> + xfs_fsblock_t fsbno)
> +{
> + xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> +}
> +
> +/* Calculate the first and last possible inode number in an AG. */
> +void
> +xfs_agino_range(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t *first,
> + xfs_agino_t *last)
> +{
> + xfs_agblock_t bno;
> + xfs_agblock_t eoag;
> +
> + eoag = xfs_ag_block_count(mp, agno);
> +
> + /*
> + * Calculate the first inode, which will be in the first
> + * cluster-aligned block after the AGFL.
> + */
> + bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> + xfs_ialloc_cluster_alignment(mp));
> + *first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> +
> + /*
> + * Calculate the last inode, which will be at the end of the
> + * last (aligned) cluster that can be allocated in the AG.
> + */
> + bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> + *last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> +}
> +
> +/*
> + * Verify that an AG inode number pointer neither points outside the AG
> + * nor points at static metadata.
> + */
> +bool
> +xfs_verify_agino(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + xfs_agino_t agino)
> +{
> + xfs_agino_t first;
> + xfs_agino_t last;
> +
> + xfs_agino_range(mp, agno, &first, &last);
> + return agino >= first && agino <= last;
> +}
> +
> +/*
> + * Verify that an FS inode number pointer neither points outside the
> + * filesystem nor points at static AG metadata.
> + */
> +bool
> +xfs_verify_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, ino);
> + xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> +
> + if (agno >= mp->m_sb.sb_agcount)
> + return false;
> + if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
> + return false;
> + return xfs_verify_agino(mp, agno, agino);
> +}
> +
> +/* Is this an internal inode number? */
> +bool
> +xfs_internal_inum(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + return ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> + (xfs_sb_version_hasquota(&mp->m_sb) &&
> + xfs_is_quota_inode(&mp->m_sb, ino));
> +}
> +
> +/*
> + * Verify that a directory entry's inode number doesn't point at an internal
> + * inode, empty space, or static AG metadata.
> + */
> +bool
> +xfs_verify_dir_ino(
> + struct xfs_mount *mp,
> + xfs_ino_t ino)
> +{
> + if (xfs_internal_inum(mp, ino))
> + return false;
> + return xfs_verify_ino(mp, ino);
> +}
> +
> +/*
> + * Verify that an realtime block number pointer doesn't point off the
> + * end of the realtime device.
> + */
> +bool
> +xfs_verify_rtbno(
> + struct xfs_mount *mp,
> + xfs_rtblock_t rtbno)
> +{
> + return rtbno < mp->m_sb.sb_rblocks;
> +}
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index b72ae343140e..4055d62f690c 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -147,4 +147,23 @@ typedef struct xfs_bmbt_irec
> xfs_exntst_t br_state; /* extent state */
> } xfs_bmbt_irec_t;
>
> +/*
> + * Type verifier functions
> + */
> +struct xfs_mount;
> +
> +xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> +bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agblock_t agbno);
> +bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
> +
> +void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t *first, xfs_agino_t *last);
> +bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
> + xfs_agino_t agino);
> +bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
> +bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
> +
> #endif /* __XFS_TYPES_H__ */
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index fb9637ff4bde..9bb0745f1ad2 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -867,7 +867,7 @@ xfs_scrub_agi(
> }
>
> /* Check inode counters */
> - xfs_ialloc_agino_range(mp, agno, &first_agino, &last_agino);
> + xfs_agino_range(mp, agno, &first_agino, &last_agino);
> icount = be32_to_cpu(agi->agi_count);
> if (icount > last_agino - first_agino + 1 ||
> icount < be32_to_cpu(agi->agi_freecount))
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-06-07 11:41 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-07 5:27 [PATCH 0/3] xfs: minor cleanups Dave Chinner
2018-06-07 5:27 ` [PATCH 1/3] xfs: move various type verifiers to common file Dave Chinner
2018-06-07 11:41 ` Brian Foster [this message]
2018-06-07 15:23 ` Darrick J. Wong
2018-06-08 6:24 ` Christoph Hellwig
2018-06-07 5:27 ` [PATCH 2/3] xfs: replace do_mod with native operations Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-07 16:01 ` Darrick J. Wong
2018-06-07 22:23 ` Dave Chinner
2018-06-07 15:54 ` Darrick J. Wong
2018-06-07 22:28 ` Dave Chinner
2018-06-08 0:43 ` [PATCH 2/3 V2] " Dave Chinner
2018-06-08 6:19 ` Christoph Hellwig
2018-06-08 7:31 ` Dave Chinner
2018-06-07 5:27 ` [PATCH 3/3] xfs: clean up MIN/MAX Dave Chinner
2018-06-07 11:42 ` Brian Foster
2018-06-08 6:23 ` Christoph Hellwig
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=20180607114146.GC7798@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