[v4] zswap: allow setting default status, compressor and allocator in Kconfig
diff mbox series

Message ID 20191117000544.1019556-1-mail@maciej.szmigiero.name
State Superseded
Headers show
Series
  • [v4] zswap: allow setting default status, compressor and allocator in Kconfig
Related show

Commit Message

Maciej S. Szmigiero Nov. 17, 2019, 12:05 a.m. UTC
The compressed cache for swap pages (zswap) currently needs from 1 to 3
extra kernel command line parameters in order to make it work: it has to be
enabled by adding a "zswap.enabled=1" command line parameter and if one
wants a different compressor or pool allocator than the default lzo / zbud
combination then these choices also need to be specified on the kernel
command line in additional parameters.

Using a different compressor and allocator for zswap is actually pretty
common as guides often recommend using the lz4 / z3fold pair instead of
the default one.
In such case it is also necessary to remember to enable the appropriate
compression algorithm and pool allocator in the kernel config manually.

Let's avoid the need for adding these kernel command line parameters and
automatically pull in the dependencies for the selected compressor
algorithm and pool allocator by adding an appropriate default switches to
Kconfig.

The default values for these options match what the code was using
previously as its defaults.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
Changes from v1:
Rename CONFIG_ZSWAP_DEFAULT_COMP_* to CONFIG_ZSWAP_COMPRESSOR_DEFAULT_*
and CONFIG_ZSWAP_DEFAULT_ZPOOL_* to CONFIG_ZSWAP_ZPOOL_DEFAULT_* while
dropping the "_NAME" suffix from the final string option in both cases.

Changes from v2:
Add zsmalloc as a pool allocator choice, add a link to a page with
benchmarks of various compression algorithms to the compression
algorithm prompt option.

Changes from v3:
Update the zswap doc in Documentation, too.

 Documentation/vm/zswap.rst |  20 ++++---
 mm/Kconfig                 | 116 ++++++++++++++++++++++++++++++++++++-
 mm/zswap.c                 |  26 +++++----
 3 files changed, 142 insertions(+), 20 deletions(-)

Comments

Vitaly Wool Nov. 18, 2019, 12:43 p.m. UTC | #1
Hi Maciej,

sorry for jumping in late into this discussion.

On Sun, Nov 17, 2019 at 1:05 AM Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
<snip>
> +config ZSWAP_COMPRESSOR_DEFAULT
> +       string
> +       default "deflate" if ZSWAP_COMPRESSOR_DEFAULT_DEFLATE
> +       default "lzo" if ZSWAP_COMPRESSOR_DEFAULT_LZO
> +       default "842" if ZSWAP_COMPRESSOR_DEFAULT_842
> +       default "lz4" if ZSWAP_COMPRESSOR_DEFAULT_LZ4
> +       default "lz4hc" if ZSWAP_COMPRESSOR_DEFAULT_LZ4HC
> +       default "zstd" if ZSWAP_COMPRESSOR_DEFAULT_ZSTD
> +       default ""

I guess this should also depend on ZSWAP. It looks a bit awkward when
this option is present in dotconfig while ZSWAP is not.

> +
> +choice
> +       prompt "Compressed cache for swap pages default allocator"
> +       depends on ZSWAP
> +       default ZSWAP_ZPOOL_DEFAULT_ZBUD
> +       help
> +         Selects the default allocator for the compressed cache for
> +         swap pages.
> +         The default is 'zbud' for compatibility, however please do
> +         read the description of each of the allocators below before
> +         making a right choice.
> +
> +         The selection made here can be overridden by using the kernel
> +         command line 'zswap.zpool=' option.
> +
> +config ZSWAP_ZPOOL_DEFAULT_ZBUD
> +       bool "zbud"
> +       select ZBUD
> +       help
> +         Use the zbud allocator as the default allocator.
> +
> +config ZSWAP_ZPOOL_DEFAULT_Z3FOLD
> +       bool "z3fold"
> +       select Z3FOLD
> +       help
> +         Use the z3fold allocator as the default allocator.
> +
> +config ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
> +       bool "zsmalloc"
> +       select ZSMALLOC
> +       help
> +         Use the zsmalloc allocator as the default allocator.
> +endchoice
> +
> +config ZSWAP_ZPOOL_DEFAULT
> +       string
> +       default "zbud" if ZSWAP_ZPOOL_DEFAULT_ZBUD
> +       default "z3fold" if ZSWAP_ZPOOL_DEFAULT_Z3FOLD
> +       default "zsmalloc" if ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
> +       default ""

