All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 9/9] xfs: move inode extent size hint validation to libxfs
Date: Wed, 21 Mar 2018 13:42:54 -0400	[thread overview]
Message-ID: <20180321174254.GK11127@bfoster.bfoster> (raw)
In-Reply-To: <152107382856.19571.17761547971193930776.stgit@magnolia>

On Wed, Mar 14, 2018 at 05:30:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Extent size hint validation is used by scrub to decide if there's an
> error, and it will be used by repair to decide to remove the hint.
> Since these use the same validation functions, move them to libxfs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c |  105 +++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_inode_buf.h |    5 ++
>  fs/xfs/scrub/inode.c          |  100 +++++----------------------------------
>  3 files changed, 122 insertions(+), 88 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 51019e5..cdd4c1d 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -651,3 +651,108 @@ xfs_iread(
>  	xfs_trans_brelse(tp, bp);
>  	return error;
>  }
> +
> +/*
> + * Validate di_extsize hint.
> + *
> + * The rules are documented at xfs_ioctl_setattr_check_extsize().
> + * These functions must be kept in sync with each other.
> + */
> +xfs_failaddr_t
> +xfs_inode_validate_extsize(
> +	struct xfs_mount		*mp,
> +	uint32_t			extsize,
> +	uint16_t			mode,
> +	uint16_t			flags)
> +{
> +	bool				rt_flag;
> +	bool				hint_flag;
> +	bool				inherit_flag;
> +	uint32_t			extsize_bytes;
> +	uint32_t			blocksize_bytes;
> +
> +	rt_flag = (flags & XFS_DIFLAG_REALTIME);
> +	hint_flag = (flags & XFS_DIFLAG_EXTSIZE);
> +	inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
> +	extsize_bytes = XFS_FSB_TO_B(mp, extsize);
> +
> +	if (rt_flag)
> +		blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
> +	else
> +		blocksize_bytes = mp->m_sb.sb_blocksize;
> +
> +	if ((hint_flag || inherit_flag) && !(S_ISDIR(mode) || S_ISREG(mode)))
> +		return __this_address;
> +
> +	if (hint_flag && !S_ISREG(mode))
> +		return __this_address;
> +
> +	if (inherit_flag && !S_ISDIR(mode))
> +		return __this_address;
> +
> +	if ((hint_flag || inherit_flag) && extsize == 0)
> +		return __this_address;
> +
> +	if (!(hint_flag || inherit_flag) && extsize != 0)
> +		return __this_address;
> +
> +	if (extsize_bytes % blocksize_bytes)
> +		return __this_address;
> +
> +	if (extsize > MAXEXTLEN)
> +		return __this_address;
> +
> +	if (!rt_flag && extsize > mp->m_sb.sb_agblocks / 2)
> +		return __this_address;
> +
> +	return NULL;
> +}
> +
> +/*
> + * Validate di_cowextsize hint.
> + *
> + * The rules are documented at xfs_ioctl_setattr_check_cowextsize().
> + * These functions must be kept in sync with each other.
> + */
> +xfs_failaddr_t
> +xfs_inode_validate_cowextsize(
> +	struct xfs_mount		*mp,
> +	uint32_t			cowextsize,
> +	uint16_t			mode,
> +	uint16_t			flags,
> +	uint64_t			flags2)
> +{
> +	bool				rt_flag;
> +	bool				hint_flag;
> +	uint32_t			cowextsize_bytes;
> +
> +	rt_flag = (flags & XFS_DIFLAG_REALTIME);
> +	hint_flag = (flags2 & XFS_DIFLAG2_COWEXTSIZE);
> +	cowextsize_bytes = XFS_FSB_TO_B(mp, cowextsize);
> +
> +	if (hint_flag && !xfs_sb_version_hasreflink(&mp->m_sb))
> +		return __this_address;
> +
> +	if (hint_flag && !(S_ISDIR(mode) || S_ISREG(mode)))
> +		return __this_address;
> +
> +	if (hint_flag && cowextsize == 0)
> +		return __this_address;
> +
> +	if (!hint_flag && cowextsize != 0)
> +		return __this_address;
> +
> +	if (hint_flag && rt_flag)
> +		return __this_address;
> +
> +	if (cowextsize_bytes % mp->m_sb.sb_blocksize)
> +		return __this_address;
> +
> +	if (cowextsize > MAXEXTLEN)
> +		return __this_address;
> +
> +	if (cowextsize > mp->m_sb.sb_agblocks / 2)
> +		return __this_address;
> +
> +	return NULL;
> +}
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 8a5e1da..d9a376a 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -84,5 +84,10 @@ void	xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
>  
>  xfs_failaddr_t xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
>  			   struct xfs_dinode *dip);
> +xfs_failaddr_t xfs_inode_validate_extsize(struct xfs_mount *mp,
> +		uint32_t extsize, uint16_t mode, uint16_t flags);
> +xfs_failaddr_t xfs_inode_validate_cowextsize(struct xfs_mount *mp,
> +		uint32_t cowextsize, uint16_t mode, uint16_t flags,
> +		uint64_t flags2);
>  
>  #endif	/* __XFS_INODE_BUF_H__ */
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index 9eadca9..25bca48 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -89,12 +89,7 @@ xfs_scrub_setup_inode(
>  
>  /* Inode core */
>  
> -/*
> - * Validate di_extsize hint.
> - *
> - * The rules are documented at xfs_ioctl_setattr_check_extsize().
> - * These functions must be kept in sync with each other.
> - */
> +/* Validate di_extsize hint. */
>  STATIC void
>  xfs_scrub_inode_extsize(
>  	struct xfs_scrub_context	*sc,
> @@ -103,52 +98,12 @@ xfs_scrub_inode_extsize(
>  	uint16_t			mode,
>  	uint16_t			flags)
>  {
> -	struct xfs_mount		*mp = sc->mp;
> -	bool				rt_flag;
> -	bool				hint_flag;
> -	bool				inherit_flag;
> -	uint32_t			extsize;
> -	uint32_t			extsize_bytes;
> -	uint32_t			blocksize_bytes;
> -
> -	rt_flag = (flags & XFS_DIFLAG_REALTIME);
> -	hint_flag = (flags & XFS_DIFLAG_EXTSIZE);
> -	inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
> -	extsize = be32_to_cpu(dip->di_extsize);
> -	extsize_bytes = XFS_FSB_TO_B(sc->mp, extsize);
> -
> -	if (rt_flag)
> -		blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
> -	else
> -		blocksize_bytes = mp->m_sb.sb_blocksize;
> -
> -	if ((hint_flag || inherit_flag) && !(S_ISDIR(mode) || S_ISREG(mode)))
> -		goto bad;
> -
> -	if (hint_flag && !S_ISREG(mode))
> -		goto bad;
> -
> -	if (inherit_flag && !S_ISDIR(mode))
> -		goto bad;
> +	xfs_failaddr_t			fa;
>  
> -	if ((hint_flag || inherit_flag) && extsize == 0)
> -		goto bad;
> -
> -	if (!(hint_flag || inherit_flag) && extsize != 0)
> -		goto bad;
> -
> -	if (extsize_bytes % blocksize_bytes)
> -		goto bad;
> -
> -	if (extsize > MAXEXTLEN)
> -		goto bad;
> -
> -	if (!rt_flag && extsize > mp->m_sb.sb_agblocks / 2)
> -		goto bad;
> -
> -	return;
> -bad:
> -	xfs_scrub_ino_set_corrupt(sc, ino);
> +	fa = xfs_inode_validate_extsize(sc->mp, be32_to_cpu(dip->di_extsize),
> +			mode, flags);
> +	if (fa)
> +		xfs_scrub_ino_set_corrupt(sc, ino);
>  }
>  
>  /*
> @@ -166,44 +121,13 @@ xfs_scrub_inode_cowextsize(
>  	uint16_t			flags,
>  	uint64_t			flags2)
>  {
> -	struct xfs_mount		*mp = sc->mp;
> -	bool				rt_flag;
> -	bool				hint_flag;
> -	uint32_t			extsize;
> -	uint32_t			extsize_bytes;
> -
> -	rt_flag = (flags & XFS_DIFLAG_REALTIME);
> -	hint_flag = (flags2 & XFS_DIFLAG2_COWEXTSIZE);
> -	extsize = be32_to_cpu(dip->di_cowextsize);
> -	extsize_bytes = XFS_FSB_TO_B(sc->mp, extsize);
> -
> -	if (hint_flag && !xfs_sb_version_hasreflink(&mp->m_sb))
> -		goto bad;
> -
> -	if (hint_flag && !(S_ISDIR(mode) || S_ISREG(mode)))
> -		goto bad;
> -
> -	if (hint_flag && extsize == 0)
> -		goto bad;
> -
> -	if (!hint_flag && extsize != 0)
> -		goto bad;
> +	xfs_failaddr_t			fa;
>  
> -	if (hint_flag && rt_flag)
> -		goto bad;
> -
> -	if (extsize_bytes % mp->m_sb.sb_blocksize)
> -		goto bad;
> -
> -	if (extsize > MAXEXTLEN)
> -		goto bad;
> -
> -	if (extsize > mp->m_sb.sb_agblocks / 2)
> -		goto bad;
> -
> -	return;
> -bad:
> -	xfs_scrub_ino_set_corrupt(sc, ino);
> +	fa = xfs_inode_validate_cowextsize(sc->mp,
> +			be32_to_cpu(dip->di_cowextsize), mode, flags,
> +			flags2);
> +	if (fa)
> +		xfs_scrub_ino_set_corrupt(sc, ino);
>  }
>  
>  /* Make sure the di_flags make sense for the inode. */
> 
> --
> 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

  reply	other threads:[~2018-03-21 17:42 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-15  0:29 [PATCH v2 0/9] xfs-4.17: online scrub fixes Darrick J. Wong
2018-03-15  0:29 ` [PATCH 1/9] xfs: sanity-check the unused space before trying to use it Darrick J. Wong
2018-03-21 13:52   ` Brian Foster
2018-03-21 17:44     ` Darrick J. Wong
2018-03-22  5:59   ` [PATCH v2 " Darrick J. Wong
2018-03-22 14:33     ` Brian Foster
2018-03-22 17:23       ` Darrick J. Wong
2018-03-22 22:04     ` Dave Chinner
2018-03-22 17:53   ` [PATCH v3 " Darrick J. Wong
2018-03-22 22:21   ` [PATCH v4 " Darrick J. Wong
2018-03-23 12:29     ` Brian Foster
2018-03-15  0:29 ` [PATCH 2/9] xfs: refactor bmap record valiation Darrick J. Wong
2018-03-21 13:55   ` Brian Foster
2018-03-21 20:30     ` Darrick J. Wong
2018-03-22  6:01   ` [PATCH v2 " Darrick J. Wong
2018-03-22 14:33     ` Brian Foster
2018-03-15  0:29 ` [PATCH 3/9] xfs: refactor inode verifier error logging Darrick J. Wong
2018-03-21 13:55   ` Brian Foster
2018-03-15  0:29 ` [PATCH 4/9] xfs: refactor inode buffer " Darrick J. Wong
2018-03-21 13:55   ` Brian Foster
2018-03-21 18:03     ` Darrick J. Wong
2018-04-24 19:51   ` Eric Sandeen
2018-03-15  0:30 ` [PATCH 5/9] xfs: bmap scrubber should do rmap xref with bmap for sparse files Darrick J. Wong
2018-03-21 17:42   ` Brian Foster
2018-03-21 18:11     ` Darrick J. Wong
2018-03-22  6:02   ` [PATCH v2 " Darrick J. Wong
2018-03-22 14:33     ` Brian Foster
2018-03-22 17:35       ` Darrick J. Wong
2018-03-15  0:30 ` [PATCH 6/9] xfs: inode scrubber shouldn't bother with raw checks Darrick J. Wong
2018-03-21 17:42   ` Brian Foster
2018-03-21 20:37     ` Darrick J. Wong
2018-03-15  0:30 ` [PATCH 7/9] xfs: remove xfs_buf parameter from inode scrub methods Darrick J. Wong
2018-03-21 17:42   ` Brian Foster
2018-03-15  0:30 ` [PATCH 8/9] xfs: record inode buf errors as a xref error in inode scrubber Darrick J. Wong
2018-03-21 17:42   ` Brian Foster
2018-03-21 20:50     ` Darrick J. Wong
2018-03-22 14:34       ` Brian Foster
2018-03-22  6:24   ` [PATCH v2 " Darrick J. Wong
2018-03-22 14:34     ` Brian Foster
2018-03-15  0:30 ` [PATCH 9/9] xfs: move inode extent size hint validation to libxfs Darrick J. Wong
2018-03-21 17:42   ` Brian Foster [this message]
2018-03-21  3:21 ` [PATCH 10/9] xfs: don't accept inode buffers with suspicious unlinked chains Darrick J. Wong
2018-03-21 17:43   ` Brian Foster
2018-03-21 20:52     ` Darrick J. Wong
2018-03-22  6:08   ` [PATCH v2 " Darrick J. Wong
2018-03-22 14:34     ` Brian Foster
2018-03-21  3:21 ` [PATCH 11/9] xfs: flag inode corruption if parent ptr doesn't get us a real inode Darrick J. Wong
2018-03-22 14:34   ` Brian Foster
2018-03-22 17:49     ` Darrick J. Wong
2018-03-22 17:57   ` [PATCH v2 " Darrick J. Wong
2018-03-23 12:29     ` Brian Foster
2018-03-22  6:19 ` [PATCH 12/9] xfs: xfs_scrub_iallocbt_xref_rmap_inodes should use xref_set_corrupt Darrick J. Wong
2018-03-22 14:34   ` Brian Foster

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=20180321174254.GK11127@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=darrick.wong@oracle.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.