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 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.