Re: [PATCH] ubifs: default to zstd compression

[Rui Salvaterra]

Hi, Richard,

On Thu, 8 Apr 2021 at 12:00, Richard Weinberger
 wrote:
>
> I was about to NACK this patch but by looking at the diff I realized
> that you change
> the default compressor only for the default filesystem as created by
> UBIFS itself.

Yes, that was the idea. :)

> Queued for the merge window. :-)

Thanks a lot!

Rui

[PATCH] ubifs: default to zstd compression

[Rui Salvaterra]

Compared to lzo and zlib, zstd is the best all-around performer, both in terms
of speed and compression ratio. Set it as the default, if available.

Signed-off-by: Rui Salvaterra 
---
 fs/ubifs/sb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index c160f718c288..e7693b94e5b5 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -53,6 +53,9 @@
 
 static int get_default_compressor(struct ubifs_info *c)
 {
+   if (ubifs_compr_present(c, UBIFS_COMPR_ZSTD))
+   return UBIFS_COMPR_ZSTD;
+
if (ubifs_compr_present(c, UBIFS_COMPR_LZO))
return UBIFS_COMPR_LZO;
 
-- 
2.31.1

Re: [RFC PATCH v2] jffs2: add support for zstd compression

[Rui Salvaterra]

Friendly ping (and also cc'ing dwmw2).

On Tue, 16 Mar 2021 at 14:19, Rui Salvaterra  wrote:
>
> Implement support for zstd compression in jffs2 at the default compression
> level (3).
>
> Lightly tested in OpenWrt, on a single CPU embedded MIPS32 system (AirGrid 
> M2).

[RFC PATCH v2] jffs2: add support for zstd compression

[Rui Salvaterra]

Implement support for zstd compression in jffs2 at the default compression
level (3).

Lightly tested in OpenWrt, on a single CPU embedded MIPS32 system (AirGrid M2).

Signed-off-by: Rui Salvaterra 
---
v2: fix blunder in compression context allocation

 fs/jffs2/Kconfig   |   9 +++
 fs/jffs2/Makefile  |   1 +
 fs/jffs2/compr.c   |  10 +++
 fs/jffs2/compr.h   |   6 ++
 fs/jffs2/compr_zstd.c  | 131 +
 fs/jffs2/super.c   |   7 ++
 include/uapi/linux/jffs2.h |   1 +
 7 files changed, 165 insertions(+)
 create mode 100644 fs/jffs2/compr_zstd.c

diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
index 7c96bc107218..31308eb7267b 100644
--- a/fs/jffs2/Kconfig
+++ b/fs/jffs2/Kconfig
@@ -136,6 +136,15 @@ config JFFS2_LZO
  This feature was added in July, 2007. Say 'N' if you need
  compatibility with older bootloaders or kernels.
 
+config JFFS2_ZSTD
+   bool "JFFS2 zstd compression support" if JFFS2_COMPRESSION_OPTIONS
+   select ZSTD_COMPRESS
+   select ZSTD_DECOMPRESS
+   depends on JFFS2_FS
+   default n
+   help
+ Zstd compression.
+
 config JFFS2_RTIME
bool "JFFS2 RTIME compression support" if JFFS2_COMPRESSION_OPTIONS
depends on JFFS2_FS
diff --git a/fs/jffs2/Makefile b/fs/jffs2/Makefile
index 5294969d5bf9..75f84b1467c5 100644
--- a/fs/jffs2/Makefile
+++ b/fs/jffs2/Makefile
@@ -19,4 +19,5 @@ jffs2-$(CONFIG_JFFS2_RUBIN)   += compr_rubin.o
 jffs2-$(CONFIG_JFFS2_RTIME)+= compr_rtime.o
 jffs2-$(CONFIG_JFFS2_ZLIB) += compr_zlib.o
 jffs2-$(CONFIG_JFFS2_LZO)  += compr_lzo.o
+jffs2-$(CONFIG_JFFS2_ZSTD) += compr_zstd.o
 jffs2-$(CONFIG_JFFS2_SUMMARY)   += summary.o
diff --git a/fs/jffs2/compr.c b/fs/jffs2/compr.c
index 4849a4c9a0e2..d65e0c39c9c5 100644
--- a/fs/jffs2/compr.c
+++ b/fs/jffs2/compr.c
@@ -237,6 +237,10 @@ uint16_t jffs2_compress(struct jffs2_sb_info *c, struct 
jffs2_inode_info *f,
ret = jffs2_selected_compress(JFFS2_COMPR_ZLIB, data_in,
cpage_out, datalen, cdatalen);
break;
+   case JFFS2_COMPR_MODE_FORCEZSTD:
+   ret = jffs2_selected_compress(JFFS2_COMPR_ZSTD, data_in,
+   cpage_out, datalen, cdatalen);
+   break;
default:
pr_err("unknown compression mode\n");
}
@@ -378,6 +382,9 @@ int __init jffs2_compressors_init(void)
 #ifdef CONFIG_JFFS2_LZO
jffs2_lzo_init();
 #endif
+#ifdef CONFIG_JFFS2_ZSTD
+   jffs2_zstd_init();
+#endif
 /* Setting default compression mode */
 #ifdef CONFIG_JFFS2_CMODE_NONE
jffs2_compression_mode = JFFS2_COMPR_MODE_NONE;
@@ -413,6 +420,9 @@ int jffs2_compressors_exit(void)
 #endif
 #ifdef CONFIG_JFFS2_ZLIB
jffs2_zlib_exit();
+#endif
+#ifdef CONFIG_JFFS2_ZSTD
+   jffs2_zstd_exit();
 #endif
return 0;
 }
diff --git a/fs/jffs2/compr.h b/fs/jffs2/compr.h
index 5e91d578f4ed..8f7032c5ecb2 100644
--- a/fs/jffs2/compr.h
+++ b/fs/jffs2/compr.h
@@ -31,6 +31,7 @@
 #define JFFS2_RTIME_PRIORITY 50
 #define JFFS2_ZLIB_PRIORITY  60
 #define JFFS2_LZO_PRIORITY   80
+#define JFFS2_ZSTD_PRIORITY  90
 
 
 #define JFFS2_RUBINMIPS_DISABLED /* RUBINs will be used only */
@@ -42,6 +43,7 @@
 #define JFFS2_COMPR_MODE_FAVOURLZO  3
 #define JFFS2_COMPR_MODE_FORCELZO   4
 #define JFFS2_COMPR_MODE_FORCEZLIB  5
+#define JFFS2_COMPR_MODE_FORCEZSTD  6
 
 #define FAVOUR_LZO_PERCENT 80
 
@@ -101,5 +103,9 @@ void jffs2_zlib_exit(void);
 int jffs2_lzo_init(void);
 void jffs2_lzo_exit(void);
 #endif
+#ifdef CONFIG_JFFS2_ZSTD
+int jffs2_zstd_init(void);
+void jffs2_zstd_exit(void);
+#endif
 
 #endif /* __JFFS2_COMPR_H__ */
diff --git a/fs/jffs2/compr_zstd.c b/fs/jffs2/compr_zstd.c
new file mode 100644
index ..bfe4607575da
--- /dev/null
+++ b/fs/jffs2/compr_zstd.c
@@ -0,0 +1,131 @@
+
+#include 
+#include "compr.h"
+
+#define ZSTD_DEF_LEVEL 3
+
+static ZSTD_CCtx *cctx;
+static ZSTD_DCtx *dctx;
+static void *cwksp;
+static void *dwksp;
+
+static ZSTD_parameters zstd_params(void)
+{
+   return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0);
+}
+
+static int zstd_comp_init(void)
+{
+   int ret = 0;
+   const ZSTD_parameters params = zstd_params();
+   const size_t wksp_size = ZSTD_CCtxWorkspaceBound(params.cParams);
+
+   cwksp = vzalloc(wksp_size);
+   if (!cwksp) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   cctx = ZSTD_initCCtx(cwksp, wksp_size);
+   if (!cctx) {
+   ret = -EINVAL;
+   goto out_free;
+   }
+out:
+   return ret;
+out_free:
+   vfree(cwksp);
+   goto out;
+}
+
+static int zstd_decomp_init(void)
+{
+   int ret = 0;
+   const size_t wksp_size = ZSTD_DCtxWorkspaceBound();
+
+   dwksp = vzalloc(wksp_size);
+   if (!dwksp) {
+   ret = -ENOMEM;
+   goto out;
+ 

Re: [RFC PATCH] jffs2: add support for zstd compression

[Rui Salvaterra]

Hi,

Please ignore this, I noticed a bug. I'll send a fixed v2 shortly.

Thanks,
Rui

On Fri, 12 Mar 2021 at 09:56, Rui Salvaterra  wrote:
>
> Implement support for zstd compression in jffs2 at the default compression
> level (3).
>
> Lightly tested in OpenWrt, on a single CPU embedded MIPS32 system (AirGrid 
> M2).
>
> Signed-off-by: Rui Salvaterra 
> ---
> Sent as RFC, since I have no idea if locking is required (the crypto API
> implementation doesn't do it, at least).
>
>  fs/jffs2/Kconfig   |   9 +++
>  fs/jffs2/Makefile  |   1 +
>  fs/jffs2/compr.c   |  10 +++
>  fs/jffs2/compr.h   |   6 ++
>  fs/jffs2/compr_zstd.c  | 131 +
>  fs/jffs2/super.c   |   7 ++
>  include/uapi/linux/jffs2.h |   1 +
>  7 files changed, 165 insertions(+)
>  create mode 100644 fs/jffs2/compr_zstd.c
>
> diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
> index 7c96bc107218..31308eb7267b 100644
> --- a/fs/jffs2/Kconfig
> +++ b/fs/jffs2/Kconfig
> @@ -136,6 +136,15 @@ config JFFS2_LZO
>   This feature was added in July, 2007. Say 'N' if you need
>   compatibility with older bootloaders or kernels.
>
> +config JFFS2_ZSTD
> +   bool "JFFS2 zstd compression support" if JFFS2_COMPRESSION_OPTIONS
> +   select ZSTD_COMPRESS
> +   select ZSTD_DECOMPRESS
> +   depends on JFFS2_FS
> +   default n
> +   help
> + Zstd compression.
> +
>  config JFFS2_RTIME
> bool "JFFS2 RTIME compression support" if JFFS2_COMPRESSION_OPTIONS
> depends on JFFS2_FS
> diff --git a/fs/jffs2/Makefile b/fs/jffs2/Makefile
> index 5294969d5bf9..75f84b1467c5 100644
> --- a/fs/jffs2/Makefile
> +++ b/fs/jffs2/Makefile
> @@ -19,4 +19,5 @@ jffs2-$(CONFIG_JFFS2_RUBIN)   += compr_rubin.o
>  jffs2-$(CONFIG_JFFS2_RTIME)+= compr_rtime.o
>  jffs2-$(CONFIG_JFFS2_ZLIB) += compr_zlib.o
>  jffs2-$(CONFIG_JFFS2_LZO)  += compr_lzo.o
> +jffs2-$(CONFIG_JFFS2_ZSTD) += compr_zstd.o
>  jffs2-$(CONFIG_JFFS2_SUMMARY)   += summary.o
> diff --git a/fs/jffs2/compr.c b/fs/jffs2/compr.c
> index 4849a4c9a0e2..d65e0c39c9c5 100644
> --- a/fs/jffs2/compr.c
> +++ b/fs/jffs2/compr.c
> @@ -237,6 +237,10 @@ uint16_t jffs2_compress(struct jffs2_sb_info *c, struct 
> jffs2_inode_info *f,
> ret = jffs2_selected_compress(JFFS2_COMPR_ZLIB, data_in,
> cpage_out, datalen, cdatalen);
> break;
> +   case JFFS2_COMPR_MODE_FORCEZSTD:
> +   ret = jffs2_selected_compress(JFFS2_COMPR_ZSTD, data_in,
> +   cpage_out, datalen, cdatalen);
> +   break;
> default:
> pr_err("unknown compression mode\n");
> }
> @@ -378,6 +382,9 @@ int __init jffs2_compressors_init(void)
>  #ifdef CONFIG_JFFS2_LZO
> jffs2_lzo_init();
>  #endif
> +#ifdef CONFIG_JFFS2_ZSTD
> +   jffs2_zstd_init();
> +#endif
>  /* Setting default compression mode */
>  #ifdef CONFIG_JFFS2_CMODE_NONE
> jffs2_compression_mode = JFFS2_COMPR_MODE_NONE;
> @@ -413,6 +420,9 @@ int jffs2_compressors_exit(void)
>  #endif
>  #ifdef CONFIG_JFFS2_ZLIB
> jffs2_zlib_exit();
> +#endif
> +#ifdef CONFIG_JFFS2_ZSTD
> +   jffs2_zstd_exit();
>  #endif
> return 0;
>  }
> diff --git a/fs/jffs2/compr.h b/fs/jffs2/compr.h
> index 5e91d578f4ed..8f7032c5ecb2 100644
> --- a/fs/jffs2/compr.h
> +++ b/fs/jffs2/compr.h
> @@ -31,6 +31,7 @@
>  #define JFFS2_RTIME_PRIORITY 50
>  #define JFFS2_ZLIB_PRIORITY  60
>  #define JFFS2_LZO_PRIORITY   80
> +#define JFFS2_ZSTD_PRIORITY  90
>
>
>  #define JFFS2_RUBINMIPS_DISABLED /* RUBINs will be used only */
> @@ -42,6 +43,7 @@
>  #define JFFS2_COMPR_MODE_FAVOURLZO  3
>  #define JFFS2_COMPR_MODE_FORCELZO   4
>  #define JFFS2_COMPR_MODE_FORCEZLIB  5
> +#define JFFS2_COMPR_MODE_FORCEZSTD  6
>
>  #define FAVOUR_LZO_PERCENT 80
>
> @@ -101,5 +103,9 @@ void jffs2_zlib_exit(void);
>  int jffs2_lzo_init(void);
>  void jffs2_lzo_exit(void);
>  #endif
> +#ifdef CONFIG_JFFS2_ZSTD
> +int jffs2_zstd_init(void);
> +void jffs2_zstd_exit(void);
> +#endif
>
>  #endif /* __JFFS2_COMPR_H__ */
> diff --git a/fs/jffs2/compr_zstd.c b/fs/jffs2/compr_zstd.c
> new file mode 100644
> index ..86b3d28670d7
> --- /dev/null
> +++ b/fs/jffs2/compr_zstd.c
> @@ -0,0 +1,131 @@
> +
> +#include 
> +#include "compr.h"
> +
> +#define ZSTD_DEF_LEVEL 3
> +
> +static ZSTD_CCtx *cctx;
> +static ZSTD_DCtx *dctx;
> +static void *cwksp;
> +static void *dwksp;
> +
> +static ZSTD_pa

[RFC PATCH] jffs2: add support for zstd compression

[Rui Salvaterra]

Implement support for zstd compression in jffs2 at the default compression
level (3).

Lightly tested in OpenWrt, on a single CPU embedded MIPS32 system (AirGrid M2).

Signed-off-by: Rui Salvaterra 
---
Sent as RFC, since I have no idea if locking is required (the crypto API
implementation doesn't do it, at least).

 fs/jffs2/Kconfig   |   9 +++
 fs/jffs2/Makefile  |   1 +
 fs/jffs2/compr.c   |  10 +++
 fs/jffs2/compr.h   |   6 ++
 fs/jffs2/compr_zstd.c  | 131 +
 fs/jffs2/super.c   |   7 ++
 include/uapi/linux/jffs2.h |   1 +
 7 files changed, 165 insertions(+)
 create mode 100644 fs/jffs2/compr_zstd.c

diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
index 7c96bc107218..31308eb7267b 100644
--- a/fs/jffs2/Kconfig
+++ b/fs/jffs2/Kconfig
@@ -136,6 +136,15 @@ config JFFS2_LZO
  This feature was added in July, 2007. Say 'N' if you need
  compatibility with older bootloaders or kernels.
 
+config JFFS2_ZSTD
+   bool "JFFS2 zstd compression support" if JFFS2_COMPRESSION_OPTIONS
+   select ZSTD_COMPRESS
+   select ZSTD_DECOMPRESS
+   depends on JFFS2_FS
+   default n
+   help
+ Zstd compression.
+
 config JFFS2_RTIME
bool "JFFS2 RTIME compression support" if JFFS2_COMPRESSION_OPTIONS
depends on JFFS2_FS
diff --git a/fs/jffs2/Makefile b/fs/jffs2/Makefile
index 5294969d5bf9..75f84b1467c5 100644
--- a/fs/jffs2/Makefile
+++ b/fs/jffs2/Makefile
@@ -19,4 +19,5 @@ jffs2-$(CONFIG_JFFS2_RUBIN)   += compr_rubin.o
 jffs2-$(CONFIG_JFFS2_RTIME)+= compr_rtime.o
 jffs2-$(CONFIG_JFFS2_ZLIB) += compr_zlib.o
 jffs2-$(CONFIG_JFFS2_LZO)  += compr_lzo.o
+jffs2-$(CONFIG_JFFS2_ZSTD) += compr_zstd.o
 jffs2-$(CONFIG_JFFS2_SUMMARY)   += summary.o
diff --git a/fs/jffs2/compr.c b/fs/jffs2/compr.c
index 4849a4c9a0e2..d65e0c39c9c5 100644
--- a/fs/jffs2/compr.c
+++ b/fs/jffs2/compr.c
@@ -237,6 +237,10 @@ uint16_t jffs2_compress(struct jffs2_sb_info *c, struct 
jffs2_inode_info *f,
ret = jffs2_selected_compress(JFFS2_COMPR_ZLIB, data_in,
cpage_out, datalen, cdatalen);
break;
+   case JFFS2_COMPR_MODE_FORCEZSTD:
+   ret = jffs2_selected_compress(JFFS2_COMPR_ZSTD, data_in,
+   cpage_out, datalen, cdatalen);
+   break;
default:
pr_err("unknown compression mode\n");
}
@@ -378,6 +382,9 @@ int __init jffs2_compressors_init(void)
 #ifdef CONFIG_JFFS2_LZO
jffs2_lzo_init();
 #endif
+#ifdef CONFIG_JFFS2_ZSTD
+   jffs2_zstd_init();
+#endif
 /* Setting default compression mode */
 #ifdef CONFIG_JFFS2_CMODE_NONE
jffs2_compression_mode = JFFS2_COMPR_MODE_NONE;
@@ -413,6 +420,9 @@ int jffs2_compressors_exit(void)
 #endif
 #ifdef CONFIG_JFFS2_ZLIB
jffs2_zlib_exit();
+#endif
+#ifdef CONFIG_JFFS2_ZSTD
+   jffs2_zstd_exit();
 #endif
return 0;
 }
diff --git a/fs/jffs2/compr.h b/fs/jffs2/compr.h
index 5e91d578f4ed..8f7032c5ecb2 100644
--- a/fs/jffs2/compr.h
+++ b/fs/jffs2/compr.h
@@ -31,6 +31,7 @@
 #define JFFS2_RTIME_PRIORITY 50
 #define JFFS2_ZLIB_PRIORITY  60
 #define JFFS2_LZO_PRIORITY   80
+#define JFFS2_ZSTD_PRIORITY  90
 
 
 #define JFFS2_RUBINMIPS_DISABLED /* RUBINs will be used only */
@@ -42,6 +43,7 @@
 #define JFFS2_COMPR_MODE_FAVOURLZO  3
 #define JFFS2_COMPR_MODE_FORCELZO   4
 #define JFFS2_COMPR_MODE_FORCEZLIB  5
+#define JFFS2_COMPR_MODE_FORCEZSTD  6
 
 #define FAVOUR_LZO_PERCENT 80
 
@@ -101,5 +103,9 @@ void jffs2_zlib_exit(void);
 int jffs2_lzo_init(void);
 void jffs2_lzo_exit(void);
 #endif
+#ifdef CONFIG_JFFS2_ZSTD
+int jffs2_zstd_init(void);
+void jffs2_zstd_exit(void);
+#endif
 
 #endif /* __JFFS2_COMPR_H__ */
diff --git a/fs/jffs2/compr_zstd.c b/fs/jffs2/compr_zstd.c
new file mode 100644
index ..86b3d28670d7
--- /dev/null
+++ b/fs/jffs2/compr_zstd.c
@@ -0,0 +1,131 @@
+
+#include 
+#include "compr.h"
+
+#define ZSTD_DEF_LEVEL 3
+
+static ZSTD_CCtx *cctx;
+static ZSTD_DCtx *dctx;
+static void *cwksp;
+static void *dwksp;
+
+static ZSTD_parameters zstd_params(void)
+{
+   return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0);
+}
+
+static int zstd_comp_init(void)
+{
+   int ret = 0;
+   const ZSTD_parameters params = zstd_params();
+   const size_t wksp_size = ZSTD_CCtxWorkspaceBound(params.cParams);
+
+   cwksp = vzalloc(wksp_size);
+   if (!cwksp) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   cctx = ZSTD_initCCtx(cwksp, wksp_size);
+   if (cctx) {
+   ret = -EINVAL;
+   goto out_free;
+   }
+out:
+   return ret;
+out_free:
+   vfree(cwksp);
+   goto out;
+}
+
+static int zstd_decomp_init(void)
+{
+   int ret = 0;
+   const size_t wksp_size = ZSTD_DCtxWorkspaceBound();
+
+   dwksp = vzalloc(wksp_size);
+   if 

Re: [PATCH] ARM: dts: turris-omnia: fix hardware buffer management

[Rui Salvaterra]

Hi, Andrew,

On Wed, 17 Feb 2021 at 16:22, Andrew Lunn  wrote:
>
> Hi Rui
>
> I don't know all the details for the MBus Windows...

Neither do I, I just followed the examples in other device trees
(namely armada-385-linksys.dtsi). I wanted to get hardware buffer
management working on my Omnia, in OpenWrt, so I started digging. :)

> Can this be set once in armada-385.dtsi ?

If all SoCs support it, I don't see why not. However, I have to defer
to someone with experience on these systems.

> Did you check the other dts files. Do any others have the same
> problem?

I did a cursory grep, yes. I see a lot of .dts files with the correct
declaration, so probably aren't that many missing (if any at all,
anymore).

