Commit d7b028f5 authored by Dan Streetman's avatar Dan Streetman Committed by Linus Torvalds

zswap: disable changing params if init fails

Add zswap_init_failed bool that prevents changing any of the module
params, if init_zswap() fails, and set zswap_enabled to false.  Change
'enabled' param to a callback, and check zswap_init_failed before
allowing any change to 'enabled', 'zpool', or 'compressor' params.

Any driver that is built-in to the kernel will not be unloaded if its
init function returns error, and its module params remain accessible for
users to change via sysfs.  Since zswap uses param callbacks, which
assume that zswap has been initialized, changing the zswap params after
a failed initialization will result in WARNING due to the param
callbacks expecting a pool to already exist.  This prevents that by
immediately exiting any of the param callbacks if initialization failed.

This was reported here:
  https://marc.info/?l=linux-mm&m=147004228125528&w=4

And fixes this WARNING:
  [  429.723476] WARNING: CPU: 0 PID: 5140 at mm/zswap.c:503 __zswap_pool_current+0x56/0x60

The warning is just noise, and not serious.  However, when init fails,
zswap frees all its percpu dstmem pages and its kmem cache.  The kmem
cache might be serious, if kmem_cache_alloc(NULL, gfp) has problems; but
the percpu dstmem pages are definitely a problem, as they're used as
temporary buffer for compressed pages before copying into place in the
zpool.

If the user does get zswap enabled after an init failure, then zswap
will likely Oops on the first page it tries to compress (or worse, start
corrupting memory).

Fixes: 90b0fc26 ("zswap: change zpool/compressor at runtime")
Link: http://lkml.kernel.org/r/20170124200259.16191-2-ddstreet@ieee.orgSigned-off-by: default avatarDan Streetman <dan.streetman@canonical.com>
Reported-by: default avatarMarcin Miroslaw <marcin@mejor.pl>
Cc: Seth Jennings <sjenning@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 79c9089f
...@@ -78,7 +78,13 @@ static u64 zswap_duplicate_entry; ...@@ -78,7 +78,13 @@ static u64 zswap_duplicate_entry;
/* Enable/disable zswap (disabled by default) */ /* Enable/disable zswap (disabled by default) */
static bool zswap_enabled; static bool zswap_enabled;
module_param_named(enabled, zswap_enabled, bool, 0644); static int zswap_enabled_param_set(const char *,
const struct kernel_param *);
static struct kernel_param_ops zswap_enabled_param_ops = {
.set = zswap_enabled_param_set,
.get = param_get_bool,
};
module_param_cb(enabled, &zswap_enabled_param_ops, &zswap_enabled, 0644);
/* Crypto compressor to use */ /* Crypto compressor to use */
#define ZSWAP_COMPRESSOR_DEFAULT "lzo" #define ZSWAP_COMPRESSOR_DEFAULT "lzo"
...@@ -176,6 +182,9 @@ static atomic_t zswap_pools_count = ATOMIC_INIT(0); ...@@ -176,6 +182,9 @@ static atomic_t zswap_pools_count = ATOMIC_INIT(0);
/* used by param callback function */ /* used by param callback function */
static bool zswap_init_started; static bool zswap_init_started;
/* fatal error during init */
static bool zswap_init_failed;
/********************************* /*********************************
* helpers and fwd declarations * helpers and fwd declarations
**********************************/ **********************************/
...@@ -624,6 +633,11 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp, ...@@ -624,6 +633,11 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp,
char *s = strstrip((char *)val); char *s = strstrip((char *)val);
int ret; int ret;
if (zswap_init_failed) {
pr_err("can't set param, initialization failed\n");
return -ENODEV;
}
/* no change required */ /* no change required */
if (!strcmp(s, *(char **)kp->arg)) if (!strcmp(s, *(char **)kp->arg))
return 0; return 0;
...@@ -703,6 +717,17 @@ static int zswap_zpool_param_set(const char *val, ...@@ -703,6 +717,17 @@ static int zswap_zpool_param_set(const char *val,
return __zswap_param_set(val, kp, NULL, zswap_compressor); return __zswap_param_set(val, kp, NULL, zswap_compressor);
} }
static int zswap_enabled_param_set(const char *val,
const struct kernel_param *kp)
{
if (zswap_init_failed) {
pr_err("can't enable, initialization failed\n");
return -ENODEV;
}
return param_set_bool(val, kp);
}
/********************************* /*********************************
* writeback code * writeback code
**********************************/ **********************************/
...@@ -1201,6 +1226,9 @@ static int __init init_zswap(void) ...@@ -1201,6 +1226,9 @@ static int __init init_zswap(void)
dstmem_fail: dstmem_fail:
zswap_entry_cache_destroy(); zswap_entry_cache_destroy();
cache_fail: cache_fail:
/* if built-in, we aren't unloaded on failure; don't allow use */
zswap_init_failed = true;
zswap_enabled = false;
return -ENOMEM; return -ENOMEM;
} }
/* must be late so crypto has time to come up */ /* must be late so crypto has time to come up */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment