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

Groups > linux.kernel > #1545383 > unrolled thread

[PATCH 0/3] Update LZ4 compressor module

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Update LZ4 compressor module Sven Schmidt <4sschmid@informatik.uni-hamburg.de> - 2016-12-20 20:10 +0100
    Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4  v1.7.2. Joe Perches <joe@perches.com> - 2016-12-20 21:00 +0100
      Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2. Sven Schmidt <4sschmid@informatik.uni-hamburg.de> - 2016-12-21 21:20 +0100
        Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4  v1.7.2. Greg KH <gregkh@linuxfoundation.org> - 2016-12-22 18:40 +0100
          Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2. Sven Schmidt <4sschmid@informatik.uni-hamburg.de> - 2016-12-22 19:40 +0100
    Re: [PATCH 0/3] Update LZ4 compressor module Greg KH <gregkh@linuxfoundation.org> - 2016-12-22 18:30 +0100
      Re: [PATCH 0/3] Update LZ4 compressor module Sven Schmidt <4sschmid@informatik.uni-hamburg.de> - 2016-12-22 19:40 +0100
        Re: [PATCH 0/3] Update LZ4 compressor module Greg KH <gregkh@linuxfoundation.org> - 2016-12-23 22:00 +0100

#1545383 — [PATCH 0/3] Update LZ4 compressor module

This patchset is for updating the LZ4 compression module to a version based
on LZ4 v1.7.2 allowing to use the fast compression algorithm aka LZ4 fast
which provides an "acceleration" parameter as a tradeoff between
compression ratio and compression speed.

We will use LZ4 fast in order to support compression in lustre. LZ4 fast empowers
us to do client-side as well as server-side compression/decompression while
being able to provide appropriate parameters to enable users to tune lustre's
behaviour to obtain the best performance/compression/etc. on their behalf
(adapative compression).

Also, it will be useful for other users of LZ4 compression,
as with LZ4 fast it is possible to enable applications to use fast and/or high
compression depending of the usecase. E.g. a developer can use very
high compression (low acceleration) for sending data over a network with
limited rate of transmission or he trades the compression ratio for higher
compression speed.

LZ4 homepage: http://www.lz4.org/
LZ4 source repository: https://github.com/lz4/lz4
Source version: 1.7.2
Compression in lustre: http://wiki.lustre.org/Enhanced_Adaptive_Compression_in_Lustre

Benchmark (taken from [1], Core i5-4300U @1.9GHz):
----------------|--------------|----------------|----------
Compressor      | Compression  | Decompression  | Ratio
----------------|--------------|----------------|----------
memcpy          |  4200 MB/s   |  4200 MB/s     | 1.000
LZ4 fast 50     |  1080 MB/s   |  2650 MB/s     | 1.375
LZ4 fast 17     |   680 MB/s   |  2220 MB/s     | 1.607
LZ4 fast 5      |   475 MB/s   |  1920 MB/s     | 1.886
LZ4 default     |   385 MB/s   |  1850 MB/s     | 2.101

[1] http://fastcompression.blogspot.de/2015/04/sampling-or-faster-lz4.html