Thanks,
Rui

Re: [PATCH] ARM: dts: turris-omnia: fix hardware buffer management

[Rui Salvaterra]

Hi again, Marek,

On Wed, 17 Feb 2021 at 16:10, Marek Behún  wrote:
>
> Rui, in the future make the subject prefix
>   [PATCH mvebu-dt]
> or
>   [PATCH mvebu/dt]
>
> so that Gregory knows its for him and for which branch.

Thanks, will do!

Cheers,
Rui

Re: [PATCH] ARM: dts: turris-omnia: fix hardware buffer management

[Rui Salvaterra]

Hi, Marek,

On Wed, 17 Feb 2021 at 15:42, Marek Behún  wrote:
>
> /o\ How did I manage to miss this?

Heh… it happens. :)

> Please wait a few minutes I am just going to do a fast compile and test.

No worries. I'm going to cook a backport for OpenWrt.

Cheers,
Rui

[PATCH] ARM: dts: turris-omnia: fix hardware buffer management

[Rui Salvaterra]

Hardware buffer management has never worked on the Turris Omnia, as the
required MBus window hadn't been reserved. Fix thusly.

Fixes: 018b88eee1a2 ("ARM: dts: turris-omnia: enable HW buffer management")

Signed-off-by: Rui Salvaterra 
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts 
b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 646a06420c77..dc80a909ea88 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -32,7 +32,8 @@ soc {
ranges = ;
+ MBUS_ID(0x09, 0x15) 0 0xf111 0x1
+ MBUS_ID(0x0c, 0x04) 0 0xf120 0x10>;
 
internal-regs {
 
-- 
2.30.1

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, again,

I haven't tested any patch or bisected, but I have another data point.
I built and tested Linux 5.8.18, with the same firmware, and it is
working correctly. I reduced the test case to just rfkilling the
connection, which showed the register dump immediately (before that I
was using the airplane toggle on the keyboard, which isn't working
correctly, it disables and immediately reenables the radio, for some
unfathomable reason).
So, now I'm inclined to believe this is some sort of race condition
between rfkill and pending transactions.

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi again, Emmanuel,

On Wed, 9 Dec 2020 at 21:07, Emmanuel Grumbach  wrote:
>
> Indeed, the bit is reverse logic. So we can put that aside.
> Frankly, I have no clue. You can try our backport tree to bisect,
> should be easier..
> What I see here is that your GP_CTRL value is 080003d8
>
> #define CSR_GP_CNTRL_REG_FLAG_HW_RF_KILL_SW  (0x0800)
> which means sense since apparently, HW RF-Kill was asserted.
> #define CSR_GP_CNTRL_REG_FLAG_GOING_TO_SLEEP (0x0010)
> Which means that the device is going to sleep... And that's the problem:
>
> iwl_trans_pcie_grab_nic_access:
> ret = iwl_poll_bit(trans, CSR_GP_CNTRL,
>CSR_GP_CNTRL_REG_VAL_MAC_ACCESS_EN,
>(CSR_GP_CNTRL_REG_FLAG_MAC_CLOCK_READY |
> CSR_GP_CNTRL_REG_FLAG_GOING_TO_SLEEP), 15000);
> if (unlikely(ret < 0)) {
> u32 cntrl = iwl_read32(trans, CSR_GP_CNTRL);
>
> WARN_ONCE(1,
>   "Timeout waiting for hardware access
> (CSR_GP_CNTRL 0x%08x)\n",
>   cntrl);
>
> but I'd expect the splat in your log...
> Or maybe you can't load the firmware?

Well, my kernel doesn't have any modules, it's all built-in. The
firmware is obviously loading fine, otherwise the card wouldn't work,
but yeah, that WARN_ONCE hasn't triggered at all.

> Can you try this:
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> index 2fffbbc8462f..748300752630 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -2121,6 +2121,7 @@ static bool
> iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans,
>  * track nic_access anyway.
>  */
> __release(_pcie->reg_lock);
> +   mdelay(1);
> return true;
>  }
>
> If that helps, then... I'd have no clue why it helps, but this
> specific device caused us trouble like bad timing after
> grab_nic_access..

I'll give it a spin. Nasty hack, but if it works, it works. :)

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, again,

On Wed, 9 Dec 2020 at 20:40, Emmanuel Grumbach  wrote:
>
> Besides, don't you get a stack dump in the vicinity of this register
> dump? That's be helpful to see.

Nope. No stack trace at all. Only the register dump.

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, Emmanuel,

On Wed, 9 Dec 2020 at 20:32, Emmanuel Grumbach  wrote:
>
> Rui, I looked at the register dump and looks like you're using AMT on
> your system?
> Can you confirm?

AMT? You mean Intel Active Management? Heavens, no, not that I know
of! This is a personal laptop (Lenovo B51-80). (And I'd personally
kill the ME with fire, if I could.)

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, guys,

On Wed, 9 Dec 2020 at 17:13, Jakub Kicinski  wrote:
>
> Any luck figuring this out, Luca? If this is a 5.10 regression we need
> to let Linus know tomorrow, so the time is ticking :(

I don't have the possibility to test other kernels at the moment, but
I will do so in a few days (at least to find a working version to
bisect). Meanwhile, I don't know if this is relevant or not, but I'm
using WPA3 PSK.

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, Luca,

On Tue, 8 Dec 2020 at 16:27, Coelho, Luciano  wrote:
>
> On Tue, 2020-12-08 at 11:27 +0000, Rui Salvaterra wrote:
> >
> > [ 3174.003910] iwlwifi :02:00.0: RF_KILL bit toggled to disable radio.
> > [ 3174.003913] iwlwifi :02:00.0: reporting RF_KILL (radio disabled)
>
> It looks like your machine is reporting RF-Kill to the WiFi device.

Yes, that's an artifact of how I tested: I rebooted the router, the
Wi-Fi interface disassociated and the dmesg was clean. However, after
the router came up, the laptop didn't reconnect (and the connection
had completely disappeared from nmtui). Afterwards, I did the rfkill
cycle you see, and only then I got the register dump.

> There seems to be some sort of race there that is causing us to still
> try to communicate with the device (and thus you see the transaction
> failed dump), but that will obviously fail when RF-Kill is enabled.

I'm not sure about that, the card was already dead before the rfkill cycle.

Thanks,
Rui

Re: [BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, Jakub,

On Tue, 8 Dec 2020 at 16:06, Jakub Kicinski  wrote:
>
> Just to confirm - is this a regression in 5.10-rc? Does 5.9 work
> smoothly in the problematic scenario?

Good question. It's definitely a regression, though I don't remember
exactly the last working version I had. I *think* 5.9 was working, but
I can't be sure without testing.

Thanks,
Rui

[BUG] iwlwifi: card unusable after firmware crash

[Rui Salvaterra]

Hi, everyone,

I'm running Linux 5.10-rc7 with this firmware/hardware:

[1.431878] iwlwifi :02:00.0: loaded firmware version
29.198743027.0 7265D-29.ucode op_mode iwlmvm
[1.431899] iwlwifi :02:00.0: Detected Intel(R) Dual Band
Wireless AC 3165, REV=0x210

Most of the time, after disabling/enabling Wi-Fi (or, say, rebooting
the router), the card dies and only comes back after reboot. This is
the relevant dmesg output:

[ 3174.003910] iwlwifi :02:00.0: RF_KILL bit toggled to disable radio.
[ 3174.003913] iwlwifi :02:00.0: reporting RF_KILL (radio disabled)
[ 3174.040788] iwlwifi :02:00.0: iwlwifi transaction failed,
dumping registers
[ 3174.040790] iwlwifi :02:00.0: iwlwifi device config registers:
[ 3174.041008] iwlwifi :02:00.0: : 31668086 00100406
02800099 0010 c114   
[ 3174.041010] iwlwifi :02:00.0: 0020:  
 42108086  00c8  01ff
[ 3174.041011] iwlwifi :02:00.0: 0040: 00020010 10008ec0
00100c10 0046e811 10110142   
[ 3174.041013] iwlwifi :02:00.0: 0060:  00080812
0005  00010001   
[ 3174.041014] iwlwifi :02:00.0: 0080:  
     
[ 3174.041016] iwlwifi :02:00.0: 00a0:  
     
[ 3174.041017] iwlwifi :02:00.0: 00c0:  
c823d001 0d00 00814005 fee00318  
[ 3174.041019] iwlwifi :02:00.0: 00e0:  
     
[ 3174.041020] iwlwifi :02:00.0: 0100: 14010001 
 00462031  2000  
[ 3174.041022] iwlwifi :02:00.0: 0120:  
     
[ 3174.041023] iwlwifi :02:00.0: 0140: 14c10003 ff75a50d
e09467ff 15410018 10031003 0001001e 00f01e1f 6003000b
[ 3174.041024] iwlwifi :02:00.0: iwlwifi device memory mapped registers:
[ 3174.041075] iwlwifi :02:00.0: : 0080 
0080     
[ 3174.041076] iwlwifi :02:00.0: 0020: 0001 080003d8
0210 d5d5  d5d5 80008040 00080042
[ 3174.041088] iwlwifi :02:00.0: iwlwifi device AER capability structure:
[ 3174.041112] iwlwifi :02:00.0: : 14010001 
 00462031  2000  
[ 3174.041113] iwlwifi :02:00.0: 0020:   
[ 3174.041114] iwlwifi :02:00.0: iwlwifi parent port
(:00:1c.5) config registers:
[ 3174.041198] iwlwifi :00:1c.5: : 9d158086 00100407
060400f1 00810010   00020200 20f0
[ 3174.041200] iwlwifi :00:1c.5: 0020: c110c110 0001fff1
   0040  000202ff
[ 3174.041201] iwlwifi :00:1c.5: 0040: 01428010 8001
0011 06724813 70110042 002cb200 0140 
[ 3174.041203] iwlwifi :00:1c.5: 0060:  0037
 000e 00010003   
[ 3174.041204] iwlwifi :00:1c.5: 0080: 00019005 fee00258
  a00d 382a17aa  
[ 3174.041206] iwlwifi :00:1c.5: 00a0: c8030001 
     
[ 3174.041207] iwlwifi :00:1c.5: 00c0:  
  07001001 1842 8b1e0008 
[ 3174.041209] iwlwifi :00:1c.5: 00e0: 00630300 
0016  0050 4c00 08230fb3 0204
[ 3174.041210] iwlwifi :00:1c.5: 0100: 1400 
 00060011 0001 2000  
[ 3174.041211] iwlwifi :00:1c.5: 0120:  
     
[ 3174.041213] iwlwifi :00:1c.5: 0140: 2001000d 000f
     
[ 3174.041214] iwlwifi :00:1c.5: 0160:  
     
[ 3174.041216] iwlwifi :00:1c.5: 0180:  
     
[ 3174.041217] iwlwifi :00:1c.5: 01a0:  
     
[ 3174.041218] iwlwifi :00:1c.5: 01c0:  
     
[ 3174.041220] iwlwifi :00:1c.5: 01e0:  
     
[ 3174.041221] iwlwifi :00:1c.5: 0200: 2201001e 0028281f 6003280b

I also had this problem with the previous firmware version. Any ideas
about what could be causing it?

Thanks,
Rui

[PATCH v7] zram: break the strict dependency from lzo

[Rui Salvaterra]

>From the beginning, the zram block device always enabled CRYPTO_LZO, since
lzo-rle is hardcoded as the fallback compression algorithm. As a consequence, on
systems where another compression algorithm is chosen (e.g. CRYPTO_ZSTD), the
lzo kernel module becomes unused, while still having to be built/loaded.

This patch removes the hardcoded lzo-rle dependency and allows the user to
select the default compression algorithm for zram at build time. The previous
behaviour is kept, as the default algorithm is still lzo-rle.

Suggested-by: Sergey Senozhatsky 
Suggested-by: Minchan Kim 
Signed-off-by: Rui Salvaterra 
---
v7: rebase against 5.10-rc7 and fix unmet direct dependencies, as reported by
Stephen Rothwell on linux-next.
v6: simplify the kconfig as per Minchan's suggestion.
v5: incorporate Minchan's feedback. Allow the user to choose a default 
algorithm.
v4: incorporate Sergey's feedback and fix a small typo.
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.

I had to change CRYPTO_LZO from "select" to "depends on", since it had the
unfortunate effect of causing unmet direct dependencies on some kernel
configurations. This also means at least one supported compression algorithm
must be selected in order for the zram block device to appear on the menu,
which isn't the perfect solution I'd hoped for, but it works for me (and make
oldconfig doesn't break existing configurations).

 drivers/block/zram/Kconfig| 42 ++-
 drivers/block/zram/zcomp.c|  2 ++
 drivers/block/zram/zram_drv.c |  2 +-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..668c6bf2554d 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,7 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
+   depends on CRYPTO_LZO || CRYPTO_ZSTD || CRYPTO_LZ4 || CRYPTO_LZ4HC || 
CRYPTO_842
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -14,6 +14,46 @@ config ZRAM
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
 
+choice
+   prompt "Default zram compressor"
+   default ZRAM_DEF_COMP_LZORLE
+   depends on ZRAM
+
+config ZRAM_DEF_COMP_LZORLE
+   bool "lzo-rle"
+   depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_ZSTD
+   bool "zstd"
+   depends on CRYPTO_ZSTD
+
+config ZRAM_DEF_COMP_LZ4
+   bool "lz4"
+   depends on CRYPTO_LZ4
+
+config ZRAM_DEF_COMP_LZO
+   bool "lzo"
+   depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_LZ4HC
+   bool "lz4hc"
+   depends on CRYPTO_LZ4HC
+
+config ZRAM_DEF_COMP_842
+   bool "842"
+   depends on CRYPTO_842
+
+endchoice
+
+config ZRAM_DEF_COMP
+   string
+   default "lzo-rle" if ZRAM_DEF_COMP_LZORLE
+   default "zstd" if ZRAM_DEF_COMP_ZSTD
+   default "lz4" if ZRAM_DEF_COMP_LZ4
+   default "lzo" if ZRAM_DEF_COMP_LZO
+   default "lz4hc" if ZRAM_DEF_COMP_LZ4HC
+   default "842" if ZRAM_DEF_COMP_842
+
 config ZRAM_WRITEBACK
bool "Write back incompressible or idle page to backing device"
depends on ZRAM
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..9ddccb968c68 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,7 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo-rle";
+static const char *default_compressor = CONFIG_ZRAM_DEF_COMP;
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
-- 
2.29.2

Re: linux-next: build failure after merge of the akpm-current tree

[Rui Salvaterra]

On Thu, 3 Dec 2020 at 09:37, Rui Salvaterra  wrote:
>
> Thanks for the heads-up, I think I know where the problem is.

Then again, maybe not. I don't have a PowerPC machine to test, at the
moment, and all my x86(-64) machines work fine. If no one beats me to
it, I can debug on an iBook G4, but only next week.

Re: linux-next: build failure after merge of the akpm-current tree

[Rui Salvaterra]

Hi, Stephen,

On Thu, 3 Dec 2020 at 09:08, Stephen Rothwell  wrote:
>
> Hi all,
>
> After merging the akpm-current tree, today's linux-next build (powerpc
> ppc44x_defconfig) failed like this:
>
[…]
>
> Caused by commit
>
>   a6d52df2d8bc ("zram: break the strict dependency from lzo")
>
> I have reverted that commit for today.

Thanks for the heads-up, I think I know where the problem is.

Cheers,
Rui

[BUG] intel_idle: build failure on Linux 5.10-rc6

[Rui Salvaterra]

Hi, Peter,

I'm sorry to bother you, but commit
6e1d2bc675bd57640f5658a4a657ae488db4c204 ("intel_idle: Fix
intel_idle() vs tracing") broke my build, when CONFIG_ACPI_PROCESSOR
is disabled (possibly because it selects CONFIG_ACPI_PROCESSOR_IDLE):

drivers/idle/intel_idle.c: In function ‘intel_idle_init_cstates_icpu’:
drivers/idle/intel_idle.c:1510:7: error: implicit declaration of
function ‘intel_idle_state_needs_timer_stop’
[-Werror=implicit-function-declaration]
 1510 |   if (intel_idle_state_needs_timer_stop(>states[drv->state_count]))
  |   ^

Reverting the aforementioned commit fixes it. Config attached. Please
let me know if you need anything else!

Thanks,
Rui


.config
Description: Binary data

Re: [PATCH v6] zram: break the strict dependency from lzo

[Rui Salvaterra]

On Wed, 25 Nov 2020 at 19:56, Minchan Kim  wrote:
>
> Acked-by: Minchan Kim 
>
> Thanks, Rui!

You're welcome, thanks for the patience! :)

[PATCH v6] zram: break the strict dependency from lzo

[Rui Salvaterra]

>From the beginning, the zram block device always enabled CRYPTO_LZO, since
lzo-rle is hardcoded as the fallback compression algorithm. As a consequence, on
systems where another compression algorithm is chosen (e.g. CRYPTO_ZSTD), the
lzo kernel module becomes unused, while still having to be built/loaded.

This patch removes the hardcoded lzo-rle dependency and allows the user to
select the default compression algorithm for zram at build time. The previous
behaviour is kept, as the default algorithm is still lzo-rle.

Suggested-by: Sergey Senozhatsky 
Suggested-by: Minchan Kim 
Signed-off-by: Rui Salvaterra 
---
v6: simplify the kconfig as per Minchan's suggestion.
v5: incorporate Minchan's feedback. Allow the user to choose a default 
algorithm.
v4: incorporate Sergey's feedback and fix a small typo.
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.

 drivers/block/zram/Kconfig| 41 ++-
 drivers/block/zram/zcomp.c|  2 ++
 drivers/block/zram/zram_drv.c |  2 +-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..6303fe5dee88 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -14,6 +13,46 @@ config ZRAM
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
 
+choice
+   prompt "Default zram compressor"
+   default ZRAM_DEF_COMP_LZORLE
+   depends on ZRAM || CRYPTO_ZSTD || CRYPTO_LZ4 || CRYPTO_LZ4HC || 
CRYPTO_842
+
+config ZRAM_DEF_COMP_LZORLE
+   bool "lzo-rle"
+   select CRYPTO_LZO
+
+config ZRAM_DEF_COMP_ZSTD
+   bool "zstd"
+   depends on CRYPTO_ZSTD
+
+config ZRAM_DEF_COMP_LZ4
+   bool "lz4"
+   depends on CRYPTO_LZ4
+
+config ZRAM_DEF_COMP_LZO
+   bool "lzo"
+   select CRYPTO_LZO
+
+config ZRAM_DEF_COMP_LZ4HC
+   bool "lz4hc"
+   depends on CRYPTO_LZ4HC
+
+config ZRAM_DEF_COMP_842
+   bool "842"
+   depends on CRYPTO_842
+
+endchoice
+
+config ZRAM_DEF_COMP
+   string
+   default "lzo-rle" if ZRAM_DEF_COMP_LZORLE
+   default "zstd" if ZRAM_DEF_COMP_ZSTD
+   default "lz4" if ZRAM_DEF_COMP_LZ4
+   default "lzo" if ZRAM_DEF_COMP_LZO
+   default "lz4hc" if ZRAM_DEF_COMP_LZ4HC
+   default "842" if ZRAM_DEF_COMP_842
+
 config ZRAM_WRITEBACK
bool "Write back incompressible or idle page to backing device"
depends on ZRAM
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..9ddccb968c68 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,7 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo-rle";
+static const char *default_compressor = CONFIG_ZRAM_DEF_COMP;
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
-- 
2.29.2

Re: [PATCH v5] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Minchan,

On Sat, 21 Nov 2020 at 00:44, Rui Salvaterra  wrote:
>
> Well, it's quite possible I mis{read,applied} your patch.

And I obviously did. Your kconfig suggestion does basically the same,
in a more simplified way. I guess a v6 is in order.

Thanks,
Rui

Re: [PATCH v5] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Minchan,

On Fri, 20 Nov 2020 at 21:40, Minchan Kim  wrote:
>
> Do I miss your point?

Well, it's quite possible I mis{read,applied} your patch. It's late
here, I'll test it with fresher brains tomorrow to see if it's
functionally equivalent.

Thanks,
Rui

Re: [PATCH v5] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Minchan,

On Thu, 19 Nov 2020 at 22:26, Minchan Kim  wrote:
>
> What's the purpose of ZRAM_AUTOSEL_ALGO?
> If you and Sergey already discussed, sorry about the missing it.

The purpose of ZRAM_AUTOSEL_ALGO is to make sure at least one of the
required compression algorithms is enabled, either as a module or
built-in. I believe Sergey agreed with the reasoning behind it, but
he'll let us know if I misunderstood. :)

> Below doesn't work for your goal?

Unfortunately, it doesn't. :( It breaks the dependency chain, allowing
you to deselect all compression algorithms in the crypto menu, and
consequently disabling zram. This is only my opinion (and I know
select directives should be very well justified), but I believe that
it should be zram to make sure its required libraries are enabled, and
not the other way around. Having to select a compression algorithm in
the crypto menu for the zram block device to appear in the block
devices menu seems backwards to me.

Thanks,
Rui

[PATCH v5] zram: break the strict dependency from lzo

[Rui Salvaterra]

>From the beginning, the zram block device always enabled CRYPTO_LZO, since
lzo-rle is hardcoded as the fallback compression algorithm. As a consequence, on
systems where another compression algorithm is chosen (e.g. CRYPTO_ZSTD), the
lzo kernel module becomes unused, while still having to be built/loaded.

This patch removes the hardcoded lzo-rle dependency and allows the user to
select the default compression algorithm for zram at build time. The previous
behaviour is kept, as the default algorithm is still lzo-rle.

Suggested-by: Sergey Senozhatsky 
Suggested-by: Minchan Kim 
Signed-off-by: Rui Salvaterra 
---
v5: incorporate Minchan's feedback. Allow the user to choose a default 
algorithm.
v4: incorporate Sergey's feedback and fix a small typo.
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.

I believe this is the final version, but it does deserve some comment. Given the
choice of having more preprocessor #if/#endif directives in C files or making
the kconfig a bit more complex, I went for the latter. However, since kconfig
choices can only be boolean, I had to devise a way to make a string selection
based on the boolean choice, hence the ZRAM_DEF_COMP symbol.
I also tried to make the ZRAM_AUTOSEL_ALGO definition a bit less painful to the
eye, while still allowing for the compression algorithms to be selected as
modules, as per Sergey's suggestion.

 drivers/block/zram/Kconfig| 49 ++-
 drivers/block/zram/zcomp.c|  2 ++
 drivers/block/zram/zram_drv.c |  2 +-
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..cde089c30ffb 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -14,6 +13,54 @@ config ZRAM
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
 
+choice
+   prompt "Default zram compression algorithm"
+   depends on ZRAM
+
+config ZRAM_DEF_COMP_LZORLE
+   bool "lzo-rle"
+   depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_ZSTD
+   bool "zstd"
+   depends on CRYPTO_ZSTD
+
+config ZRAM_DEF_COMP_LZ4
+   bool "lz4"
+   depends on CRYPTO_LZ4
+
+config ZRAM_DEF_COMP_LZO
+   bool "lzo"
+   depends on CRYPTO_LZO
+
+config ZRAM_DEF_COMP_LZ4HC
+   bool "lz4hc"
+   depends on CRYPTO_LZ4HC
+
+config ZRAM_DEF_COMP_842
+   bool "842"
+   depends on CRYPTO_842
+
+endchoice
+
+config ZRAM_DEF_COMP
+   string
+   default "lzo-rle" if ZRAM_DEF_COMP_LZORLE
+   default "zstd" if ZRAM_DEF_COMP_ZSTD
+   default "lz4" if ZRAM_DEF_COMP_LZ4
+   default "lzo" if ZRAM_DEF_COMP_LZO
+   default "lz4hc" if ZRAM_DEF_COMP_LZ4HC
+   default "842" if ZRAM_DEF_COMP_842
+
+config ZRAM_AUTOSEL_ALGO
+   def_bool y
+   depends on ZRAM && \
+   !(CRYPTO_LZ4=m   || CRYPTO_LZ4=y   || \
+ CRYPTO_LZ4HC=m || CRYPTO_LZ4HC=y || \
+ CRYPTO_842=m   || CRYPTO_842=y   || \
+ CRYPTO_ZSTD=m  || CRYPTO_ZSTD=y)
+   select CRYPTO_LZO
+
 config ZRAM_WRITEBACK
bool "Write back incompressible or idle page to backing device"
depends on ZRAM
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..9ddccb968c68 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,7 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo-rle";
+static const char *default_compressor = CONFIG_ZRAM_DEF_COMP;
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
-- 
2.29.2

Re: [PATCH v4] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Minchan,

On Tue, 3 Nov 2020 at 21:29, Minchan Kim  wrote:
>
> Can't we just provide choice/endchoice in Kconfig to select default
> comp algorithm from admin?

I'm fine with whatever you guys decide, just let me know what works
best for everyone.

Thanks,
Rui

Re: [PATCH v3] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, again,

On Wed, 28 Oct 2020 at 18:22, Sergey Senozhatsky
 wrote:
>
> Right, but well we also need to select ZSMALLOC and CRYPTO for
> zram to become visible (the thing that I found out recently is
> that you can always check the hidden/blocked items by hitting
> 'z' in menuconfig).

Sure, I can fix that too. Should I do it now, or wait for
Andrew's/Minchan's feedback?
And that 'z' key… wow. Did you read the kconfig code? I have no idea
how you found that one out, but it sure deserves to be documented
somewhere, it's too useful to be true.

Thanks,
Rui

[PATCH v4] zram: break the strict dependency from lzo

[Rui Salvaterra]

There's nothing special about zram and lzo. It works just fine without it, so
as long as at least one of the other supported compression algorithms is
selected.

Suggested-by: Sergey Senozhatsky 
Signed-off-by: Rui Salvaterra 
---
v4: incorporate Sergey's feedback and fix a small typo.
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.

 drivers/block/zram/Kconfig|  6 +-
 drivers/block/zram/zcomp.c| 17 +
 drivers/block/zram/zcomp.h|  1 +
 drivers/block/zram/zram_drv.c |  5 +++--
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..141ce0ebad06 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -14,6 +13,11 @@ config ZRAM
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
 
+config ZRAM_AUTOSEL_ALGO
+   def_bool y
+   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
CRYPTO_ZSTD)
+   select CRYPTO_LZO
+
 config ZRAM_WRITEBACK
bool "Write back incompressible or idle page to backing device"
depends on ZRAM
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..4fad177c78c4 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
@@ -202,6 +204,21 @@ void zcomp_destroy(struct zcomp *comp)
kfree(comp);
 }
 
+const char *default_compressor(void)
+{
+   /*
+* Pick the first available one (there should be at least one).
+*
+* In theory, we can drop all the ifdefs from backends[] and
+* just iterate backends array doing crypto_has_comp(comp, 0, 0)
+* for each entry and return the first one which is recognized by
+* crypto. But crypto_has_comp() modprobes compression drivers,
+* so we may end up with extra loaded drivers, when the 'default'
+* compressor is not what zram is configured to use.
+*/
+   return backends[0];
+}
+
 /*
  * search available compressors for requested algorithm.
  * allocate new zcomp and initialize it. return compressing
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 40f6420f4b2e..f104be9eae9c 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -27,6 +27,7 @@ int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node);
 ssize_t zcomp_available_show(const char *comp, char *buf);
 bool zcomp_available_algorithm(const char *comp);
 
+const char *default_compressor(void);
 struct zcomp *zcomp_create(const char *comp);
 void zcomp_destroy(struct zcomp *comp);
 
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..f02ee050c7bf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,6 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo-rle";
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
@@ -1960,7 +1959,9 @@ static int zram_add(void)
blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue);
device_add_disk(NULL, zram->disk, zram_disk_attr_groups);
 
-   strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor));
+   strlcpy(zram->compressor,
+   default_compressor(),
+   sizeof(zram->compressor));
 
zram_debugfs_register(zram);
pr_info("Added device: %s\n", zram->disk->disk_name);
-- 
2.29.1

Re: [PATCH v3] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Sergey,

On Wed, 28 Oct 2020 at 10:19, Sergey Senozhatsky
 wrote:
>
> Can the following work then?

Almost! See below. :)

> Completely untested.
>
> ===8<===
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index fe7a4b7d30cf..f93eed40e155 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -2,7 +2,7 @@
>  config ZRAM
> tristate "Compressed RAM block device support"
> depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
> -   select CRYPTO_LZO
> +   depends on (CRYPTO_LZO || CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
> CRYPTO_ZSTD)

This reverses the dependency order, as now we have to select a
supported compression algorithm in order for zram to be visible in the
block device drivers list. This is why I wrote the ZRAM_AUTOSEL_ALGO
kconfig symbol, which automatically selects lzo as a fallback. If the
user chooses to select another supported algorithm, he will then be
allowed to deselect lzo. We thus follow the principle of least
surprise, IMHO.

> help
>   Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
>   Pages written to these disks are compressed and stored in memory
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 33e3b76c4fa9..98c7c46c9c3a 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -15,8 +15,10 @@
>  #include "zcomp.h"
>
>  static const char * const backends[] = {
> +#if IS_ENABLED(CONFIG_CRYPTO_LZO)
> "lzo",
> "lzo-rle",
> +#endif
>  #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
> "lz4",
>  #endif
> @@ -202,6 +204,21 @@ void zcomp_destroy(struct zcomp *comp)
> kfree(comp);
>  }
>
> +const char *default_compressor(void)
> +{
> +   /*
> +* Pick the first available one (there should be at least one).
> +*
> +* In theory, we can drop all the ifdefs from backends[] and
> +* just iterate backends array doing crypto_has_comp(comp, 0, 0)
> +* for each entry and return the first one which is recognized by
> +* crypto. But crypto_has_comp() modprobes compression drivers,
> +* so we may endup with extra loaded drivers, when the 'default'
> +* compressor is not what zram is configured to use.
> +*/
> +   return backends[0];
> +}

This is much more elegant, indeed, and I completely agree with just
returning the first one. I'll incorporate your feedback and send a v4
soon.

Thanks a lot,
Rui

Re: [PATCH v3] zram: break the strict dependency from lzo

[Rui Salvaterra]

Hi, Sergey,

On Tue, 27 Oct 2020 at 01:22, Sergey Senozhatsky
 wrote:
>
> Honestly, I'm not entirely excited. lzo is a fallback compression
> algorithm. If you want to use zram with something else thenconfigure
> zram to use something else. What do all these #if/#elif buy us?

The idea is to allow us to select a single compression algorithm at
build time, if we're sure to use something other than lzo. The status
quo only allows us to select additional algorithms, as lzo is a hard
dependency. I dislike the "iffery" as much as the next guy, but in
this case the default selection stops being static (as lzo may not be
available at run time), so we have to fall back to an algorithm which
is enabled, otherwise zram won't work out of the box (we'd always need
to choose the algorithm manually in sysfs).
Personally, I always use zram with zstd, and the only lzo dependency I
have is zram. Disabling lzo saves me about 3 kiB in the final
(xz-compressed) vmlinuz image. It's not much, for sure, but when your
total storage is 4 MiB (and your RAM is 32 MiB), every bit counts. :)

