Re: [PATCH v7 00/47] xfs: add reverse mapping support

Dave Chinner
2016-08-04T23:50:23Z

On Thu, Aug 04, 2016 at 08:48:45AM -0700, Darrick J. Wong wrote:
> On Wed, Aug 03, 2016 at 07:18:52PM -0700, Mark Fasheh wrote:
> > On Wed, Aug 03, 2016 at 05:58:43PM -0700, Darrick J. Wong wrote:
> > > On Wed, Aug 03, 2016 at 01:55:20PM -0700, Darrick J. Wong wrote:
> > > > On Wed, Aug 03, 2016 at 12:45:36PM -0700, Mark Fasheh wrote:
> > > > > Where can I the patches to enable dedupe_range on xfs? I tested your
> > > > > previous devel branch based on Linux v4.7-rc3 with duperemove
> > > > > (https://github.com/markfasheh/duperemove) and it worked extremely well -
> > > > > even handling some cases that btrfs still has issues with. I actually
> > > > > committed the code to enable xfs support in duperemove so anyone can test on
> > > > > xfs with the dedupe_range patches.
> > > > >
> > > > > I'd gladly test your latest patches by doing my usual 'large' duperemove
> > > > > tests once I can get ahold of the dedupe_range work :)
> > > >
> > > > Your best bets are probably the -experimental trees:
> > > > https://github.com/djwong/linux/commits/djwong-experimental
> > > > https://github.com/djwong/xfsprogs/commits/djwong-experimental
> > > >
> > > > I haven't updated them in a while because I've been busy trying
> > > > to get reverse-mapping (the start of those patchbombs) into 4.8.
> > > >
> > > > Just as a warning, don't put anything critical on those XFS filesystems
> > > > because there's going to be a disk format update between now and the
> > > > next time I post the patches because Dave and I decided to cache the
> > > > block counts for the new btrees in order to speed up mounting. I don't
> > > > anticipate having time to clean up my dev tree and push to github until
> > > > a week or two after the merge window closes.
> > >
> > > That said, all the craziness from the last two weeks (xfs_scrub sprint
> > > and the rmapbt review fixes) are now in the -wtf tree, which /should/
> > > behave. I've dumped everything there in completely not cleaned up
> > > format, but this does have the AGF btree block counter stuff I talked
> > > about above.
> > >
> > > https://github.com/djwong/linux/commits/djwong-wtf
> > > https://github.com/djwong/xfsprogs/commits/djwong-wtf
> >
> > Fantastic. Don't worry about corrupting data, I've been doing this long
> > enough to know to use this only on scratch partitions for now :) I'm in the
> > middle of a couple other tests but once I'm done there I'll grab those
> > branches and give them another spin. Typically I'm running through 800G-1TB
> > on these (with varying duplicated data percentages).
> >
> > Silly question which I could just answer by looking at the patches - are you
> > reporting FIEMAP_EXTENT_SHARED yet for extents with more than one owner? We
> > use that flag (by comparing SHARED bytes before and after dedupe) to give
> > people an estimate of how much space was saved. I presume figuring the
> > shared state of an extent would be as easy as querying the rmap btree,
> > correct?
>
> Yes, it should report FIEMAP_EXTENT_SHARED. In the past it would
> report exactly which parts of an extent were shared. If a file mapped
> blocks 30-40 and block 35 was shared, it would report 30-34, 35
> (shared), and 36-40. However, btrfs only reports a single extent
> "30-40 (shared)" so I stopped doing that.

I'd much prefer that fiemap gives exact information about shared
extents. FIEMAP is a diagnostic tool and as such we need it to
accurately reflect the exact extent map of the inode being queried
so we aren't mislead about the layout of the file during trouble
shooting.

Hence I think what btrfs does is sub-optimal and we shouldn't copy
it. Let's do the right thing with XFS and if people complain it is
different to btrfs then someone can go make btrfs report an accurate
extent map, too.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com