Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1708993

Re: [PATCH v5 2/5] lib: Add zstd modules

Show all headers | View raw

On 2017-08-10 15:25, Hugo Mills wrote:
> On Thu, Aug 10, 2017 at 01:41:21PM -0400, Chris Mason wrote:
>> On 08/10/2017 04:30 AM, Eric Biggers wrote:
>>>
>>> Theses benchmarks are misleading because they compress the whole file as a
>>> single stream without resetting the dictionary, which isn't how data will
>>> typically be compressed in kernel mode.  With filesystem compression the data
>>> has to be divided into small chunks that can each be decompressed independently.
>>> That eliminates one of the primary advantages of Zstandard (support for large
>>> dictionary sizes).
>>
>> I did btrfs benchmarks of kernel trees and other normal data sets as
>> well.  The numbers were in line with what Nick is posting here.
>> zstd is a big win over both lzo and zlib from a btrfs point of view.
>>
>> It's true Nick's patches only support a single compression level in
>> btrfs, but that's because btrfs doesn't have a way to pass in the
>> compression ratio.  It could easily be a mount option, it was just
>> outside the scope of Nick's initial work.
> 
>     Could we please not add more mount options? I get that they're easy
> to implement, but it's a very blunt instrument. What we tend to see
> (with both nodatacow and compress) is people using the mount options,
> then asking for exceptions, discovering that they can't do that, and
> then falling back to doing it with attributes or btrfs properties.
> Could we just start with btrfs properties this time round, and cut out
> the mount option part of this cycle.
AFAIUI, the intent is to extend the compression type specification for 
both the mount options and the property, not to add a new mount option. 
I think we all agree that `mount -o compress=zstd3` is a lot better than 
`mount -o compress=zstd,compresslevel=3`.
> 
>     In the long run, it'd be great to see most of the btrfs-specific
> mount options get deprecated and ultimately removed entirely, in
> favour of attributes/properties, where feasible.
Are properties set on the root subvolume inherited properly?  Because 
unless they are, we can't get the same semantics.

Two other counter arguments on completely removing BTRFS-specific mount 
options:
1. It's a lot easier and a lot more clearly defined to change things 
that affect global behavior of the FS by a remount than having to 
iterate everything in the FS to update properties.  If I'm disabling 
autodefrag, I'd much rather just `mount -o remount,noautodefrag` than 
`find / -xdev -exec btrfs property set \{\} autodefrag false`, as the 
first will take effect for everything simultaneously and run 
exponentially quicker.
2. There are some things that don't make sense as per-object settings or 
are otherwise nonsensical on objects.  Many, but not all, of the BTRFS 
specific mount options fall into this category IMO, with the notable 
exception of compress[-force], [no]autodefrag, [no]datacow, and 
[no]datasum.  Some other options do make sense as properties of the 
filesystem (commit, flushoncommit, {inode,space}_cache, max_inline, 
metadata_ratio, [no]ssd, and [no]treelog are such options), but many are 
one-off options that affect behavior on mount (like skip_balance, 
clear_cache, nologreplay, norecovery, usebbackuproot, and subvol).

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v5 0/5] Add xxhash and zstd modules Nick Terrell <terrelln@fb.com> - 2017-08-10 04:40 +0200
  [PATCH v5 5/5] crypto: Add zstd support Nick Terrell <terrelln@fb.com> - 2017-08-10 04:50 +0200
  [PATCH v5 4/5] squashfs: Add zstd support Nick Terrell <terrelln@fb.com> - 2017-08-10 04:50 +0200
  Re: [PATCH v5 2/5] lib: Add zstd modules Eric Biggers <ebiggers3@gmail.com> - 2017-08-10 10:40 +0200
    Re: [PATCH v5 2/5] lib: Add zstd modules "Austin S. Hemmelgarn" <ahferroin7@gmail.com> - 2017-08-10 13:40 +0200
      Re: [PATCH v5 2/5] lib: Add zstd modules "Austin S. Hemmelgarn" <ahferroin7@gmail.com> - 2017-08-10 17:00 +0200
        Re: [PATCH v5 2/5] lib: Add zstd modules Eric Biggers <ebiggers3@gmail.com> - 2017-08-10 19:40 +0200
      Re: [PATCH v5 2/5] lib: Add zstd modules Eric Biggers <ebiggers3@gmail.com> - 2017-08-10 19:30 +0200
        Re: [PATCH v5 2/5] lib: Add zstd modules "Austin S. Hemmelgarn" <ahferroin7@gmail.com> - 2017-08-10 19:50 +0200
          Re: [PATCH v5 2/5] lib: Add zstd modules Nick Terrell <terrelln@fb.com> - 2017-08-10 21:30 +0200
    Re: [PATCH v5 2/5] lib: Add zstd modules Eric Biggers <ebiggers3@gmail.com> - 2017-08-10 21:10 +0200
    Re: [PATCH v5 2/5] lib: Add zstd modules Nick Terrell <terrelln@fb.com> - 2017-08-10 21:20 +0200
    Re: [PATCH v5 2/5] lib: Add zstd modules Hugo Mills <hugo@carfax.org.uk> - 2017-08-10 21:50 +0200
      Re: [PATCH v5 2/5] lib: Add zstd modules "Austin S. Hemmelgarn" <ahferroin7@gmail.com> - 2017-08-10 22:00 +0200

csiph-web