Thanks,
Rui

[PATCH v3] zram: break the strict dependency from lzo

[Rui Salvaterra]

There's nothing special about zram and lzo. It works just fine without it, so
as long as at least one of the other supported compression algorithms is
selected.

Signed-off-by: Rui Salvaterra 
---
v3: fix the default selection when lzo isn't present. Rebase against 5.10-rc1.
v2: fix the dependency on CRYPTO.

 drivers/block/zram/Kconfig|  6 +-
 drivers/block/zram/zcomp.c|  2 ++
 drivers/block/zram/zram_drv.c | 14 +-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..14b2b098d662 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -37,3 +36,8 @@ config ZRAM_MEMORY_TRACKING
  /sys/kernel/debug/zram/zramX/block_state.
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
+
+config ZRAM_AUTOSEL_ALGO
+   def_bool y
+   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
CRYPTO_ZSTD)
+   select CRYPTO_LZO
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b697208d661..2ae09561ba79 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,7 +42,19 @@ static DEFINE_IDR(zram_index_idr);
 static DEFINE_MUTEX(zram_index_mutex);
 
 static int zram_major;
-static const char *default_compressor = "lzo-rle";
+
+static const char *default_compressor =
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
+   "lzo-rle";
+#elif IS_ENABLED(CONFIG_CRYPTO_LZ4)
+   "lz4";
+#elif IS_ENABLED(CONFIG_CRYPTO_LZ4HC)
+   "lz4hc";
+#elif IS_ENABLED(CONFIG_CRYPTO_842)
+   "842";
+#elif IS_ENABLED(CONFIG_CRYPTO_ZSTD)
+   "zstd";
+#endif
 
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
-- 
2.28.0