Same here.

> +
> +config ZSWAP_DEFAULT_ON
> +       bool "Enable the compressed cache for swap pages by default"
> +       depends on ZSWAP
> +       help
> +         If selected, the compressed cache for swap pages will be enabled
> +         at boot, otherwise it will be disabled.
> +
> +         The selection made here can be overridden by using the kernel
> +         command line 'zswap.enabled=' option.
> +
>  config ZPOOL
>         tristate "Common API for compressed memory storage"
>         help
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 46a322316e52..71795b6f5b71 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -71,8 +71,12 @@ static u64 zswap_duplicate_entry;
>
>  #define ZSWAP_PARAM_UNSET ""
>
> -/* Enable/disable zswap (disabled by default) */
> +/* Enable/disable zswap */
> +#ifdef CONFIG_ZSWAP_DEFAULT_ON
> +static bool zswap_enabled = true;
> +#else
>  static bool zswap_enabled;
> +#endif

Wouldn't it be better to have 'static bool zswap_enabled =
IS_ENABLED(CONFIG_ZSWAP_DEFAULT_ON);' instead?

>  static int zswap_enabled_param_set(const char *,
>                                    const struct kernel_param *);
>  static struct kernel_param_ops zswap_enabled_param_ops = {
> @@ -82,8 +86,7 @@ static struct kernel_param_ops zswap_enabled_param_ops = {
>  module_param_cb(enabled, &zswap_enabled_param_ops, &zswap_enabled, 0644);
>
>  /* Crypto compressor to use */
> -#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
> -static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> +static char *zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
>  static int zswap_compressor_param_set(const char *,
>                                       const struct kernel_param *);
>  static struct kernel_param_ops zswap_compressor_param_ops = {
> @@ -95,8 +98,7 @@ module_param_cb(compressor, &zswap_compressor_param_ops,
>                 &zswap_compressor, 0644);
>
>  /* Compressed storage zpool to use */
> -#define ZSWAP_ZPOOL_DEFAULT "zbud"
> -static char *zswap_zpool_type = ZSWAP_ZPOOL_DEFAULT;
> +static char *zswap_zpool_type = CONFIG_ZSWAP_ZPOOL_DEFAULT;
>  static int zswap_zpool_param_set(const char *, const struct kernel_param *);
>  static struct kernel_param_ops zswap_zpool_param_ops = {
>         .set =          zswap_zpool_param_set,
> @@ -569,11 +571,12 @@ static __init struct zswap_pool *__zswap_pool_create_fallback(void)
>         bool has_comp, has_zpool;
>
>         has_comp = crypto_has_comp(zswap_compressor, 0, 0);
> -       if (!has_comp && strcmp(zswap_compressor, ZSWAP_COMPRESSOR_DEFAULT)) {
> +       if (!has_comp && strcmp(zswap_compressor,
> +                               CONFIG_ZSWAP_COMPRESSOR_DEFAULT)) {
>                 pr_err("compressor %s not available, using default %s\n",
> -                      zswap_compressor, ZSWAP_COMPRESSOR_DEFAULT);
> +                      zswap_compressor, CONFIG_ZSWAP_COMPRESSOR_DEFAULT);
>                 param_free_charp(&zswap_compressor);
> -               zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> +               zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
>                 has_comp = crypto_has_comp(zswap_compressor, 0, 0);
>         }
>         if (!has_comp) {
> @@ -584,11 +587,12 @@ static __init struct zswap_pool *__zswap_pool_create_fallback(void)
>         }
>
>         has_zpool = zpool_has_pool(zswap_zpool_type);
> -       if (!has_zpool && strcmp(zswap_zpool_type, ZSWAP_ZPOOL_DEFAULT)) {
> +       if (!has_zpool && strcmp(zswap_zpool_type,
> +                                CONFIG_ZSWAP_ZPOOL_DEFAULT)) {
>                 pr_err("zpool %s not available, using default %s\n",
> -                      zswap_zpool_type, ZSWAP_ZPOOL_DEFAULT);
> +                      zswap_zpool_type, CONFIG_ZSWAP_ZPOOL_DEFAULT);
>                 param_free_charp(&zswap_zpool_type);
> -               zswap_zpool_type = ZSWAP_ZPOOL_DEFAULT;
> +               zswap_zpool_type = CONFIG_ZSWAP_ZPOOL_DEFAULT;
>                 has_zpool = zpool_has_pool(zswap_zpool_type);
>         }
>         if (!has_zpool) {
Maciej S. Szmigiero Nov. 18, 2019, 11:03 p.m. UTC | #2
Hi Vitaly,

On 18.11.2019 13:43, Vitaly Wool wrote:
> Hi Maciej,
> 
> sorry for jumping in late into this discussion.

Thanks for your comments, implemented them all now in v5.

Maciej

Patch
diff mbox series

diff --git a/Documentation/vm/zswap.rst b/Documentation/vm/zswap.rst
index 1444ecd40911..2df6ee6b7a1d 100644
--- a/Documentation/vm/zswap.rst
+++ b/Documentation/vm/zswap.rst
@@ -35,9 +35,11 @@  Zswap evicts pages from compressed cache on an LRU basis to the backing swap
 device when the compressed pool reaches its size limit.  This requirement had
 been identified in prior community discussions.
 
-Zswap is disabled by default but can be enabled at boot time by setting
-the ``enabled`` attribute to 1 at boot time. ie: ``zswap.enabled=1``.  Zswap
-can also be enabled and disabled at runtime using the sysfs interface.
+Whether Zswap is enabled at the boot time depends on whether
+the ``CONFIG_ZSWAP_DEFAULT_ON`` Kconfig option is enabled or not.
+This setting can then be overridden by providing the kernel command line
+``zswap.enabled=`` option, for example ``zswap.enabled=0``.
+Zswap can also be enabled and disabled at runtime using the sysfs interface.
 An example command to enable zswap at runtime, assuming sysfs is mounted
 at ``/sys``, is::
 
@@ -64,9 +66,10 @@  allocation in zpool is not directly accessible by address.  Rather, a handle is
 returned by the allocation routine and that handle must be mapped before being
 accessed.  The compressed memory pool grows on demand and shrinks as compressed
 pages are freed.  The pool is not preallocated.  By default, a zpool
-of type zbud is created, but it can be selected at boot time by
-setting the ``zpool`` attribute, e.g. ``zswap.zpool=zbud``. It can
-also be changed at runtime using the sysfs ``zpool`` attribute, e.g.::
+of type selected in ``CONFIG_ZSWAP_ZPOOL_DEFAULT`` Kconfig option is created,
+but it can be overridden at boot time by setting the ``zpool`` attribute,
+e.g. ``zswap.zpool=zbud``. It can also be changed at runtime using the sysfs
+``zpool`` attribute, e.g.::
 
 	echo zbud > /sys/module/zswap/parameters/zpool
 
@@ -97,8 +100,9 @@  controlled policy:
 * max_pool_percent - The maximum percentage of memory that the compressed
   pool can occupy.
 
-The default compressor is lzo, but it can be selected at boot time by
-setting the ``compressor`` attribute, e.g. ``zswap.compressor=lzo``.
+The default compressor is selected in ``CONFIG_ZSWAP_COMPRESSOR_DEFAULT``
+Kconfig option, but it can be overridden at boot time by setting the
+``compressor`` attribute, e.g. ``zswap.compressor=lzo``.
 It can also be changed at runtime using the sysfs "compressor"
 attribute, e.g.::
 
diff --git a/mm/Kconfig b/mm/Kconfig
index a5dae9a7eb51..4a00d9ebf774 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -525,7 +525,6 @@  config MEM_SOFT_DIRTY
 config ZSWAP
 	bool "Compressed cache for swap pages (EXPERIMENTAL)"
 	depends on FRONTSWAP && CRYPTO=y
-	select CRYPTO_LZO
 	select ZPOOL
 	help
 	  A lightweight compressed cache for swap pages.  It takes
@@ -541,6 +540,121 @@  config ZSWAP
 	  they have not be fully explored on the large set of potential
 	  configurations and workloads that exist.
 
+choice
+	prompt "Compressed cache for swap pages default compressor"
+	depends on ZSWAP
+	default ZSWAP_COMPRESSOR_DEFAULT_LZO
+	help
+	  Selects the default compression algorithm for the compressed cache
+	  for swap pages.
+
+	  For an overview what kind of performance can be expected from
+	  a particular compression algorithm please refer to the benchmarks
+	  available at the following LWN page:
+	  https://lwn.net/Articles/751795/
+
+	  If in doubt, select 'LZO'.
+
+	  The selection made here can be overridden by using the kernel
+	  command line 'zswap.compressor=' option.
+
+config ZSWAP_COMPRESSOR_DEFAULT_DEFLATE
+	bool "Deflate"
+	select CRYPTO_DEFLATE
+	help
+	  Use the Deflate algorithm as the default compression algorithm.
+
+config ZSWAP_COMPRESSOR_DEFAULT_LZO
+	bool "LZO"
+	select CRYPTO_LZO
+	help
+	  Use the LZO algorithm as the default compression algorithm.
+
+config ZSWAP_COMPRESSOR_DEFAULT_842
+	bool "842"
+	select CRYPTO_842
+	help
+	  Use the 842 algorithm as the default compression algorithm.
+
+config ZSWAP_COMPRESSOR_DEFAULT_LZ4
+	bool "LZ4"
+	select CRYPTO_LZ4
+	help
+	  Use the LZ4 algorithm as the default compression algorithm.
+
+config ZSWAP_COMPRESSOR_DEFAULT_LZ4HC
+	bool "LZ4HC"
+	select CRYPTO_LZ4HC
+	help
+	  Use the LZ4HC algorithm as the default compression algorithm.
+
+config ZSWAP_COMPRESSOR_DEFAULT_ZSTD
+	bool "zstd"
+	select CRYPTO_ZSTD
+	help
+	  Use the zstd algorithm as the default compression algorithm.
+endchoice
+
+config ZSWAP_COMPRESSOR_DEFAULT
+       string
+       default "deflate" if ZSWAP_COMPRESSOR_DEFAULT_DEFLATE
+       default "lzo" if ZSWAP_COMPRESSOR_DEFAULT_LZO
+       default "842" if ZSWAP_COMPRESSOR_DEFAULT_842
+       default "lz4" if ZSWAP_COMPRESSOR_DEFAULT_LZ4
+       default "lz4hc" if ZSWAP_COMPRESSOR_DEFAULT_LZ4HC
+       default "zstd" if ZSWAP_COMPRESSOR_DEFAULT_ZSTD
+       default ""
+
+choice
+	prompt "Compressed cache for swap pages default allocator"
+	depends on ZSWAP
+	default ZSWAP_ZPOOL_DEFAULT_ZBUD
+	help
+	  Selects the default allocator for the compressed cache for
+	  swap pages.
+	  The default is 'zbud' for compatibility, however please do
+	  read the description of each of the allocators below before
+	  making a right choice.
+
+	  The selection made here can be overridden by using the kernel
+	  command line 'zswap.zpool=' option.
+
+config ZSWAP_ZPOOL_DEFAULT_ZBUD
+	bool "zbud"
+	select ZBUD
+	help
+	  Use the zbud allocator as the default allocator.
+
+config ZSWAP_ZPOOL_DEFAULT_Z3FOLD
+	bool "z3fold"
+	select Z3FOLD
+	help
+	  Use the z3fold allocator as the default allocator.
+
+config ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
+	bool "zsmalloc"
+	select ZSMALLOC
+	help
+	  Use the zsmalloc allocator as the default allocator.
+endchoice
+
+config ZSWAP_ZPOOL_DEFAULT
+       string
+       default "zbud" if ZSWAP_ZPOOL_DEFAULT_ZBUD
+       default "z3fold" if ZSWAP_ZPOOL_DEFAULT_Z3FOLD
+       default "zsmalloc" if ZSWAP_ZPOOL_DEFAULT_ZSMALLOC
+       default ""
+
+config ZSWAP_DEFAULT_ON
+	bool "Enable the compressed cache for swap pages by default"
+	depends on ZSWAP
+	help
+	  If selected, the compressed cache for swap pages will be enabled
+	  at boot, otherwise it will be disabled.
+
+	  The selection made here can be overridden by using the kernel
+	  command line 'zswap.enabled=' option.
+
 config ZPOOL
 	tristate "Common API for compressed memory storage"
 	help
diff --git a/mm/zswap.c b/mm/zswap.c
index 46a322316e52..71795b6f5b71 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -71,8 +71,12 @@  static u64 zswap_duplicate_entry;
 
 #define ZSWAP_PARAM_UNSET ""
 
-/* Enable/disable zswap (disabled by default) */
+/* Enable/disable zswap */
+#ifdef CONFIG_ZSWAP_DEFAULT_ON
+static bool zswap_enabled = true;
+#else
 static bool zswap_enabled;
+#endif
 static int zswap_enabled_param_set(const char *,
 				   const struct kernel_param *);
 static struct kernel_param_ops zswap_enabled_param_ops = {
@@ -82,8 +86,7 @@  static struct kernel_param_ops zswap_enabled_param_ops = {
 module_param_cb(enabled, &zswap_enabled_param_ops, &zswap_enabled, 0644);
 
 /* Crypto compressor to use */
-#define ZSWAP_COMPRESSOR_DEFAULT "lzo"
-static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
+static char *zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
 static int zswap_compressor_param_set(const char *,
 				      const struct kernel_param *);
 static struct kernel_param_ops zswap_compressor_param_ops = {
@@ -95,8 +98,7 @@  module_param_cb(compressor, &zswap_compressor_param_ops,
 		&zswap_compressor, 0644);
 
 /* Compressed storage zpool to use */
-#define ZSWAP_ZPOOL_DEFAULT "zbud"
-static char *zswap_zpool_type = ZSWAP_ZPOOL_DEFAULT;
+static char *zswap_zpool_type = CONFIG_ZSWAP_ZPOOL_DEFAULT;
 static int zswap_zpool_param_set(const char *, const struct kernel_param *);
 static struct kernel_param_ops zswap_zpool_param_ops = {
 	.set =		zswap_zpool_param_set,
@@ -569,11 +571,12 @@  static __init struct zswap_pool *__zswap_pool_create_fallback(void)
 	bool has_comp, has_zpool;
 
 	has_comp = crypto_has_comp(zswap_compressor, 0, 0);
-	if (!has_comp && strcmp(zswap_compressor, ZSWAP_COMPRESSOR_DEFAULT)) {
+	if (!has_comp && strcmp(zswap_compressor,
+				CONFIG_ZSWAP_COMPRESSOR_DEFAULT)) {
 		pr_err("compressor %s not available, using default %s\n",
-		       zswap_compressor, ZSWAP_COMPRESSOR_DEFAULT);
+		       zswap_compressor, CONFIG_ZSWAP_COMPRESSOR_DEFAULT);
 		param_free_charp(&zswap_compressor);
-		zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
+		zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
 		has_comp = crypto_has_comp(zswap_compressor, 0, 0);
 	}
 	if (!has_comp) {
@@ -584,11 +587,12 @@  static __init struct zswap_pool *__zswap_pool_create_fallback(void)
 	}
 
 	has_zpool = zpool_has_pool(zswap_zpool_type);
-	if (!has_zpool && strcmp(zswap_zpool_type, ZSWAP_ZPOOL_DEFAULT)) {
+	if (!has_zpool && strcmp(zswap_zpool_type,
+				 CONFIG_ZSWAP_ZPOOL_DEFAULT)) {
 		pr_err("zpool %s not available, using default %s\n",
-		       zswap_zpool_type, ZSWAP_ZPOOL_DEFAULT);
+		       zswap_zpool_type, CONFIG_ZSWAP_ZPOOL_DEFAULT);
 		param_free_charp(&zswap_zpool_type);
-		zswap_zpool_type = ZSWAP_ZPOOL_DEFAULT;
+		zswap_zpool_type = CONFIG_ZSWAP_ZPOOL_DEFAULT;
 		has_zpool = zpool_has_pool(zswap_zpool_type);
 	}
 	if (!has_zpool) {