Re: [PATCH 10/14] xfs: minimize impact to non-reflink files via reflink per-inode flag

[Dave Chinner <david@fromorbit.com>]

On Thu, Jun 25, 2015 at 04:40:16PM -0700, Darrick J. Wong wrote:
> Gate all the reflink functions (which generally involve an expensive
> trip to the reflink btree) on an inode flag which is applied to both
> inodes at reflink time.  This minimizes reflink's impact on non-CoW
> files.

Ah, I see you do this reflink inode flag here. This should be one of
the first patches, not the last.  i.e. the patch series should
build up all the supporting infrastructure in individual patches
before adding any of the actual reflink implementation....

Also, the flag needs to go into the di_flags2 field, as the last
flag in the di_flags field is reserved for a "more flags" flag if we
ever need to add more flags to a v2 inode in a v4 filesystem...

> +/*
> + * xfs_is_reflink_inode() -- Decide if an inode needs to be checked for CoW.
> + *
> + * @ip: XFS inode
> + */
> +bool
> +xfs_is_reflink_inode(
> +	struct xfs_inode	*ip)		/* XFS inode */
> +{
> +	struct xfs_mount	*mp = ip->i_mount;
> +
> +	if (!xfs_sb_version_hasreflink(&mp->m_sb))
> +		return false;
> +	if (!(ip->i_d.di_flags & XFS_DIFLAG_REFLINK))
> +		return false;
> +
> +	ASSERT(!XFS_IS_REALTIME_INODE(ip));
> +	return true;

I would have thought you only need to check the inode flag here
because the only time it will be set is on a reflink enabled
filesystem. i.e. that flag being set implies we've already done
all the "reflink is supported in this filesystem and it's not a
realtime file" checks when setting the flag.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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