Re: [PATCH v2] zram: break the strict dependency from lzo

[Rui Salvaterra]

Sigh. Please disregard this too, I also need to fix the default
selection. I'll send a new version against 5.10-rc1, next week.

On Thu, 22 Oct 2020 at 22:30, Rui Salvaterra  wrote:
>
> There's nothing special about zram and lzo. It works just fine without it, so
> as long as at least one of the other supported compression algorithms is
> selected.
>
> Signed-off-by: Rui Salvaterra 
> ---
> v2: fix the dependency on CRYPTO.
>
>  drivers/block/zram/Kconfig | 6 +-
>  drivers/block/zram/zcomp.c | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index fe7a4b7d30cf..14b2b098d662 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -2,7 +2,6 @@
>  config ZRAM
> tristate "Compressed RAM block device support"
> depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
> -   select CRYPTO_LZO
> help
>   Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
>   Pages written to these disks are compressed and stored in memory
> @@ -37,3 +36,8 @@ config ZRAM_MEMORY_TRACKING
>   /sys/kernel/debug/zram/zramX/block_state.
>
>   See Documentation/admin-guide/blockdev/zram.rst for more 
> information.
> +
> +config ZRAM_AUTOSEL_ALGO
> +   def_bool y
> +   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
> CRYPTO_ZSTD)
> +   select CRYPTO_LZO
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 33e3b76c4fa9..052aa3f65514 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -15,8 +15,10 @@
>  #include "zcomp.h"
>
>  static const char * const backends[] = {
> +#if IS_ENABLED(CONFIG_CRYPTO_LZO)
> "lzo",
> "lzo-rle",
> +#endif
>  #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
> "lz4",
>  #endif
> --
> 2.28.0
>

[PATCH v2] zram: break the strict dependency from lzo

[Rui Salvaterra]

There's nothing special about zram and lzo. It works just fine without it, so
as long as at least one of the other supported compression algorithms is
selected.

Signed-off-by: Rui Salvaterra 
---
v2: fix the dependency on CRYPTO.

 drivers/block/zram/Kconfig | 6 +-
 drivers/block/zram/zcomp.c | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..14b2b098d662 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -2,7 +2,6 @@
 config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -37,3 +36,8 @@ config ZRAM_MEMORY_TRACKING
  /sys/kernel/debug/zram/zramX/block_state.
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
+
+config ZRAM_AUTOSEL_ALGO
+   def_bool y
+   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
CRYPTO_ZSTD)
+   select CRYPTO_LZO
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
-- 
2.28.0

Re: [PATCH] zram: break the strict dependency from lzo

[Rui Salvaterra]

Oops, this is broken, I'll send a v2 soon.

On Thu, 22 Oct 2020 at 17:11, Rui Salvaterra  wrote:
>
> There's nothing special about zram and lzo. It works just fine without it, so
> as long as at least one of the other supported compression algorithms is
> selected.
>
> Additionally, drop the explicit dependency from CRYPTO, as it's implied by the
> selection of the algorithms themselves.
>
> Signed-off-by: Rui Salvaterra 
> ---
>  drivers/block/zram/Kconfig | 8 ++--
>  drivers/block/zram/zcomp.c | 2 ++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index fe7a4b7d30cf..2641b86f8677 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -1,8 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  config ZRAM
> tristate "Compressed RAM block device support"
> -   depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
> -   select CRYPTO_LZO
> +   depends on BLOCK && SYSFS && ZSMALLOC
> help
>   Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
>   Pages written to these disks are compressed and stored in memory
> @@ -37,3 +36,8 @@ config ZRAM_MEMORY_TRACKING
>   /sys/kernel/debug/zram/zramX/block_state.
>
>   See Documentation/admin-guide/blockdev/zram.rst for more 
> information.
> +
> +config ZRAM_AUTOSEL_ALGO
> +   def_bool y
> +   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
> CRYPTO_ZSTD)
> +   select CRYPTO_LZO
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 33e3b76c4fa9..052aa3f65514 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -15,8 +15,10 @@
>  #include "zcomp.h"
>
>  static const char * const backends[] = {
> +#if IS_ENABLED(CONFIG_CRYPTO_LZO)
> "lzo",
> "lzo-rle",
> +#endif
>  #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
> "lz4",
>  #endif
> --
> 2.28.0
>

[PATCH] zram: break the strict dependency from lzo

[Rui Salvaterra]

There's nothing special about zram and lzo. It works just fine without it, so
as long as at least one of the other supported compression algorithms is
selected.

Additionally, drop the explicit dependency from CRYPTO, as it's implied by the
selection of the algorithms themselves.

Signed-off-by: Rui Salvaterra 
---
 drivers/block/zram/Kconfig | 8 ++--
 drivers/block/zram/zcomp.c | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index fe7a4b7d30cf..2641b86f8677 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -1,8 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 config ZRAM
tristate "Compressed RAM block device support"
-   depends on BLOCK && SYSFS && ZSMALLOC && CRYPTO
-   select CRYPTO_LZO
+   depends on BLOCK && SYSFS && ZSMALLOC
help
  Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
  Pages written to these disks are compressed and stored in memory
@@ -37,3 +36,8 @@ config ZRAM_MEMORY_TRACKING
  /sys/kernel/debug/zram/zramX/block_state.
 
  See Documentation/admin-guide/blockdev/zram.rst for more information.
+
+config ZRAM_AUTOSEL_ALGO
+   def_bool y
+   depends on ZRAM && !(CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842 || 
CRYPTO_ZSTD)
+   select CRYPTO_LZO
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 33e3b76c4fa9..052aa3f65514 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -15,8 +15,10 @@
 #include "zcomp.h"
 
 static const char * const backends[] = {
+#if IS_ENABLED(CONFIG_CRYPTO_LZO)
"lzo",
"lzo-rle",
+#endif
 #if IS_ENABLED(CONFIG_CRYPTO_LZ4)
"lz4",
 #endif
-- 
2.28.0

Re: [BISECTED] bug/regression, x86-64: completely unbootable machine

[Rui Salvaterra]

On Sat, 25 Apr 2020 at 17:01, Giovanni Gherdovich  wrote:
>
> There is an easy way to tell (besides compiling with those patches on top and
> check if it works): run the command "turbostat --interval 1 sleep 0", the
> output should tell you the content of the register MSR_TURBO_RATIO_LIMIT.
>
> If bits 31:24 are zero, you see the bug (the code divides by that value),
> otherwise you don't. Some 2 cores / 4 threads CPU have a non-zero value there
> (even if it doesn't mean much), some others have zero instead.
>
> The Intel Software Developer Manual (SDM) says the register content is like
> this:
>
> Bit Fields  Bit Description
> 7:0 Maximum turbo ratio limit of 1 core active.
> 15:8Maximum turbo ratio limit of 2 core active.
> 23:16   Maximum turbo ratio limit of 3 core active.
> 31:24   Maximum turbo ratio limit of 4 core active.
> 39:32   Maximum turbo ratio limit of 5 core active.
> 47:40   Maximum turbo ratio limit of 6 core active.
> 55:48   Maximum turbo ratio limit of 7 core active.
> 63:56   Maximum turbo ratio limit of 8 core active.
>
> As I wrote above, some 2c/4t CPUs will say (correctly) their 4 cores turbo
> frequency is zero, such as this Intel Core i5-430M (Arrandale) where I've seen
> turbostat saying:
>
> cpu1: MSR_TURBO_RATIO_LIMIT: 0x1313
> 19 * 133.3 = 2533.3 MHz max turbo 2 active cores
> 19 * 133.3 = 2533.3 MHz max turbo 1 active cores
>
> On the contrary, my laptop has an Intel Core i5-5300U (Broadwell, also
> 2 cores / 4 threads) and it has:
>
> cpu3: MSR_TURBO_RATIO_LIMIT: 0x1b1b1b1b1b1d
> 27 * 100.0 = 2700.0 MHz max turbo 6 active cores
> 27 * 100.0 = 2700.0 MHz max turbo 5 active cores
> 27 * 100.0 = 2700.0 MHz max turbo 4 active cores
> 27 * 100.0 = 2700.0 MHz max turbo 3 active cores
> 27 * 100.0 = 2700.0 MHz max turbo 2 active cores
> 29 * 100.0 = 2900.0 MHz max turbo 1 active cores
>
> You can see above that the 4 cores turbo freq is declared as 2.7GHz even if
> it's nonsense because there aren't 4 cores. In any case, this cpu wouldn't
> trigger the bug, just as your skylake.
>
>
> Thanks,
> Giovanni

Hi again, Giovanni,

Thanks for the detailed and insightful explanation and sorry for not
replying earlier. This was indeed the bug I was hitting, as my
Arrandale laptop is now booting 5.7-rc3 just fine.

Best regards,
Rui

Re: [PATCHv2] Bluetooth: trivial: tidy up printk message output from btrtl.

[Rui Salvaterra]

Hi, Marcel,

On Thu, 26 Sep 2019 at 07:34, Marcel Holtmann  wrote:
>
> Hi Rui,

[patch snipped]

> I have some similar patch in my tree. Can you check what is still missing and 
> send a new version. Thanks.

Yeah, from a cursory look at your tree, it seems about the same as
mine. I'll take a deeper look later on (can't access the laptops to
test, ATM) and send a patch with any missing stuff, if needed.

Thanks,
Rui

Re: [PATCH] netfilter: trivial: remove extraneous space from message

[Rui Salvaterra]

Friendly ping.

On Wed, 24 Jul 2019 at 15:37, Rui Salvaterra  wrote:
>
> Pure ocd, but this one has been bugging me for a while.
>
> Signed-off-by: Rui Salvaterra 
> ---
>  net/netfilter/nf_conntrack_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_helper.c 
> b/net/netfilter/nf_conntrack_helper.c
> index 8d729e7c36ff..209123f35b4a 100644
> --- a/net/netfilter/nf_conntrack_helper.c
> +++ b/net/netfilter/nf_conntrack_helper.c
> @@ -218,7 +218,7 @@ nf_ct_lookup_helper(struct nf_conn *ct, struct net *net)
> return NULL;
> pr_info("nf_conntrack: default automatic helper assignment "
> "has been turned off for security reasons and 
> CT-based "
> -   " firewall rule not found. Use the iptables CT target 
> "
> +   "firewall rule not found. Use the iptables CT target "
> "to attach helpers instead.\n");
> net->ct.auto_assign_helper_warned = 1;
> return NULL;
> --
> 2.22.0
>

[PATCHv2] Bluetooth: trivial: tidy up printk message output from btrtl.

[Rui Salvaterra]

From: Rui 

v2: also remove new line from hci_h5.c

The rtl_dev_* calls in the Realtek USB Bluetooth driver add unnecessary
device prefixes and new lines at the end of most messages, which make the
dmesg output look like this:

[5.667121] sd 0:0:0:0: [sda] Attached SCSI disk
[5.736869] Bluetooth: hci0: RTL: rtl: examining hci_ver=06 hci_rev=000a 
lmp_ver=06 lmp_subver=8821

[5.737853] Bluetooth: hci0: RTL: rom_version status=0 version=1

[5.737857] Bluetooth: hci0: RTL: rtl: loading rtl_bt/rtl8821a_fw.bin

[5.737946] Bluetooth: hci0: RTL: rtl: loading rtl_bt/rtl8821a_config.bin

[5.737965] bluetooth hci0: Direct firmware load for 
rtl_bt/rtl8821a_config.bin failed with error -2
[5.737972] Bluetooth: hci0: RTL: cfg_sz -2, total sz 17428

[5.997375] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)

Clean this up by removing extraneous new lines and redundant device prefixes,
with the following result:

[5.667008] sd 0:0:0:0: [sda] Attached SCSI disk
[5.716945] Bluetooth: hci0: rtl: examining hci_ver=06 hci_rev=000a 
lmp_ver=06 lmp_subver=8821
[5.718745] Bluetooth: hci0: rtl: rom_version status=0 version=1
[5.718749] Bluetooth: hci0: rtl: loading rtl_bt/rtl8821a_fw.bin
[5.718830] Bluetooth: hci0: rtl: loading rtl_bt/rtl8821a_config.bin
[5.718849] bluetooth hci0: Direct firmware load for 
rtl_bt/rtl8821a_config.bin failed with error -2
[5.718858] Bluetooth: hci0: rtl: cfg_sz -2, total sz 17428
[5.997400] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)

Signed-off-by: Rui Salvaterra 
---
 drivers/bluetooth/btrtl.c  | 62 +++---
 drivers/bluetooth/btrtl.h  |  8 ++---
 drivers/bluetooth/hci_h5.c |  2 +-
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 4f75a9b61d09..0c9c16444ce5 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -186,19 +186,19 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 
*version)
/* Read RTL ROM version command */
skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
-   rtl_dev_err(hdev, "Read ROM version failed (%ld)\n",
+   rtl_dev_err(hdev, "Read ROM version failed (%ld)",
PTR_ERR(skb));
return PTR_ERR(skb);
}
 
if (skb->len != sizeof(*rom_version)) {
-   rtl_dev_err(hdev, "RTL version event length mismatch\n");
+   rtl_dev_err(hdev, "RTL version event length mismatch");
kfree_skb(skb);
return -EIO;
}
 
rom_version = (struct rtl_rom_version_evt *)skb->data;
-   rtl_dev_info(hdev, "rom_version status=%x version=%x\n",
+   rtl_dev_info(hdev, "rom_version status=%x version=%x",
 rom_version->status, rom_version->version);
 
*version = rom_version->version;
@@ -242,7 +242,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
 
fwptr = btrtl_dev->fw_data + btrtl_dev->fw_len - sizeof(extension_sig);
if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {
-   rtl_dev_err(hdev, "extension section signature mismatch\n");
+   rtl_dev_err(hdev, "extension section signature mismatch");
return -EINVAL;
}
 
@@ -263,7 +263,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
break;
 
if (length == 0) {
-   rtl_dev_err(hdev, "found instruction with length 0\n");
+   rtl_dev_err(hdev, "found instruction with length 0");
return -EINVAL;
}
 
@@ -276,7 +276,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
}
 
if (project_id < 0) {
-   rtl_dev_err(hdev, "failed to find version instruction\n");
+   rtl_dev_err(hdev, "failed to find version instruction");
return -EINVAL;
}
 
@@ -287,13 +287,13 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
}
 
if (i >= ARRAY_SIZE(project_id_to_lmp_subver)) {
-   rtl_dev_err(hdev, "unknown project id %d\n", project_id);
+   rtl_dev_err(hdev, "unknown project id %d", project_id);
return -EINVAL;
}
 
