linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Chandan Babu R <chandan.babu@oracle.com>
Cc: linux-xfs@vger.kernel.org, david@fromorbit.com
Subject: Re: [PATCH V4 04/16] xfs: Introduce xfs_dfork_nextents() helper
Date: Tue, 4 Jan 2022 15:48:40 -0800	[thread overview]
Message-ID: <20220104234840.GK31583@magnolia> (raw)
In-Reply-To: <20211214084519.759272-5-chandan.babu@oracle.com>

On Tue, Dec 14, 2021 at 02:15:07PM +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
> 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 <chandan.babu@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_format.h     |  4 ----
>  fs/xfs/libxfs/xfs_inode_buf.c  | 16 +++++++++++-----
>  fs/xfs/libxfs/xfs_inode_fork.c | 10 ++++++----
>  fs/xfs/libxfs/xfs_inode_fork.h | 32 ++++++++++++++++++++++++++++++++
>  fs/xfs/scrub/inode.c           | 18 ++++++++++--------
>  5 files changed, 59 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index d75e5b16da7e..e5654b578ec0 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -925,10 +925,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))

Yay, I love watching macros die.  Especially when the replacement
validates its arguments completely!

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  
>  /*
>   * 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 5c95a5428fc7..860d32816909 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -336,9 +336,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(dip, whichfork);
> +
>  	switch (XFS_DFORK_FORMAT(dip, whichfork)) {
>  	case XFS_DINODE_FMT_LOCAL:
>  		/*
> @@ -405,6 +407,8 @@ xfs_dinode_verify(
>  	uint16_t		flags;
>  	uint64_t		flags2;
>  	uint64_t		di_size;
> +	xfs_extnum_t            nextents;
> +	xfs_filblks_t		nblocks;
>  
>  	if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
>  		return __this_address;
> @@ -435,10 +439,12 @@ xfs_dinode_verify(
>  	if ((S_ISLNK(mode) || S_ISDIR(mode)) && di_size == 0)
>  		return __this_address;
>  
> +	nextents = xfs_dfork_data_extents(dip);
> +	nextents += xfs_dfork_attr_extents(dip);
> +	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)
> @@ -495,7 +501,7 @@ xfs_dinode_verify(
>  		default:
>  			return __this_address;
>  		}
> -		if (dip->di_anextents)
> +		if (xfs_dfork_attr_extents(dip))
>  			return __this_address;
>  	}
>  
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index a17c4d87520a..829739e249b6 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -105,7 +105,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(dip, whichfork);
>  	int			size = nex * sizeof(xfs_bmbt_rec_t);
>  	struct xfs_iext_cursor	icur;
>  	struct xfs_bmbt_rec	*dp;
> @@ -230,7 +230,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_data_extents(dip);
>  
>  	switch (inode->i_mode & S_IFMT) {
>  	case S_IFIFO:
> @@ -295,14 +295,16 @@ xfs_iformat_attr_fork(
>  	struct xfs_inode	*ip,
>  	struct xfs_dinode	*dip)
>  {
> +	xfs_extnum_t		naextents;
>  	int			error = 0;
>  
> +	naextents = xfs_dfork_attr_extents(dip);
> +
>  	/*
>  	 * 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, naextents);
>  
>  	switch (ip->i_afp->if_format) {
>  	case XFS_DINODE_FMT_LOCAL:
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 2605f7ff8fc1..7ed2ecb51bca 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -141,6 +141,38 @@ static inline xfs_extnum_t xfs_iext_max_nextents(int whichfork)
>  	return MAXAEXTNUM;
>  }
>  
> +static inline xfs_extnum_t
> +xfs_dfork_data_extents(
> +	struct xfs_dinode	*dip)
> +{
> +	return be32_to_cpu(dip->di_nextents);
> +}
> +
> +static inline xfs_extnum_t
> +xfs_dfork_attr_extents(
> +	struct xfs_dinode	*dip)
> +{
> +	return be16_to_cpu(dip->di_anextents);
> +}
> +
> +static inline xfs_extnum_t
> +xfs_dfork_nextents(
> +	struct xfs_dinode	*dip,
> +	int			whichfork)
> +{
> +	switch (whichfork) {
> +	case XFS_DATA_FORK:
> +		return xfs_dfork_data_extents(dip);
> +	case XFS_ATTR_FORK:
> +		return xfs_dfork_attr_extents(dip);
> +	default:
> +		ASSERT(0);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
>  struct xfs_ifork *xfs_ifork_alloc(enum xfs_dinode_fmt format,
>  				xfs_extnum_t nextents);
>  struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index aefdf8fe1372..a601b04fe408 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -233,6 +233,7 @@ xchk_dinode(
>  	unsigned long long	isize;
>  	uint64_t		flags2;
>  	xfs_extnum_t		nextents;
> +	xfs_extnum_t		naextents;
>  	uint16_t		flags;
>  	uint16_t		mode;
>  
> @@ -377,7 +378,7 @@ xchk_dinode(
>  	xchk_inode_extsize(sc, dip, ino, mode, flags);
>  
>  	/* di_nextents */
> -	nextents = be32_to_cpu(dip->di_nextents);
> +	nextents = xfs_dfork_data_extents(dip);
>  	fork_recs =  XFS_DFORK_DSIZE(dip, mp) / sizeof(struct xfs_bmbt_rec);
>  	switch (dip->di_format) {
>  	case XFS_DINODE_FMT_EXTENTS:
> @@ -394,10 +395,12 @@ xchk_dinode(
>  		break;
>  	}
>  
> +	naextents = xfs_dfork_attr_extents(dip);
> +
>  	/* 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);
> @@ -409,19 +412,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);
>  	}
>  
> @@ -499,14 +501,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_data_extents(dip))
>  		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_attr_extents(dip))
>  		xchk_ino_xref_set_corrupt(sc, sc->ip->i_ino);
>  
>  	/* Check nblocks against the inode. */
> -- 
> 2.30.2
> 

  reply	other threads:[~2022-01-04 23:48 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-14  8:45 [PATCH V4 00/16] xfs: Extend per-inode extent counters Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 01/16] xfs: Move extent count limits to xfs_format.h Chandan Babu R
2022-01-04 23:28   ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 02/16] xfs: Introduce xfs_iext_max_nextents() helper Chandan Babu R
2022-01-04 23:30   ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 03/16] xfs: Use xfs_extnum_t instead of basic data types Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 04/16] xfs: Introduce xfs_dfork_nextents() helper Chandan Babu R
2022-01-04 23:48   ` Darrick J. Wong [this message]
2021-12-14  8:45 ` [PATCH V4 05/16] xfs: Use basic types to define xfs_log_dinode's di_nextents and di_anextents Chandan Babu R
2022-01-04 23:50   ` Darrick J. Wong
2022-01-05 13:43     ` Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 06/16] xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits respectively Chandan Babu R
2021-12-14 14:54   ` kernel test robot
2021-12-14 15:05   ` kernel test robot
2021-12-14 15:15   ` kernel test robot
2021-12-15  9:19     ` Chandan Babu R
2022-01-04 23:54       ` Darrick J. Wong
2022-01-05 14:14         ` Chandan Babu R
2022-01-05 17:21           ` Darrick J. Wong
2022-01-06  7:03             ` Chandan Babu R
2022-01-06 20:31               ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 07/16] xfs: Introduce XFS_SB_FEAT_INCOMPAT_NREXT64 and associated per-fs feature bit Chandan Babu R
2022-01-05  0:03   ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 08/16] xfs: Introduce XFS_FSOP_GEOM_FLAGS_NREXT64 Chandan Babu R
2022-01-05  0:05   ` Darrick J. Wong
2022-01-05 13:44     ` Chandan Babu R
2022-01-05 17:22       ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 09/16] xfs: Introduce XFS_DIFLAG2_NREXT64 and associated helpers Chandan Babu R
2022-01-05  0:43   ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 10/16] xfs: Use xfs_rfsblock_t to count maximum blocks that can be used by BMBT Chandan Babu R
2021-12-14 18:15   ` kernel test robot
2021-12-14  8:45 ` [PATCH V4 11/16] xfs: Introduce macros to represent new maximum extent counts for data/attr forks Chandan Babu R
2022-01-05  0:42   ` Darrick J. Wong
2022-01-05 13:46     ` Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 12/16] xfs: Introduce per-inode 64-bit extent counters Chandan Babu R
2022-01-05  1:04   ` Darrick J. Wong
2022-01-05 13:47     ` Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 13/16] xfs: Conditionally upgrade existing inodes to use " Chandan Babu R
2022-01-05  0:18   ` Darrick J. Wong
2022-01-05 13:49     ` Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 14/16] xfs: Enable bulkstat ioctl to support 64-bit per-inode " Chandan Babu R
2022-01-05  0:28   ` Darrick J. Wong
2022-01-05 13:50     ` Chandan Babu R
2021-12-14  8:45 ` [PATCH V4 15/16] xfs: Add XFS_SB_FEAT_INCOMPAT_NREXT64 to the list of supported flags Chandan Babu R
2022-01-05  0:47   ` Darrick J. Wong
2021-12-14  8:45 ` [PATCH V4 16/16] xfs: Define max extent length based on on-disk format definition Chandan Babu R
2022-01-05  0:47   ` Darrick J. Wong
2022-01-05 13:51     ` 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=20220104234840.GK31583@magnolia \
    --to=djwong@kernel.org \
    --cc=chandan.babu@oracle.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;
as well as URLs for NNTP newsgroup(s).