Re: [PATCH v4 4/5] squashfs: Add zstd support

[Phillip Lougher]

On Sun, Aug 13, 2017 at 9:31 AM, Geert Uytterhoeven
 wrote:
> On Fri, Aug 4, 2017 at 10:19 PM, Nick Terrell  wrote:
>> --- a/fs/squashfs/decompressor.c
>> +++ b/fs/squashfs/decompressor.c
>> @@ -65,6 +65,12 @@ static const struct squashfs_decompressor 
>> squashfs_zlib_comp_ops = {
>>  };
>>  #endif
>>
>> +#ifndef CONFIG_SQUASHFS_ZSTD
>> +static const struct squashfs_decompressor squashfs_zstd_comp_ops = {
>> +   NULL, NULL, NULL, NULL, ZSTD_COMPRESSION, "zstd", 0
>
> Can you please use designated initializers?

I prefer it as it is.  It matches the coding style that I used in the
rest of that file to declare the "unsupported" compressor entries (if
this patch uses a different style it would look odd).There's no
pointers to functions being assigned here, and it makes it a short and
concise one-line.

Phillip

>
>> +};
>> +#endif
>> +
>>  static const struct squashfs_decompressor squashfs_unknown_comp_ops = {
>> NULL, NULL, NULL, NULL, 0, "unknown", 0
>> ;
>
>> +const struct squashfs_decompressor squashfs_zstd_comp_ops = {
>> +   .init = zstd_init,
>> +   .free = zstd_free,
>> +   .decompress = zstd_uncompress,
>> +   .id = ZSTD_COMPRESSION,
>> +   .name = "zstd",
>> +   .supported = 1
>> +};
>
> ... like you did here.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
--
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

Re: [PATCH v4 4/5] squashfs: Add zstd support

[Geert Uytterhoeven]

On Fri, Aug 4, 2017 at 10:19 PM, Nick Terrell  wrote:
> --- a/fs/squashfs/decompressor.c
> +++ b/fs/squashfs/decompressor.c
> @@ -65,6 +65,12 @@ static const struct squashfs_decompressor 
> squashfs_zlib_comp_ops = {
>  };
>  #endif
>
> +#ifndef CONFIG_SQUASHFS_ZSTD
> +static const struct squashfs_decompressor squashfs_zstd_comp_ops = {
> +   NULL, NULL, NULL, NULL, ZSTD_COMPRESSION, "zstd", 0

Can you please use designated initializers?

> +};
> +#endif
> +
>  static const struct squashfs_decompressor squashfs_unknown_comp_ops = {
> NULL, NULL, NULL, NULL, 0, "unknown", 0
> ;

> +const struct squashfs_decompressor squashfs_zstd_comp_ops = {
> +   .init = zstd_init,
> +   .free = zstd_free,
> +   .decompress = zstd_uncompress,
> +   .id = ZSTD_COMPRESSION,
> +   .name = "zstd",
> +   .supported = 1
> +};

... like you did here.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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

Re: [PATCH v4 4/5] squashfs: Add zstd support

[Sean Purcell]

Signed-off-by: Sean Purcell 

On Fri, Aug 4, 2017 at 4:19 PM, Nick Terrell  wrote:
> Add zstd compression and decompression support to SquashFS. zstd is a
> great fit for SquashFS because it can compress at ratios approaching xz,
> while decompressing twice as fast as zlib. For SquashFS in particular,
> it can decompress as fast as lzo and lz4. It also has the flexibility
> to turn down the compression ratio for faster compression times.
>
> The compression benchmark is run on the file tree from the SquashFS archive
> found in ubuntu-16.10-desktop-amd64.iso [1]. It uses `mksquashfs` with the
> default block size (128 KB) and and various compression algorithms/levels.
> xz and zstd are also benchmarked with 256 KB blocks. The decompression
> benchmark times how long it takes to `tar` the file tree into `/dev/null`.
> See the benchmark file in the upstream zstd source repository located under
> `contrib/linux-kernel/squashfs-benchmark.sh` [2] for details.
>
> I ran the benchmarks on a Ubuntu 14.04 VM with 2 cores and 4 GiB of RAM.
> The VM is running on a MacBook Pro with a 3.1 GHz Intel Core i7 processor,
> 16 GB of RAM, and a SSD.
>
> | Method | Ratio | Compression MB/s | Decompression MB/s |
> ||---|--||
> | gzip   |  2.92 |   15 |128 |
> | lzo|  2.64 |  9.5 |217 |
> | lz4|  2.12 |   94 |218 |
> | xz |  3.43 |  5.5 | 35 |
> | xz 256 KB  |  3.53 |  5.4 | 40 |
> | zstd 1 |  2.71 |   96 |210 |
> | zstd 5 |  2.93 |   69 |198 |
> | zstd 10|  3.01 |   41 |225 |
> | zstd 15|  3.13 | 11.4 |224 |
> | zstd 16 256 KB |  3.24 |  8.1 |210 |
>
> This patch was written by Sean Purcell , but I will be
> taking over the submission process.
>
> [1] http://releases.ubuntu.com/16.10/
> [2] 
> https://github.com/facebook/zstd/blob/dev/contrib/linux-kernel/squashfs-benchmark.sh
>
> zstd source repository: https://github.com/facebook/zstd
>
> Cc: Sean Purcell 
> Signed-off-by: Nick Terrell 
> ---
> v3 -> v4:
> - Fix minor linter warnings
>
>  fs/squashfs/Kconfig|  14 +
>  fs/squashfs/Makefile   |   1 +
>  fs/squashfs/decompressor.c |   7 +++
>  fs/squashfs/decompressor.h |   4 ++
>  fs/squashfs/squashfs_fs.h  |   1 +
>  fs/squashfs/zstd_wrapper.c | 149 
> +
>  6 files changed, 176 insertions(+)
>  create mode 100644 fs/squashfs/zstd_wrapper.c
>
> diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
> index ffb093e..1adb334 100644
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -165,6 +165,20 @@ config SQUASHFS_XZ
>
>   If unsure, say N.
>
> +config SQUASHFS_ZSTD
> +   bool "Include support for ZSTD compressed file systems"
> +   depends on SQUASHFS
> +   select ZSTD_DECOMPRESS
> +   help
> + Saying Y here includes support for reading Squashfs file systems
> + compressed with ZSTD compression.  ZSTD gives better compression 
> than
> + the default ZLIB compression, while using less CPU.
> +
> + ZSTD is not the standard compression used in Squashfs and so most
> + file systems will be readable without selecting this option.
> +
> + If unsure, say N.
> +
>  config SQUASHFS_4K_DEVBLK_SIZE
> bool "Use 4K device block size?"
> depends on SQUASHFS
> diff --git a/fs/squashfs/Makefile b/fs/squashfs/Makefile
> index 246a6f3..6655631 100644
> --- a/fs/squashfs/Makefile
> +++ b/fs/squashfs/Makefile
> @@ -15,3 +15,4 @@ squashfs-$(CONFIG_SQUASHFS_LZ4) += lz4_wrapper.o
>  squashfs-$(CONFIG_SQUASHFS_LZO) += lzo_wrapper.o
>  squashfs-$(CONFIG_SQUASHFS_XZ) += xz_wrapper.o
>  squashfs-$(CONFIG_SQUASHFS_ZLIB) += zlib_wrapper.o
> +squashfs-$(CONFIG_SQUASHFS_ZSTD) += zstd_wrapper.o
> diff --git a/fs/squashfs/decompressor.c b/fs/squashfs/decompressor.c
> index d2bc136..8366398 100644
> --- a/fs/squashfs/decompressor.c
> +++ b/fs/squashfs/decompressor.c
> @@ -65,6 +65,12 @@ static const struct squashfs_decompressor 
> squashfs_zlib_comp_ops = {
>  };
>  #endif
>
> +#ifndef CONFIG_SQUASHFS_ZSTD
> +static const struct squashfs_decompressor squashfs_zstd_comp_ops = {
> +   NULL, NULL, NULL, NULL, ZSTD_COMPRESSION, "zstd", 0
> +};
> +#endif
> +
>  static const struct squashfs_decompressor squashfs_unknown_comp_ops = {
> NULL, NULL, NULL, NULL, 0, "unknown", 0
>  };
> @@ -75,6 +81,7 @@ static const struct squashfs_decompressor *decompressor[] = 
> {
> _lzo_comp_ops,
> _xz_comp_ops,
> _lzma_unsupported_comp_ops,
> +   _zstd_comp_ops,
> _unknown_comp_ops

Re: [PATCH v4 4/5] squashfs: Add zstd support

[Nick Terrell]

On 8/4/17, 3:10 PM, "linus...@gmail.com on behalf of Linus Torvalds" 
 wrote:
> On Fri, Aug 4, 2017 at 1:19 PM, Nick Terrell  wrote:
> >
> > This patch was written by Sean Purcell , but I will be
> > taking over the submission process.
> 
> Please, if so, get Sean's sign-off, and also make sure that the patch
> gets submitted with
> 
>From: Sean Purcell 
> 
> at the top of the body of the email so that authorship gets properly
> attributed by all the usual tools.
> 
>  Linus
> 

Thanks for the help, I'll fix it for the next version.

Re: [PATCH v4 4/5] squashfs: Add zstd support

[Linus Torvalds]

On Fri, Aug 4, 2017 at 1:19 PM, Nick Terrell  wrote:
>
> This patch was written by Sean Purcell , but I will be
> taking over the submission process.

Please, if so, get Sean's sign-off, and also make sure that the patch
gets submitted with

   From: Sean Purcell 

at the top of the body of the email so that authorship gets properly
attributed by all the usual tools.

 Linus
--
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