if (btrtl_dev->ic_info->lmp_subver !=
project_id_to_lmp_subver[i].lmp_subver) {
-   rtl_dev_err(hdev, "firmware is for %x but this is a %x\n",
+   rtl_dev_err(hdev, "firmware is for %x but this is a %x",
project_id_to_lmp_subver[i].lmp_subver,
btrtl_dev->ic

[PATCH] Bluetooth: trivial: tidy up printk message output from btrtl.

[Rui Salvaterra]

The rtl_dev_* calls in the Realtek USB Bluetooth driver add unnecessary
device prefixes and new lines at the end of most messages, which make the
dmesg output look like this:

[5.667121] sd 0:0:0:0: [sda] Attached SCSI disk
[5.736869] Bluetooth: hci0: RTL: rtl: examining hci_ver=06 hci_rev=000a 
lmp_ver=06 lmp_subver=8821

[5.737853] Bluetooth: hci0: RTL: rom_version status=0 version=1

[5.737857] Bluetooth: hci0: RTL: rtl: loading rtl_bt/rtl8821a_fw.bin

[5.737946] Bluetooth: hci0: RTL: rtl: loading rtl_bt/rtl8821a_config.bin

[5.737965] bluetooth hci0: Direct firmware load for 
rtl_bt/rtl8821a_config.bin failed with error -2
[5.737972] Bluetooth: hci0: RTL: cfg_sz -2, total sz 17428

[5.997375] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)

Clean this up by removing extraneous new lines and redundant device prefixes,
with the following result:

[5.667008] sd 0:0:0:0: [sda] Attached SCSI disk
[5.716945] Bluetooth: hci0: rtl: examining hci_ver=06 hci_rev=000a 
lmp_ver=06 lmp_subver=8821
[5.718745] Bluetooth: hci0: rtl: rom_version status=0 version=1
[5.718749] Bluetooth: hci0: rtl: loading rtl_bt/rtl8821a_fw.bin
[5.718830] Bluetooth: hci0: rtl: loading rtl_bt/rtl8821a_config.bin
[5.718849] bluetooth hci0: Direct firmware load for 
rtl_bt/rtl8821a_config.bin failed with error -2
[5.718858] Bluetooth: hci0: rtl: cfg_sz -2, total sz 17428
[5.997400] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)

Signed-off-by: Rui Salvaterra 
---
 drivers/bluetooth/btrtl.c | 62 +++
 drivers/bluetooth/btrtl.h |  8 ++---
 2 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 4f75a9b61d09..0c9c16444ce5 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -186,19 +186,19 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 
*version)
/* Read RTL ROM version command */
skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
-   rtl_dev_err(hdev, "Read ROM version failed (%ld)\n",
+   rtl_dev_err(hdev, "Read ROM version failed (%ld)",
PTR_ERR(skb));
return PTR_ERR(skb);
}
 
if (skb->len != sizeof(*rom_version)) {
-   rtl_dev_err(hdev, "RTL version event length mismatch\n");
+   rtl_dev_err(hdev, "RTL version event length mismatch");
kfree_skb(skb);
return -EIO;
}
 
rom_version = (struct rtl_rom_version_evt *)skb->data;
-   rtl_dev_info(hdev, "rom_version status=%x version=%x\n",
+   rtl_dev_info(hdev, "rom_version status=%x version=%x",
 rom_version->status, rom_version->version);
 
*version = rom_version->version;
@@ -242,7 +242,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
 
fwptr = btrtl_dev->fw_data + btrtl_dev->fw_len - sizeof(extension_sig);
if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {
-   rtl_dev_err(hdev, "extension section signature mismatch\n");
+   rtl_dev_err(hdev, "extension section signature mismatch");
return -EINVAL;
}
 
@@ -263,7 +263,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
break;
 
if (length == 0) {
-   rtl_dev_err(hdev, "found instruction with length 0\n");
+   rtl_dev_err(hdev, "found instruction with length 0");
return -EINVAL;
}
 
@@ -276,7 +276,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
}
 
if (project_id < 0) {
-   rtl_dev_err(hdev, "failed to find version instruction\n");
+   rtl_dev_err(hdev, "failed to find version instruction");
return -EINVAL;
}
 
@@ -287,13 +287,13 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
}
 
if (i >= ARRAY_SIZE(project_id_to_lmp_subver)) {
-   rtl_dev_err(hdev, "unknown project id %d\n", project_id);
+   rtl_dev_err(hdev, "unknown project id %d", project_id);
return -EINVAL;
}
 
if (btrtl_dev->ic_info->lmp_subver !=
project_id_to_lmp_subver[i].lmp_subver) {
-   rtl_dev_err(hdev, "firmware is for %x but this is a %x\n",
+   rtl_dev_err(hdev, "firmware is for %x but this is a %x",
project_id_to_lmp_subver[i].lmp_subver,
btrtl_dev->ic_info->lmp_subver);
return -EINVAL;
@@ -301,7 +301,7 @@ static int rtlbt_parse_firmwar

Re: [BUG] Linux 5.3-rc1: timer problem on x86-64 (Pentium D)

[Rui Salvaterra]

Hi, Thomas,

On Thu, 25 Jul 2019 at 10:37, Thomas Gleixner  wrote:
>
> Duh. Yes, this explains it nicely.
>
> > [1.123548] clocksource: timekeeping watchdog on CPU1: Marking
> > clocksource 'tsc-early' as unstable because the skew is too large:
> > [1.123552] clocksource:   'hpet' wd_now: 33
> > wd_last: 33 mask: 
>
> The HPET counter check succeeded, but the early enable and the following
> reconfiguration confused it completely. So the HPET is not counting:
>
> 'hpet' wd_now: 33 wd_last: 33 mask: 
>
> Which is a full explanation for the boot fail because if the counter is not
> working then the HPET timer is not expiring and the early boot is waiting
> for HPET to fire forever.

Interesting. Thanks for the detailed explanation!

> That's consistent with the above. 5.3-rc1 unpatched would of course boot as
> well with hpet=disable now that we know the root cause.

I guess that's the only thing I hadn't tried, hindsight is 20/20. :)

> I'll write a changelog and route it to Linus for -rc2.
>
> Thanks a lot for debugging this and providing all the information!

My pleasure! I'm glad to have this old workhorse chugging along again.

Thanks,
Rui

Re: [BUG] Linux 5.3-rc1: timer problem on x86-64 (Pentium D)

[Rui Salvaterra]

On Wed, 24 Jul 2019 at 12:02, Thomas Gleixner  wrote:
>
> Rui,
>
> On Wed, 24 Jul 2019, Rui Salvaterra wrote:
> > I don't know if this has been reported before, but from a cursory
> > search it doesn't seem to be the case.
> > I have a x86-64 Pentium (4) D machine which always worked perfectly
> > with Linux 5.2 using the TSC as the clock source. With Linux 5.3-rc1 I
> > can't, for the life of me, boot it with anything other than
> > clocksource=jiffies, it completely hangs without even a backtrace.
>
> The obvious candidate for this is the following section:
>
> c8c4076723da ("x86/timer: Skip PIT initialization on modern chipsets")
> dde3626f815e ("x86/apic: Use non-atomic operations when possible")
> 748b170ca19a ("x86/apic: Make apic_bsp_setup() static")
> 2420a0b1798d ("x86/tsc: Set LAPIC timer period to crystal clock frequency")
> 52ae346bd26c ("x86/apic: Rename 'lapic_timer_frequency' to 
> 'lapic_timer_period'")
> 604dc9170f24 ("x86/tsc: Use CPUID.0x16 to calculate missing crystal 
> frequency")

Hi again, everyone,

Looks like we have a winner. Actually, I did a full bisection between
5.2 and 5.3-rc1. Full log follows:

git bisect start
# good: [0ecfebd2b52404ae0c54a878c872bb93363ada36] Linux 5.2
git bisect good 0ecfebd2b52404ae0c54a878c872bb93363ada36
# bad: [5f9e832c137075045d15cd6899ab0505cfb2ca4b] Linus 5.3-rc1
git bisect bad 5f9e832c137075045d15cd6899ab0505cfb2ca4b
# bad: [e786741ff1b52769b044b7f4407f39cd13ee5d2d] Merge tag
'staging-5.3-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect bad e786741ff1b52769b044b7f4407f39cd13ee5d2d
# bad: [8f6ccf6159aed1f04c6d179f61f6fb2691261e84] Merge tag
'clone3-v5.3' of
git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux
git bisect bad 8f6ccf6159aed1f04c6d179f61f6fb2691261e84
# bad: [ed63b9c873601ca113da5c7b1745e3946493e9f3] Merge tag
'media/v5.3-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect bad ed63b9c873601ca113da5c7b1745e3946493e9f3
# bad: [4d2fa8b44b891f0da5ceda3e5a1402ccf0ab6f26] Merge branch 'linus'
of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect bad 4d2fa8b44b891f0da5ceda3e5a1402ccf0ab6f26
# bad: [46f1ec23a46940846f86a91c46f7119d8a8b5de1] Merge branch
'core-rcu-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 46f1ec23a46940846f86a91c46f7119d8a8b5de1
# good: [2a1ccd31420a7b1acd6ca37b2bec2d723aa093e4] Merge branch
'irq-core-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 2a1ccd31420a7b1acd6ca37b2bec2d723aa093e4
# good: [ab2486a9ee3243c8342549f58a13cdfa9abb497a] Merge branch
'x86-fpu-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good ab2486a9ee3243c8342549f58a13cdfa9abb497a
# bad: [2f0f6503e37551eb8d8d5e4d27c78d28a30fed5a] Merge branch
'x86-timers-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 2f0f6503e37551eb8d8d5e4d27c78d28a30fed5a
# good: [fd329f276ecaad7a371d6f91b9bbea031d0c3440] x86/mtrr: Skip
cache flushes on CPUs with cache self-snooping
git bisect good fd329f276ecaad7a371d6f91b9bbea031d0c3440
# bad: [e37f0881e9d9ec8b12f242cc2b78d93259aa7f0f] x86/hpet: Introduce
struct hpet_base and struct hpet_channel
git bisect bad e37f0881e9d9ec8b12f242cc2b78d93259aa7f0f
# good: [8c273f2c81f0756f65b24771196c0eff7ac90e7b] x86/hpet: Move
static and global variables to one place
git bisect good 8c273f2c81f0756f65b24771196c0eff7ac90e7b
# bad: [3535aa12f7f26fc755514b13aee8fac15741267e] x86/hpet:
Decapitalize and rename EVT_TO_HPET_DEV
git bisect bad 3535aa12f7f26fc755514b13aee8fac15741267e
# bad: [3222daf970f30133cc4c639cbecdc29c4ae91b2b] x86/hpet: Separate
counter check out of clocksource register code
git bisect bad 3222daf970f30133cc4c639cbecdc29c4ae91b2b
# good: [6bdec41a0cbcbda35c9044915fc8f45503a595a0] x86/hpet: Shuffle
code around for readability sake
git bisect good 6bdec41a0cbcbda35c9044915fc8f45503a595a0
# first bad commit: [3222daf970f30133cc4c639cbecdc29c4ae91b2b]
x86/hpet: Separate counter check out of clocksource register code

I haven't tried reverting this commit and recompiling (it's already
past 1:00 here, need to sleep), but I'll try it tomorrow, if required.
Note that on this machine (i945G) the HPET is disabled at boot and is
forcefully enabled by the kernel…

[0.147527] pci :00:1f.0: Force enabled HPET at 0xfed0

… and the commit message reads…

"The init code checks whether the HPET counter works late in the init
function when the clocksource is registered. That should happen right
with the other sanity checks."

… maybe now the check is happening a bit too early…?

Thanks,
Rui

[PATCH] netfilter: trivial: remove extraneous space from message

[Rui Salvaterra]

Pure ocd, but this one has been bugging me for a while.

Signed-off-by: Rui Salvaterra 
---
 net/netfilter/nf_conntrack_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 8d729e7c36ff..209123f35b4a 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -218,7 +218,7 @@ nf_ct_lookup_helper(struct nf_conn *ct, struct net *net)
return NULL;
pr_info("nf_conntrack: default automatic helper assignment "
"has been turned off for security reasons and CT-based "
-   " firewall rule not found. Use the iptables CT target "
+   "firewall rule not found. Use the iptables CT target "
"to attach helpers instead.\n");
net->ct.auto_assign_helper_warned = 1;
return NULL;
-- 
2.22.0

[BUG] Linux 5.3-rc1: timer problem on x86-64 (Pentium D)

[Rui Salvaterra]

Hi, everyone,

I don't know if this has been reported before, but from a cursory
search it doesn't seem to be the case.
I have a x86-64 Pentium (4) D machine which always worked perfectly
with Linux 5.2 using the TSC as the clock source. With Linux 5.3-rc1 I
can't, for the life of me, boot it with anything other than
clocksource=jiffies, it completely hangs without even a backtrace.
At the moment I'm at work and, for obvious reasons, I'm not able to
bisect this remotely. In any case, if anyone has any idea about the
cause, I'll be glad to help debugging and testing any patches.

Thanks in advance,
Rui

Re: use generic DMA mapping code in powerpc V4

[Rui Salvaterra]

On Mon, 10 Dec 2018 at 20:49, Benjamin Herrenschmidt
 wrote:
>

[snip]

>
> AGP is a gigantic nightmare :-) It's not just cache coherency issues
> (some implementations are coherent, some aren't, Apple's is ... weird).
>
> Apple has all sort of bugs, and Darwin source code only sheds light on
> some of them. Some implementation can only read, not write I think, for
> example. There are issues with transfers crossing some boundaries I
> beleive, but it's all unclear.
>
> Apple makes this work with a combination of hacks in the AGP "driver"
> and the closed source GPU driver, which we don't see.
>
> I have given up trying to make that stuff work reliably a decade ago :)
>
> Cheers,
> Ben.

That's what I was afraid of… what a mess. At least now I have a
definitive answer from one of the original authors of the code, thanks
a lot, Ben. :)
I have an unresearched belief that AGP support was hacked in the Mac
series as an afterthought (weren't they supposed to be PCI/PCI-X
only?), and your explanation surely seems to corroborate. :/

Re: use generic DMA mapping code in powerpc V4

[Rui Salvaterra]

On Mon, 10 Dec 2018 at 19:33, Christoph Hellwig  wrote:
>
> On Mon, Dec 10, 2018 at 05:04:46PM +0000, Rui Salvaterra wrote:
> > Hi, Christoph and Ben,
> >
> > It just came to my mind (and this is most likely a stupid question,
> > but still)… Is there any possibility of these changes having an
> > (positive) effect on the long-standing problem of Power Mac machines
> > with AGP graphics cards (which have to be limited to PCI transfers,
> > otherwise they'll hang, due to coherence issues)? If so, I have a G4
> > machine where I'd gladly test them.
>
> These patches themselves are not going to affect that directly.
> But IFF the problem really is that the AGP needs to be treated as not
> cache coherent (I have no idea if that is true) the generic direct
> mapping code has full support for a per-device coherent flag, so
> support for a non-coherent AGP slot could be implemented relatively
> simply.

Thanks for the insight, Christoph. Well, the problem[1] is real, and
it's been known for a long time, though I can't be sure if it's *only*
a coherence issue. If someone who knows the hardware manages to cook
up a patch (as hacky is it may be), I'll definitely fire up old my G4
laptop to test it! :)

[1] https://bugs.freedesktop.org/show_bug.cgi?id=95017

Re: use generic DMA mapping code in powerpc V4

[Rui Salvaterra]

On Sat, 8 Dec 2018 at 17:17, Christoph Hellwig  wrote:
>
> On Sun, Dec 02, 2018 at 05:11:02PM +1100, Benjamin Herrenschmidt wrote:
> > Talking of which ... Christoph, not sure if we can do something about
> > this at the DMA API level or keep hacks but some adapters such as the
> > nVidia GPUs have a HW hack we can use to work around their limitations
> > in that case.
> >
> > They have a register that can program a fixed value for the top bits
> > that they don't support.
> >
> > This works fine for any linear mapping with an offset, provided they
> > can program the offset in that register and they have enough DMA range
> > to cover all memory from that offset.
> >
> > I can probably get the info about this from them so we can exploit it
> > in nouveau.
>
> I think we can expose the direct mapping offset if people care enough,
> we just have to be very careful designing the API.  I'll happily leave
> that to those that actually want to use it, but I'll gladly help
> reviewing it.

Hi, Christoph and Ben,

