From: "Darrick J. Wong" <djwong@kernel.org>
To: Chandan Babu R <chandanrlinux@gmail.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH V2 05/12] xfs: Introduce xfs_dfork_nextents() helper
Date: Tue, 27 Jul 2021 15:10:16 -0700 [thread overview]
Message-ID: <20210727221016.GO559212@magnolia> (raw)
In-Reply-To: <20210726114541.24898-6-chandanrlinux@gmail.com>
On Mon, Jul 26, 2021 at 05:15:34PM +0530, Chandan Babu R wrote:
> This commit replaces the macro XFS_DFORK_NEXTENTS() with the helper function
> xfs_dfork_nextents(). As of this commit, xfs_dfork_nextents() returns the same
> value as XFS_DFORK_NEXTENTS(). A future commit which extends inode's extent
Yay fewer shouty macros!
> counter fields will add more logic to this helper.
>
> This commit also replaces direct accesses to xfs_dinode->di_[a]nextents
> with calls to xfs_dfork_nextents().
>
> No functional changes have been made.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
> fs/xfs/libxfs/xfs_format.h | 4 ----
> fs/xfs/libxfs/xfs_inode_buf.c | 41 +++++++++++++++++++++++++++++-----
> fs/xfs/libxfs/xfs_inode_buf.h | 2 ++
> fs/xfs/libxfs/xfs_inode_fork.c | 12 ++++++----
> fs/xfs/scrub/inode.c | 19 +++++++++-------
> fs/xfs/scrub/inode_repair.c | 38 +++++++++++++++++++------------
> 6 files changed, 81 insertions(+), 35 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 920e3f9c418f..001a4077a7c6 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1166,10 +1166,6 @@ enum xfs_dinode_fmt {
> ((w) == XFS_DATA_FORK ? \
> (dip)->di_format : \
> (dip)->di_aformat)
> -#define XFS_DFORK_NEXTENTS(dip,w) \
> - ((w) == XFS_DATA_FORK ? \
> - be32_to_cpu((dip)->di_nextents) : \
> - be16_to_cpu((dip)->di_anextents))
>
> /*
> * For block and character special files the 32bit dev_t is stored at the
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index cba9a38f3270..6bef0757fca4 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -342,9 +342,11 @@ xfs_dinode_verify_fork(
> struct xfs_mount *mp,
> int whichfork)
> {
> - xfs_extnum_t di_nextents = XFS_DFORK_NEXTENTS(dip, whichfork);
> + xfs_extnum_t di_nextents;
> xfs_extnum_t max_extents;
>
> + di_nextents = xfs_dfork_nextents(mp, dip, whichfork);
> +
> switch (XFS_DFORK_FORMAT(dip, whichfork)) {
> case XFS_DINODE_FMT_LOCAL:
> /*
> @@ -375,6 +377,31 @@ xfs_dinode_verify_fork(
> return NULL;
> }
>
> +xfs_extnum_t
> +xfs_dfork_nextents(
> + struct xfs_mount *mp,
> + struct xfs_dinode *dip,
> + int whichfork)
> +{
> + xfs_extnum_t nextents = 0;
> +
> + switch (whichfork) {
> + case XFS_DATA_FORK:
> + nextents = be32_to_cpu(dip->di_nextents);
> + break;
> +
> + case XFS_ATTR_FORK:
> + nextents = be16_to_cpu(dip->di_anextents);
> + break;
> +
> + default:
> + ASSERT(0);
> + break;
> + }
> +
> + return nextents;
> +}
This could be a static inline function taking the place of
XFS_DFORK_NEXTENTS instead of another new function with a stack frame,
etc:
static inline xfs_extnum_t
xfs_dfork_nextents(
struct xfs_mount *mp,
struct xfs_dinode *dip,
int whichfork)
{
switch (whichfork) {
case XFS_DATA_FORK:
return be32_to_cpu(dip->di_nextents);
case XFS_ATTR_FORK:
return be16_to_cpu(dip->di_anextents);
default:
ASSERT(0);
return 0;
}
}
> +
> static xfs_failaddr_t
> xfs_dinode_verify_forkoff(
> struct xfs_dinode *dip,
> @@ -474,6 +501,8 @@ xfs_dinode_verify(
> uint16_t flags;
> uint64_t flags2;
> uint64_t di_size;
> + xfs_extnum_t nextents;
> + int64_t nblocks;
xfs_rfsblock_t, since that's the type we use for nblocks in xfs_inode.
>
> if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
> return __this_address;
> @@ -504,10 +533,12 @@ xfs_dinode_verify(
> if ((S_ISLNK(mode) || S_ISDIR(mode)) && di_size == 0)
> return __this_address;
>
> + nextents = xfs_dfork_nextents(mp, dip, XFS_DATA_FORK);
> + nextents += xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK);
> + nblocks = be64_to_cpu(dip->di_nblocks);
> +
> /* Fork checks carried over from xfs_iformat_fork */
> - if (mode &&
> - be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
> - be64_to_cpu(dip->di_nblocks))
> + if (mode && nextents > nblocks)
> return __this_address;
>
> if (mode && XFS_DFORK_BOFF(dip) > mp->m_sb.sb_inodesize)
> @@ -564,7 +595,7 @@ xfs_dinode_verify(
> default:
> return __this_address;
> }
> - if (dip->di_anextents)
> + if (xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK))
> return __this_address;
> }
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index a30b7676098a..ea2c35091609 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -36,6 +36,8 @@ xfs_failaddr_t xfs_inode_validate_extsize(struct xfs_mount *mp,
> xfs_failaddr_t xfs_inode_validate_cowextsize(struct xfs_mount *mp,
> uint32_t cowextsize, uint16_t mode, uint16_t flags,
> uint64_t flags2);
> +xfs_extnum_t xfs_dfork_nextents(struct xfs_mount *mp, struct xfs_dinode *dip,
> + int whichfork);
>
> static inline uint64_t xfs_inode_encode_bigtime(struct timespec64 tv)
> {
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index a1e40df585a3..38dd2dfc31fa 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -107,7 +107,7 @@ xfs_iformat_extents(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> int state = xfs_bmap_fork_to_state(whichfork);
> - xfs_extnum_t nex = XFS_DFORK_NEXTENTS(dip, whichfork);
> + xfs_extnum_t nex = xfs_dfork_nextents(mp, dip, whichfork);
> int size = nex * sizeof(xfs_bmbt_rec_t);
> struct xfs_iext_cursor icur;
> struct xfs_bmbt_rec *dp;
> @@ -226,6 +226,7 @@ xfs_iformat_data_fork(
> struct xfs_inode *ip,
> struct xfs_dinode *dip)
> {
> + struct xfs_mount *mp = ip->i_mount;
> struct inode *inode = VFS_I(ip);
> int error;
>
> @@ -234,7 +235,7 @@ xfs_iformat_data_fork(
> * depend on it.
> */
> ip->i_df.if_format = dip->di_format;
> - ip->i_df.if_nextents = be32_to_cpu(dip->di_nextents);
> + ip->i_df.if_nextents = xfs_dfork_nextents(mp, dip, XFS_DATA_FORK);
>
> switch (inode->i_mode & S_IFMT) {
> case S_IFIFO:
> @@ -301,14 +302,17 @@ xfs_iformat_attr_fork(
> struct xfs_inode *ip,
> struct xfs_dinode *dip)
> {
> + struct xfs_mount *mp = ip->i_mount;
> + xfs_extnum_t nextents;
naextents for consistency?
> int error = 0;
>
> + nextents = xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK);
> +
> /*
> * Initialize the extent count early, as the per-format routines may
> * depend on it.
> */
> - ip->i_afp = xfs_ifork_alloc(dip->di_aformat,
> - be16_to_cpu(dip->di_anextents));
> + ip->i_afp = xfs_ifork_alloc(dip->di_aformat, nextents);
>
> switch (ip->i_afp->if_format) {
> case XFS_DINODE_FMT_LOCAL:
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index 246d11ca133f..a161dac31a6f 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -220,6 +220,7 @@ xchk_dinode(
> unsigned long long isize;
> uint64_t flags2;
> xfs_extnum_t nextents;
> + xfs_extnum_t naextents;
> prid_t prid;
> uint16_t flags;
> uint16_t mode;
> @@ -378,7 +379,7 @@ xchk_dinode(
> xchk_inode_extsize(sc, dip, ino, mode, flags);
>
> /* di_nextents */
> - nextents = be32_to_cpu(dip->di_nextents);
> + nextents = xfs_dfork_nextents(mp, dip, XFS_DATA_FORK);
> fork_recs = XFS_DFORK_DSIZE(dip, mp) / sizeof(struct xfs_bmbt_rec);
> switch (dip->di_format) {
> case XFS_DINODE_FMT_EXTENTS:
> @@ -395,10 +396,12 @@ xchk_dinode(
> break;
> }
>
> + naextents = xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK);
> +
> /* di_forkoff */
> if (XFS_DFORK_APTR(dip) >= (char *)dip + mp->m_sb.sb_inodesize)
> xchk_ino_set_corrupt(sc, ino);
> - if (dip->di_anextents != 0 && dip->di_forkoff == 0)
> + if (naextents != 0 && dip->di_forkoff == 0)
> xchk_ino_set_corrupt(sc, ino);
> if (dip->di_forkoff == 0 && dip->di_aformat != XFS_DINODE_FMT_EXTENTS)
> xchk_ino_set_corrupt(sc, ino);
> @@ -410,19 +413,18 @@ xchk_dinode(
> xchk_ino_set_corrupt(sc, ino);
>
> /* di_anextents */
> - nextents = be16_to_cpu(dip->di_anextents);
> fork_recs = XFS_DFORK_ASIZE(dip, mp) / sizeof(struct xfs_bmbt_rec);
> switch (dip->di_aformat) {
> case XFS_DINODE_FMT_EXTENTS:
> - if (nextents > fork_recs)
> + if (naextents > fork_recs)
> xchk_ino_set_corrupt(sc, ino);
> break;
> case XFS_DINODE_FMT_BTREE:
> - if (nextents <= fork_recs)
> + if (naextents <= fork_recs)
> xchk_ino_set_corrupt(sc, ino);
> break;
> default:
> - if (nextents != 0)
> + if (naextents != 0)
> xchk_ino_set_corrupt(sc, ino);
> }
>
> @@ -487,6 +489,7 @@ xchk_inode_xref_bmap(
> struct xfs_scrub *sc,
> struct xfs_dinode *dip)
> {
> + struct xfs_mount *mp = sc->mp;
> xfs_extnum_t nextents;
> xfs_filblks_t count;
> xfs_filblks_t acount;
> @@ -500,14 +503,14 @@ xchk_inode_xref_bmap(
> &nextents, &count);
> if (!xchk_should_check_xref(sc, &error, NULL))
> return;
> - if (nextents < be32_to_cpu(dip->di_nextents))
> + if (nextents < xfs_dfork_nextents(mp, dip, XFS_DATA_FORK))
> xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
>
> error = xfs_bmap_count_blocks(sc->tp, sc->ip, XFS_ATTR_FORK,
> &nextents, &acount);
> if (!xchk_should_check_xref(sc, &error, NULL))
> return;
> - if (nextents != be16_to_cpu(dip->di_anextents))
> + if (nextents != xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK))
> xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
>
> /* Check nblocks against the inode. */
> diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
> index 042c7d0bc0f5..bdb4685923c0 100644
> --- a/fs/xfs/scrub/inode_repair.c
> +++ b/fs/xfs/scrub/inode_repair.c
Hey waitaminute, is this based off of djwong-dev??
> @@ -602,7 +602,7 @@ xrep_dinode_bad_extents_fork(
> int i;
> int fork_size;
>
> - nex = XFS_DFORK_NEXTENTS(dip, whichfork);
> + nex = xfs_dfork_nextents(sc->mp, dip, whichfork);
> fork_size = nex * sizeof(struct xfs_bmbt_rec);
> if (fork_size < 0 || fork_size > dfork_size)
> return true;
> @@ -636,7 +636,7 @@ xrep_dinode_bad_btree_fork(
> int nrecs;
> int level;
>
> - if (XFS_DFORK_NEXTENTS(dip, whichfork) <=
> + if (xfs_dfork_nextents(sc->mp, dip, whichfork) <=
> dfork_size / sizeof(struct xfs_bmbt_rec))
> return true;
>
> @@ -831,12 +831,16 @@ xrep_dinode_ensure_forkoff(
> struct xrep_dinode_stats *dis)
> {
> struct xfs_bmdr_block *bmdr;
> + xfs_extnum_t anextents, dnextents;
> size_t bmdr_minsz = xfs_bmdr_space_calc(1);
> unsigned int lit_sz = XFS_LITINO(sc->mp);
> unsigned int afork_min, dfork_min;
>
> trace_xrep_dinode_ensure_forkoff(sc, dip);
>
> + dnextents = xfs_dfork_nextents(sc->mp, dip, XFS_DATA_FORK);
> + anextents = xfs_dfork_nextents(sc->mp, dip, XFS_ATTR_FORK);
> +
> /*
> * Before calling this function, xrep_dinode_core ensured that both
> * forks actually fit inside their respective literal areas. If this
> @@ -857,15 +861,14 @@ xrep_dinode_ensure_forkoff(
> afork_min = XFS_DFORK_SIZE(dip, sc->mp, XFS_ATTR_FORK);
> break;
> case XFS_DINODE_FMT_EXTENTS:
> - if (dip->di_anextents) {
> + if (anextents) {
> /*
> * We must maintain sufficient space to hold the entire
> * extent map array in the data fork. Note that we
> * previously zapped the fork if it had no chance of
> * fitting in the inode.
> */
> - afork_min = sizeof(struct xfs_bmbt_rec) *
> - be16_to_cpu(dip->di_anextents);
> + afork_min = sizeof(struct xfs_bmbt_rec) * anextents;
> } else if (dis->attr_extents > 0) {
> /*
> * The attr fork thinks it has zero extents, but we
> @@ -908,15 +911,14 @@ xrep_dinode_ensure_forkoff(
> dfork_min = be64_to_cpu(dip->di_size);
> break;
> case XFS_DINODE_FMT_EXTENTS:
> - if (dip->di_nextents) {
> + if (dnextents) {
> /*
> * We must maintain sufficient space to hold the entire
> * extent map array in the data fork. Note that we
> * previously zapped the fork if it had no chance of
> * fitting in the inode.
> */
> - dfork_min = sizeof(struct xfs_bmbt_rec) *
> - be32_to_cpu(dip->di_nextents);
> + dfork_min = sizeof(struct xfs_bmbt_rec) * dnextents;
> } else if (dis->data_extents > 0 || dis->rt_extents > 0) {
> /*
> * The data fork thinks it has zero extents, but we
> @@ -956,7 +958,7 @@ xrep_dinode_ensure_forkoff(
> * recovery fork, move the attr fork up.
> */
> if (dip->di_format == XFS_DINODE_FMT_EXTENTS &&
> - dip->di_nextents == 0 &&
> + dnextents == 0 &&
> (dis->data_extents > 0 || dis->rt_extents > 0) &&
> bmdr_minsz > XFS_DFORK_DSIZE(dip, sc->mp)) {
> if (bmdr_minsz + afork_min > lit_sz) {
> @@ -982,7 +984,7 @@ xrep_dinode_ensure_forkoff(
> * recovery fork, move the attr fork down.
> */
> if (dip->di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dip->di_anextents == 0 &&
> + anextents == 0 &&
> dis->attr_extents > 0 &&
> bmdr_minsz > XFS_DFORK_ASIZE(dip, sc->mp)) {
> if (dip->di_format == XFS_DINODE_FMT_BTREE) {
> @@ -1019,6 +1021,9 @@ xrep_dinode_zap_forks(
> struct xfs_dinode *dip,
> struct xrep_dinode_stats *dis)
> {
> + uint64_t nblocks;
xfs_rfsblock_t.
--D
> + xfs_extnum_t nextents;
> + xfs_extnum_t naextents;
> uint16_t mode;
> bool zap_datafork = false;
> bool zap_attrfork = false;
> @@ -1028,12 +1033,17 @@ xrep_dinode_zap_forks(
> mode = be16_to_cpu(dip->di_mode);
>
> /* Inode counters don't make sense? */
> - if (be32_to_cpu(dip->di_nextents) > be64_to_cpu(dip->di_nblocks))
> + nblocks = be64_to_cpu(dip->di_nblocks);
> +
> + nextents = xfs_dfork_nextents(sc->mp, dip, XFS_DATA_FORK);
> + if (nextents > nblocks)
> zap_datafork = true;
> - if (be16_to_cpu(dip->di_anextents) > be64_to_cpu(dip->di_nblocks))
> +
> + naextents = xfs_dfork_nextents(sc->mp, dip, XFS_ATTR_FORK);
> + if (naextents > nblocks)
> zap_attrfork = true;
> - if (be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
> - be64_to_cpu(dip->di_nblocks))
> +
> + if (nextents + naextents > nblocks)
> zap_datafork = zap_attrfork = true;
>
> if (!zap_datafork)
> --
> 2.30.2
>
next prev parent reply other threads:[~2021-07-27 22:10 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 11:45 [PATCH V2 00/12] xfs: Extend per-inode extent counters Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 01/12] xfs: Move extent count limits to xfs_format.h Chandan Babu R
2021-07-26 18:00 ` Darrick J. Wong
2021-07-27 8:07 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 02/12] xfs: Rename MAXEXTNUM, MAXAEXTNUM to XFS_IFORK_EXTCNT_MAXS32, XFS_IFORK_EXTCNT_MAXS16 Chandan Babu R
2021-07-27 21:56 ` Darrick J. Wong
2021-07-27 22:03 ` Darrick J. Wong
2021-07-28 3:15 ` Chandan Babu R
2021-08-23 4:18 ` Chandan Babu R
2021-08-23 7:17 ` Chandan Babu R
2021-08-23 18:16 ` Darrick J. Wong
2021-07-26 11:45 ` [PATCH V2 03/12] xfs: Introduce xfs_iext_max() helper Chandan Babu R
2021-07-27 21:58 ` Darrick J. Wong
2021-07-28 3:17 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 04/12] xfs: Use xfs_extnum_t instead of basic data types Chandan Babu R
2021-07-27 21:59 ` Darrick J. Wong
2021-07-28 3:38 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 05/12] xfs: Introduce xfs_dfork_nextents() helper Chandan Babu R
2021-07-27 22:10 ` Darrick J. Wong [this message]
2021-07-28 4:06 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 06/12] xfs: xfs_dfork_nextents: Return extent count via an out argument Chandan Babu R
2021-07-27 22:22 ` Darrick J. Wong
2021-07-28 4:21 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 07/12] xfs: Rename inode's extent counter fields based on their width Chandan Babu R
2021-07-27 22:50 ` Darrick J. Wong
2021-07-28 5:48 ` Chandan Babu R
2021-07-28 19:04 ` Darrick J. Wong
2021-07-26 11:45 ` [PATCH V2 08/12] xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits respectively Chandan Babu R
2021-07-27 22:29 ` Darrick J. Wong
2021-07-26 11:45 ` [PATCH V2 09/12] xfs: Rename XFS_IOC_BULKSTAT to XFS_IOC_BULKSTAT_V5 Chandan Babu R
2021-07-27 22:54 ` Darrick J. Wong
2021-07-27 23:00 ` Darrick J. Wong
2021-07-27 23:17 ` Dave Chinner
2021-07-28 6:56 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 10/12] xfs: Enable bulkstat ioctl to support 64-bit extent counters Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 11/12] xfs: Extend per-inode extent counter widths Chandan Babu R
2021-07-27 23:09 ` Darrick J. Wong
2021-07-28 7:17 ` Chandan Babu R
2021-07-26 11:45 ` [PATCH V2 12/12] xfs: Error tag to test if v5 bulkstat skips inodes with large extent count Chandan Babu R
2021-07-27 23:10 ` Darrick J. Wong
2021-07-28 7:23 ` Chandan Babu R
2021-07-28 7:38 ` Chandan Babu R
2021-07-28 19:06 ` Darrick J. Wong
2021-07-28 21:27 ` [PATCH V2 00/12] xfs: Extend per-inode extent counters Darrick J. Wong
2021-07-29 6:40 ` Chandan Babu R
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=20210727221016.GO559212@magnolia \
--to=djwong@kernel.org \
--cc=chandanrlinux@gmail.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;
as well as URLs for NNTP newsgroup(s).