All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/4] xfs: remove xfs_bmbt_validate_extent
Date: Mon, 23 Oct 2017 08:41:19 -0700	[thread overview]
Message-ID: <20171023154119.GC5483@magnolia> (raw)
In-Reply-To: <20171023063017.11520-3-hch@lst.de>

On Mon, Oct 23, 2017 at 08:30:15AM +0200, Christoph Hellwig wrote:
> We have stop supporting file systems without unwritten extent bit support
> a long time ago, so remove the debug code for it which will get in the way
> of future changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c       |  8 +-------
>  fs/xfs/libxfs/xfs_bmap_btree.h | 14 --------------
>  fs/xfs/libxfs/xfs_inode_fork.c |  7 -------
>  3 files changed, 1 insertion(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 46813b71dd74..19ec8b1f99dd 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1166,8 +1166,7 @@ xfs_bmap_add_attrfork(
>  /*
>   * Read in the extents to if_extents.
>   * All inode fields are set up by caller, we just traverse the btree
> - * and copy the records in. If the file system cannot contain unwritten
> - * extents, the records are checked for no "state" flags.
> + * and copy the records in.
>   */
>  int					/* error */
>  xfs_bmap_read_extents(
> @@ -1255,11 +1254,6 @@ xfs_bmap_read_extents(
>  			xfs_bmbt_rec_host_t *trp = xfs_iext_get_ext(ifp, i);
>  			trp->l0 = be64_to_cpu(frp->l0);
>  			trp->l1 = be64_to_cpu(frp->l1);
> -			if (!xfs_bmbt_validate_extent(mp, whichfork, trp)) {
> -				XFS_ERROR_REPORT("xfs_bmap_read_extents(2)",
> -						 XFS_ERRLEVEL_LOW, mp);
> -				goto error0;
> -			}
>  			trace_xfs_read_extent(ip, i, state, _THIS_IP_);
>  		}
>  		xfs_trans_brelse(tp, bp);
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.h b/fs/xfs/libxfs/xfs_bmap_btree.h
> index 6f891eeb88f6..82d397de8e00 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.h
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.h
> @@ -123,18 +123,4 @@ extern int xfs_bmbt_change_owner(struct xfs_trans *tp, struct xfs_inode *ip,
>  extern struct xfs_btree_cur *xfs_bmbt_init_cursor(struct xfs_mount *,
>  		struct xfs_trans *, struct xfs_inode *, int);
>  
> -/*
> - * Check that the extent does not contain an invalid unwritten extent flag.
> - */
> -static inline bool xfs_bmbt_validate_extent(struct xfs_mount *mp, int whichfork,
> -		struct xfs_bmbt_rec_host *ep)
> -{
> -	if (ep->l0 >> (64 - BMBT_EXNTFLAG_BITLEN) == 0)
> -		return true;
> -	if (whichfork == XFS_DATA_FORK &&
> -	    xfs_sb_version_hasextflgbit(&mp->m_sb))
> -		return true;
> -	return false;

By removing this, I think we no longer report corruption if we encounter
'unwritten' attr fork extents, which (AFAIK) aren't allowed.

--D

> -}
> -
>  #endif	/* __XFS_BMAP_BTREE_H__ */
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index b1e69734c450..48a5dec360cd 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -373,11 +373,6 @@ xfs_iformat_extents(
>  			xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
>  			ep->l0 = get_unaligned_be64(&dp->l0);
>  			ep->l1 = get_unaligned_be64(&dp->l1);
> -			if (!xfs_bmbt_validate_extent(mp, whichfork, ep)) {
> -				XFS_ERROR_REPORT("xfs_iformat_extents(2)",
> -						 XFS_ERRLEVEL_LOW, mp);
> -				return -EFSCORRUPTED;
> -			}



>  			trace_xfs_read_extent(ip, i, state, _THIS_IP_);
>  		}
>  	}
> @@ -801,8 +796,6 @@ xfs_iextents_copy(
>  	for (i = 0; i < nrecs; i++) {
>  		xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
>  
> -		ASSERT(xfs_bmbt_validate_extent(ip->i_mount, whichfork, ep));
> -
>  		start_block = xfs_bmbt_get_startblock(ep);
>  		if (isnullstartblock(start_block)) {
>  			/*
> -- 
> 2.14.2
> 
> --
> 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:[~2017-10-23 15:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-23  6:30 even more extent mapping cleanups Christoph Hellwig
2017-10-23  6:30 ` [PATCH 1/4] xfs: add asserts for the mmap lock in xfs_{insert,collapse}_file_space Christoph Hellwig
2017-10-23 15:42   ` Darrick J. Wong
2017-10-23  6:30 ` [PATCH 2/4] xfs: remove xfs_bmbt_validate_extent Christoph Hellwig
2017-10-23 15:41   ` Darrick J. Wong [this message]
2017-10-24  7:54     ` Christoph Hellwig
2017-10-25  5:24       ` Darrick J. Wong
2017-10-25  6:19         ` Christoph Hellwig
2017-10-23  6:30 ` [PATCH 3/4] xfs: merge xfs_bmap_read_extents into xfs_iread_extents Christoph Hellwig
2017-10-23 23:19   ` Darrick J. Wong
2017-10-23  6:30 ` [PATCH 4/4] xfs: add a new xfs_iext_lookup_extent_before helper Christoph Hellwig
2017-10-23 23:32   ` Darrick J. Wong

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=20171023154119.GC5483@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=hch@lst.de \
    --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.