It just came to my mind (and this is most likely a stupid question,
but still)… Is there any possibility of these changes having an
(positive) effect on the long-standing problem of Power Mac machines
with AGP graphics cards (which have to be limited to PCI transfers,
otherwise they'll hang, due to coherence issues)? If so, I have a G4
machine where I'd gladly test them.

Thanks,

Rui

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Thu, 13 Sep 2018 at 16:49, Ben Hutchings  wrote:
>
> Like I said, please send the error messages; we're not going to guess.
>
> Ben.
>
> --
> Ben Hutchings
> Computers are not intelligent.  They only think they are.

I'm an idiot. Turns out it's something completely unrelated. Here's
the relevant part of the build failure:

[snip]

In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,
 from arch/powerpc/boot/decompress.c:42:
arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:
linux/crc32poly.h: No such file or directory
 #include 
  ^~~
compilation terminated.
make[4]: *** [arch/powerpc/boot/Makefile:217:
arch/powerpc/boot/decompress.o] Error 1
make[3]: *** [arch/powerpc/Makefile:288: zImage] Error 2

[snip]

It's the xz kernel compression option which is failing, gzip builds
fine (but it's still pointless, powerpc kernels seem to be always
installed uncompressed). Broken .config attached, if relevant.

Sorry for the misdirection.


.config
Description: Binary data

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Thu, 13 Sep 2018 at 16:49, Ben Hutchings  wrote:
>
> Like I said, please send the error messages; we're not going to guess.
>
> Ben.
>
> --
> Ben Hutchings
> Computers are not intelligent.  They only think they are.

I'm an idiot. Turns out it's something completely unrelated. Here's
the relevant part of the build failure:

[snip]

In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,
 from arch/powerpc/boot/decompress.c:42:
arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:
linux/crc32poly.h: No such file or directory
 #include 
  ^~~
compilation terminated.
make[4]: *** [arch/powerpc/boot/Makefile:217:
arch/powerpc/boot/decompress.o] Error 1
make[3]: *** [arch/powerpc/Makefile:288: zImage] Error 2

[snip]

It's the xz kernel compression option which is failing, gzip builds
fine (but it's still pointless, powerpc kernels seem to be always
installed uncompressed). Broken .config attached, if relevant.

Sorry for the misdirection.


.config
Description: Binary data

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Thu, 13 Sep 2018 at 13:32, Ben Hutchings  wrote:
>
> On Tue, 2018-09-11 at 15:58 +0100, Rui Salvaterra wrote:
> > Greetings! I'm having trouble building the kernel on my ppc64 machine
> > (Power Mac G5) running Debian Sid. The make bindeb-pkg target is failing
> > ever since 4.19-rc1. Is this a known issue? If not, let me know if you need
> > anything else in order to help debug this. Thanks in advance!
>
> It sounds like you don't actually have a ppc64 installation, but a
> powerpc installation with a 64-bit kernel.  In that case you will now
> need to add ppc64 as a secondary architecture by running:
>
> dpkg --add-architecture ppc64
>
> If that doesn't help, please send the error messages you're getting.
>
> Ben.
>
> --
> Ben Hutchings
> Computers are not intelligent.  They only think they are.
>

Hi, Ben!

Thanks for the hint, I had added the ppc64 architecture already, but
no luck. I spent yesterday bisecting between v4.18 and v4.19-rc1, but
the result pointed to an obviously wrong commit.
Serendipitously (by building a v4.19-rc3 allnoconfig), I discovered
32-bit builds fine, but 64-bit big endian doesn't.
I started digging in the git history for some clues. I tried reverting
this commit[1], which seemed suspicious, but the build still fails
(the initial dpkg-buildpackage warnings are gone, though).
Let me know what else can I do to help.

Thanks,

Rui

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=50d511ba6246c46654857b8ef64aefe76a87e455

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Thu, 13 Sep 2018 at 13:32, Ben Hutchings  wrote:
>
> On Tue, 2018-09-11 at 15:58 +0100, Rui Salvaterra wrote:
> > Greetings! I'm having trouble building the kernel on my ppc64 machine
> > (Power Mac G5) running Debian Sid. The make bindeb-pkg target is failing
> > ever since 4.19-rc1. Is this a known issue? If not, let me know if you need
> > anything else in order to help debug this. Thanks in advance!
>
> It sounds like you don't actually have a ppc64 installation, but a
> powerpc installation with a 64-bit kernel.  In that case you will now
> need to add ppc64 as a secondary architecture by running:
>
> dpkg --add-architecture ppc64
>
> If that doesn't help, please send the error messages you're getting.
>
> Ben.
>
> --
> Ben Hutchings
> Computers are not intelligent.  They only think they are.
>

Hi, Ben!

Thanks for the hint, I had added the ppc64 architecture already, but
no luck. I spent yesterday bisecting between v4.18 and v4.19-rc1, but
the result pointed to an obviously wrong commit.
Serendipitously (by building a v4.19-rc3 allnoconfig), I discovered
32-bit builds fine, but 64-bit big endian doesn't.
I started digging in the git history for some clues. I tried reverting
this commit[1], which seemed suspicious, but the build still fails
(the initial dpkg-buildpackage warnings are gone, though).
Let me know what else can I do to help.

Thanks,

Rui

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=50d511ba6246c46654857b8ef64aefe76a87e455

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Wed, 12 Sep 2018 at 10:32,  wrote:
>
> Hi, Rui,
>
>
>
> Could you cater to ‘git bisect’ ?
>
>
>
> Thanks
>
>
>
>
>
> Greetings! I'm having trouble building the kernel on my ppc64 machine (Power 
> Mac G5) running Debian Sid. The make bindeb-pkg target is failing ever since 
> 4.19-rc1. Is this a known issue? If not, let me know if you need anything 
> else in order to help debug this. Thanks in advance!

Hi! I never did a git bisection before, but I'll try that and get back
to you. Thanks!

Re: 4.19-rc3: bindeb-pkg target is failing on ppc64

[Rui Salvaterra]

On Wed, 12 Sep 2018 at 10:32,  wrote:
>
> Hi, Rui,
>
>
>
> Could you cater to ‘git bisect’ ?
>
>
>
> Thanks
>
>
>
>
>
> Greetings! I'm having trouble building the kernel on my ppc64 machine (Power 
> Mac G5) running Debian Sid. The make bindeb-pkg target is failing ever since 
> 4.19-rc1. Is this a known issue? If not, let me know if you need anything 
> else in order to help debug this. Thanks in advance!

Hi! I never did a git bisection before, but I'll try that and get back
to you. Thanks!

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Rui Salvaterra]

On 8 January 2017 at 11:25, Greg KH  wrote:
> On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
> Meta-comment.  Does this update include all of the security fixes that
> we have made over the past few years to the lz4 code?  I don't want to
> be adding back insecure functions that will cause us problems.
>
> Specifically look at the changes I made in 2014 in this directory for an
> example of what I am talking about here.
>
> thanks,
>
> greg k-h

Also, this series must be tested on big endian, to make sure the last
fixes we made don't regress.


Thanks,

Rui

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Rui Salvaterra]

On 8 January 2017 at 11:25, Greg KH  wrote:
> On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
> Meta-comment.  Does this update include all of the security fixes that
> we have made over the past few years to the lz4 code?  I don't want to
> be adding back insecure functions that will cause us problems.
>
> Specifically look at the changes I made in 2014 in this directory for an
> example of what I am talking about here.
>
> thanks,
>
> greg k-h

Also, this series must be tested on big endian, to make sure the last
fixes we made don't regress.


Thanks,

Rui

Re: [RFC] [PATCH] arch: x86: change GCC optimisation target from atom to bonnell

[Rui Salvaterra]

2016-10-11 1:11 GMT+01:00  <h...@zytor.com>:
> On October 10, 2016 2:45:38 PM PDT, Rui Salvaterra <rsalvate...@gmail.com> 
> wrote:
>>Hi, Thomas, Ingo, Peter,
>>
>>(Sending as RFC, since I don't know if this patch is acceptable.)
>>
>>The GCC team has deprecated atom as a march/mtune target since almost
>>three
>>years ago, according to this mailing list thread [1], in favour of
>>specific
>>microarchitecture names (bonnell, silvermont). This patch changes the
>>Atom
>>optimisation target in the kernel to bonnell, as it was originally
>>intended.
>>
>>Tested on an x86-64 Atom 330 machine. No functional changes.
>>
>>[1] https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01805.html
>>
>>Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
>>---
>> arch/x86/Makefile| 4 ++--
>> arch/x86/Makefile_32.cpu | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>index 2d44933..b7d615f 100644
>>--- a/arch/x86/Makefile
>>+++ b/arch/x86/Makefile
>>@@ -109,8 +109,8 @@ else
>>
>> cflags-$(CONFIG_MCORE2) += \
>>$(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
>>-  cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
>>-  $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
>>+  cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell) \
>>+  $(call cc-option,-mtune=bonnell,$(call 
>>cc-option,-mtune=generic))
>>   cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
>> KBUILD_CFLAGS += $(cflags-y)
>>
>>diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
>>index 6647ed4..d66e5e3 100644
>>--- a/arch/x86/Makefile_32.cpu
>>+++ b/arch/x86/Makefile_32.cpu
>>@@ -32,8 +32,8 @@ cflags-$(CONFIG_MCYRIXIII)   += $(call
>>cc-option,-march=c3,-march=i486) $(align)-f
>> cflags-$(CONFIG_MVIAC3_2) += $(call cc-option,-march=c3-2,-march=i686)
>> cflags-$(CONFIG_MVIAC7)   += -march=i686
>> cflags-$(CONFIG_MCORE2)   += -march=i686 $(call tune,core2)
>>-cflags-$(CONFIG_MATOM)+= $(call cc-option,-march=atom,$(call
>>cc-option,-march=core2,-march=i686)) \
>>-  $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
>>+cflags-$(CONFIG_MATOM)+= $(call 
>>cc-option,-march=bonnell,$(call
>>cc-option,-march=core2,-march=i686)) \
>>+  $(call cc-option,-mtune=bonnell,$(call cc-option,-mtune=generic))
>>
>> # AMD Elan support
>> cflags-$(CONFIG_MELAN)+= -march=i486
>
> This just breaks backwards compatibility with older gcc.  If you want to add 
> a comment that's fine, though.
>
> Nacked-by: hpa
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.


Hi, Hans,

I suspected that (hence the RFC), but I couldn't find the minimum supported
gcc version. I was just worried the bonnell microarch could get neglected in
the long term, when it needs all the (compiler) help it can get, being in-order.

Thanks,

Rui

Re: [RFC] [PATCH] arch: x86: change GCC optimisation target from atom to bonnell

[Rui Salvaterra]

2016-10-11 1:11 GMT+01:00  :
> On October 10, 2016 2:45:38 PM PDT, Rui Salvaterra  
> wrote:
>>Hi, Thomas, Ingo, Peter,
>>
>>(Sending as RFC, since I don't know if this patch is acceptable.)
>>
>>The GCC team has deprecated atom as a march/mtune target since almost
>>three
>>years ago, according to this mailing list thread [1], in favour of
>>specific
>>microarchitecture names (bonnell, silvermont). This patch changes the
>>Atom
>>optimisation target in the kernel to bonnell, as it was originally
>>intended.
>>
>>Tested on an x86-64 Atom 330 machine. No functional changes.
>>
>>[1] https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01805.html
>>
>>Signed-off-by: Rui Salvaterra 
>>---
>> arch/x86/Makefile| 4 ++--
>> arch/x86/Makefile_32.cpu | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>index 2d44933..b7d615f 100644
>>--- a/arch/x86/Makefile
>>+++ b/arch/x86/Makefile
>>@@ -109,8 +109,8 @@ else
>>
>> cflags-$(CONFIG_MCORE2) += \
>>$(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
>>-  cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
>>-  $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
>>+  cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell) \
>>+  $(call cc-option,-mtune=bonnell,$(call 
>>cc-option,-mtune=generic))
>>   cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
>> KBUILD_CFLAGS += $(cflags-y)
>>
>>diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
>>index 6647ed4..d66e5e3 100644
>>--- a/arch/x86/Makefile_32.cpu
>>+++ b/arch/x86/Makefile_32.cpu
>>@@ -32,8 +32,8 @@ cflags-$(CONFIG_MCYRIXIII)   += $(call
>>cc-option,-march=c3,-march=i486) $(align)-f
>> cflags-$(CONFIG_MVIAC3_2) += $(call cc-option,-march=c3-2,-march=i686)
>> cflags-$(CONFIG_MVIAC7)   += -march=i686
>> cflags-$(CONFIG_MCORE2)   += -march=i686 $(call tune,core2)
>>-cflags-$(CONFIG_MATOM)+= $(call cc-option,-march=atom,$(call
>>cc-option,-march=core2,-march=i686)) \
>>-  $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
>>+cflags-$(CONFIG_MATOM)+= $(call 
>>cc-option,-march=bonnell,$(call
>>cc-option,-march=core2,-march=i686)) \
>>+  $(call cc-option,-mtune=bonnell,$(call cc-option,-mtune=generic))
>>
>> # AMD Elan support
>> cflags-$(CONFIG_MELAN)+= -march=i486
>
> This just breaks backwards compatibility with older gcc.  If you want to add 
> a comment that's fine, though.
>
> Nacked-by: hpa
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.


Hi, Hans,

I suspected that (hence the RFC), but I couldn't find the minimum supported
gcc version. I was just worried the bonnell microarch could get neglected in
the long term, when it needs all the (compiler) help it can get, being in-order.

Thanks,

Rui

[RFC] [PATCH] arch: x86: change GCC optimisation target from atom to bonnell

[Rui Salvaterra]

Hi, Thomas, Ingo, Peter,

(Sending as RFC, since I don't know if this patch is acceptable.)

The GCC team has deprecated atom as a march/mtune target since almost three
years ago, according to this mailing list thread [1], in favour of specific
microarchitecture names (bonnell, silvermont). This patch changes the Atom
optimisation target in the kernel to bonnell, as it was originally intended.

Tested on an x86-64 Atom 330 machine. No functional changes.

[1] https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01805.html

Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
---
 arch/x86/Makefile| 4 ++--
 arch/x86/Makefile_32.cpu | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..b7d615f 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -109,8 +109,8 @@ else
 
 cflags-$(CONFIG_MCORE2) += \
 $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
-   cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
-   $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+   cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell) \
+   $(call cc-option,-mtune=bonnell,$(call 
cc-option,-mtune=generic))
 cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
 KBUILD_CFLAGS += $(cflags-y)
 
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..d66e5e3 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -32,8 +32,8 @@ cflags-$(CONFIG_MCYRIXIII)+= $(call 
cc-option,-march=c3,-march=i486) $(align)-f
 cflags-$(CONFIG_MVIAC3_2)  += $(call cc-option,-march=c3-2,-march=i686)
 cflags-$(CONFIG_MVIAC7)+= -march=i686
 cflags-$(CONFIG_MCORE2)+= -march=i686 $(call tune,core2)
-cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom,$(call 
cc-option,-march=core2,-march=i686)) \
-   $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell,$(call 
cc-option,-march=core2,-march=i686)) \
+   $(call cc-option,-mtune=bonnell,$(call cc-option,-mtune=generic))
 
 # AMD Elan support
 cflags-$(CONFIG_MELAN) += -march=i486
-- 
2.10.1

[RFC] [PATCH] arch: x86: change GCC optimisation target from atom to bonnell

[Rui Salvaterra]

Hi, Thomas, Ingo, Peter,

(Sending as RFC, since I don't know if this patch is acceptable.)

The GCC team has deprecated atom as a march/mtune target since almost three
years ago, according to this mailing list thread [1], in favour of specific
microarchitecture names (bonnell, silvermont). This patch changes the Atom
optimisation target in the kernel to bonnell, as it was originally intended.

Tested on an x86-64 Atom 330 machine. No functional changes.

[1] https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01805.html

Signed-off-by: Rui Salvaterra 
---
 arch/x86/Makefile| 4 ++--
 arch/x86/Makefile_32.cpu | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..b7d615f 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -109,8 +109,8 @@ else
 
 cflags-$(CONFIG_MCORE2) += \
 $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
-   cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
-   $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+   cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell) \
+   $(call cc-option,-mtune=bonnell,$(call 
cc-option,-mtune=generic))
 cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
 KBUILD_CFLAGS += $(cflags-y)
 
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..d66e5e3 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -32,8 +32,8 @@ cflags-$(CONFIG_MCYRIXIII)+= $(call 
cc-option,-march=c3,-march=i486) $(align)-f
 cflags-$(CONFIG_MVIAC3_2)  += $(call cc-option,-march=c3-2,-march=i686)
 cflags-$(CONFIG_MVIAC7)+= -march=i686
 cflags-$(CONFIG_MCORE2)+= -march=i686 $(call tune,core2)
-cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom,$(call 
cc-option,-march=core2,-march=i686)) \
-   $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+cflags-$(CONFIG_MATOM) += $(call cc-option,-march=bonnell,$(call 
cc-option,-march=core2,-march=i686)) \
+   $(call cc-option,-mtune=bonnell,$(call cc-option,-mtune=generic))
 
 # AMD Elan support
 cflags-$(CONFIG_MELAN) += -march=i486
-- 
2.10.1

[PATCH] powerpc: wire up preadv2 and pwritev2 syscalls

[Rui Salvaterra]

Wire up preadv2/pwritev2 in the same way as preadv/pwritev. Fixes two
build warnings on ppc64.

Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
---
 arch/powerpc/include/asm/systbl.h  | 2 ++
 arch/powerpc/include/asm/unistd.h  | 2 +-
 arch/powerpc/include/uapi/asm/unistd.h | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/systbl.h 
b/arch/powerpc/include/asm/systbl.h
index 3fa9df7..2fc5d4d 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -384,3 +384,5 @@ SYSCALL(ni_syscall)
 SYSCALL(ni_syscall)
 SYSCALL(mlock2)
 SYSCALL(copy_file_range)
+COMPAT_SYS_SPU(preadv2)
+COMPAT_SYS_SPU(pwritev2)
diff --git a/arch/powerpc/include/asm/unistd.h 
b/arch/powerpc/include/asm/unistd.h
index 1f2594d..cf12c58 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,7 +12,7 @@
 #include 
 
 
-#define NR_syscalls380
+#define NR_syscalls382
 
 #define __NR__exit __NR_exit
 
diff --git a/arch/powerpc/include/uapi/asm/unistd.h 
b/arch/powerpc/include/uapi/asm/unistd.h
index 940290d..e9f5f41 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -390,5 +390,7 @@
 #define __NR_membarrier365
 #define __NR_mlock2378
 #define __NR_copy_file_range   379
+#define __NR_preadv2   380
+#define __NR_pwritev2  381
 
 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
-- 
2.7.4

[PATCH] powerpc: wire up preadv2 and pwritev2 syscalls

[Rui Salvaterra]

Wire up preadv2/pwritev2 in the same way as preadv/pwritev. Fixes two
build warnings on ppc64.

