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

[Dave Chinner <david@fromorbit.com>]

ping?

On Tue, Apr 06, 2010 at 06:53:04PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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@redhat.com>
> ---
>  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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs