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

[Darrick J. Wong]

On Wed, Jul 01, 2015 at 11:58:43AM +1000, Dave Chinner wrote:
> 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....

Pardon all the dust, I figured that it'd be better to get all the patches
out for earlier review than to make everyone wait until I could get a
reasonable refactoring done once.

> 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...

Ok.

> > +/*
> > + * 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.

Sure.  The reason for so many ASSERTs everywhere is to help me check my
own sanity while cobbling together the first version.  I imagine I could
eliminate a lot of them, but since they all compile out on !XFS_DEBUG &&
!XFS_WARN, I didn't think it was a serious problem. :)

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david at fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs