[PATCH v4 0/7] lib/lzo: performance improvements

[PATCH v4 0/7] lib/lzo: performance improvements

[Dave Rodgman]

This patch series introduces performance improvements for lzo.

The previous version of this patchset is here:
https://lkml.org/lkml/2018/11/30/807

This version of the patchset fixes a maybe-used-uninitialized warning
(although the previous version was still safe).

Dave

[PATCH 1/8] lib/lzo: tidy-up ifdefs

[Dave Rodgman]

Modify the ifdefs in lzodefs.h to be more consistent with normal kernel
macros (e.g., change __aarch64__ to CONFIG_ARM64).

Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzodefs.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index 4edefd2f540c..497f9c9f03a8 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -15,7 +15,7 @@
 
 #define COPY4(dst, src)	\
 		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
-#if defined(__x86_64__)
+#if defined(CONFIG_X86_64)
 #define COPY8(dst, src)	\
 		put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))
 #else
@@ -25,12 +25,12 @@
 
 #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
 #error "conflicting endian definitions"
-#elif defined(__x86_64__)
+#elif defined(CONFIG_X86_64)
 #define LZO_USE_CTZ64	1
 #define LZO_USE_CTZ32	1
-#elif defined(__i386__) || defined(__powerpc__)
+#elif defined(CONFIG_X86) || defined(CONFIG_PPC)
 #define LZO_USE_CTZ32	1
-#elif defined(__arm__) && (__LINUX_ARM_ARCH__ >= 5)
+#elif defined(CONFIG_ARM) && (__LINUX_ARM_ARCH__ >= 5)
 #define LZO_USE_CTZ32	1
 #endif
 
-- 
2.17.1

Re: [PATCH 1/8] lib/lzo: tidy-up ifdefs

[Markus F.X.J. Oberhumer]

Acked-by: Markus F.X.J. Oberhumer <markus@oberhumer.com>


On 2018-11-30 15:26, Dave Rodgman wrote:
> Modify the ifdefs in lzodefs.h to be more consistent with normal kernel
> macros (e.g., change __aarch64__ to CONFIG_ARM64).
> 
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Matt Sealey <matt.sealey@arm.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  lib/lzo/lzodefs.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index 4edefd2f540c..497f9c9f03a8 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -15,7 +15,7 @@
>  
>  #define COPY4(dst, src)	\
>  		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
> -#if defined(__x86_64__)
> +#if defined(CONFIG_X86_64)
>  #define COPY8(dst, src)	\
>  		put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))
>  #else
> @@ -25,12 +25,12 @@
>  
>  #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
>  #error "conflicting endian definitions"
> -#elif defined(__x86_64__)
> +#elif defined(CONFIG_X86_64)
>  #define LZO_USE_CTZ64	1
>  #define LZO_USE_CTZ32	1
> -#elif defined(__i386__) || defined(__powerpc__)
> +#elif defined(CONFIG_X86) || defined(CONFIG_PPC)
>  #define LZO_USE_CTZ32	1
> -#elif defined(__arm__) && (__LINUX_ARM_ARCH__ >= 5)
> +#elif defined(CONFIG_ARM) && (__LINUX_ARM_ARCH__ >= 5)
>  #define LZO_USE_CTZ32	1
>  #endif
>  
> 

-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 2/8] lib/lzo: clean-up by introducing COPY16

[Dave Rodgman]

From: Matt Sealey <matt.sealey@arm.com>

Most compilers should be able to merge adjacent loads/stores of sizes
which are less than but effect a multiple of a machine word size (in
effect a memcpy() of a constant amount). However the semantics of the
macro are that it just does the copy, the pointer increment is in the
code, hence we see

    *a = *b
    a += 8
    b += 8
    *a = *b
    a += 8
    b += 8

This introduces a dependency between the two groups of statements which
seems to defeat said compiler optimizers and generate some very strange
sequences of addition and subtraction of address offsets (i.e. it is
overcomplicated).

Since COPY8 is only ever used to copy amounts of 16 bytes (in pairs),
just define COPY16 as COPY8,COPY8. We leave the definition to preserve
the need to do unaligned accesses to machine-sized words per the
original code intent, we just don't use it in the code proper.

COPY16 then gives us code like:

    *a = *b
    *(a+8) = *(b+8)
    a += 16
    b += 16

This seems to allow compilers to generate much better code by using
base register writeback or simply positively incrementing offsets which
seems to positively affect performance. It is, at least, fewer
instructions to do the same job.

Link: http://lkml.kernel.org/r/20181127161913.23863-3-dave.rodgman@arm.com
Signed-off-by: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzo1x_compress.c        |  9 +++------
 lib/lzo/lzo1x_decompress_safe.c | 18 ++++++------------
 lib/lzo/lzodefs.h               |  3 +++
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
index 236eb21167b5..82fb5571ce5e 100644
--- a/lib/lzo/lzo1x_compress.c
+++ b/lib/lzo/lzo1x_compress.c
@@ -60,8 +60,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 				op += t;
 			} else if (t <= 16) {
 				*op++ = (t - 3);
-				COPY8(op, ii);
-				COPY8(op + 8, ii + 8);
+				COPY16(op, ii);
 				op += t;
 			} else {
 				if (t <= 18) {
@@ -76,8 +75,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 					*op++ = tt;
 				}
 				do {
-					COPY8(op, ii);
-					COPY8(op + 8, ii + 8);
+					COPY16(op, ii);
 					op += 16;
 					ii += 16;
 					t -= 16;
@@ -255,8 +253,7 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
 			*op++ = tt;
 		}
 		if (t >= 16) do {
-			COPY8(op, ii);
-			COPY8(op + 8, ii + 8);
+			COPY16(op, ii);
 			op += 16;
 			ii += 16;
 			t -= 16;
diff --git a/lib/lzo/lzo1x_decompress_safe.c b/lib/lzo/lzo1x_decompress_safe.c
index a1c387f6afba..aa95d3066b7d 100644
--- a/lib/lzo/lzo1x_decompress_safe.c
+++ b/lib/lzo/lzo1x_decompress_safe.c
@@ -86,12 +86,9 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
 					const unsigned char *ie = ip + t;
 					unsigned char *oe = op + t;
 					do {
-						COPY8(op, ip);
-						op += 8;
-						ip += 8;
-						COPY8(op, ip);
-						op += 8;
-						ip += 8;
+						COPY16(op, ip);
+						op += 16;
+						ip += 16;
 					} while (ip < ie);
 					ip = ie;
 					op = oe;
@@ -187,12 +184,9 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
 			unsigned char *oe = op + t;
 			if (likely(HAVE_OP(t + 15))) {
 				do {
-					COPY8(op, m_pos);
-					op += 8;
-					m_pos += 8;
-					COPY8(op, m_pos);
-					op += 8;
-					m_pos += 8;
+					COPY16(op, m_pos);
+					op += 16;
+					m_pos += 16;
 				} while (op < oe);
 				op = oe;
 				if (HAVE_IP(6)) {
diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index 497f9c9f03a8..e1b3cf6459a9 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -23,6 +23,9 @@
 		COPY4(dst, src); COPY4((dst) + 4, (src) + 4)
 #endif
 
+#define COPY16(dst, src) \
+	do { COPY8(dst, src); COPY8((dst) + 8, (src) + 8); } while (0)
+
 #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
 #error "conflicting endian definitions"
 #elif defined(CONFIG_X86_64)
-- 
2.17.1

Re: [PATCH 2/8] lib/lzo: clean-up by introducing COPY16

[Markus F.X.J. Oberhumer]

*NOT* acked by Markus Oberhumer, original author of the LZO data compression
library and author of the current Linux kernel LZO implementation.

Please see my email reply to the patch series header for more info.


On 2018-11-30 15:26, Dave Rodgman wrote:
> From: Matt Sealey <matt.sealey@arm.com>
> 
> Most compilers should be able to merge adjacent loads/stores of sizes
> which are less than but effect a multiple of a machine word size (in
> effect a memcpy() of a constant amount). However the semantics of the
> macro are that it just does the copy, the pointer increment is in the
> code, hence we see
> 
>     *a = *b
>     a += 8
>     b += 8
>     *a = *b
>     a += 8
>     b += 8
> 
> This introduces a dependency between the two groups of statements which
> seems to defeat said compiler optimizers and generate some very strange
> sequences of addition and subtraction of address offsets (i.e. it is
> overcomplicated).
> 
> Since COPY8 is only ever used to copy amounts of 16 bytes (in pairs),
> just define COPY16 as COPY8,COPY8. We leave the definition to preserve
> the need to do unaligned accesses to machine-sized words per the
> original code intent, we just don't use it in the code proper.
> 
> COPY16 then gives us code like:
> 
>     *a = *b
>     *(a+8) = *(b+8)
>     a += 16
>     b += 16
> 
> This seems to allow compilers to generate much better code by using
> base register writeback or simply positively incrementing offsets which
> seems to positively affect performance. It is, at least, fewer
> instructions to do the same job.
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-3-dave.rodgman@arm.com
> Signed-off-by: Matt Sealey <matt.sealey@arm.com>
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  lib/lzo/lzo1x_compress.c        |  9 +++------
>  lib/lzo/lzo1x_decompress_safe.c | 18 ++++++------------
>  lib/lzo/lzodefs.h               |  3 +++
>  3 files changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
> index 236eb21167b5..82fb5571ce5e 100644
> --- a/lib/lzo/lzo1x_compress.c
> +++ b/lib/lzo/lzo1x_compress.c
> @@ -60,8 +60,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  				op += t;
>  			} else if (t <= 16) {
>  				*op++ = (t - 3);
> -				COPY8(op, ii);
> -				COPY8(op + 8, ii + 8);
> +				COPY16(op, ii);
>  				op += t;
>  			} else {
>  				if (t <= 18) {
> @@ -76,8 +75,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  					*op++ = tt;
>  				}
>  				do {
> -					COPY8(op, ii);
> -					COPY8(op + 8, ii + 8);
> +					COPY16(op, ii);
>  					op += 16;
>  					ii += 16;
>  					t -= 16;
> @@ -255,8 +253,7 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
>  			*op++ = tt;
>  		}
>  		if (t >= 16) do {
> -			COPY8(op, ii);
> -			COPY8(op + 8, ii + 8);
> +			COPY16(op, ii);
>  			op += 16;
>  			ii += 16;
>  			t -= 16;
> diff --git a/lib/lzo/lzo1x_decompress_safe.c b/lib/lzo/lzo1x_decompress_safe.c
> index a1c387f6afba..aa95d3066b7d 100644
> --- a/lib/lzo/lzo1x_decompress_safe.c
> +++ b/lib/lzo/lzo1x_decompress_safe.c
> @@ -86,12 +86,9 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
>  					const unsigned char *ie = ip + t;
>  					unsigned char *oe = op + t;
>  					do {
> -						COPY8(op, ip);
> -						op += 8;
> -						ip += 8;
> -						COPY8(op, ip);
> -						op += 8;
> -						ip += 8;
> +						COPY16(op, ip);
> +						op += 16;
> +						ip += 16;
>  					} while (ip < ie);
>  					ip = ie;
>  					op = oe;
> @@ -187,12 +184,9 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
>  			unsigned char *oe = op + t;
>  			if (likely(HAVE_OP(t + 15))) {
>  				do {
> -					COPY8(op, m_pos);
> -					op += 8;
> -					m_pos += 8;
> -					COPY8(op, m_pos);
> -					op += 8;
> -					m_pos += 8;
> +					COPY16(op, m_pos);
> +					op += 16;
> +					m_pos += 16;
>  				} while (op < oe);
>  				op = oe;
>  				if (HAVE_IP(6)) {
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index 497f9c9f03a8..e1b3cf6459a9 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -23,6 +23,9 @@
>  		COPY4(dst, src); COPY4((dst) + 4, (src) + 4)
>  #endif
>  
> +#define COPY16(dst, src) \
> +	do { COPY8(dst, src); COPY8((dst) + 8, (src) + 8); } while (0)
> +
>  #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
>  #error "conflicting endian definitions"
>  #elif defined(CONFIG_X86_64)
> 


-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 3/8] lib/lzo: enable 64-bit CTZ on Arm

[Dave Rodgman]

From: Matt Sealey <matt.sealey@arm.com>

ARMv6 Thumb state introduced an RBIT instruction which, combined with CLZ
as present in ARMv5, introduces an extremely fast path for counting
trailing zeroes.

Enable the use of the GCC builtin for this on ARMv6+ with
CONFIG_THUMB2_KERNEL to ensure we get the 'new' instruction usage.

We do not bother enabling LZO_USE_CTZ64 support for ARMv5 as the builtin
code path does the same thing as the LZO_USE_CTZ32 code, only with more
register pressure.

Link: http://lkml.kernel.org/r/20181127161913.23863-4-dave.rodgman@arm.com
Signed-off-by: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzodefs.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index e1b3cf6459a9..c0193f726db0 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -33,9 +33,14 @@
 #define LZO_USE_CTZ32	1
 #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
 #define LZO_USE_CTZ32	1
-#elif defined(CONFIG_ARM) && (__LINUX_ARM_ARCH__ >= 5)
+#elif defined(CONFIG_ARM)
+#if (__LINUX_ARM_ARCH__ >= 5)
 #define LZO_USE_CTZ32	1
 #endif
+#if (__LINUX_ARM_ARCH__ >= 6) && defined(CONFIG_THUMB2_KERNEL)
+#define LZO_USE_CTZ64	1
+#endif
+#endif
 
 #define M1_MAX_OFFSET	0x0400
 #define M2_MAX_OFFSET	0x0800
-- 
2.17.1

Re: [PATCH 3/8] lib/lzo: enable 64-bit CTZ on Arm

[Markus F.X.J. Oberhumer]

I think that LZO_USE_CTZ64 should only be defined for ARM64 and
not for 32-bit ARM.


On 2018-11-30 15:26, Dave Rodgman wrote:
> From: Matt Sealey <matt.sealey@arm.com>
> 
> ARMv6 Thumb state introduced an RBIT instruction which, combined with CLZ
> as present in ARMv5, introduces an extremely fast path for counting
> trailing zeroes.
> 
> Enable the use of the GCC builtin for this on ARMv6+ with
> CONFIG_THUMB2_KERNEL to ensure we get the 'new' instruction usage.
> 
> We do not bother enabling LZO_USE_CTZ64 support for ARMv5 as the builtin
> code path does the same thing as the LZO_USE_CTZ32 code, only with more
> register pressure.
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-4-dave.rodgman@arm.com
> Signed-off-by: Matt Sealey <matt.sealey@arm.com>
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  lib/lzo/lzodefs.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index e1b3cf6459a9..c0193f726db0 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -33,9 +33,14 @@
>  #define LZO_USE_CTZ32	1
>  #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
>  #define LZO_USE_CTZ32	1
> -#elif defined(CONFIG_ARM) && (__LINUX_ARM_ARCH__ >= 5)
> +#elif defined(CONFIG_ARM)
> +#if (__LINUX_ARM_ARCH__ >= 5)
>  #define LZO_USE_CTZ32	1
>  #endif
> +#if (__LINUX_ARM_ARCH__ >= 6) && defined(CONFIG_THUMB2_KERNEL)
> +#define LZO_USE_CTZ64	1
> +#endif
> +#endif
>  
>  #define M1_MAX_OFFSET	0x0400
>  #define M2_MAX_OFFSET	0x0800
> 

-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 4/8] lib/lzo: 64-bit CTZ on arm64

[Dave Rodgman]

From: Matt Sealey <matt.sealey@arm.com>

LZO leaves some performance on the table by not realising that arm64 can
optimize count-trailing-zeros bit operations.

Add CONFIG_ARM64 to the checked definitions alongside CONFIG_X86_64 to
enable the use of rbit/clz instructions on full 64-bit quantities.

Link: http://lkml.kernel.org/r/20181127161913.23863-5-dave.rodgman@arm.com
Signed-off-by: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzodefs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index c0193f726db0..c8965dc181df 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -28,7 +28,7 @@
 
 #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
 #error "conflicting endian definitions"
-#elif defined(CONFIG_X86_64)
+#elif defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
 #define LZO_USE_CTZ64	1
 #define LZO_USE_CTZ32	1
 #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
-- 
2.17.1

Re: [PATCH 4/8] lib/lzo: 64-bit CTZ on arm64

[Markus F.X.J. Oberhumer]

Acked-by: Markus F.X.J. Oberhumer <markus@oberhumer.com>



On 2018-11-30 15:26, Dave Rodgman wrote:
> From: Matt Sealey <matt.sealey@arm.com>
> 
> LZO leaves some performance on the table by not realising that arm64 can
> optimize count-trailing-zeros bit operations.
> 
> Add CONFIG_ARM64 to the checked definitions alongside CONFIG_X86_64 to
> enable the use of rbit/clz instructions on full 64-bit quantities.
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-5-dave.rodgman@arm.com
> Signed-off-by: Matt Sealey <matt.sealey@arm.com>
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  lib/lzo/lzodefs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index c0193f726db0..c8965dc181df 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -28,7 +28,7 @@
>  
>  #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
>  #error "conflicting endian definitions"
> -#elif defined(CONFIG_X86_64)
> +#elif defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
>  #define LZO_USE_CTZ64	1
>  #define LZO_USE_CTZ32	1
>  #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
> 

-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 5/8] lib/lzo: fast 8-byte copy on arm64

[Dave Rodgman]

From: Matt Sealey <matt.sealey@arm.com>

Enable faster 8-byte copies on arm64.

Link: http://lkml.kernel.org/r/20181127161913.23863-6-dave.rodgman@arm.com
Signed-off-by: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzodefs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index c8965dc181df..06fa83a38e0a 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -15,7 +15,7 @@
 
 #define COPY4(dst, src)	\
 		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
-#if defined(CONFIG_X86_64)
+#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
 #define COPY8(dst, src)	\
 		put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))
 #else
-- 
2.17.1

Re: [PATCH 5/8] lib/lzo: fast 8-byte copy on arm64

[Markus F.X.J. Oberhumer]

Acked-by: Markus F.X.J. Oberhumer <markus@oberhumer.com>



On 2018-11-30 15:26, Dave Rodgman wrote:
> From: Matt Sealey <matt.sealey@arm.com>
> 
> Enable faster 8-byte copies on arm64.
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-6-dave.rodgman@arm.com
> Signed-off-by: Matt Sealey <matt.sealey@arm.com>
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  lib/lzo/lzodefs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index c8965dc181df..06fa83a38e0a 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -15,7 +15,7 @@
>  
>  #define COPY4(dst, src)	\
>  		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
> -#if defined(CONFIG_X86_64)
> +#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
>  #define COPY8(dst, src)	\
>  		put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))
>  #else
> 

-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 6/8] lib/lzo: implement run-length encoding

[Dave Rodgman]

When using zram, we frequently encounter long runs of zero bytes.
This adds a special case which identifies runs of zeros and encodes
them using run-length encoding.

This is faster for both compression and decompresion. For
high-entropy data which doesn't hit this case, impact is minimal.

Compression ratio is within a few percent in all cases.

This modifies the bitstream in a way which is backwards compatible
(i.e., we can decompress old bitstreams, but old versions of lzo
cannot decompress new bitstreams).

Link: http://lkml.kernel.org/r/20181127161913.23863-7-dave.rodgman@arm.com
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Matt Sealey <matt.sealey@arm.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 Documentation/lzo.txt           |  35 ++++++++---
 include/linux/lzo.h             |   2 +-
 lib/lzo/lzo1x_compress.c        | 100 ++++++++++++++++++++++++++++----
 lib/lzo/lzo1x_decompress_safe.c |  75 ++++++++++++++++--------
 lib/lzo/lzodefs.h               |  12 +++-
 5 files changed, 181 insertions(+), 43 deletions(-)

diff --git a/Documentation/lzo.txt b/Documentation/lzo.txt
index 6fa6a93d0949..306c60344ca7 100644
--- a/Documentation/lzo.txt
+++ b/Documentation/lzo.txt
@@ -78,16 +78,30 @@ Description
      is an implementation design choice independent on the algorithm or
      encoding.
 
+Versions
+
+0: Original version
+1: LZO-RLE
+
+Version 1 of LZO implements an extension to encode runs of zeros using run
+length encoding. This improves speed for data with many zeros, which is a
+common case for zram. This modifies the bitstream in a backwards compatible way
+(v1 can correctly decompress v0 compressed data, but v0 cannot read v1 data).
+
 Byte sequences
 ==============
 
   First byte encoding::
 
-      0..17   : follow regular instruction encoding, see below. It is worth
-                noting that codes 16 and 17 will represent a block copy from
-                the dictionary which is empty, and that they will always be
+      0..16   : follow regular instruction encoding, see below. It is worth
+                noting that code 16 will represent a block copy from the
+                dictionary which is empty, and that it will always be
                 invalid at this place.
 
+      17      : bitstream version. If the first byte is 17, the next byte
+                gives the bitstream version. If the first byte is not 17,
+                the bitstream version is 0.
+
       18..21  : copy 0..3 literals
                 state = (byte - 17) = 0..3  [ copy <state> literals ]
                 skip byte
@@ -140,6 +154,11 @@ Byte sequences
            state = S (copy S literals after this block)
            End of stream is reached if distance == 16384
 
+        In version 1, this instruction is also used to encode a run of zeros if
+        distance = 0xbfff, i.e. H = 1 and the D bits are all 1.
+           In this case, it is followed by a fourth byte, X.
+           run length = ((X << 3) | (0 0 0 0 0 L L L)) + 4.
+
       0 0 1 L L L L L  (32..63)
            Copy of small block within 16kB distance (preferably less than 34B)
            length = 2 + (L ?: 31 + (zero_bytes * 255) + non_zero_byte)
@@ -165,7 +184,9 @@ Authors
 =======
 
   This document was written by Willy Tarreau <w@1wt.eu> on 2014/07/19 during an
-  analysis of the decompression code available in Linux 3.16-rc5. The code is
-  tricky, it is possible that this document contains mistakes or that a few
-  corner cases were overlooked. In any case, please report any doubt, fix, or
-  proposed updates to the author(s) so that the document can be updated.
+  analysis of the decompression code available in Linux 3.16-rc5, and updated
+  by Dave Rodgman <dave.rodgman@arm.com> on 2018/10/30 to introduce run-length
+  encoding. The code is tricky, it is possible that this document contains
+  mistakes or that a few corner cases were overlooked. In any case, please
+  report any doubt, fix, or proposed updates to the author(s) so that the
+  document can be updated.
diff --git a/include/linux/lzo.h b/include/linux/lzo.h
index 2ae27cb89927..547a86c71e1b 100644
--- a/include/linux/lzo.h
+++ b/include/linux/lzo.h
@@ -18,7 +18,7 @@
 #define LZO1X_1_MEM_COMPRESS	(8192 * sizeof(unsigned short))
 #define LZO1X_MEM_COMPRESS	LZO1X_1_MEM_COMPRESS
 
