On 2017年09月05日 22:21, Hans van Kranenburg wrote:
On 09/05/2017 04:02 PM, Qu Wenruo wrote:


On 2017年09月05日 03:52, Timofey Titovets wrote:
2017-09-04 21:42 GMT+03:00 Adam Borowski <kilob...@angband.pl>:
On Mon, Sep 04, 2017 at 07:07:25PM +0300, Timofey Titovets wrote:
2017-09-04 18:11 GMT+03:00 Adam Borowski <kilob...@angband.pl>:
Here's an utility to measure used compression type + ratio on a set
of files
or directories: https://github.com/kilobyte/compsize

It should be of great help for users, and also if you:
* muck with compression levels
* add new compression types
* add heurestics that could err on withholding compression too much

Did a brief review, and the result looks quite good.
Especially same disk bytenr is handled well, so same file extent
referring to different part of the large extent won't get count twice.

Nice job.

But still some smaller improvement can be done:
(Please keep in mind I can go totally wrong since I'm not doing a
comprehensive review)

Search key min_type and max_type can be set to BTRFS_EXTENT_DATA_KEY,
which should filtered out unrelated results.

No, it does not.

https://patchwork.kernel.org/patch/9767619/

Why not?

Min key = ino, EXTENT_DATA, 0
Max key = ino, EXTENT_DATA, -1

With that min_key and max_key, the result is just what we want.

This also filtered out any item not belongs to this ino, and other things like XATTR or whatever.

Thanks,
Qu

And to improve readability, using BTRFS_SETGET_STACK_FUNCS() defined
functions will be a big improvement for reviewers.
(So I can check if the magic numbers are right or not, since I'm a lazy
bone and don't want to manually calculate the offset)


Packaged to AUR:
https://aur.archlinux.org/packages/compsize-git/

Nice, I don't even need to build it myself!
(Well, no much dependency anyway)


Cool!  I'd wait until people say the code is sane (I don't really
know these
ioctls) but if you want to make poor AUR folks our beta testers,
that's ok.

The code is sane!
And it even considered inline extent! (Which I didn't consider BTW as
inline extent counts as metadata, not data so my first thought just is
to just ignore them).


This just are too handy =>>
However, one issue: I did not set a license; your packaging says GPL3.
It would be better to have something compatible with btrfs-progs
which are
GPL2-only.  What about GPL2-or-higher?

Sorry for license, just copy-paste error, fixed

After adding some related info (like wasted space in pinned extents,
reuse
of extents), it'd be nice to have this tool inside btrfs-progs,
either as a
part of "fi du" or another command.

That will be useful =>
If improved, I think there is the chance to get it into btrfs-progs.

Thanks,
Qu


P.S.
your code work amazing fast on my ssd and data %)
150Gb data
-O0 2.12s
-O2 0.51s

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to