Signed-off-by: Rui Salvaterra 
---
 arch/powerpc/include/asm/systbl.h  | 2 ++
 arch/powerpc/include/asm/unistd.h  | 2 +-
 arch/powerpc/include/uapi/asm/unistd.h | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/systbl.h 
b/arch/powerpc/include/asm/systbl.h
index 3fa9df7..2fc5d4d 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -384,3 +384,5 @@ SYSCALL(ni_syscall)
 SYSCALL(ni_syscall)
 SYSCALL(mlock2)
 SYSCALL(copy_file_range)
+COMPAT_SYS_SPU(preadv2)
+COMPAT_SYS_SPU(pwritev2)
diff --git a/arch/powerpc/include/asm/unistd.h 
b/arch/powerpc/include/asm/unistd.h
index 1f2594d..cf12c58 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,7 +12,7 @@
 #include 
 
 
-#define NR_syscalls380
+#define NR_syscalls382
 
 #define __NR__exit __NR_exit
 
diff --git a/arch/powerpc/include/uapi/asm/unistd.h 
b/arch/powerpc/include/uapi/asm/unistd.h
index 940290d..e9f5f41 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -390,5 +390,7 @@
 #define __NR_membarrier365
 #define __NR_mlock2378
 #define __NR_copy_file_range   379
+#define __NR_preadv2   380
+#define __NR_pwritev2  381
 
 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
-- 
2.7.4

[PATCH v2 1/2] lib: lz4: fixed zram with lz4 on big endian machines

[Rui Salvaterra]

Based on Sergey's test patch [1], this fixes zram with lz4 compression
on big endian cpus.

Note that the 64-bit preprocessor test is not a cleanup, it's part of
the fix, since those identifiers are bogus (for example, __ppc64__
isn't defined anywhere else in the kernel, which means we'd fall into
the 32-bit definitions on ppc64).

Tested on ppc64 with no regression on x86_64.

[1] http://marc.info/?l=linux-kernel=145994470805853=4

Cc: sta...@vger.kernel.org
Suggested-by: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
---
 lib/lz4/lz4defs.h | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index abcecdc..0710a62 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -11,8 +11,7 @@
 /*
  * Detects 64 bits mode
  */
-#if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
-   || defined(__ppc64__) || defined(__LP64__))
+#if defined(CONFIG_64BIT)
 #define LZ4_ARCH64 1
 #else
 #define LZ4_ARCH64 0
@@ -35,6 +34,10 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #define PUT4(s, d) (A32(d) = A32(s))
 #define PUT8(s, d) (A64(d) = A64(s))
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - A16(p))
+
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
do {\
A16(p) = v; \
@@ -51,10 +54,13 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define PUT8(s, d) \
put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
 
-#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
-   do {\
-   put_unaligned(v, (u16 *)(p)); \
-   p += 2; \
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - get_unaligned_le16(p))
+
+#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
+   do {\
+   put_unaligned_le16(v, (u16 *)(p));  \
+   p += 2; \
} while (0)
 #endif
 
@@ -140,9 +146,6 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #endif
 
-#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
-   (d = s - get_unaligned_le16(p))
-
 #define LZ4_WILDCOPY(s, d, e)  \
do {\
LZ4_COPYPACKET(s, d);   \
-- 
2.7.4

[PATCH v2 1/2] lib: lz4: fixed zram with lz4 on big endian machines

[Rui Salvaterra]

Based on Sergey's test patch [1], this fixes zram with lz4 compression
on big endian cpus.

Note that the 64-bit preprocessor test is not a cleanup, it's part of
the fix, since those identifiers are bogus (for example, __ppc64__
isn't defined anywhere else in the kernel, which means we'd fall into
the 32-bit definitions on ppc64).

Tested on ppc64 with no regression on x86_64.

[1] http://marc.info/?l=linux-kernel=145994470805853=4

Cc: sta...@vger.kernel.org
Suggested-by: Sergey Senozhatsky 
Signed-off-by: Rui Salvaterra 
---
 lib/lz4/lz4defs.h | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index abcecdc..0710a62 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -11,8 +11,7 @@
 /*
  * Detects 64 bits mode
  */
-#if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
-   || defined(__ppc64__) || defined(__LP64__))
+#if defined(CONFIG_64BIT)
 #define LZ4_ARCH64 1
 #else
 #define LZ4_ARCH64 0
@@ -35,6 +34,10 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #define PUT4(s, d) (A32(d) = A32(s))
 #define PUT8(s, d) (A64(d) = A64(s))
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - A16(p))
+
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
do {\
A16(p) = v; \
@@ -51,10 +54,13 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define PUT8(s, d) \
put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
 
-#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
-   do {\
-   put_unaligned(v, (u16 *)(p)); \
-   p += 2; \
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - get_unaligned_le16(p))
+
+#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
+   do {\
+   put_unaligned_le16(v, (u16 *)(p));  \
+   p += 2; \
} while (0)
 #endif
 
@@ -140,9 +146,6 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #endif
 
-#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
-   (d = s - get_unaligned_le16(p))
-
 #define LZ4_WILDCOPY(s, d, e)  \
do {\
LZ4_COPYPACKET(s, d);   \
-- 
2.7.4

[PATCH v2 2/2] lib: lz4: cleanup unaligned access efficiency detection

[Rui Salvaterra]

These identifiers are bogus. The interested architectures should define
HAVE_EFFICIENT_UNALIGNED_ACCESS whenever relevant to do so. If this
isn't true for some arch, it should be fixed in the arch definition.

Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
---
 lib/lz4/lz4defs.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index 0710a62..c79d7ea 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -24,9 +24,7 @@
 typedef struct _U16_S { u16 v; } U16_S;
 typedef struct _U32_S { u32 v; } U32_S;
 typedef struct _U64_S { u64 v; } U64_S;
-#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)\
-   || defined(CONFIG_ARM) && __LINUX_ARM_ARCH__ >= 6   \
-   && defined(ARM_EFFICIENT_UNALIGNED_ACCESS)
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
 
 #define A16(x) (((U16_S *)(x))->v)
 #define A32(x) (((U32_S *)(x))->v)
-- 
2.7.4

[PATCH v2 0/2] lib: lz4: fix for big endian and cleanup

[Rui Salvaterra]

v2:
 - Addressed GregKH's review and comments.


Hi,

The first patch fixes zram with lz4 compression on ppc64 (and big endian
architectures with efficient unaligned access), the second is just a
cleanup.

Thanks,

Rui


Rui Salvaterra (2):
  lib: lz4: fixed zram with lz4 on big endian machines
  lib: lz4: cleanup unaligned access efficiency detection

 lib/lz4/lz4defs.h | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH v2 2/2] lib: lz4: cleanup unaligned access efficiency detection

[Rui Salvaterra]

These identifiers are bogus. The interested architectures should define
HAVE_EFFICIENT_UNALIGNED_ACCESS whenever relevant to do so. If this
isn't true for some arch, it should be fixed in the arch definition.

Signed-off-by: Rui Salvaterra 
---
 lib/lz4/lz4defs.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index 0710a62..c79d7ea 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -24,9 +24,7 @@
 typedef struct _U16_S { u16 v; } U16_S;
 typedef struct _U32_S { u32 v; } U32_S;
 typedef struct _U64_S { u64 v; } U64_S;
-#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)\
-   || defined(CONFIG_ARM) && __LINUX_ARM_ARCH__ >= 6   \
-   && defined(ARM_EFFICIENT_UNALIGNED_ACCESS)
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
 
 #define A16(x) (((U16_S *)(x))->v)
 #define A32(x) (((U32_S *)(x))->v)
-- 
2.7.4

[PATCH v2 0/2] lib: lz4: fix for big endian and cleanup

[Rui Salvaterra]

v2:
 - Addressed GregKH's review and comments.


Hi,

The first patch fixes zram with lz4 compression on ppc64 (and big endian
architectures with efficient unaligned access), the second is just a
cleanup.

Thanks,

Rui


Rui Salvaterra (2):
  lib: lz4: fixed zram with lz4 on big endian machines
  lib: lz4: cleanup unaligned access efficiency detection

 lib/lz4/lz4defs.h | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH] lib: lz4: fixed zram with lz4 on big endian machines

[Rui Salvaterra]

Based on Sergey's test patch [1], this fixes zram with lz4 compression on big 
endian cpus. Tested on ppc64 with no regression on x86_64.

[1] http://marc.info/?l=linux-kernel=145994470805853=4

Cc: sta...@vger.kernel.org
Signed-off-by: Rui Salvaterra <rsalvate...@gmail.com>
---
 lib/lz4/lz4defs.h | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index abcecdc..a98c08c 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -11,8 +11,7 @@
 /*
  * Detects 64 bits mode
  */
-#if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
-   || defined(__ppc64__) || defined(__LP64__))
+#if defined(CONFIG_64BIT)
 #define LZ4_ARCH64 1
 #else
 #define LZ4_ARCH64 0
@@ -25,9 +24,7 @@
 typedef struct _U16_S { u16 v; } U16_S;
 typedef struct _U32_S { u32 v; } U32_S;
 typedef struct _U64_S { u64 v; } U64_S;
-#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)\
-   || defined(CONFIG_ARM) && __LINUX_ARM_ARCH__ >= 6   \
-   && defined(ARM_EFFICIENT_UNALIGNED_ACCESS)
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
 
 #define A16(x) (((U16_S *)(x))->v)
 #define A32(x) (((U32_S *)(x))->v)
@@ -35,6 +32,10 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #define PUT4(s, d) (A32(d) = A32(s))
 #define PUT8(s, d) (A64(d) = A64(s))
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - A16(p))
+
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
do {\
A16(p) = v; \
@@ -51,12 +52,15 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define PUT8(s, d) \
put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
 
-#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
-   do {\
-   put_unaligned(v, (u16 *)(p)); \
-   p += 2; \
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - get_unaligned_le16(p))
+
+#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
+   do {\
+   put_unaligned_le16(v, (u16 *)(p));  \
+   p += 2; \
} while (0)
-#endif
+#endif /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
 
 #define COPYLENGTH 8
 #define ML_BITS  4
@@ -138,10 +142,7 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define LZ4_NBCOMMONBYTES(val) (__builtin_ctz(val) >> 3)
 #endif
 
-#endif
-
-#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
-   (d = s - get_unaligned_le16(p))
+#endif /* LZ4_ARCH64 */
 
 #define LZ4_WILDCOPY(s, d, e)  \
do {\
-- 
2.7.4

[PATCH] lib: lz4: fixed zram with lz4 on big endian machines

[Rui Salvaterra]

Based on Sergey's test patch [1], this fixes zram with lz4 compression on big 
endian cpus. Tested on ppc64 with no regression on x86_64.

[1] http://marc.info/?l=linux-kernel=145994470805853=4

Cc: sta...@vger.kernel.org
Signed-off-by: Rui Salvaterra 
---
 lib/lz4/lz4defs.h | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index abcecdc..a98c08c 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -11,8 +11,7 @@
 /*
  * Detects 64 bits mode
  */
-#if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
-   || defined(__ppc64__) || defined(__LP64__))
+#if defined(CONFIG_64BIT)
 #define LZ4_ARCH64 1
 #else
 #define LZ4_ARCH64 0
@@ -25,9 +24,7 @@
 typedef struct _U16_S { u16 v; } U16_S;
 typedef struct _U32_S { u32 v; } U32_S;
 typedef struct _U64_S { u64 v; } U64_S;
-#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)\
-   || defined(CONFIG_ARM) && __LINUX_ARM_ARCH__ >= 6   \
-   && defined(ARM_EFFICIENT_UNALIGNED_ACCESS)
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
 
 #define A16(x) (((U16_S *)(x))->v)
 #define A32(x) (((U32_S *)(x))->v)
@@ -35,6 +32,10 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #define PUT4(s, d) (A32(d) = A32(s))
 #define PUT8(s, d) (A64(d) = A64(s))
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - A16(p))
+
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
do {\
A16(p) = v; \
@@ -51,12 +52,15 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define PUT8(s, d) \
put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
 
-#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
-   do {\
-   put_unaligned(v, (u16 *)(p)); \
-   p += 2; \
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
+   (d = s - get_unaligned_le16(p))
+
+#define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
+   do {\
+   put_unaligned_le16(v, (u16 *)(p));  \
+   p += 2; \
} while (0)
-#endif
+#endif /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
 
 #define COPYLENGTH 8
 #define ML_BITS  4
@@ -138,10 +142,7 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define LZ4_NBCOMMONBYTES(val) (__builtin_ctz(val) >> 3)
 #endif
 
-#endif
-
-#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
-   (d = s - get_unaligned_le16(p))
+#endif /* LZ4_ARCH64 */
 
 #define LZ4_WILDCOPY(s, d, e)  \
do {\
-- 
2.7.4

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-07 15:07 GMT+01:00 Sergey Senozhatsky <sergey.senozhat...@gmail.com>:
> On (04/07/16 13:33), Rui Salvaterra wrote:
> [..]
>> Hi again, Sergey
>
> Hello,
>
>> Thanks for the patch, I'll test it as soon as possible. I agree with
>> your second option, usually one selects lz4 when (especially
>> decompression) speed is paramount, so it needs all the help it can
>> get.
>
> thanks!
>
>> Speaking of fishy, the 64-bit detection code also looks suspiciously
>> bogus. Some of the identifiers don't even exist anywhere in the kernel
>> (__ppc64__, por example, after grepping all .c and .h files).
>> Shouldn't we instead check for CONFIG_64BIT or BITS_PER_LONG == 64?
>
> definitely a good question. personally, I'd prefer to test for
> CONFIG_64BIT only, looking at this hairy
>
>   /* Detects 64 bits mode */
>   #if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
>  || defined(__ppc64__) || defined(__LP64__))
>
> and remove/rewrite a bunch of other stuff. but the thing with cleanups
> is that they don't fix anything, while potentially can introduce bugs.
> it's more risky to touch the stable code. /* well, removing those 'ghost'
> identifiers is sort of OK to me */. but that's just my opinion, I'll
> leave it to you and Greg.
>
> -ss

Hi again, Sergey

I finally was able to test your patch but, as I suspected, it wasn't
enough. However, based on it, I was able to write a (hopefully)
correct one, which I'll send soon (tested on ppc64, with no
regressions on x86_64).

Thanks,

Rui

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-07 15:07 GMT+01:00 Sergey Senozhatsky :
> On (04/07/16 13:33), Rui Salvaterra wrote:
> [..]
>> Hi again, Sergey
>
> Hello,
>
>> Thanks for the patch, I'll test it as soon as possible. I agree with
>> your second option, usually one selects lz4 when (especially
>> decompression) speed is paramount, so it needs all the help it can
>> get.
>
> thanks!
>
>> Speaking of fishy, the 64-bit detection code also looks suspiciously
>> bogus. Some of the identifiers don't even exist anywhere in the kernel
>> (__ppc64__, por example, after grepping all .c and .h files).
>> Shouldn't we instead check for CONFIG_64BIT or BITS_PER_LONG == 64?
>
> definitely a good question. personally, I'd prefer to test for
> CONFIG_64BIT only, looking at this hairy
>
>   /* Detects 64 bits mode */
>   #if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
>  || defined(__ppc64__) || defined(__LP64__))
>
> and remove/rewrite a bunch of other stuff. but the thing with cleanups
> is that they don't fix anything, while potentially can introduce bugs.
> it's more risky to touch the stable code. /* well, removing those 'ghost'
> identifiers is sort of OK to me */. but that's just my opinion, I'll
> leave it to you and Greg.
>
> -ss

Hi again, Sergey

I finally was able to test your patch but, as I suspected, it wasn't
enough. However, based on it, I was able to write a (hopefully)
correct one, which I'll send soon (tested on ppc64, with no
regressions on x86_64).

Thanks,

