Message ID | 512D1C12.4080109@oberhumer.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, 26 Feb 2013, 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 To be fair, you should also take into account the compressed size of a typical ARM kernel. Sometimes a slightly slower decompressor may be faster overall if the compressed image to work on is smaller. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "Nicolas" == Nicolas Pitre <nico@fluxnic.net> 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)
On Tue, 26 Feb 2013, Peter Korsgaard wrote: > >>>>> "Nicolas" == Nicolas Pitre <nico@fluxnic.net> 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) OK. If LZO is now faster than LZ4 while still compressing more then I have no argument. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 26, 2013 at 10:58:02PM +0100, Peter Korsgaard wrote: > >>>>> "Nicolas" == Nicolas Pitre <nico@fluxnic.net> 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? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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". -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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!
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 27 Feb 2013 09:51:39 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(removed Richard Purdie and Albin Tonnerre as their email addresses seem to be bounding) > 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: :) Meddling... You sound like one of those nameless villains on Scooby Doo. If only I had a cool nickname like Dave "Shaggy" Kliekamp. I guess you'd have to call me Velma. > [ 927.235546] \x01\x01\x01\x01\x01\x01\x01\x01 > ... > [ 927.241505] \x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01 > > 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". > ^^^^^ Yeah, sorry. I thought that the zero was after any concatenation like .c. Learned something. 'preciate that. Would have appreciated a polite "you broke it" email too. > ??? Yea, right, meanwhile breaking the ability of stuff to produce > kernel messages. Fortunately, that's the only .S instance. > > > > > > > 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. My opinion is it's useful to update LZO. cheers, Joe (aka: Velma) -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
commit 8745b927fcfcd6953ada9bd1220a73083db5948a Author: Markus F.X.J. Oberhumer <markus@oberhumer.com> Date: Mon Feb 4 02:26:14 2013 +0100 lib/lzo: huge LZO decompression speedup on ARM by using unaligned access Signed-off-by: Markus F.X.J. Oberhumer <markus@oberhumer.com> diff --git a/lib/lzo/lzo1x_decompress_safe.c b/lib/lzo/lzo1x_decompress_safe.c index 569985d..e3edc5f 100644 --- a/lib/lzo/lzo1x_decompress_safe.c +++ b/lib/lzo/lzo1x_decompress_safe.c @@ -72,9 +72,11 @@ copy_literal_run: COPY8(op, ip); op += 8; ip += 8; +# if !defined(__arm__) COPY8(op, ip); op += 8; ip += 8; +# endif } while (ip < ie); ip = ie; op = oe; @@ -159,9 +161,11 @@ copy_literal_run: COPY8(op, m_pos); op += 8; m_pos += 8; +# if !defined(__arm__) COPY8(op, m_pos); op += 8; m_pos += 8; +# endif } while (op < oe); op = oe; if (HAVE_IP(6)) { diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h index 5a4beb2..b230601 100644 --- a/lib/lzo/lzodefs.h +++ b/lib/lzo/lzodefs.h @@ -12,8 +12,14 @@ */ +#if 1 && defined(__arm__) && ((__LINUX_ARM_ARCH__ >= 6) || defined(__ARM_FEATURE_UNALIGNED)) +#define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1 +#define COPY4(dst, src) \ + * (u32 *) (void *) (dst) = * (const u32 *) (const void *) (src) +#else #define COPY4(dst, src) \ put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst)) +#endif #if defined(__x86_64__) #define COPY8(dst, src) \ put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))