| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 18 Nov 2019 13:43:58 +0100
From: Vitaly Wool <vitalywool@...il.com>
To: "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>
Cc: Seth Jennings <sjenning@...hat.com>,
Dan Streetman <ddstreet@...e.org>,
Vlastimil Babka <vbabka@...e.cz>,
Linux-MM <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] zswap: allow setting default status, compressor and
allocator in Kconfig
Hi Maciej,
sorry for jumping in late into this discussion.
On Sun, Nov 17, 2019 at 1:05 AM Maciej S. Szmigiero
<mail@...iej.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) {
Powered by blists - more mailing lists