On Wed, Aug 03, 2016 at 02:16:27AM -0700, Christoph Hellwig wrote: > On Wed, Aug 03, 2016 at 08:39:50AM +1000, Dave Chinner wrote: > > Rather than have to make major changes to core infrastructure now, > > let's work this out as a separate patchset to clean up the rmap and > > reflink code in the next couple of releases. It's going to be better > > to get working code out there now under the experimental tag than it > > is is to keep it as an out of tree patchset for another cycle. > > The problm is that this does not only affect the rmap code (for which > I suspect it actually is fine), but also regresses the freed extent > logging. If you want minimal changes we should simply drop the patches > to move over the freed extent tracking to the new deferred ops > mechanism for now. I haven't said I want "minimal changes". What I don't want to do is make large, untested changes at the last minute, especially when there's no evidence that there is a regression caused by the code being changed. We need this kind of infrastructure for reflink, and there's no point rewriting the rmap code to remove it just to then have to immediately re-introduce it back into to the rmap code so it works with reflink. It's pointless churn and invalidates all the testing we've done on rmap up to this point. I don't understand what problem or regression you think this code causes, mainly because it hasn't been explained or demonstrated. I don't understand the solution being proposed, because the little that has been described tells me nothing about what the problem it solves is, nor how it solves the atomicity and ordering requirements that updating the bmbt, refcountbt and rmapbt have. I have no issues with fixing the code if it is broken, but first everyone needs to undestand how it is broken. It's time to make progress on this functionality - if we wait for the code to be perfect before it gets merged, we'll never make any progress. So, please explain in more detail what the problem is and what the proposed solution is so I (and probably Darrick, too) have some understanding of the issue you see with this code. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
