Re: Supporting reflinked/cow files in coreutils

[Jeff liu]

Hi Philipp,

Thanks for taking a look at this feature, and sorry for my late response!

Hi Jim,

Recently, I have discussed with a colleague(Bo Liu CC-ed) who is focus on Btrfs 
development, and we
decided to teach Btrfs aware of reflinked extents per file for du(1) purpose. :)


> Philipp Thomas wrote:
>> Jim,
>> 
>> * Jeff liu (address@hidden) [20110418 15:54]:
>> 
>>> FYI, I once implemented a similar patch to teach du(1) to show the disk
>>> footprint with reflinked stuff for OCFS2 only.
>>> 
>>> It is make use of rb-tree to figure out the shared extents and
>>> calculate the exactly used space eventually.
>>> http://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007293.html
>>> http://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007288.html
>>> http://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007287.html
>>> 
>>> I have not submited the patch set to Coreutils since I guess it is hard to
>>> accept by upstream as reflink and FIEMAP_EXTENT_SHARED flag are not spread
>>> over the general filesystems.
>> 
>> in this thread Jeff asked you on your opinions regarding such a patch set
>> but somehow you never responded. Could you possibly do so now as I would
>> still like something like this to be included upstream.
> 
> Hi Philipp,
> 
> Glancing through the patch in the first message,
> 
>    http://oss.oracle.com/pipermail/ocfs2-devel/2010-September/007293.html
> 
> I have a hard time justifying the addition of so much code for a
> relatively small feature that will be useful only for OCFS2 file systems:
> 
>  coreutils-6.9/src/du.c |  479 
> +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 473 insertions(+), 6 deletions(-)
> 
> That's a lot of new always-compiled code that will be used only by a
> very small fraction of users.  And to add insult to injury, it extends
> the ugly tentacles of FIEMAP into du.c.  It was bad enough to have to
> use it in copy.c, and we're all impatient for something better, but
> SEEK_HOLE and SEEK_DATA aren't yet widely available.
> 
> That's a lot of new code, not even counting these:
> 
>  * lib/rbtree.c: Source file of rbtree.
>  * lib/rbtree.h: Header file of rbtree.
>  * lib/Makefile.am (libcoreutils_a_SOURCES): Add both of them.
>  ---
>   coreutils-6.9/lib/Makefile.am |    3 +-
>   coreutils-6.9/lib/Makefile.in |    4 +-
>   coreutils-6.9/lib/rbtree.c    |  403 
> +++++++++++++++++++++++++++++++++++++++++
>   coreutils-6.9/lib/rbtree.h    |  143 +++++++++++++++
>   4 files changed, 550 insertions(+), 3 deletions(-)
> 
> Besides which, there is no reason to add new rbtree.[ch] code
> when there is already plenty of rbtree code in gnulib.
Yes. Originally, I wrote this patch set for a shared-du utility which only 
available on our Oracle Linux release for OCFS2 purpose, so I
didn't make use of rbtree code in gnulib at that time.

> 
> If it can be made to work cleanly with BTRFS as well, and preferably
> *without* FIEMAP (if possible, though I fear that FIEMAP is the only
> interface we have for now), that would be even better.
I also hate FIEMAP, however, for BTRFS, we have to make it works through FIEMAP 
as well...

> 
> If you and/or Jeff are motivated enough, a good first step would be to
> post a rebased patch to bug-coreutils, preferably after you've switched
> out that rbtree.[ch] for gnulib's code.  I hesitate even to suggest
> that you rebase, because we might end up deciding it's not yet worth
> the cost, but let's hear what others have to say.  Maybe enough people
> use du and are bothered because the space used by their reflinked files
> is over-reported.
 
We can find time to rebase it according to your suggestions if you like. 

> 
> A final question: why use an rbtree rather than a hash table?
> du is already using hash tables at the required scale, and new code
> would be required at all.

Consider scalability, I think we can use hash table, but rbtree is also works 
well, except bitmap.


Thanks,
-Jeff