Rui

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-06 14:09 GMT+01:00 Sergey Senozhatsky <sergey.senozhat...@gmail.com>:
> Cc Chanho Min, Kyungsik Lee
>
>
> Hello,
>
> On (04/06/16 10:39), Rui Salvaterra wrote:
>> > may we please ask you to test the patch first? quite possible there
>> > is nothing to fix there; I've no access to mips h/w but the patch
>> > seems correct to me.
>> >
>> > LZ4_READ_LITTLEENDIAN_16 does get_unaligned_le16(), so
>> > LZ4_WRITE_LITTLEENDIAN_16 must do put_unaligned_le16() /* not 
>> > put_unaligned() */
>> >
> [..]
>> Consequentially, while I believe the patch will fix the mips case, I'm
>> not so sure about ppc (or any other big endian architecture with
>> efficient unaligned accesses).
>
> frankly, yes, I took a quick look today (after I sent my initial
> message, tho) ... and it is fishy, I agree. was going to followup
> on my email but somehow got interrupted, sorry.
>
> so we have, write:
> ((U16_S *)(p)) = vORput_unaligned(v, (u16 *)(p))
>
> and only one read:
> get_unaligned_le16(p))
>
> I guess it's either read part also must depend on
> HAVE_EFFICIENT_UNALIGNED_ACCESS, or write path
> should stop doing so.
>
> I ended up with two patches, NONE was tested (!!!). like at all.
>
> 1) provide CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS-dependent
>LZ4_READ_LITTLEENDIAN_16
>
> 2) provide common LZ4_WRITE_LITTLEENDIAN_16 and LZ4_READ_LITTLEENDIAN_16
>regardless CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
>
>
> assuming that common LZ4_WRITE_LITTLEENDIAN_16 will somehow hit the
> performance, I'd probably prefer option #1.
>
> the patch is below. would be great if you can help testing it.
>
> ---
>
>  lib/lz4/lz4defs.h | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
> index abcecdc..a23e6c2 100644
> --- a/lib/lz4/lz4defs.h
> +++ b/lib/lz4/lz4defs.h
> @@ -36,10 +36,14 @@ typedef struct _U64_S { u64 v; } U64_S;
>  #define PUT4(s, d) (A32(d) = A32(s))
>  #define PUT8(s, d) (A64(d) = A64(s))
>  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
> -   do {\
> -   A16(p) = v; \
> -   p += 2; \
> +   do {\
> +   A16(p) = v; \
> +   p += 2; \
> } while (0)
> +
> +#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
> +   (d = s - A16(p))
> +
>  #else /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
>
>  #define A64(x) get_unaligned((u64 *)&(((U16_S *)(x))->v))
> @@ -52,10 +56,13 @@ typedef struct _U64_S { u64 v; } U64_S;
> put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
>
>  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
> -   do {\
> -   put_unaligned(v, (u16 *)(p)); \
> -   p += 2; \
> +   do {\
> +   put_unaligned_le16(v, (u16 *)(p));  \
> +   p += 2; \
> } while (0)
> +
> +#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
> +   (d = s - get_unaligned_le16(p))
>  #endif
>
>  #define COPYLENGTH 8
> @@ -140,9 +147,6 @@ typedef struct _U64_S { u64 v; } U64_S;
>
>  #endif
>
> -#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
> -   (d = s - get_unaligned_le16(p))
> -
>  #define LZ4_WILDCOPY(s, d, e)  \
> do {\
> LZ4_COPYPACKET(s, d);   \
>


Hi again, Sergey


Thanks for the patch, I'll test it as soon as possible. I agree with
your second option, usually one selects lz4 when (especially
decompression) speed is paramount, so it needs all the help it can
get.

Speaking of fishy, the 64-bit detection code also looks suspiciously
bogus. Some of the identifiers don't even exist anywhere in the kernel
(__ppc64__, por example, after grepping all .c and .h files).
Shouldn't we instead check for CONFIG_64BIT or BITS_PER_LONG == 64?


Thanks,

Rui

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-06 14:09 GMT+01:00 Sergey Senozhatsky :
> Cc Chanho Min, Kyungsik Lee
>
>
> Hello,
>
> On (04/06/16 10:39), Rui Salvaterra wrote:
>> > may we please ask you to test the patch first? quite possible there
>> > is nothing to fix there; I've no access to mips h/w but the patch
>> > seems correct to me.
>> >
>> > LZ4_READ_LITTLEENDIAN_16 does get_unaligned_le16(), so
>> > LZ4_WRITE_LITTLEENDIAN_16 must do put_unaligned_le16() /* not 
>> > put_unaligned() */
>> >
> [..]
>> Consequentially, while I believe the patch will fix the mips case, I'm
>> not so sure about ppc (or any other big endian architecture with
>> efficient unaligned accesses).
>
> frankly, yes, I took a quick look today (after I sent my initial
> message, tho) ... and it is fishy, I agree. was going to followup
> on my email but somehow got interrupted, sorry.
>
> so we have, write:
> ((U16_S *)(p)) = vORput_unaligned(v, (u16 *)(p))
>
> and only one read:
> get_unaligned_le16(p))
>
> I guess it's either read part also must depend on
> HAVE_EFFICIENT_UNALIGNED_ACCESS, or write path
> should stop doing so.
>
> I ended up with two patches, NONE was tested (!!!). like at all.
>
> 1) provide CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS-dependent
>LZ4_READ_LITTLEENDIAN_16
>
> 2) provide common LZ4_WRITE_LITTLEENDIAN_16 and LZ4_READ_LITTLEENDIAN_16
>regardless CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
>
>
> assuming that common LZ4_WRITE_LITTLEENDIAN_16 will somehow hit the
> performance, I'd probably prefer option #1.
>
> the patch is below. would be great if you can help testing it.
>
> ---
>
>  lib/lz4/lz4defs.h | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
> index abcecdc..a23e6c2 100644
> --- a/lib/lz4/lz4defs.h
> +++ b/lib/lz4/lz4defs.h
> @@ -36,10 +36,14 @@ typedef struct _U64_S { u64 v; } U64_S;
>  #define PUT4(s, d) (A32(d) = A32(s))
>  #define PUT8(s, d) (A64(d) = A64(s))
>  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
> -   do {\
> -   A16(p) = v; \
> -   p += 2; \
> +   do {\
> +   A16(p) = v; \
> +   p += 2; \
> } while (0)
> +
> +#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
> +   (d = s - A16(p))
> +
>  #else /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
>
>  #define A64(x) get_unaligned((u64 *)&(((U16_S *)(x))->v))
> @@ -52,10 +56,13 @@ typedef struct _U64_S { u64 v; } U64_S;
> put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
>
>  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)\
> -   do {\
> -   put_unaligned(v, (u16 *)(p)); \
> -   p += 2; \
> +   do {\
> +   put_unaligned_le16(v, (u16 *)(p));  \
> +   p += 2; \
> } while (0)
> +
> +#define LZ4_READ_LITTLEENDIAN_16(d, s, p)  \
> +   (d = s - get_unaligned_le16(p))
>  #endif
>
>  #define COPYLENGTH 8
> @@ -140,9 +147,6 @@ typedef struct _U64_S { u64 v; } U64_S;
>
>  #endif
>
> -#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
> -   (d = s - get_unaligned_le16(p))
> -
>  #define LZ4_WILDCOPY(s, d, e)  \
> do {\
> LZ4_COPYPACKET(s, d);   \
>


Hi again, Sergey


Thanks for the patch, I'll test it as soon as possible. I agree with
your second option, usually one selects lz4 when (especially
decompression) speed is paramount, so it needs all the help it can
get.

Speaking of fishy, the 64-bit detection code also looks suspiciously
bogus. Some of the identifiers don't even exist anywhere in the kernel
(__ppc64__, por example, after grepping all .c and .h files).
Shouldn't we instead check for CONFIG_64BIT or BITS_PER_LONG == 64?


Thanks,

Rui

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-06 6:33 GMT+01:00 Sergey Senozhatsky
<sergey.senozhatsky.w...@gmail.com>:
> On (04/05/16 17:02), Rui Salvaterra wrote:
> [..]
>> > For some reason it never got merged, sorry, I don't remember why.
>> >
>> > Have you tested this patch?  If so, can you resend it with your
>> > tested-by: line added to it?
>> >
>> > thanks,
>> >
>> > greg k-h
>>
>> Hi, Greg
>>
>>
>> No, I haven't tested the patch at all. I want to do so, and fix if if
>> necessary, but I still need to learn how to (meaning, I need to watch
>> your "first kernel patch" presentation again). I'd love to get
>> involved in kernel development, and this seems to be a good
>> opportunity, if none of the kernel gods beat me to it (I may need a
>> month, but then again nobody complained about this bug in almost two
>> years).
>
> Hello Rui,
>
> may we please ask you to test the patch first? quite possible there
> is nothing to fix there; I've no access to mips h/w but the patch
> seems correct to me.
>
> LZ4_READ_LITTLEENDIAN_16 does get_unaligned_le16(), so
> LZ4_WRITE_LITTLEENDIAN_16 must do put_unaligned_le16() /* not put_unaligned() 
> */
>
> -ss

Hi, Sergey


Besides ppc64, I have ppc32, x86 and x86_64 hardware readily
available. The only mips (74kc, also big endian) hardware I have
access to is my router, running OpenWrt, I can try to test it there
too, but it will be more complicated. Still, after reading the
existing code [1] more thoroughly, I can't see how Eunbong Song's
patch [2] would fix the ppc case (please correct me if I'm wrong,
which is highly likely, since my C preprocessor knowledge varies
between nonexistent to very superficial).

Now, LZ4_READ_LITTLEENDIAN_16 is unconditionally defined as:

#define LZ4_READ_LITTLEENDIAN_16(d, s, p)
(d = s - get_unaligned_le16(p))

As far as I can tell, and unlike ppc, mips doesn't define
HAVE_EFFICIENT_UNALIGNED_ACCESS, which means for mips case,
LZ4_WRITE_LITTLEENDIAN_16 will be defined as:

#define LZ4_WRITE_LITTLEENDIAN_16(p, v)
do {
put_unaligned(v, (u16 *)(p));
p += 2;
} while (0)

Whereas for ppc, which defines HAVE_EFFICIENT_UNALIGNED_ACCESS,
LZ4_WRITE_LITTLEENDIAN_16 will be defined as:

#define LZ4_WRITE_LITTLEENDIAN_16(p, v)
do {
A16(p) = v;
p += 2;
} while (0)

Consequentially, while I believe the patch will fix the mips case, I'm
not so sure about ppc (or any other big endian architecture with
efficient unaligned accesses).


Thanks,

Rui

[1] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/lib/lz4/lz4defs.h?h=v4.4.6
[2] http://permalink.gmane.org/gmane.linux.kernel/1752745

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-06 6:33 GMT+01:00 Sergey Senozhatsky
:
> On (04/05/16 17:02), Rui Salvaterra wrote:
> [..]
>> > For some reason it never got merged, sorry, I don't remember why.
>> >
>> > Have you tested this patch?  If so, can you resend it with your
>> > tested-by: line added to it?
>> >
>> > thanks,
>> >
>> > greg k-h
>>
>> Hi, Greg
>>
>>
>> No, I haven't tested the patch at all. I want to do so, and fix if if
>> necessary, but I still need to learn how to (meaning, I need to watch
>> your "first kernel patch" presentation again). I'd love to get
>> involved in kernel development, and this seems to be a good
>> opportunity, if none of the kernel gods beat me to it (I may need a
>> month, but then again nobody complained about this bug in almost two
>> years).
>
> Hello Rui,
>
> may we please ask you to test the patch first? quite possible there
> is nothing to fix there; I've no access to mips h/w but the patch
> seems correct to me.
>
> LZ4_READ_LITTLEENDIAN_16 does get_unaligned_le16(), so
> LZ4_WRITE_LITTLEENDIAN_16 must do put_unaligned_le16() /* not put_unaligned() 
> */
>
> -ss

Hi, Sergey


Besides ppc64, I have ppc32, x86 and x86_64 hardware readily
available. The only mips (74kc, also big endian) hardware I have
access to is my router, running OpenWrt, I can try to test it there
too, but it will be more complicated. Still, after reading the
existing code [1] more thoroughly, I can't see how Eunbong Song's
patch [2] would fix the ppc case (please correct me if I'm wrong,
which is highly likely, since my C preprocessor knowledge varies
between nonexistent to very superficial).

Now, LZ4_READ_LITTLEENDIAN_16 is unconditionally defined as:

#define LZ4_READ_LITTLEENDIAN_16(d, s, p)
(d = s - get_unaligned_le16(p))

As far as I can tell, and unlike ppc, mips doesn't define
HAVE_EFFICIENT_UNALIGNED_ACCESS, which means for mips case,
LZ4_WRITE_LITTLEENDIAN_16 will be defined as:

#define LZ4_WRITE_LITTLEENDIAN_16(p, v)
do {
put_unaligned(v, (u16 *)(p));
p += 2;
} while (0)

Whereas for ppc, which defines HAVE_EFFICIENT_UNALIGNED_ACCESS,
LZ4_WRITE_LITTLEENDIAN_16 will be defined as:

#define LZ4_WRITE_LITTLEENDIAN_16(p, v)
do {
A16(p) = v;
p += 2;
} while (0)

Consequentially, while I believe the patch will fix the mips case, I'm
not so sure about ppc (or any other big endian architecture with
efficient unaligned accesses).


Thanks,

Rui

[1] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/lib/lz4/lz4defs.h?h=v4.4.6
[2] http://permalink.gmane.org/gmane.linux.kernel/1752745

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-05 16:34 GMT+01:00 Greg KH <gre...@linuxfoundation.org>:
> On Tue, Apr 05, 2016 at 03:07:48PM +0100, Rui Salvaterra wrote:
>> Hi,
>>
>>
>> I apologise in advance if I've cc'ed too many/the wrong people/lists.
>>
>> Whenever I try to use zram with lz4, on my Power Mac G5 (tested with
>> kernel 4.4.0-16-powerpc64-smp from Ubuntu 16.04 LTS), I get the
>> following on my dmesg:
>>
>> [13150.675820] zram: Added device: zram0
>> [13150.704133] zram0: detected capacity change from 0 to 5131976704
>> [13150.715960] zram: Decompression failed! err=-1, page=0
>> [13150.716008] zram: Decompression failed! err=-1, page=0
>> [13150.716027] zram: Decompression failed! err=-1, page=0
>> [13150.716032] Buffer I/O error on dev zram0, logical block 0, async page 
>> read
>>
>> I believe Eunbong Song wrote a patch [1] to fix this (or a very
>> identical) bug on MIPS, but it never got merged (maybe
>> incorrect/incomplete?). Is there any hope of seeing this bug fixed?
>>
>>
>> Thanks,
>>
>> Rui Salvaterra
>>
>>
>> [1] http://comments.gmane.org/gmane.linux.kernel/1752745
>
> For some reason it never got merged, sorry, I don't remember why.
>
> Have you tested this patch?  If so, can you resend it with your
> tested-by: line added to it?
>
> thanks,
>
> greg k-h

Hi, Greg


No, I haven't tested the patch at all. I want to do so, and fix if if
necessary, but I still need to learn how to (meaning, I need to watch
your "first kernel patch" presentation again). I'd love to get
involved in kernel development, and this seems to be a good
opportunity, if none of the kernel gods beat me to it (I may need a
month, but then again nobody complained about this bug in almost two
years).


Thanks,

Rui

Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

2016-04-05 16:34 GMT+01:00 Greg KH :
> On Tue, Apr 05, 2016 at 03:07:48PM +0100, Rui Salvaterra wrote:
>> Hi,
>>
>>
>> I apologise in advance if I've cc'ed too many/the wrong people/lists.
>>
>> Whenever I try to use zram with lz4, on my Power Mac G5 (tested with
>> kernel 4.4.0-16-powerpc64-smp from Ubuntu 16.04 LTS), I get the
>> following on my dmesg:
>>
>> [13150.675820] zram: Added device: zram0
>> [13150.704133] zram0: detected capacity change from 0 to 5131976704
>> [13150.715960] zram: Decompression failed! err=-1, page=0
>> [13150.716008] zram: Decompression failed! err=-1, page=0
>> [13150.716027] zram: Decompression failed! err=-1, page=0
>> [13150.716032] Buffer I/O error on dev zram0, logical block 0, async page 
>> read
>>
>> I believe Eunbong Song wrote a patch [1] to fix this (or a very
>> identical) bug on MIPS, but it never got merged (maybe
>> incorrect/incomplete?). Is there any hope of seeing this bug fixed?
>>
>>
>> Thanks,
>>
>> Rui Salvaterra
>>
>>
>> [1] http://comments.gmane.org/gmane.linux.kernel/1752745
>
> For some reason it never got merged, sorry, I don't remember why.
>
> Have you tested this patch?  If so, can you resend it with your
> tested-by: line added to it?
>
> thanks,
>
> greg k-h

Hi, Greg


No, I haven't tested the patch at all. I want to do so, and fix if if
necessary, but I still need to learn how to (meaning, I need to watch
your "first kernel patch" presentation again). I'd love to get
involved in kernel development, and this seems to be a good
opportunity, if none of the kernel gods beat me to it (I may need a
month, but then again nobody complained about this bug in almost two
years).


Thanks,

Rui

[BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

Hi,


I apologise in advance if I've cc'ed too many/the wrong people/lists.

Whenever I try to use zram with lz4, on my Power Mac G5 (tested with
kernel 4.4.0-16-powerpc64-smp from Ubuntu 16.04 LTS), I get the
following on my dmesg:

[13150.675820] zram: Added device: zram0
[13150.704133] zram0: detected capacity change from 0 to 5131976704
[13150.715960] zram: Decompression failed! err=-1, page=0
[13150.716008] zram: Decompression failed! err=-1, page=0
[13150.716027] zram: Decompression failed! err=-1, page=0
[13150.716032] Buffer I/O error on dev zram0, logical block 0, async page read

I believe Eunbong Song wrote a patch [1] to fix this (or a very
identical) bug on MIPS, but it never got merged (maybe
incorrect/incomplete?). Is there any hope of seeing this bug fixed?


Thanks,

Rui Salvaterra


[1] http://comments.gmane.org/gmane.linux.kernel/1752745

[BUG] lib: zram lz4 compression/decompression still broken on big endian

[Rui Salvaterra]

Hi,


I apologise in advance if I've cc'ed too many/the wrong people/lists.

Whenever I try to use zram with lz4, on my Power Mac G5 (tested with
kernel 4.4.0-16-powerpc64-smp from Ubuntu 16.04 LTS), I get the
following on my dmesg:

[13150.675820] zram: Added device: zram0
[13150.704133] zram0: detected capacity change from 0 to 5131976704
[13150.715960] zram: Decompression failed! err=-1, page=0
[13150.716008] zram: Decompression failed! err=-1, page=0
[13150.716027] zram: Decompression failed! err=-1, page=0
[13150.716032] Buffer I/O error on dev zram0, logical block 0, async page read

I believe Eunbong Song wrote a patch [1] to fix this (or a very
identical) bug on MIPS, but it never got merged (maybe
incorrect/incomplete?). Is there any hope of seeing this bug fixed?


Thanks,

Rui Salvaterra


[1] http://comments.gmane.org/gmane.linux.kernel/1752745