xfs
[Top] [All Lists]

Re: [PATCH] xfs: more swap extent fixes for dynamic fork offsets

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH] xfs: more swap extent fixes for dynamic fork offsets
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 13 Apr 2010 15:05:13 +1000
In-reply-to: <1270543984-20598-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1270543984-20598-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
ping?

On Tue, Apr 06, 2010 at 06:53:04PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> A new xfsqa test (226) with a prototype xfs_fsr change to try to
> handle dynamic fork offsets better triggers an assertion failure
> where the inode data fork is in btree format, yet there is room in
> the inode for it to be in extent format. The two inodes look like:
> 
> before: ino 0x101 (target), num_extents 11, Max in-fork extents 6, broot size 
> 40, fork offset 96
> before: ino 0x115 (temp),  num_extents 5, Max in-fork extents 3, broot size 
> 40, fork offset 56
> after: ino 0x101 (target), num_extents 5, Max in-fork extents 6, broot size 
> 40, fork offset 96
> after: ino 0x115 (temp), num_extents 11, Max in-fork extents 3, broot size 
> 40, fork offset 56
> 
> Basically the target inode ends up with 5 extents in btree format,
> but it had space for 6 extents in extent format, so ends up
> incorrect. Notably here the broot size is the same, and that is
> where the kernel code is going wrong - the btree root will fit, so
> it lets the swap go ahead.
> 
> The check should not allow the swap to take place if the number of
> extents while in btree format is less than the number of extents
> that can fit in the inode in extent format. Adding that check will
> prevent this swap and corruption from occurring.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_dfrag.c |   22 ++++++++++++++++------
>  1 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
> index cd27c9d..5bba29a 100644
> --- a/fs/xfs/xfs_dfrag.c
> +++ b/fs/xfs/xfs_dfrag.c
> @@ -177,16 +177,26 @@ xfs_swap_extents_check_format(
>           XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) > tip->i_df.if_ext_max)
>               return EINVAL;
>  
> -     /* Check root block of temp in btree form to max in target */
> +     /*
> +      * If we are in a btree format, check that the temp root block will fit
> +      * in the target and that it has enough extents to be in btree format
> +      * in the target.
> +      *
> +      * Note that we have to be careful to allow btree->extent conversions
> +      * (a common defrag case) which will occur when the temp inode is in
> +      * extent format...
> +      */
>       if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE &&
> -         XFS_IFORK_BOFF(ip) &&
> -         tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip))
> +         ((XFS_IFORK_BOFF(ip) &&
> +           tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip)) ||
> +          XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <= ip->i_df.if_ext_max))
>               return EINVAL;
>  
> -     /* Check root block of target in btree form to max in temp */
> +     /* Reciprocal target->temp btree format checks */
>       if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE &&
> -         XFS_IFORK_BOFF(tip) &&
> -         ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip))
> +         ((XFS_IFORK_BOFF(tip) &&
> +           ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip)) ||
> +          XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <= tip->i_df.if_ext_max))
>               return EINVAL;
>  
>       return 0;
> -- 
> 1.6.5
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

-- 
Dave Chinner
david@xxxxxxxxxxxxx

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