On Wed, 27 Feb 2013 09:51:39 +0000
Russell King - ARM Linux <linux@...> wrote:

On Wed, Feb 27, 2013 at 04:36:47PM +0900, Kyungsik Lee wrote:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

That is pretty conclusive - it shows an 8% increase in image size vs a
66% increase in decompression speed. It will take a _lot_ to offset
that increase in decompression speed.

So, what I think is that yes, we should accept LZ4 and drop LZO from
the kernel - the "fast but may not be small" compression title has
clearly been taken by LZ4.

Akpm - what's your thoughts?

It sounds like we should merge both.

I've sent Linus a little reminder for Markus's 3.9 pull request. Let's
get down and review and test this new code?

David's review comments were useful.

I'd like to also see a Kconfig patch which makes x86 and arm kernels
default to the new LZ4 code. Then I can sneak that patch into
linux-next so the new code will get some testing. If we don't do that,
very few people will run it.

On Wed, Feb 27, 2013 at 09:39:47AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 12:16 -0500, Nicolas Pitre wrote:

On Wed, 27 Feb 2013, Joe Perches wrote:

On Wed, 2013-02-27 at 16:31 +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

We disagree.

I'm curious - what in your mind qualifies "significant value" ?

faster boot time. smaller, faster overall code.

Sorry, but you certainly successfully got me confused, and probably
others as well.

RMK says that "66% increase in decompression speed over LZO" is
significant. You apparently disagree with that.

Yeah, I can see how that can be interpreted.
I'm referring only to the new LZO.

I guess Russell has not reviewed the new LZO.

There is apparently no speed increase for LZ4 over
the new LZO.

Total claptrap. I've no idea where you're getting your data from, but
it's franky wrong and you're now being totally misleading to anyone
else reading this thread.

I explicitly asked for a comparison of the _new_ LZO vs the LZ4 code,
and this is what I received from Kyungsik Lee in this thread:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

And my statement of a "66% increase in speed" of LZ4 is comparing the
_new_ LZO code with unaligned access with the LZ4 code.

Now, you refer to Markus' results - but Markus' results do not say what
they're comparing - they don't say what the size of the compressed image
is, nor what the size of the uncompressed image was.

Now, Markus' results show a 42% increase in speed between the LZO-2012
and LZO-2013-UA versions (do the calculation yourself - I'm sure you're
capable of that? If not, we can turn this into a maths lesson too).
The above shows a 53% increase in speed between the existing LZO code
and the new LZO code with unaligned accesses.

_But_ the above shows an additional 66% increase between the new LZO
code with unaligned accesses and LZ4. Or, a whopping 150% increase
in speed over the _existing_ LZO code.

So please, stop stating what I have and have not reviewed. Unlike you,
I _have_ been following everything that's been said in this thread, and
- unlike you - I have analysed the figures put forward and drawn
conclusions which are fully supported by the published data from them,
and stated them - now many times.

On Wed, 27 Feb 2013, Joe Perches wrote:

On Wed, 2013-02-27 at 12:16 -0500, Nicolas Pitre wrote:

RMK says that "66% increase in decompression speed over LZO" is
significant. You apparently disagree with that.

Yeah, I can see how that can be interpreted.
I'm referring only to the new LZO.

I guess Russell has not reviewed the new LZO.

There is apparently no speed increase for LZ4 over
the new LZO.

I believe Markus has shown comparison testing in
this very thread.

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

Right.

Can the new LZO code be merged by Linus now? It has been sitting in
linux-next for quite some time. Afterwards we could revisit lz4
worthiness without all the present confusion.

BTW, I still wonder what that patch requiring ARM people approval is.


Nicolas

On Wed, 2013-02-27 at 12:16 -0500, Nicolas Pitre wrote:

On Wed, 27 Feb 2013, Joe Perches wrote:

On Wed, 2013-02-27 at 16:31 +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

We disagree.

I'm curious - what in your mind qualifies "significant value" ?

faster boot time. smaller, faster overall code.

Sorry, but you certainly successfully got me confused, and probably
others as well.

RMK says that "66% increase in decompression speed over LZO" is
significant. You apparently disagree with that.

Yeah, I can see how that can be interpreted.
I'm referring only to the new LZO.

I guess Russell has not reviewed the new LZO.

