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