-#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
+#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3 + 2)
 
 /* This requires 'wrkmem' of size LZO1X_1_MEM_COMPRESS */
 int lzo1x_1_compress(const unsigned char *src, size_t src_len,
diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
index 82fb5571ce5e..03834b99a5a1 100644
--- a/lib/lzo/lzo1x_compress.c
+++ b/lib/lzo/lzo1x_compress.c
@@ -20,7 +20,7 @@
 static noinline size_t
 lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 		    unsigned char *out, size_t *out_len,
-		    size_t ti, void *wrkmem)
+		    size_t ti, void *wrkmem, signed char *state_offset)
 {
 	const unsigned char *ip;
 	unsigned char *op;
@@ -35,27 +35,85 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 	ip += ti < 4 ? 4 - ti : 0;
 
 	for (;;) {
-		const unsigned char *m_pos;
+		const unsigned char *m_pos = NULL;
 		size_t t, m_len, m_off;
 		u32 dv;
+		u32 run_length = 0;
 literal:
 		ip += 1 + ((ip - ii) >> 5);
 next:
 		if (unlikely(ip >= ip_end))
 			break;
 		dv = get_unaligned_le32(ip);
-		t = ((dv * 0x1824429d) >> (32 - D_BITS)) & D_MASK;
-		m_pos = in + dict[t];
-		dict[t] = (lzo_dict_t) (ip - in);
-		if (unlikely(dv != get_unaligned_le32(m_pos)))
-			goto literal;
+
+		if (dv == 0) {
+			const unsigned char *ir = ip + 4;
+			const unsigned char *limit = ip_end
+				< (ip + MAX_ZERO_RUN_LENGTH + 1)
+				? ip_end : ip + MAX_ZERO_RUN_LENGTH + 1;
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && \
+	defined(LZO_FAST_64BIT_MEMORY_ACCESS)
+			u64 dv64;
+
+			for (; (ir + 32) <= limit; ir += 32) {
+				dv64 = get_unaligned((u64 *)ir);
+				dv64 |= get_unaligned((u64 *)ir + 1);
+				dv64 |= get_unaligned((u64 *)ir + 2);
+				dv64 |= get_unaligned((u64 *)ir + 3);
+				if (dv64)
+					break;
+			}
+			for (; (ir + 8) <= limit; ir += 8) {
+				dv64 = get_unaligned((u64 *)ir);
+				if (dv64) {
+#  if defined(__LITTLE_ENDIAN)
+					ir += __builtin_ctzll(dv64) >> 3;
+#  elif defined(__BIG_ENDIAN)
+					ir += __builtin_clzll(dv64) >> 3;
+#  else
+#    error "missing endian definition"
+#  endif
+					break;
+				}
+			}
+#else
+			while ((ir < (const unsigned char *)
+					ALIGN((uintptr_t)ir, 4)) &&
+					(ir < limit) && (*ir == 0))
+				ir++;
+			for (; (ir + 4) <= limit; ir += 4) {
+				dv = *((u32 *)ir);
+				if (dv) {
+#  if defined(__LITTLE_ENDIAN)
+					ir += __builtin_ctz(dv) >> 3;
+#  elif defined(__BIG_ENDIAN)
+					ir += __builtin_clz(dv) >> 3;
+#  else
+#    error "missing endian definition"
+#  endif
+					break;
+				}
+			}
+#endif
+			while (likely(ir < limit) && unlikely(*ir == 0))
+				ir++;
+			run_length = ir - ip;
+			if (run_length > MAX_ZERO_RUN_LENGTH)
+				run_length = MAX_ZERO_RUN_LENGTH;
+		} else {
+			t = ((dv * 0x1824429d) >> (32 - D_BITS)) & D_MASK;
+			m_pos = in + dict[t];
+			dict[t] = (lzo_dict_t) (ip - in);
+			if (unlikely(dv != get_unaligned_le32(m_pos)))
+				goto literal;
+		}
 
 		ii -= ti;
 		ti = 0;
 		t = ip - ii;
 		if (t != 0) {
 			if (t <= 3) {
-				op[-2] |= t;
+				op[*state_offset] |= t;
 				COPY4(op, ii);
 				op += t;
 			} else if (t <= 16) {
@@ -86,6 +144,17 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 			}
 		}
 
+		if (unlikely(run_length)) {
+			ip += run_length;
+			run_length -= MIN_ZERO_RUN_LENGTH;
+			put_unaligned_le32((run_length << 21) | 0xfffc18
+					   | (run_length & 0x7), op);
+			op += 4;
+			run_length = 0;
+			*state_offset = -3;
+			goto finished_writing_instruction;
+		}
+
 		m_len = 4;
 		{
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && defined(LZO_USE_CTZ64)
@@ -168,7 +237,6 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 
 		m_off = ip - m_pos;
 		ip += m_len;
-		ii = ip;
 		if (m_len <= M2_MAX_LEN && m_off <= M2_MAX_OFFSET) {
 			m_off -= 1;
 			*op++ = (((m_len - 1) << 5) | ((m_off & 7) << 2));
@@ -205,6 +273,9 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 			*op++ = (m_off << 2);
 			*op++ = (m_off >> 6);
 		}
+		*state_offset = -2;
+finished_writing_instruction:
+		ii = ip;
 		goto next;
 	}
 	*out_len = op - out;
@@ -219,6 +290,12 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
 	unsigned char *op = out;
 	size_t l = in_len;
 	size_t t = 0;
+	signed char state_offset = -2;
+
+	// LZO v0 will never write 17 as first byte,
+	// so this is used to version the bitstream
+	*op++ = 17;
+	*op++ = LZO_VERSION;
 
 	while (l > 20) {
 		size_t ll = l <= (M4_MAX_OFFSET + 1) ? l : (M4_MAX_OFFSET + 1);
@@ -227,7 +304,8 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
 			break;
 		BUILD_BUG_ON(D_SIZE * sizeof(lzo_dict_t) > LZO1X_1_MEM_COMPRESS);
 		memset(wrkmem, 0, D_SIZE * sizeof(lzo_dict_t));
-		t = lzo1x_1_do_compress(ip, ll, op, out_len, t, wrkmem);
+		t = lzo1x_1_do_compress(ip, ll, op, out_len,
+					t, wrkmem, &state_offset);
 		ip += ll;
 		op += *out_len;
 		l  -= ll;
@@ -240,7 +318,7 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
 		if (op == out && t <= 238) {
 			*op++ = (17 + t);
 		} else if (t <= 3) {
-			op[-2] |= t;
+			op[state_offset] |= t;
 		} else if (t <= 18) {
 			*op++ = (t - 3);
 		} else {
diff --git a/lib/lzo/lzo1x_decompress_safe.c b/lib/lzo/lzo1x_decompress_safe.c
index aa95d3066b7d..b8f88d5ea3ff 100644
--- a/lib/lzo/lzo1x_decompress_safe.c
+++ b/lib/lzo/lzo1x_decompress_safe.c
@@ -46,11 +46,23 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
 	const unsigned char * const ip_end = in + in_len;
 	unsigned char * const op_end = out + *out_len;
 
+	unsigned char bitstream_version;
+
 	op = out;
 	ip = in;
 
 	if (unlikely(in_len < 3))
 		goto input_overrun;
+
+	if (likely(*ip == 17)) {
+		bitstream_version = ip[1];
+		ip += 2;
+		if (unlikely(in_len < 5))
+			goto input_overrun;
+	} else {
+		bitstream_version = 0;
+	}
+
 	if (*ip > 17) {
 		t = *ip++ - 17;
 		if (t < 4) {
@@ -151,32 +163,49 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
 			m_pos -= next >> 2;
 			next &= 3;
 		} else {
-			m_pos = op;
-			m_pos -= (t & 8) << 11;
-			t = (t & 7) + (3 - 1);
-			if (unlikely(t == 2)) {
-				size_t offset;
-				const unsigned char *ip_last = ip;
+			NEED_IP(2);
+			next = get_unaligned_le16(ip);
+			if (((next & 0xfffc) == 0xfffc) &&
+			    ((t & 0xf8) == 0x18) &&
+			    likely(bitstream_version)) {
+				NEED_IP(3);
+				t &= 7;
+				t |= ip[2] << 3;
+				t += MIN_ZERO_RUN_LENGTH;
+				NEED_OP(t);
+				memset(op, 0, t);
+				op += t;
+				next &= 3;
+				ip += 3;
+				goto match_next;
+			} else {
+				m_pos = op;
+				m_pos -= (t & 8) << 11;
+				t = (t & 7) + (3 - 1);
+				if (unlikely(t == 2)) {
+					size_t offset;
+					const unsigned char *ip_last = ip;
 
-				while (unlikely(*ip == 0)) {
-					ip++;
-					NEED_IP(1);
-				}
-				offset = ip - ip_last;
-				if (unlikely(offset > MAX_255_COUNT))
-					return LZO_E_ERROR;
+					while (unlikely(*ip == 0)) {
+						ip++;
+						NEED_IP(1);
+					}
+					offset = ip - ip_last;
+					if (unlikely(offset > MAX_255_COUNT))
+						return LZO_E_ERROR;
 
-				offset = (offset << 8) - offset;
-				t += offset + 7 + *ip++;
-				NEED_IP(2);
+					offset = (offset << 8) - offset;
+					t += offset + 7 + *ip++;
+					NEED_IP(2);
+					next = get_unaligned_le16(ip);
+				}
+				ip += 2;
+				m_pos -= next >> 2;
+				next &= 3;
+				if (m_pos == op)
+					goto eof_found;
+				m_pos -= 0x4000;
 			}
-			next = get_unaligned_le16(ip);
-			ip += 2;
-			m_pos -= next >> 2;
-			next &= 3;
-			if (m_pos == op)
-				goto eof_found;
-			m_pos -= 0x4000;
 		}
 		TEST_LB(m_pos);
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index 06fa83a38e0a..682359058b3c 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -13,6 +13,12 @@
  */
 
 
+/* Version
+ * 0: original lzo version
+ * 1: lzo with support for RLE
+ */
+#define LZO_VERSION 1
+
 #define COPY4(dst, src)	\
 		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
 #if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
@@ -31,6 +37,7 @@
 #elif defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
 #define LZO_USE_CTZ64	1
 #define LZO_USE_CTZ32	1
+#define LZO_FAST_64BIT_MEMORY_ACCESS
 #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
 #define LZO_USE_CTZ32	1
 #elif defined(CONFIG_ARM)
@@ -45,7 +52,7 @@
 #define M1_MAX_OFFSET	0x0400
 #define M2_MAX_OFFSET	0x0800
 #define M3_MAX_OFFSET	0x4000
-#define M4_MAX_OFFSET	0xbfff
+#define M4_MAX_OFFSET	0xbffe
 
 #define M1_MIN_LEN	2
 #define M1_MAX_LEN	2
@@ -61,6 +68,9 @@
 #define M3_MARKER	32
 #define M4_MARKER	16
 
+#define MIN_ZERO_RUN_LENGTH	4
+#define MAX_ZERO_RUN_LENGTH	(2047 + MIN_ZERO_RUN_LENGTH)
+
 #define lzo_dict_t      unsigned short
 #define D_BITS		13
 #define D_SIZE		(1u << D_BITS)
-- 
2.17.1

Re: [PATCH 6/8] lib/lzo: implement run-length encoding

[Markus F.X.J. Oberhumer]

*NOT* acked by Markus Oberhumer, original author of the LZO data compression
library and author of the current Linux kernel LZO implementation.

Please see my email reply to the patch series header for more info.


On 2018-11-30 15:26, Dave Rodgman wrote:
> When using zram, we frequently encounter long runs of zero bytes.
> This adds a special case which identifies runs of zeros and encodes
> them using run-length encoding.
> 
> This is faster for both compression and decompresion. For
> high-entropy data which doesn't hit this case, impact is minimal.
> 
> Compression ratio is within a few percent in all cases.
> 
> This modifies the bitstream in a way which is backwards compatible
> (i.e., we can decompress old bitstreams, but old versions of lzo
> cannot decompress new bitstreams).
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-7-dave.rodgman@arm.com
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Matt Sealey <matt.sealey@arm.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  Documentation/lzo.txt           |  35 ++++++++---
>  include/linux/lzo.h             |   2 +-
>  lib/lzo/lzo1x_compress.c        | 100 ++++++++++++++++++++++++++++----
>  lib/lzo/lzo1x_decompress_safe.c |  75 ++++++++++++++++--------
>  lib/lzo/lzodefs.h               |  12 +++-
>  5 files changed, 181 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/lzo.txt b/Documentation/lzo.txt
> index 6fa6a93d0949..306c60344ca7 100644
> --- a/Documentation/lzo.txt
> +++ b/Documentation/lzo.txt
> @@ -78,16 +78,30 @@ Description
>       is an implementation design choice independent on the algorithm or
>       encoding.
>  
> +Versions
> +
> +0: Original version
> +1: LZO-RLE
> +
> +Version 1 of LZO implements an extension to encode runs of zeros using run
> +length encoding. This improves speed for data with many zeros, which is a
> +common case for zram. This modifies the bitstream in a backwards compatible way
> +(v1 can correctly decompress v0 compressed data, but v0 cannot read v1 data).
> +
>  Byte sequences
>  ==============
>  
>    First byte encoding::
>  
> -      0..17   : follow regular instruction encoding, see below. It is worth
> -                noting that codes 16 and 17 will represent a block copy from
> -                the dictionary which is empty, and that they will always be
> +      0..16   : follow regular instruction encoding, see below. It is worth
> +                noting that code 16 will represent a block copy from the
> +                dictionary which is empty, and that it will always be
>                  invalid at this place.
>  
> +      17      : bitstream version. If the first byte is 17, the next byte
> +                gives the bitstream version. If the first byte is not 17,
> +                the bitstream version is 0.
> +
>        18..21  : copy 0..3 literals
>                  state = (byte - 17) = 0..3  [ copy <state> literals ]
>                  skip byte
> @@ -140,6 +154,11 @@ Byte sequences
>             state = S (copy S literals after this block)
>             End of stream is reached if distance == 16384
>  
> +        In version 1, this instruction is also used to encode a run of zeros if
> +        distance = 0xbfff, i.e. H = 1 and the D bits are all 1.
> +           In this case, it is followed by a fourth byte, X.
> +           run length = ((X << 3) | (0 0 0 0 0 L L L)) + 4.
> +
>        0 0 1 L L L L L  (32..63)
>             Copy of small block within 16kB distance (preferably less than 34B)
>             length = 2 + (L ?: 31 + (zero_bytes * 255) + non_zero_byte)
> @@ -165,7 +184,9 @@ Authors
>  =======
>  
>    This document was written by Willy Tarreau <w@1wt.eu> on 2014/07/19 during an
> -  analysis of the decompression code available in Linux 3.16-rc5. The code is
> -  tricky, it is possible that this document contains mistakes or that a few
> -  corner cases were overlooked. In any case, please report any doubt, fix, or
> -  proposed updates to the author(s) so that the document can be updated.
> +  analysis of the decompression code available in Linux 3.16-rc5, and updated
> +  by Dave Rodgman <dave.rodgman@arm.com> on 2018/10/30 to introduce run-length
> +  encoding. The code is tricky, it is possible that this document contains
> +  mistakes or that a few corner cases were overlooked. In any case, please
> +  report any doubt, fix, or proposed updates to the author(s) so that the
> +  document can be updated.
> diff --git a/include/linux/lzo.h b/include/linux/lzo.h
> index 2ae27cb89927..547a86c71e1b 100644
> --- a/include/linux/lzo.h
> +++ b/include/linux/lzo.h
> @@ -18,7 +18,7 @@
>  #define LZO1X_1_MEM_COMPRESS	(8192 * sizeof(unsigned short))
>  #define LZO1X_MEM_COMPRESS	LZO1X_1_MEM_COMPRESS
>  
> -#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
> +#define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3 + 2)
>  
>  /* This requires 'wrkmem' of size LZO1X_1_MEM_COMPRESS */
>  int lzo1x_1_compress(const unsigned char *src, size_t src_len,
> diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
> index 82fb5571ce5e..03834b99a5a1 100644
> --- a/lib/lzo/lzo1x_compress.c
> +++ b/lib/lzo/lzo1x_compress.c
> @@ -20,7 +20,7 @@
>  static noinline size_t
>  lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  		    unsigned char *out, size_t *out_len,
> -		    size_t ti, void *wrkmem)
> +		    size_t ti, void *wrkmem, signed char *state_offset)
>  {
>  	const unsigned char *ip;
>  	unsigned char *op;
> @@ -35,27 +35,85 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  	ip += ti < 4 ? 4 - ti : 0;
>  
>  	for (;;) {
> -		const unsigned char *m_pos;
> +		const unsigned char *m_pos = NULL;
>  		size_t t, m_len, m_off;
>  		u32 dv;
> +		u32 run_length = 0;
>  literal:
>  		ip += 1 + ((ip - ii) >> 5);
>  next:
>  		if (unlikely(ip >= ip_end))
>  			break;
>  		dv = get_unaligned_le32(ip);
> -		t = ((dv * 0x1824429d) >> (32 - D_BITS)) & D_MASK;
> -		m_pos = in + dict[t];
> -		dict[t] = (lzo_dict_t) (ip - in);
> -		if (unlikely(dv != get_unaligned_le32(m_pos)))
> -			goto literal;
> +
> +		if (dv == 0) {
> +			const unsigned char *ir = ip + 4;
> +			const unsigned char *limit = ip_end
> +				< (ip + MAX_ZERO_RUN_LENGTH + 1)
> +				? ip_end : ip + MAX_ZERO_RUN_LENGTH + 1;
> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && \
> +	defined(LZO_FAST_64BIT_MEMORY_ACCESS)
> +			u64 dv64;
> +
> +			for (; (ir + 32) <= limit; ir += 32) {
> +				dv64 = get_unaligned((u64 *)ir);
> +				dv64 |= get_unaligned((u64 *)ir + 1);
> +				dv64 |= get_unaligned((u64 *)ir + 2);
> +				dv64 |= get_unaligned((u64 *)ir + 3);
> +				if (dv64)
> +					break;
> +			}
> +			for (; (ir + 8) <= limit; ir += 8) {
> +				dv64 = get_unaligned((u64 *)ir);
> +				if (dv64) {
> +#  if defined(__LITTLE_ENDIAN)
> +					ir += __builtin_ctzll(dv64) >> 3;
> +#  elif defined(__BIG_ENDIAN)
> +					ir += __builtin_clzll(dv64) >> 3;
> +#  else
> +#    error "missing endian definition"
> +#  endif
> +					break;
> +				}
> +			}
> +#else
> +			while ((ir < (const unsigned char *)
> +					ALIGN((uintptr_t)ir, 4)) &&
> +					(ir < limit) && (*ir == 0))
> +				ir++;
> +			for (; (ir + 4) <= limit; ir += 4) {
> +				dv = *((u32 *)ir);
> +				if (dv) {
> +#  if defined(__LITTLE_ENDIAN)
> +					ir += __builtin_ctz(dv) >> 3;
> +#  elif defined(__BIG_ENDIAN)
> +					ir += __builtin_clz(dv) >> 3;
> +#  else
> +#    error "missing endian definition"
> +#  endif
> +					break;
> +				}
> +			}
> +#endif
> +			while (likely(ir < limit) && unlikely(*ir == 0))
> +				ir++;
> +			run_length = ir - ip;
> +			if (run_length > MAX_ZERO_RUN_LENGTH)
> +				run_length = MAX_ZERO_RUN_LENGTH;
> +		} else {
> +			t = ((dv * 0x1824429d) >> (32 - D_BITS)) & D_MASK;
> +			m_pos = in + dict[t];
> +			dict[t] = (lzo_dict_t) (ip - in);
> +			if (unlikely(dv != get_unaligned_le32(m_pos)))
> +				goto literal;
> +		}
>  
>  		ii -= ti;
>  		ti = 0;
>  		t = ip - ii;
>  		if (t != 0) {
>  			if (t <= 3) {
> -				op[-2] |= t;
> +				op[*state_offset] |= t;
>  				COPY4(op, ii);
>  				op += t;
>  			} else if (t <= 16) {
> @@ -86,6 +144,17 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  			}
>  		}
>  
> +		if (unlikely(run_length)) {
> +			ip += run_length;
> +			run_length -= MIN_ZERO_RUN_LENGTH;
> +			put_unaligned_le32((run_length << 21) | 0xfffc18
> +					   | (run_length & 0x7), op);
> +			op += 4;
> +			run_length = 0;
> +			*state_offset = -3;
> +			goto finished_writing_instruction;
> +		}
> +
>  		m_len = 4;
>  		{
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && defined(LZO_USE_CTZ64)
> @@ -168,7 +237,6 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  
>  		m_off = ip - m_pos;
>  		ip += m_len;
> -		ii = ip;
>  		if (m_len <= M2_MAX_LEN && m_off <= M2_MAX_OFFSET) {
>  			m_off -= 1;
>  			*op++ = (((m_len - 1) << 5) | ((m_off & 7) << 2));
> @@ -205,6 +273,9 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
>  			*op++ = (m_off << 2);
>  			*op++ = (m_off >> 6);
>  		}
> +		*state_offset = -2;
> +finished_writing_instruction:
> +		ii = ip;
>  		goto next;
>  	}
>  	*out_len = op - out;
> @@ -219,6 +290,12 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
>  	unsigned char *op = out;
>  	size_t l = in_len;
>  	size_t t = 0;
> +	signed char state_offset = -2;
> +
> +	// LZO v0 will never write 17 as first byte,
> +	// so this is used to version the bitstream
> +	*op++ = 17;
> +	*op++ = LZO_VERSION;
>  
>  	while (l > 20) {
>  		size_t ll = l <= (M4_MAX_OFFSET + 1) ? l : (M4_MAX_OFFSET + 1);
> @@ -227,7 +304,8 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
>  			break;
>  		BUILD_BUG_ON(D_SIZE * sizeof(lzo_dict_t) > LZO1X_1_MEM_COMPRESS);
>  		memset(wrkmem, 0, D_SIZE * sizeof(lzo_dict_t));
> -		t = lzo1x_1_do_compress(ip, ll, op, out_len, t, wrkmem);
> +		t = lzo1x_1_do_compress(ip, ll, op, out_len,
> +					t, wrkmem, &state_offset);
>  		ip += ll;
>  		op += *out_len;
>  		l  -= ll;
> @@ -240,7 +318,7 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
>  		if (op == out && t <= 238) {
>  			*op++ = (17 + t);
>  		} else if (t <= 3) {
> -			op[-2] |= t;
> +			op[state_offset] |= t;
>  		} else if (t <= 18) {
>  			*op++ = (t - 3);
>  		} else {
> diff --git a/lib/lzo/lzo1x_decompress_safe.c b/lib/lzo/lzo1x_decompress_safe.c
> index aa95d3066b7d..b8f88d5ea3ff 100644
> --- a/lib/lzo/lzo1x_decompress_safe.c
> +++ b/lib/lzo/lzo1x_decompress_safe.c
> @@ -46,11 +46,23 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
>  	const unsigned char * const ip_end = in + in_len;
>  	unsigned char * const op_end = out + *out_len;
>  
> +	unsigned char bitstream_version;
> +
>  	op = out;
>  	ip = in;
>  
>  	if (unlikely(in_len < 3))
>  		goto input_overrun;
> +
> +	if (likely(*ip == 17)) {
> +		bitstream_version = ip[1];
> +		ip += 2;
> +		if (unlikely(in_len < 5))
> +			goto input_overrun;
> +	} else {
> +		bitstream_version = 0;
> +	}
> +
>  	if (*ip > 17) {
>  		t = *ip++ - 17;
>  		if (t < 4) {
> @@ -151,32 +163,49 @@ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
>  			m_pos -= next >> 2;
>  			next &= 3;
>  		} else {
> -			m_pos = op;
> -			m_pos -= (t & 8) << 11;
> -			t = (t & 7) + (3 - 1);
> -			if (unlikely(t == 2)) {
> -				size_t offset;
> -				const unsigned char *ip_last = ip;
> +			NEED_IP(2);
> +			next = get_unaligned_le16(ip);
> +			if (((next & 0xfffc) == 0xfffc) &&
> +			    ((t & 0xf8) == 0x18) &&
> +			    likely(bitstream_version)) {
> +				NEED_IP(3);
> +				t &= 7;
> +				t |= ip[2] << 3;
> +				t += MIN_ZERO_RUN_LENGTH;
> +				NEED_OP(t);
> +				memset(op, 0, t);
> +				op += t;
> +				next &= 3;
> +				ip += 3;
> +				goto match_next;
> +			} else {
> +				m_pos = op;
> +				m_pos -= (t & 8) << 11;
> +				t = (t & 7) + (3 - 1);
> +				if (unlikely(t == 2)) {
> +					size_t offset;
> +					const unsigned char *ip_last = ip;
>  
> -				while (unlikely(*ip == 0)) {
> -					ip++;
> -					NEED_IP(1);
> -				}
> -				offset = ip - ip_last;
> -				if (unlikely(offset > MAX_255_COUNT))
> -					return LZO_E_ERROR;
> +					while (unlikely(*ip == 0)) {
> +						ip++;
> +						NEED_IP(1);
> +					}
> +					offset = ip - ip_last;
> +					if (unlikely(offset > MAX_255_COUNT))
> +						return LZO_E_ERROR;
>  
> -				offset = (offset << 8) - offset;
> -				t += offset + 7 + *ip++;
> -				NEED_IP(2);
> +					offset = (offset << 8) - offset;
> +					t += offset + 7 + *ip++;
> +					NEED_IP(2);
> +					next = get_unaligned_le16(ip);
> +				}
> +				ip += 2;
> +				m_pos -= next >> 2;
> +				next &= 3;
> +				if (m_pos == op)
> +					goto eof_found;
> +				m_pos -= 0x4000;
>  			}
> -			next = get_unaligned_le16(ip);
> -			ip += 2;
> -			m_pos -= next >> 2;
> -			next &= 3;
> -			if (m_pos == op)
> -				goto eof_found;
> -			m_pos -= 0x4000;
>  		}
>  		TEST_LB(m_pos);
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
> index 06fa83a38e0a..682359058b3c 100644
> --- a/lib/lzo/lzodefs.h
> +++ b/lib/lzo/lzodefs.h
> @@ -13,6 +13,12 @@
>   */
>  
>  
> +/* Version
> + * 0: original lzo version
> + * 1: lzo with support for RLE
> + */
> +#define LZO_VERSION 1
> +
>  #define COPY4(dst, src)	\
>  		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
>  #if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
> @@ -31,6 +37,7 @@
>  #elif defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
>  #define LZO_USE_CTZ64	1
>  #define LZO_USE_CTZ32	1
> +#define LZO_FAST_64BIT_MEMORY_ACCESS
>  #elif defined(CONFIG_X86) || defined(CONFIG_PPC)
>  #define LZO_USE_CTZ32	1
>  #elif defined(CONFIG_ARM)
> @@ -45,7 +52,7 @@
>  #define M1_MAX_OFFSET	0x0400
>  #define M2_MAX_OFFSET	0x0800
>  #define M3_MAX_OFFSET	0x4000
> -#define M4_MAX_OFFSET	0xbfff
> +#define M4_MAX_OFFSET	0xbffe
>  
>  #define M1_MIN_LEN	2
>  #define M1_MAX_LEN	2
> @@ -61,6 +68,9 @@
>  #define M3_MARKER	32
>  #define M4_MARKER	16
>  
> +#define MIN_ZERO_RUN_LENGTH	4
> +#define MAX_ZERO_RUN_LENGTH	(2047 + MIN_ZERO_RUN_LENGTH)
> +
>  #define lzo_dict_t      unsigned short
>  #define D_BITS		13
>  #define D_SIZE		(1u << D_BITS)
> 

-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

[PATCH 7/8] lib/lzo: separate lzo-rle from lzo

[Dave Rodgman]

To prevent any issues with persistent data, separate lzo-rle
from lzo so that it is treated as a separate algorithm, and
lzo is still available.

Link: http://lkml.kernel.org/r/20181127161913.23863-8-dave.rodgman@arm.com
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Matt Sealey <matt.sealey@arm.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
---
 Documentation/lzo.txt      |  12 ++-
 crypto/Makefile            |   2 +-
 crypto/lzo-rle.c           | 175 +++++++++++++++++++++++++++++++++++++
 crypto/tcrypt.c            |   4 +-
 drivers/block/zram/zcomp.c |   1 +
 include/linux/lzo.h        |   4 +
 lib/lzo/lzo1x_compress.c   |  42 +++++++--
 lib/lzo/lzodefs.h          |   3 +-
 8 files changed, 226 insertions(+), 17 deletions(-)
 create mode 100644 crypto/lzo-rle.c

diff --git a/Documentation/lzo.txt b/Documentation/lzo.txt
index 306c60344ca7..f79934225d8d 100644
--- a/Documentation/lzo.txt
+++ b/Documentation/lzo.txt
@@ -88,6 +88,10 @@ length encoding. This improves speed for data with many zeros, which is a
 common case for zram. This modifies the bitstream in a backwards compatible way
 (v1 can correctly decompress v0 compressed data, but v0 cannot read v1 data).
 
+For maximum compatibility, both versions are available under different names
+(lzo and lzo-rle). Differences in the encoding are noted in this document with
+e.g.: version 1 only.
+
 Byte sequences
 ==============
 
@@ -99,8 +103,8 @@ Byte sequences
                 invalid at this place.
 
       17      : bitstream version. If the first byte is 17, the next byte
-                gives the bitstream version. If the first byte is not 17,
-                the bitstream version is 0.
+                gives the bitstream version (version 1 only). If the first byte
+                is not 17, the bitstream version is 0.
 
       18..21  : copy 0..3 literals
                 state = (byte - 17) = 0..3  [ copy <state> literals ]
@@ -154,8 +158,8 @@ Byte sequences
            state = S (copy S literals after this block)
            End of stream is reached if distance == 16384
 
-        In version 1, this instruction is also used to encode a run of zeros if
-        distance = 0xbfff, i.e. H = 1 and the D bits are all 1.
+        In version 1 only, this instruction is also used to encode a run of
+        zeros if distance = 0xbfff, i.e. H = 1 and the D bits are all 1.
            In this case, it is followed by a fourth byte, X.
            run length = ((X << 3) | (0 0 0 0 0 L L L)) + 4.
 
diff --git a/crypto/Makefile b/crypto/Makefile
index 5e789dc2d4fd..23491b70e601 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -127,7 +127,7 @@ obj-$(CONFIG_CRYPTO_CRC32C) += crc32c_generic.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32_generic.o
 obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_common.o crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
-obj-$(CONFIG_CRYPTO_LZO) += lzo.o
+obj-$(CONFIG_CRYPTO_LZO) += lzo.o lzo-rle.o
 obj-$(CONFIG_CRYPTO_LZ4) += lz4.o
 obj-$(CONFIG_CRYPTO_LZ4HC) += lz4hc.o
 obj-$(CONFIG_CRYPTO_842) += 842.o
diff --git a/crypto/lzo-rle.c b/crypto/lzo-rle.c
new file mode 100644
index 000000000000..ea9c75b1db49
--- /dev/null
+++ b/crypto/lzo-rle.c
@@ -0,0 +1,175 @@
+/*
+ * Cryptographic API.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/crypto.h>
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/lzo.h>
+#include <crypto/internal/scompress.h>
+
+struct lzorle_ctx {
+	void *lzorle_comp_mem;
+};
+
+static void *lzorle_alloc_ctx(struct crypto_scomp *tfm)
+{
+	void *ctx;
+
+	ctx = kvmalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	return ctx;
+}
+
+static int lzorle_init(struct crypto_tfm *tfm)
+{
+	struct lzorle_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	ctx->lzorle_comp_mem = lzorle_alloc_ctx(NULL);
+	if (IS_ERR(ctx->lzorle_comp_mem))
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void lzorle_free_ctx(struct crypto_scomp *tfm, void *ctx)
+{
+	kvfree(ctx);
+}
+
+static void lzorle_exit(struct crypto_tfm *tfm)
+{
+	struct lzorle_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	lzorle_free_ctx(NULL, ctx->lzorle_comp_mem);
+}
+
+static int __lzorle_compress(const u8 *src, unsigned int slen,
+			  u8 *dst, unsigned int *dlen, void *ctx)
+{
+	size_t tmp_len = *dlen; /* size_t(ulong) <-> uint on 64 bit */
+	int err;
+
+	err = lzorle1x_1_compress(src, slen, dst, &tmp_len, ctx);
+
+	if (err != LZO_E_OK)
+		return -EINVAL;
+
+	*dlen = tmp_len;
+	return 0;
+}
+
+static int lzorle_compress(struct crypto_tfm *tfm, const u8 *src,
+			unsigned int slen, u8 *dst, unsigned int *dlen)
+{
+	struct lzorle_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	return __lzorle_compress(src, slen, dst, dlen, ctx->lzorle_comp_mem);
+}
+
+static int lzorle_scompress(struct crypto_scomp *tfm, const u8 *src,
+			 unsigned int slen, u8 *dst, unsigned int *dlen,
+			 void *ctx)
+{
+	return __lzorle_compress(src, slen, dst, dlen, ctx);
+}
+
+static int __lzorle_decompress(const u8 *src, unsigned int slen,
+			    u8 *dst, unsigned int *dlen)
+{
+	int err;
+	size_t tmp_len = *dlen; /* size_t(ulong) <-> uint on 64 bit */
+
+	err = lzo1x_decompress_safe(src, slen, dst, &tmp_len);
+
+	if (err != LZO_E_OK)
+		return -EINVAL;
+
+	*dlen = tmp_len;
+	return 0;
+}
+
+static int lzorle_decompress(struct crypto_tfm *tfm, const u8 *src,
+			  unsigned int slen, u8 *dst, unsigned int *dlen)
+{
+	return __lzorle_decompress(src, slen, dst, dlen);
+}
+
+static int lzorle_sdecompress(struct crypto_scomp *tfm, const u8 *src,
+			   unsigned int slen, u8 *dst, unsigned int *dlen,
+			   void *ctx)
+{
+	return __lzorle_decompress(src, slen, dst, dlen);
+}
+
+static struct crypto_alg alg = {
+	.cra_name		= "lzo-rle",
+	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
+	.cra_ctxsize		= sizeof(struct lzorle_ctx),
+	.cra_module		= THIS_MODULE,
+	.cra_init		= lzorle_init,
+	.cra_exit		= lzorle_exit,
+	.cra_u			= { .compress = {
+	.coa_compress		= lzorle_compress,
+	.coa_decompress		= lzorle_decompress } }
+};
+
+static struct scomp_alg scomp = {
+	.alloc_ctx		= lzorle_alloc_ctx,
+	.free_ctx		= lzorle_free_ctx,
+	.compress		= lzorle_scompress,
+	.decompress		= lzorle_sdecompress,
+	.base			= {
+		.cra_name	= "lzo-rle",
+		.cra_driver_name = "lzo-rle-scomp",
+		.cra_module	 = THIS_MODULE,
+	}
+};
+
+static int __init lzorle_mod_init(void)
+{
+	int ret;
+
+	ret = crypto_register_alg(&alg);
+	if (ret)
+		return ret;
+
+	ret = crypto_register_scomp(&scomp);
+	if (ret) {
+		crypto_unregister_alg(&alg);
+		return ret;
+	}
+
+	return ret;
+}
+
+static void __exit lzorle_mod_fini(void)
+{
+	crypto_unregister_alg(&alg);
+	crypto_unregister_scomp(&scomp);
+}
+
+module_init(lzorle_mod_init);
+module_exit(lzorle_mod_fini);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LZO-RLE Compression Algorithm");
+MODULE_ALIAS_CRYPTO("lzo-rle");
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 0590a9204562..c1c56c9771cf 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -76,8 +76,8 @@ static char *check[] = {
 	"cast6", "arc4", "michael_mic", "deflate", "crc32c", "tea", "xtea",
 	"khazad", "wp512", "wp384", "wp256", "tnepres", "xeta",  "fcrypt",
 	"camellia", "seed", "salsa20", "rmd128", "rmd160", "rmd256", "rmd320",
-	"lzo", "cts", "sha3-224", "sha3-256", "sha3-384", "sha3-512",
-	"streebog256", "streebog512",
+	"lzo", "lzo-rle", "cts", "sha3-224", "sha3-256", "sha3-384",
+	"sha3-512", "streebog256", "streebog512",
 	NULL
 };
 
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 4ed0a78fdc09..4d9a38890965 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,6 +20,7 @@
 
 static const char * const backends[] = {
 	"lzo",
+	"lzo-rle",
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
 	"lz4",
 #endif
diff --git a/include/linux/lzo.h b/include/linux/lzo.h
index 547a86c71e1b..e95c7d1092b2 100644
--- a/include/linux/lzo.h
+++ b/include/linux/lzo.h
@@ -24,6 +24,10 @@
 int lzo1x_1_compress(const unsigned char *src, size_t src_len,
 		     unsigned char *dst, size_t *dst_len, void *wrkmem);
 
+/* This requires 'wrkmem' of size LZO1X_1_MEM_COMPRESS */
+int lzorle1x_1_compress(const unsigned char *src, size_t src_len,
+		     unsigned char *dst, size_t *dst_len, void *wrkmem);
+
 /* safe decompression with overrun testing */
 int lzo1x_decompress_safe(const unsigned char *src, size_t src_len,
 			  unsigned char *dst, size_t *dst_len);
diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
index 03834b99a5a1..c4384ea14a50 100644
--- a/lib/lzo/lzo1x_compress.c
+++ b/lib/lzo/lzo1x_compress.c
@@ -20,7 +20,8 @@
 static noinline size_t
 lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 		    unsigned char *out, size_t *out_len,
-		    size_t ti, void *wrkmem, signed char *state_offset)
+		    size_t ti, void *wrkmem, signed char *state_offset,
+		    const unsigned char bitstream_version)
 {
 	const unsigned char *ip;
 	unsigned char *op;
@@ -46,7 +47,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 			break;
 		dv = get_unaligned_le32(ip);
 
-		if (dv == 0) {
+		if (dv == 0 && bitstream_version) {
 			const unsigned char *ir = ip + 4;
 			const unsigned char *limit = ip_end
 				< (ip + MAX_ZERO_RUN_LENGTH + 1)
@@ -282,30 +283,36 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 	return in_end - (ii - ti);
 }
 
-int lzo1x_1_compress(const unsigned char *in, size_t in_len,
+int lzogeneric1x_1_compress(const unsigned char *in, size_t in_len,
 		     unsigned char *out, size_t *out_len,
-		     void *wrkmem)
+		     void *wrkmem, const unsigned char bitstream_version)
 {
 	const unsigned char *ip = in;
 	unsigned char *op = out;
 	size_t l = in_len;
 	size_t t = 0;
 	signed char state_offset = -2;
+	unsigned int m4_max_offset;
 
 	// LZO v0 will never write 17 as first byte,
 	// so this is used to version the bitstream
-	*op++ = 17;
-	*op++ = LZO_VERSION;
+	if (bitstream_version > 0) {
+		*op++ = 17;
+		*op++ = bitstream_version;
+		m4_max_offset = M4_MAX_OFFSET_V1;
+	} else {
+		m4_max_offset = M4_MAX_OFFSET_V0;
+	}
 
 	while (l > 20) {
-		size_t ll = l <= (M4_MAX_OFFSET + 1) ? l : (M4_MAX_OFFSET + 1);
+		size_t ll = l <= (m4_max_offset + 1) ? l : (m4_max_offset + 1);
 		uintptr_t ll_end = (uintptr_t) ip + ll;
 		if ((ll_end + ((t + ll) >> 5)) <= ll_end)
 			break;
 		BUILD_BUG_ON(D_SIZE * sizeof(lzo_dict_t) > LZO1X_1_MEM_COMPRESS);
 		memset(wrkmem, 0, D_SIZE * sizeof(lzo_dict_t));
-		t = lzo1x_1_do_compress(ip, ll, op, out_len,
-					t, wrkmem, &state_offset);
+		t = lzo1x_1_do_compress(ip, ll, op, out_len, t, wrkmem,
+					&state_offset, bitstream_version);
 		ip += ll;
 		op += *out_len;
 		l  -= ll;
@@ -348,7 +355,24 @@ int lzo1x_1_compress(const unsigned char *in, size_t in_len,
 	*out_len = op - out;
 	return LZO_E_OK;
 }
+
+int lzo1x_1_compress(const unsigned char *in, size_t in_len,
+		     unsigned char *out, size_t *out_len,
+		     void *wrkmem)
+{
+	return lzogeneric1x_1_compress(in, in_len, out, out_len, wrkmem, 0);
+}
+
+int lzorle1x_1_compress(const unsigned char *in, size_t in_len,
+		     unsigned char *out, size_t *out_len,
+		     void *wrkmem)
+{
+	return lzogeneric1x_1_compress(in, in_len, out, out_len,
+				       wrkmem, LZO_VERSION);
+}
+
 EXPORT_SYMBOL_GPL(lzo1x_1_compress);
+EXPORT_SYMBOL_GPL(lzorle1x_1_compress);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("LZO1X-1 Compressor");
diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index 682359058b3c..c0657441a35d 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -52,7 +52,8 @@
 #define M1_MAX_OFFSET	0x0400
 #define M2_MAX_OFFSET	0x0800
 #define M3_MAX_OFFSET	0x4000
-#define M4_MAX_OFFSET	0xbffe
+#define M4_MAX_OFFSET_V0	0xbfff
+#define M4_MAX_OFFSET_V1	0xbffe
 
 #define M1_MIN_LEN	2
 #define M1_MAX_LEN	2
-- 
2.17.1

Re: [PATCH 7/8] lib/lzo: separate lzo-rle from lzo

[Herbert Xu]

On Fri, Nov 30, 2018 at 02:26:30PM +0000, Dave Rodgman wrote:
> To prevent any issues with persistent data, separate lzo-rle
> from lzo so that it is treated as a separate algorithm, and
> lzo is still available.
> 
> Link: http://lkml.kernel.org/r/20181127161913.23863-8-dave.rodgman@arm.com
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
> Cc: Matt Sealey <matt.sealey@arm.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Nitin Gupta <nitingupta910@gmail.com>
> Cc: Richard Purdie <rpurdie@openedhand.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: Sonny Rao <sonnyrao@google.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
> ---
>  Documentation/lzo.txt      |  12 ++-
>  crypto/Makefile            |   2 +-
>  crypto/lzo-rle.c           | 175 +++++++++++++++++++++++++++++++++++++
>  crypto/tcrypt.c            |   4 +-
>  drivers/block/zram/zcomp.c |   1 +
>  include/linux/lzo.h        |   4 +
>  lib/lzo/lzo1x_compress.c   |  42 +++++++--
>  lib/lzo/lzodefs.h          |   3 +-
>  8 files changed, 226 insertions(+), 17 deletions(-)
>  create mode 100644 crypto/lzo-rle.c

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 8/8] zram: default to lzo-rle instead of lzo

[Dave Rodgman]

lzo-rle gives higher performance and similar compression ratios to lzo.

Testing with 80 browser tabs showed a 27% reduction in total time spent
(de)compressing data during swapping.

Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
---
 drivers/block/zram/zram_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 58b025c5c83e..b8dc0976e385 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -41,7 +41,7 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo";
+static const char *default_compressor = "lzo-rle";
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
-- 
2.17.1

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Sergey Senozhatsky]

On (11/30/18 14:26), Dave Rodgman wrote:
> 
> This patch series introduces performance improvements for lzo.
> 
> The previous version of this patchset is here:
> https://lkml.org/lkml/2018/11/30/807
> 
> This version of the patchset fixes a maybe-used-uninitialized warning
> (although the previous version was still safe).

Hi Dave,

Notices this warning today:

lib/lzo/lzo1x_compress.c: In function ‘lzo1x_1_do_compress’:
lib/lzo/lzo1x_compress.c:239:14: warning: ‘m_pos’ may be used uninitialized in this function [-Wmaybe-uninitialized]
239 |   m_off = ip - m_pos;

Care to take a look? (could be false positive)

	-ss

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Dave Rodgman]

On 05/12/2018 7:30 am, Sergey Senozhatsky wrote:
> Hi Dave,
> 
> Notices this warning today:
> 
> lib/lzo/lzo1x_compress.c: In function ‘lzo1x_1_do_compress’:
> lib/lzo/lzo1x_compress.c:239:14: warning: ‘m_pos’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 239 |   m_off = ip - m_pos;
> 
> Care to take a look? (could be false positive)

Hi Sergey,

This warning (it's a false positive only seen with gcc) was fixed by v4 of this patchset. I've just tested the latest 
linux-next/master (next-20181205) and it doesn't have this issue.

cheers

Dave

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Sergey Senozhatsky]

On (12/05/18 09:50), Dave Rodgman wrote:
> > lib/lzo/lzo1x_compress.c: In function ‘lzo1x_1_do_compress’:
> > lib/lzo/lzo1x_compress.c:239:14: warning: ‘m_pos’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > 239 |   m_off = ip - m_pos;
> > 
> > Care to take a look? (could be false positive)
> 
> Hi Sergey,

Hi,

> This warning (it's a false positive only seen with gcc) was fixed
> by v4 of this patchset.

OK, cool.

> I've just tested the latest linux-next/master (next-20181205)
> and it doesn't have this issue.

Indeed.

	-ss

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Markus F.X.J. Oberhumer]

On 2018-11-30 15:26, Dave Rodgman wrote:
> This patch series introduces performance improvements for lzo.
> 
> The previous version of this patchset is here:
> https://lkml.org/lkml/2018/11/30/807
> 
> This version of the patchset fixes a maybe-used-uninitialized warning
> (although the previous version was still safe).
> 
> Dave

Hi Dave,

as indicated in my previous mail please split your series into three
distinct pull requests.


Request 1 - ARM64 improvements; acked by me

   [PATCH 1/8] lib/lzo: tidy-up ifdefs
   [PATCH 3/8] lib/lzo: enable 64-bit CTZ on Arm
   [PATCH 4/8] lib/lzo: 64-bit CTZ on arm64
   [PATCH 5/8] lib/lzo: fast 8-byte copy on arm64

are simple arch patches that give a nice speedup on ARM64 and should
get merged ASAP.



Request 2 - add COPY16; *NOT* acked by me

  [PATCH 2/8] lib/lzo: clean-up by introducing COPY16

is still not correct because of possible overlapping copies. I'll
address this on the weekend.



Request 3 - add lzo-rle; *NOT* acked by me

   [PATCH 6/8] lib/lzo: implement run-length encoding
   [PATCH 7/8] lib/lzo: separate lzo-rle from lzo
   [PATCH 8/8] zram: default to lzo-rle instead of lzo

This can *NOT* be applied in the current implementation.

It (1) silently changes the compressed data format, (2) crashes on MIPS,
and (3) makes compression and decompression on typical data 10% slower on
X86_64 with our internal benchmarks, and (4) has to be carefully checked
for buffer overflows.

I understand that we want some optimizations for data with many zeros like
in the typical ZRAM use case, but the implementation will clearly need some
more work. I'll also have a look at the weekend - eg I have a nice idea
how to deal with (1).



As a final comment, I question the quality your benchmarks - combining
arch-related ARM64 improvements and algorithmic changes into one
benchmark comparision is just unprofessional marketing.

Cheers,
Markus



-- 
Markus Oberhumer, <markus@oberhumer.com>, http://www.oberhumer.com/

RE: [PATCH v4 0/7] lib/lzo: performance improvements

[Matt Sealey]

Markus,

> Request 2 - add COPY16; *NOT* acked by me
> 
>   [PATCH 2/8] lib/lzo: clean-up by introducing COPY16
> 
> is still not correct because of possible overlapping copies. I'll
> address this on the weekend.

Can you give a syndrome as to why

{
	COPY8(op, ip);
	COPY8(op+8,ip+8);
	ip+=16;
	op+=16;
}

or

{ 
	COPY8(op, ip);
	ip+=8;
	op+=8;
	COPY8(op, ip);
	ip+=8;
	op+=8;
}

vs.

#define COPY16(dst,src) COPY8(dst,src); COPY8(dst+8,src+8)

{
	COPY16(op, ip);
	ip+=16;
	op+=16;
}

.. causes "overlapping copies"?

COPY8 was only ever used in pairs as above and the second method
broke compiler optimizers since it adds an artificial barrier
between the two groups. The only difference was that decompress
and compress had the pointer increments spread out. If we need
to fix that then that's a good reason, but your reasoning continues
to elude me.

I can refactor the patch to align the second method with the first
and make compress and decompress get the same codegen, which is
functionally identical to the COPY16 patch, but that would seem to
in your opinion be the whole problem..

I'll see what you've got after the weekend ;D

Ta
Matt Sealey

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Dave Rodgman]

Hi Markus,

On 06/12/2018 3:47 pm, Markus F.X.J. Oberhumer wrote:> Request 3 - add lzo-rle; *NOT* acked by me
 >
 >     [PATCH 6/8] lib/lzo: implement run-length encoding
 >     [PATCH 7/8] lib/lzo: separate lzo-rle from lzo
 >     [PATCH 8/8] zram: default to lzo-rle instead of lzo
 >
 > It (1) silently changes the compressed data format

I'm not sure this is relevant: as a separate algorithm, there's no reason
to retain the same format (although backwards compatibility can help with
migration). If you know of a way to improve the compatibility aspect
though, that would be great!

 > (2) crashes on MIPS,

Please could you provide more detail? I tested on x86-32, x86-64, arm,
arm64 and big-endian MIPS64, but if there is an issue I missed I'd like to
address it.

 > and (3) makes compression and decompression on typical data 10% slower on
 > X86_64 with our internal benchmarks,

It is of course data-dependent. In my testing, as I mentioned previously, RLE
without the other patches does regress slightly on high-entropy data, but
offers a win on low-entropy data. For the right applications (e.g., zram),
this makes it overall beneficial.

 > and (4) has to be carefully checked for buffer overflows.

This has been reviewed prior to sharing on LKML, and of course tested,
but further review is of course welcome.

 > As a final comment, I question the quality your benchmarks - combining
 > arch-related ARM64 improvements and algorithmic changes into one
 > benchmark comparision is just unprofessional marketing.

I felt it was helpful to show overall performance with the complete patchset:
this is what end-users experience. However, as you can see below, I also
previously shared a summary of the two main components of the patchset to
try and address this sort of concern:

 >> As a quick summary of the impact of these patches on bigger chunks of
 >> data, I've compared the performance of four different variants of lzo
 >> on two large (~40 MB) files. The numbers show round-trip throughput
 >> in MB/s:
 >>
 >> Variant         | Low-entropy | High-entropy
 >> Current lzo     |  242        | 157
 >> Arm opts        |  290        | 159
 >> RLE             |  876        | 151
 >> Arm opts + RLE  | 1150        | 181

cheers

Dave

Re: [PATCH v4 0/7] lib/lzo: performance improvements

[Dave Rodgman]

Hi Markus,

What are your thoughts on how we should proceed with this patchset? You raised a few concerns in December - however, I'm 
not sure what further changes might be needed, if any. IMO this could be merged as it stands.

Regarding compatibility concerns: patchset v4 does not modify the behaviour of existing lzo. It introduces an 
independent algorithm (closely based on lzo); and also introduces some Arm performance benefits for existing lzo, 
without modifying the behaviour. So I don't see a compatibility risk.

You mentioned a crash on MIPS - do you have any details on this please? I have not seen any crashes in my testing so I'm 
not able to look into this without more data.

On 07/12/2018 3:54 pm, Dave Rodgman wrote:
> Hi Markus,
> 
> On 06/12/2018 3:47 pm, Markus F.X.J. Oberhumer wrote:> Request 3 - add lzo-rle; *NOT* acked by me
>  >
>  >     [PATCH 6/8] lib/lzo: implement run-length encoding
>  >     [PATCH 7/8] lib/lzo: separate lzo-rle from lzo
>  >     [PATCH 8/8] zram: default to lzo-rle instead of lzo
>  >
>  > It (1) silently changes the compressed data format
> 
> I'm not sure this is relevant: as a separate algorithm, there's no reason
> to retain the same format (although backwards compatibility can help with
> migration). If you know of a way to improve the compatibility aspect
> though, that would be great!
> 
>  > (2) crashes on MIPS,
> 
> Please could you provide more detail? I tested on x86-32, x86-64, arm,
> arm64 and big-endian MIPS64, but if there is an issue I missed I'd like to
> address it.
> 
>  > and (3) makes compression and decompression on typical data 10% slower on
>  > X86_64 with our internal benchmarks,
> 
> It is of course data-dependent. In my testing, as I mentioned previously, RLE
> without the other patches does regress slightly on high-entropy data, but
> offers a win on low-entropy data. For the right applications (e.g., zram),
> this makes it overall beneficial.
> 
>  > and (4) has to be carefully checked for buffer overflows.
> 
> This has been reviewed prior to sharing on LKML, and of course tested,
> but further review is of course welcome.
> 
>  > As a final comment, I question the quality your benchmarks - combining
>  > arch-related ARM64 improvements and algorithmic changes into one
>  > benchmark comparision is just unprofessional marketing.
> 
> I felt it was helpful to show overall performance with the complete patchset:
> this is what end-users experience. However, as you can see below, I also
> previously shared a summary of the two main components of the patchset to
> try and address this sort of concern:
> 
>  >> As a quick summary of the impact of these patches on bigger chunks of
>  >> data, I've compared the performance of four different variants of lzo
>  >> on two large (~40 MB) files. The numbers show round-trip throughput
>  >> in MB/s:
>  >>
>  >> Variant         | Low-entropy | High-entropy
>  >> Current lzo     |  242        | 157
>  >> Arm opts        |  290        | 159
>  >> RLE             |  876        | 151
>  >> Arm opts + RLE  | 1150        | 181

Regards

Dave

[PATCH v3 0/7] lib/lzo: performance improvements

[Dave Rodgman]

This patch series introduces performance improvements for lzo.

The previous version of this patchset is here:
https://lkml.org/lkml/2018/11/27/1086

On 29/11/2018 8:32 pm, Andrew Morton wrote:
> On Thu, 29 Nov 2018 10:21:53 +0000 Dave Rodgman <dave.rodgman@arm.com> wrote:
>>> OK, so it's not just "separate lzo-rle", it's also "switch zram to
>>> a new compression algorithm by default". I'd say that usually I'd
>>> expect this to be separate patches.
>>
>> Yes, fair point. akpm has picked this up now though, so probably a bit
>> late to break it out into a separate patch?
> 
> That's fine.  akpm is flexible ;)  Whatever produces the best overall
> result in the permanent record, please.

Thanks - I've separated this as suggested. I've also fixed up the commit
message for patch 1 (tidy-up ifdefs) which looks like it was accidentally
changed (please ignore if the change was intentional).

Dave

[PATCH 1/8] lib/lzo: tidy-up ifdefs

[Dave Rodgman]

Modify the ifdefs in lzodefs.h to be more consistent with normal kernel
macros (e.g., change __aarch64__ to CONFIG_ARM64).

Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: David S. Miller <davem@davemloft.net>
Cc: Nitin Gupta <nitingupta910@gmail.com>
Cc: Richard Purdie <rpurdie@openedhand.com>
Cc: Markus F.X.J. Oberhumer <markus@oberhumer.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sonny Rao <sonnyrao@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Matt Sealey <matt.sealey@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 lib/lzo/lzodefs.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/lzo/lzodefs.h b/lib/lzo/lzodefs.h
index 4edefd2f540c..497f9c9f03a8 100644
--- a/lib/lzo/lzodefs.h
+++ b/lib/lzo/lzodefs.h
@@ -15,7 +15,7 @@
 
 #define COPY4(dst, src)	\
 		put_unaligned(get_unaligned((const u32 *)(src)), (u32 *)(dst))
-#if defined(__x86_64__)
+#if defined(CONFIG_X86_64)
 #define COPY8(dst, src)	\
 		put_unaligned(get_unaligned((const u64 *)(src)), (u64 *)(dst))
 #else
@@ -25,12 +25,12 @@
 
 #if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN)
 #error "conflicting endian definitions"
-#elif defined(__x86_64__)
+#elif defined(CONFIG_X86_64)
 #define LZO_USE_CTZ64	1
 #define LZO_USE_CTZ32	1
-#elif defined(__i386__) || defined(__powerpc__)
+#elif defined(CONFIG_X86) || defined(CONFIG_PPC)
 #define LZO_USE_CTZ32	1
-#elif defined(__arm__) && (__LINUX_ARM_ARCH__ >= 5)
+#elif defined(CONFIG_ARM) && (__LINUX_ARM_ARCH__ >= 5)
 #define LZO_USE_CTZ32	1
 #endif
 
-- 
2.17.1