[PATCH 1/3] crypto: Change lz4 modules to work with new lz4
[PATCH 2/3] fs/pstore: fs/squashfs: Change lz4 compressor functions
[PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[toc] | [next] | [standalone]

#1545451 — Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
> The kernel module is inspired by the previous work by Chanho Min.
> The updated LZ4 module will not break existing code since there were alias
> methods added to ensure backwards compatibility.

[]

> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
[]
> @@ -1,87 +1,218 @@
>  #ifndef __LZ4_H__
>  #define __LZ4_H__
> +
>  /*
>   * LZ4 Kernel Interface
>   *
> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee <kyungsik.lee@lge.com>
> + * Copyright (C) 2016, Sven Schmidt <4sschmid@informatik.uni-hamburg.de>

Deleting copyright notices is poor form and shouldn't be done.

I didn't look at the rest

[toc] | [prev] | [next] | [standalone]

#1545984 — Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

On 12/20/2016 08:52 PM, Joe Perches wrote:
> On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
>[]
>
>> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
> []
>> @@ -1,87 +1,218 @@
>>  #ifndef __LZ4_H__
>>  #define __LZ4_H__
>> +
>>  /*
>>   * LZ4 Kernel Interface
>>   *
>> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee <kyungsik.lee@lge.com>
>> + * Copyright (C) 2016, Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
>
> Deleting copyright notices is poor form and shouldn't be done.
>
> I didn't look at the rest
>

Thanks Joe for your time. If you would take a look at the rest, you may notice
that I changed almost the whole file. That's why I also changed the copyright note.
If you think I should keep the original note, I will do that.

What I did change:
- I included additional information stating that the file is based on Yann Collets
original header file (especially in matters of the function comments)

Sven

[toc] | [prev] | [next] | [standalone]

#1546517 — Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

On Wed, Dec 21, 2016 at 09:04:02PM +0100, Sven Schmidt wrote:
> On 12/20/2016 08:52 PM, Joe Perches wrote:
> > On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
> >> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
> >> The kernel module is inspired by the previous work by Chanho Min.
> >> The updated LZ4 module will not break existing code since there were alias
> >> methods added to ensure backwards compatibility.
> >
> >[]
> >
> >> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
> > []
> >> @@ -1,87 +1,218 @@
> >>  #ifndef __LZ4_H__
> >>  #define __LZ4_H__
> >> +
> >>  /*
> >>   * LZ4 Kernel Interface
> >>   *
> >> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee <kyungsik.lee@lge.com>
> >> + * Copyright (C) 2016, Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
> >
> > Deleting copyright notices is poor form and shouldn't be done.
> >
> > I didn't look at the rest
> >
> 
> Thanks Joe for your time. If you would take a look at the rest, you may notice
> that I changed almost the whole file. That's why I also changed the copyright note.
> If you think I should keep the original note, I will do that.

You have to, please consult a lawyer if you have questions about
copyright issues.  Or better yet, there's a free presentation/course
online about copyright on the linuxfoundation.org website somewhere that
should answer all of your questions here.  I recommend taking a few
minutes and going through that.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1546535 — Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

On 12/22/2016 06:31 PM, Greg KH wrote:
>On Wed, Dec 21, 2016 at 09:04:02PM +0100, Sven Schmidt wrote:
>> On 12/20/2016 08:52 PM, Joe Perches wrote:
>> > On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
>> >> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> >> The kernel module is inspired by the previous work by Chanho Min.
>> >> The updated LZ4 module will not break existing code since there were alias
>> >> methods added to ensure backwards compatibility.
>> >
>> >[]
>> >
>> >> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
>> > []
>> >> @@ -1,87 +1,218 @@
>> >>  #ifndef __LZ4_H__
>> >>  #define __LZ4_H__
>> >> +
>> >>  /*
>> >>   * LZ4 Kernel Interface
>> >>   *
>> >> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee <kyungsik.lee@lge.com>
>> >> + * Copyright (C) 2016, Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
>> >
>> > Deleting copyright notices is poor form and shouldn't be done.
>> >
>> > I didn't look at the rest
>> >
>>
>> Thanks Joe for your time. If you would take a look at the rest, you may notice
>> that I changed almost the whole file. That's why I also changed the copyright note.
>> If you think I should keep the original note, I will do that.
>
>You have to, please consult a lawyer if you have questions about
>copyright issues.  Or better yet, there's a free presentation/course
>online about copyright on the linuxfoundation.org website somewhere that
>should answer all of your questions here.  I recommend taking a few
>minutes and going through that.
>
>thanks,
>
>greg k-h
>

Will do, thanks Greg!

[toc] | [prev] | [next] | [standalone]

#1546512

On Tue, Dec 20, 2016 at 07:53:09PM +0100, Sven Schmidt wrote:
> 
> This patchset is for updating the LZ4 compression module to a version based
> on LZ4 v1.7.2 allowing to use the fast compression algorithm aka LZ4 fast
> which provides an "acceleration" parameter as a tradeoff between
> compression ratio and compression speed.

But why do this?

> We will use LZ4 fast in order to support compression in lustre. LZ4 fast empowers
> us to do client-side as well as server-side compression/decompression while
> being able to provide appropriate parameters to enable users to tune lustre's
> behaviour to obtain the best performance/compression/etc. on their behalf
> (adapative compression).

We don't care about lustre, especially as it is not merged into the main
portion of the kernel tree :)

Seriously, work on fixing up the known issues in lustre before adding
additional features, I've only been saying this for a few _years_ now...

> Also, it will be useful for other users of LZ4 compression,
> as with LZ4 fast it is possible to enable applications to use fast and/or high
> compression depending of the usecase. E.g. a developer can use very
> high compression (low acceleration) for sending data over a network with
> limited rate of transmission or he trades the compression ratio for higher
> compression speed.

This whole patch series is broken, always test-build your code, there's
nothing we could do with these patches even if we wanted to :(

greg k-h

[toc] | [prev] | [next] | [standalone]

#1546533

On 12/22/2016 06:29 PM, Greg KH wrote:
> On Tue, Dec 20, 2016 at 07:53:09PM +0100, Sven Schmidt wrote:
>>
>> This patchset is for updating the LZ4 compression module to a version based
>> on LZ4 v1.7.2 allowing to use the fast compression algorithm aka LZ4 fast
>> which provides an "acceleration" parameter as a tradeoff between
>> compression ratio and compression speed.
>
> But why do this?
>
>> We will use LZ4 fast in order to support compression in lustre. LZ4 fast empowers
>> us to do client-side as well as server-side compression/decompression while
>> being able to provide appropriate parameters to enable users to tune lustre's
>> behaviour to obtain the best performance/compression/etc. on their behalf
>> (adapative compression).
>
> We don't care about lustre, especially as it is not merged into the main
> portion of the kernel tree :)
>
> Seriously, work on fixing up the known issues in lustre before adding
> additional features, I've only been saying this for a few _years_ now...
>

Hey Greg and thanks for your time. Actually, we're not the guys behind lustre,
hence I'd leave fixing the bugs in lustre to them ;)

I'm working with the research group for scientific computing on the University
of Hamburg on the German Climate Computing Centre. The research group is working
with high performance storage systems etc. In this case, we investigate
data reduction techniques in behalf of the increasing gap between computational speed,
network speed and storage capacity. That's why we're ultimately aiming
for compression support in lustre.

Initial studies have shown that an adequate compression ratio for scientific data
can be archieved using LZ4. Since the currently available version of LZ4
in the kernel is about three years old, we would love if you accept 
our work on getting a more current version into the kernel.

>> Also, it will be useful for other users of LZ4 compression,
>> as with LZ4 fast it is possible to enable applications to use fast and/or high
>> compression depending of the usecase. E.g. a developer can use very
>> high compression (low acceleration) for sending data over a network with
>> limited rate of transmission or he trades the compression ratio for higher
>> compression speed.
>
> This whole patch series is broken, always test-build your code, there's
> nothing we could do with these patches even if we wanted to :(
>
> greg k-h
>

I'm sorry, this is my first patchset, so please be kind :(
Already got rid of the issues on my local machine and reviewed the output from
the buildbots. Will send an updated patchset later!

Thanks
Sven

[toc] | [prev] | [next] | [standalone]

#1546971

On Thu, Dec 22, 2016 at 07:35:14PM +0100, Sven Schmidt wrote:
> On 12/22/2016 06:29 PM, Greg KH wrote:
> > On Tue, Dec 20, 2016 at 07:53:09PM +0100, Sven Schmidt wrote:
> >>
> >> This patchset is for updating the LZ4 compression module to a version based
> >> on LZ4 v1.7.2 allowing to use the fast compression algorithm aka LZ4 fast
> >> which provides an "acceleration" parameter as a tradeoff between
> >> compression ratio and compression speed.
> >
> > But why do this?
> >
> >> We will use LZ4 fast in order to support compression in lustre. LZ4 fast empowers
> >> us to do client-side as well as server-side compression/decompression while
> >> being able to provide appropriate parameters to enable users to tune lustre's
> >> behaviour to obtain the best performance/compression/etc. on their behalf
> >> (adapative compression).
> >
> > We don't care about lustre, especially as it is not merged into the main
> > portion of the kernel tree :)
> >
> > Seriously, work on fixing up the known issues in lustre before adding
> > additional features, I've only been saying this for a few _years_ now...
> >
> 
> Hey Greg and thanks for your time. Actually, we're not the guys behind lustre,
> hence I'd leave fixing the bugs in lustre to them ;)
> 
> I'm working with the research group for scientific computing on the University
> of Hamburg on the German Climate Computing Centre. The research group is working
> with high performance storage systems etc. In this case, we investigate
> data reduction techniques in behalf of the increasing gap between computational speed,
> network speed and storage capacity. That's why we're ultimately aiming
> for compression support in lustre.
> 
> Initial studies have shown that an adequate compression ratio for scientific data
> can be archieved using LZ4. Since the currently available version of LZ4
> in the kernel is about three years old, we would love if you accept 
> our work on getting a more current version into the kernel.

But the LZ4 code isn't really used much in the kernel today, so if you
change it, what is really going to benefit?  And if you want to archive
things, you aren't using the in-kernel lz4 code, right?

If you want to add support for compression for lustre, great, I suggest
working with those developers, but note, I can't take any new features
for lustre until that code is out of drivers/staging/.

> >> Also, it will be useful for other users of LZ4 compression,
> >> as with LZ4 fast it is possible to enable applications to use fast and/or high
> >> compression depending of the usecase. E.g. a developer can use very
> >> high compression (low acceleration) for sending data over a network with
> >> limited rate of transmission or he trades the compression ratio for higher
> >> compression speed.
> >
> > This whole patch series is broken, always test-build your code, there's
> > nothing we could do with these patches even if we wanted to :(
> >
> > greg k-h
> >
> 
> I'm sorry, this is my first patchset, so please be kind :(

Not a problem, just always test-build your patches, in series, so that
we don't get grumpy for obvious problems :)

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web