There is apparently no speed increase for LZ4 over
the new LZO.

I believe Markus has shown comparison testing in
this very thread.

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

Then you say that faster boot time is significant.

Increasing speed in incumbent code without adding
defects is always useful no?. Replacing incumbent
code with new code should be debated for utility.

I still think there's not much value in adding LZ4.
LZ4 is not not faster than LZO, it's just more code.

On Wed, Feb 27, 2013 at 09:04:48AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 16:31 +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

We disagree.

ROTFL.

I'm curious - what in your mind qualifies "significant value" ?

faster boot time. smaller, faster overall code.

ROTFL again! Because you've just disagreed with your above statement.
"66% increase in decompression speed" as far as I know _is_ "faster
boot time" !

Maybe "significant value" is a patch which buggily involves converting
all those "<n>" printk format strings in assembly files to KERN_* macros,
thereby breaking those strings because you've not paid attention to what
.asciz means? (Yes, I've just cleaned that crap up after you...)

If you mean commit 0cc41e4a21d43, perhaps you could clarify with an
example. I don't see any relevant changes by you in -next, but
maybe I'm not looking in the right spot.

While recently asking someone to enable VFP debugging, so I could help
sort out a problem they had reported, this is the debug output I was
greeted by thanks to your meddling:

[ 927.235546] \x01\x01\x01\x01\x01\x01\x01\x01
...
[ 927.241505] \x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01

Yes, really useful debug output isn't it? You can really see what's
going on there. These are coming from ultimately two commits - the
one you refer to above, which on its own would've changed the printk
string to be merely "<7>" - and the follow on commit changing the
way printk levels are dealt with.

The above output is produced by:

#define KERN_SOH "\001" /* ASCII Start Of Header */
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */

.asciz KERN_DEBUG "VFP: \str\n"

7.6 `.asciz "STRING"'...
========================

`.asciz' is just like `.ascii', but each string is followed by a zero
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
byte. The "z" in `.asciz' stands for "zero".
^^^^^

0000 01003700 5646503a 20696e73 74722025 ..7.VFP: instr %
^^ ^^
0010 30387820 70632025 30387820 73746174 08x pc %08x stat
0020 65202570 0a000100 37005646 503a2066 e %p....7.VFP: f
^^
...

That is: \x01 \x00 7 \x00 VFP: instr %08x pc %08x state %p \x00

See - three separately terminated strings because you changed:

.asciz "<7>VFP: \str\n"

to:

.asciz "<7>" "VFP: \str\n"

which turned it into _two_ separately NUL-terminated strings, and then
the follow-on changes to printk kern levels changed this to:

.asciz "\001" "7" "VFP: \str\n"

producing _three_ separately NUL-terminated strings.

The commit is not in mainline, nor linux-next, but in my tree as of
yesterday (e36815e2e), ready to be pushed out when I've finished working
on fixing other problems with VFP - or when I decide to push it out ready
for submission during this merge window.

The change did enable reducing code size.

??? Yea, right, meanwhile breaking the ability of stuff to produce
kernel messages.

Why would the LZO code not be updated?

I'm not saying that the LZO code should not be updated.

You said:

so that we can see whether it's worth updating the LZO code

Sounded as if you were doubtful to me.

_In_ the decompressor. We're talking about the _decompressor_ in
this thread.

On Wed, 27 Feb 2013, Joe Perches wrote:

On Wed, 2013-02-27 at 16:31 +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

We disagree.

I'm curious - what in your mind qualifies "significant value" ?

faster boot time. smaller, faster overall code.

Sorry, but you certainly successfully got me confused, and probably
others as well.

RMK says that "66% increase in decompression speed over LZO" is
significant. You apparently disagree with that.

Then you say that faster boot time is significant.

Again, can you (or anyone else) provide comprehensive test results in a
single email with both compression methods?


Nicolas

On Wed, 2013-02-27 at 16:31 +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

We disagree.

I'm curious - what in your mind qualifies "significant value" ?

faster boot time. smaller, faster overall code.

Maybe "significant value" is a patch which buggily involves converting
all those "<n>" printk format strings in assembly files to KERN_* macros,
thereby breaking those strings because you've not paid attention to what
.asciz means? (Yes, I've just cleaned that crap up after you...)

If you mean commit 0cc41e4a21d43, perhaps you could clarify with an
example. I don't see any relevant changes by you in -next, but
maybe I'm not looking in the right spot.

The change did enable reducing code size.

Why would the LZO code not be updated?

I'm not saying that the LZO code should not be updated.

You said:

so that we can see whether it's worth updating the LZO code

Sounded as if you were doubtful to me.

I'm saying that
the kernel boot time decompressor is not a play ground for an ever
increasing number of "my favourite compression method" crap.

Completely agree.

On Wed, Feb 27, 2013 at 04:31:18PM +0000, Russell King - ARM Linux wrote:

I'm not saying that the LZO code should not be updated. I'm saying
that the kernel boot time decompressor is not a play ground for an
ever increasing number of "my favourite compression method" crap.
We don't need four, five or even six compression methods there. We
just need three - a "fast but large", "small but slow" and "all round
popular medium".

Hell yeah!

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

On Wed, Feb 27, 2013 at 07:49:12AM -0800, Joe Perches wrote:

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Sorry, a 66% increase in decompression speed over the updated LZO code
isn't "significant value" ?

I'm curious - what in your mind qualifies "significant value" ?

Maybe "significant value" is a patch which buggily involves converting
all those "<n>" printk format strings in assembly files to KERN_* macros,
thereby breaking those strings because you've not paid attention to what
.asciz means? (Yes, I've just cleaned that crap up after you...)

Why would the LZO code not be updated?

I'm not saying that the LZO code should not be updated. I'm saying that
the kernel boot time decompressor is not a play ground for an ever
increasing number of "my favourite compression method" crap. We don't
need four, five or even six compression methods there. We just need
three - a "fast but large", "small but slow" and "all round popular
medium".

On Wed, 27 Feb 2013, Joe Perches wrote:

https://lkml.org/lkml/2013/1/29/145

Connecting to lkml.org (lkml.org)|87.253.128.182|:443... connected.
HTTP request sent, awaiting response... 500 Server Error

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

Please someone post a comprehensive comparison with all the results in
the same email.

The new LZO code is faster than ever and it's
a standalone improvement.

Why would the LZO code not be updated?

It is used by filesystems, etc. So of course it needs to be updated to
faster code.

Markus has posted what seems a clean git pull
request. It was not cc'd to arm or linux-arch.

http://linux-kernel.2935.n7.nabble.com/GIT-PULL-Update-LZO-compression-code-for-v3-9-td605184.html

Maybe a reminder should be sent to Linus about this.

From the above we can see this:

**LZO-2013-UA : updated LZO version available in linux-next plus
experimental ARM Unaligned Access patch. This needs approval from
some ARM maintainer ist NOT YET INCLUDED.

What is that experimental patch in need of approval?


Nicolas

On Wed, 2013-02-27 at 09:56 +0000, Russell King - ARM Linux wrote:

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

https://lkml.org/lkml/2013/1/29/145

I did not and do not see significant value in
adding LZ4 given Markus' LZO improvements.

I asked about LZO.

Why would the LZO code not be updated?

The new LZO code is faster than ever and it's
a standalone improvement.

Markus has posted what seems a clean git pull
request. It was not cc'd to arm or linux-arch.

http://linux-kernel.2935.n7.nabble.com/GIT-PULL-Update-LZO-compression-code-for-v3-9-td605184.html

On Wed, 27 Feb 2013, Johannes Stezenbach wrote:

On Wed, Feb 27, 2013 at 09:51:39AM +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 04:36:47PM +0900, Kyungsik Lee wrote:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

That is pretty conclusive - it shows an 8% increase in image size vs a
66% increase in decompression speed. It will take a _lot_ to offset
that increase in decompression speed.

So, what I think is that yes, we should accept LZ4 and drop LZO from
the kernel - the "fast but may not be small" compression title has
clearly been taken by LZ4.

I think LZO may be used by squashfs, jffs2 and btrfs, thus you
cannot drop it without breaking on disk storage formats.

It is not about dropping LZO from the kernel entirely. It's about
removing support for compressing zImage using LZO (and some others).
There is no compatibility issue as zImage embeds its own decompression
code.


Nicolas

On Wed, Feb 27, 2013 at 09:51:39AM +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 04:36:47PM +0900, Kyungsik Lee wrote:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

That is pretty conclusive - it shows an 8% increase in image size vs a
66% increase in decompression speed. It will take a _lot_ to offset
that increase in decompression speed.

So, what I think is that yes, we should accept LZ4 and drop LZO from
the kernel - the "fast but may not be small" compression title has
clearly been taken by LZ4.

I have read the comments regarding how many compressors the kernel
should support and understand that it can not support all the
compressors available.

However, I don't think that LZO can be replaced by LZ4 in all the
cases. The benchmark above shows only about improved decompression
speed.

Thanks,
Kyungsik

On Wed, Feb 27, 2013 at 09:51:39AM +0000, Russell King - ARM Linux wrote:

On Wed, Feb 27, 2013 at 04:36:47PM +0900, Kyungsik Lee wrote:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

That is pretty conclusive - it shows an 8% increase in image size vs a
66% increase in decompression speed. It will take a _lot_ to offset
that increase in decompression speed.

So, what I think is that yes, we should accept LZ4 and drop LZO from
the kernel - the "fast but may not be small" compression title has
clearly been taken by LZ4.

I think LZO may be used by squashfs, jffs2 and btrfs, thus you
cannot drop it without breaking on disk storage formats.

Johannes

On Tue, Feb 26, 2013 at 05:40:34PM -0800, Joe Perches wrote:

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

Please read the comments against the previous posting of these patches
where I first stated this argument - and with agreement from those
following the thread. The thread started on 26 Jan 2013. Thanks.

On Wed, Feb 27, 2013 at 04:36:47PM +0900, Kyungsik Lee wrote:

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

That is pretty conclusive - it shows an 8% increase in image size vs a
66% increase in decompression speed. It will take a _lot_ to offset
that increase in decompression speed.

So, what I think is that yes, we should accept LZ4 and drop LZO from
the kernel - the "fast but may not be small" compression title has
clearly been taken by LZ4.

Akpm - what's your thoughts?

On Tue, Feb 26, 2013 at 09:33:22PM +0100, Markus F.X.J. Oberhumer wrote:

On 2013-02-26 07:24, Kyungsik Lee wrote:

Hi,

[...]

Through the benchmark, it was found that -Os Compiler flag for
decompress.o brought better decompression performance in most of cases
(ex, different compiler and hardware spec.) in ARM architecture.

Lastly, CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is not always the best
option even though it is supported. The decompression speed can be
slightly slower in some cases.

This patchset is based on 3.8.

Any comments are appreciated.

Did you actually *try* the new LZO version and the patch (which is attached
once again) as explained in https://lkml.org/lkml/2013/2/3/367 ?

Because the new LZO version is faster than LZ4 in my testing, at least
when comparing apples with apples and enabling unaligned access in
BOTH versions:

armv7 (Cortex-A9), Linaro gcc-4.6 -O3, Silesia test corpus, 256 kB block-size:

compression speed decompression speed

LZO-2012 : 44 MB/sec 117 MB/sec no unaligned access
LZO-2013-UA : 47 MB/sec 167 MB/sec Unaligned Access
LZ4 r88 UA : 46 MB/sec 154 MB/sec Unaligned Access

I agree that the new LZO version provided shows better decompression
speed than 3.7 based. It is much improved especially for UA.

Compiler: Linaro ARM gcc 4.6.2
2. ARMv7, 1.7GHz based board
Kernel: linux 3.7
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.0MB 34.1MB/s Old
----------------------------------------
6.0MB 34.7MB/s New
6.0MB 52.2MB/s(UA)
=============================================
LZ4 6.5MB 86.7MB/s
UA: Unaligned memory Access support

One thing I can say that the code you may have used, guessing
"lz4demo" is not the same code provided in this patch.
It has been ported for the kernel and uses different function
not like the "lz4demo".

Thanks,
Kyungsik

On Tue, Feb 26, 2013 at 02:12:06PM +0100, David Sterba wrote:

On Tue, Feb 26, 2013 at 03:24:27PM +0900, Kyungsik Lee wrote:

This patch adds support for LZ4 decompression in the Linux Kernel.
LZ4 Decompression APIs for kernel are based on LZ4 implementation
by Yann Collet.

LZ4 homepage : http://fastcompression.blogspot.com/p/lz4.html
LZ4 source repository : http://code.google.com/p/lz4/

What SVN version did you use?

It's based on r88.

+/*
+ * LZ4_COMPRESSBOUND()
+ * Provides the maximum size that LZ4 may output in a "worst case" scenario
+ * (input data not compressible)
+ */
+#define LZ4_COMPRESSBOUND(isize) (isize + ((isize)/255) + 16)

For safety reasons I suggest to add a temporary variable to avoid double
evaluation of isize.

Yes, Good point.

--- /dev/null
+++ b/lib/lz4/lz4_decompress.c
+ }
+ cpy = op + length - (STEPSIZE - 4);
+ if (cpy > oend - COPYLENGTH) {
+
+ /* Error: request to write beyond destination buffer */
+ if (cpy > oend)
+ goto _output_error;
+ LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
+ while (op < cpy)
+ *op++ = *ref++;
+ op = cpy;
+ /*
+ * Check EOF (should never happen, since last 5 bytes
+ * are supposed to be literals)
+ */
+ if (op == oend)
+ goto _output_error;
+ continue;
+ }
+ LZ4_SECURECOPY(ref, op, cpy);
+ op = cpy; /* correction */
+ }

Does this compile? The } is an extra one, and does not match the
original sources.

It's about indent errors. The } is a pair of braces regarding
"while (1) {". However it compiled. It will be fixed.

+#define LZ4_COPYPACKET(s, d) \
+ do { \
+ LZ4_COPYSTEP(s, d); \
+ LZ4_COPYSTEP(s, d); \
+ } while (0)
+
+#define LZ4_SECURECOPY LZ4_WILDCOPY
+#endif
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
+ (d = s - get_unaligned_le16(p))
+#define LZ4_WILDCOPY(s, d, e) \
+ do { \
+ LZ4_COPYPACKET(s, d); \
+ } while (d < e)

All the \ at the ends of lines would look better aligned in one column.

Yes, right. It looks better if it's aligned.

Thanks,
Kyungsik

On Tue, 2013-02-26 at 22:10 +0000, Russell King - ARM Linux wrote:

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?

How could it be questionable that it's worth updating the LZO code?

On Tue, Feb 26, 2013 at 10:58:02PM +0100, Peter Korsgaard wrote:

"Nicolas" == Nicolas Pitre <nico@...> writes:

Hi,

>> Did you actually *try* the new LZO version and the patch (which is attached
>> once again) as explained in https://lkml.org/lkml/2013/2/3/367 ?
>>
>> Because the new LZO version is faster than LZ4 in my testing, at least
>> when comparing apples with apples and enabling unaligned access in
>> BOTH versions:
>>
>> armv7 (Cortex-A9), Linaro gcc-4.6 -O3, Silesia test corpus, 256 kB block-size:
>>
>> compression speed decompression speed
>>
>> LZO-2012 : 44 MB/sec 117 MB/sec no unaligned access
>> LZO-2013-UA : 47 MB/sec 167 MB/sec Unaligned Access
>> LZ4 r88 UA : 46 MB/sec 154 MB/sec Unaligned Access

Nicolas> To be fair, you should also take into account the compressed
Nicolas> size of a typical ARM kernel. Sometimes a slightly slower
Nicolas> decompressor may be faster overall if the compressed image to
Nicolas> work on is smaller.

Yes, but notice that lzo compressed BETTER than lz4 - E.G. from the
introduction mail:

1. ARMv7, 1.5GHz based board
Kernel: linux 3.4
Uncompressed Kernel Size: 14MB
Compressed Size Decompression Speed
LZO 6.7MB 21.1MB/s
LZ4 7.3MB 29.1MB/s, 45.6MB/s(UA)

Well, until someone can put all the pieces together so that a reasonably
meaningful test between:

- The new LZO code
- The new LZ4 code

then you're all comparing different things. TBH, I'm disappointed that
all the comments about this from the previous posting of LZ4 have been
totally ignored, and we _still_ don't really have this information. It
seems like replying to these threads is a waste of time.

So... for a selected kernel version of a particular size, can we please
have a comparison between the new LZO code and this LZ4 code, so that
we can see whether it's worth updating the LZO code or replacing the
LZO code with LZ4?