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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

It looks to me like di_flags2 only exists in a v3 inode, and v3 inodes
only exist on v5 filesystems.  I don't really mind using di_flags2 for
reflink (on the off chance you want to use bit 15 of di_flags for a
v2 inode) but I'm wondering how is it possible to have di_flags on a v4 fs?

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

Yeah, probably these checks are all unnecessary.

--D

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

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



[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux