On Fri, Sep 01, 2017 at 08:19:52AM +1000, Dave Chinner wrote:
> On Wed, Aug 30, 2017 at 11:40:33PM -0700, Darrick J. Wong wrote:
> > On Wed, Aug 30, 2017 at 04:54:00PM +0200, Christoph Hellwig wrote:
> > > But reject reflink + DAX file systems for now until the code to
> > > support reflinks on DAX is actually implemented.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > > ---
> > > fs/xfs/xfs_super.c | 8 +++-----
> > > 1 file changed, 3 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index 38aaacdbb8b3..92521032468e 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -1634,7 +1634,9 @@ xfs_fs_fill_super(
> > > }
> > > if (xfs_sb_version_hasreflink(&mp->m_sb))
> > > xfs_alert(mp,
> > > - "DAX and reflink have not been tested together!");
> > > + "DAX and reflink can not be used together!");
> > > + error = -EINVAL;
> > > + goto out_filestream_unmount;
> >
> > /This/ hunk seems fine, but...
> >
> > > }
> > >
> > > if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
> > > @@ -1648,10 +1650,6 @@ xfs_fs_fill_super(
> > > "EXPERIMENTAL reverse mapping btree feature enabled. Use at your own risk!");
> > > }
> > >
> > > - if (xfs_sb_version_hasreflink(&mp->m_sb))
> > > - xfs_alert(mp,
> > > - "EXPERIMENTAL reflink feature enabled. Use at your own risk!");
> > > -
> >
> > ...I frankly would rather wait until we land and stabilize the incore
> > extent rework, because I'd rather not have the reflink story be: 1) we
> > declare reflink stable in 4.14; 2) immediately people start loading up
> > XFSes with sparse VM images that blow up on high order allocations and
> > then 3) we get a whole lot of complaints about it. Then in 4.15 we 4)
> > land the incore extent map only now we're dragging those same users
> > through the mud while /that/ stabilizes, with the result 5) that
> > everyone thinks we've gone off the deep end and doesn't trust us
> > anymore.
>
> Which is a lose-lose proposition. IOWs, to be avoided at all costs.
>
> > I wish that didn't also mean waiting another 6 months for something that
> > none of us /developers/ have seen in practice,
>
> Oh, I've seen it plenty of times in practice. Restoring large
> metadumps with (tens of) millions of tiny extents is the common
> cause for me.
>
> > especially since the
> > enterprise distros will have plenty of time to backport all this stuff
> > before their next big releases if it /does/ become a problem. It's fine
> > enough for me (and Christoph's customers, evidently) but is that enough?
>
> FYI, this has been a problem on RHEL for a long time. It's a long
> standing, well known problem and that's one of the reasons why
> it's at the top of my list to fix right now.
>
> > <shrug> Not sure what to do about my own fear factor. :) Anyone want to
> > offer further comments? A quick git history trip says sparse inodes
> > took about 13 months to go from initial commit to EXPERIMENTAL removed?
>
> I always aimed to remove the experimental tag 4 kernel releases
> after the initial merge, assuming testing and required functionality
> was all there and complete early on in the stabilisation process.
> Generally I had a set of criteria that needed to be fulfilled before
> I'd remove the tag, and ticked them off as fixes were merged.
>
> e.g. CRCs were introduced in 3.10, the experimental tag was removed
> in 3.15, so 5 releases in that case. We didn't get CRCs working
> sanely until 3.11 and there was still a lot of userspace stuff that
> lagged (and having a fully working userspace was a criteria for
> removing experimental from the kernel code), so it took longer to
> stabilise than something smaller like sparse inodes.
>
> Fixing the extent list memory allocation problem was always
> something on my list of "need to fix before reflink can be deployed
> in anger", so I'd much prefer that we are conservative in removing
> the experimental tag. It doesn't hurt anyone if we keep it on until
> we get the extent tree rework merged, and it's better to err on the
> side of keeping it too long than not long enough.
Hmm, it's been 4 months since we last picked up this thread. We've
had 7 releases since reflink itself went upstream, which for most
features would be long enough to remove the EXPERIMENTAL tag for 4.16.
On the other hand, the extent list memory allocation fix series hasn't
had much time to stabilize, which could be a reason to leave the tag.
Thoughts?
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
