xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfs: bmapbt checking on debug kernels too expensive

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfs: bmapbt checking on debug kernels too expensive
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 7 Jan 2016 07:53:14 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1452123087-13892-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1452123087-13892-1-git-send-email-david@xxxxxxxxxxxxx> <1452123087-13892-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.24 (2015-08-30)
On Thu, Jan 07, 2016 at 10:31:27AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> For large sparse or fragmented files, checking every single entry in
> the bmapbt on every operation is prohibitively expensive. Especially
> as such checks rarely discover problems during normal operations on
> high extent coutn files. Our regression tests don't tend to exercise

              count

> files with hundreds of thousands to millions of extents, so mostly
> this isn't noticed.
> 
> However, trying to run things like xfs_mdrestore of large filesystem
> dumps on a debug kernel quickly becomes impossible as the CPU is
> completely burnt up repeatedly walking the sparse file bmapbt that
> is generated for every allocation that is made.
> 
> Hence, if the file has more than 10,000 extents, just don't bother
> with walking the tree to check it exhaustively. The btree code has
> checks that ensure that the newly inserted/removed/modified record
> is correctly ordered, so the entrie tree walk in thses cases has

                                                   these

> limited additional value.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 7388495..bc7e7d5 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -325,9 +325,11 @@ xfs_check_block(
>  
>  /*
>   * Check that the extents for the inode ip are in the right order in all
> - * btree leaves.
> + * btree leaves. THis becomes prohibitively expensive for large extent count

                    This

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

> + * files, so don't bother with inodes that have more than 10,000 extents in
> + * them. The btree record ordering checks will still be done, so for such 
> large
> + * bmapbt constructs that is going to catch most corruptions.
>   */
> -
>  STATIC void
>  xfs_bmap_check_leaf_extents(
>       xfs_btree_cur_t         *cur,   /* btree cursor or null */
> @@ -352,6 +354,10 @@ xfs_bmap_check_leaf_extents(
>               return;
>       }
>  
> +     /* skip large extent count inodes */
> +     if (ip->i_d.di_nextents > 10000)
> +             return;
> +
>       bno = NULLFSBLOCK;
>       mp = ip->i_mount;
>       ifp = XFS_IFORK_PTR(ip, whichfork);
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>