| From d7b028f56a971a2e4d8d7887540a144eeefcd4ab Mon Sep 17 00:00:00 2001 |
| From: Dan Streetman <ddstreet@ieee.org> |
| Date: Fri, 3 Feb 2017 13:13:09 -0800 |
| Subject: zswap: disable changing params if init fails |
| |
| From: Dan Streetman <ddstreet@ieee.org> |
| |
| commit d7b028f56a971a2e4d8d7887540a144eeefcd4ab upstream. |
| |
| 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: 90b0fc26d5db ("zswap: change zpool/compressor at runtime") |
| Link: http://lkml.kernel.org/r/20170124200259.16191-2-ddstreet@ieee.org |
| Signed-off-by: Dan Streetman <dan.streetman@canonical.com> |
| Reported-by: Marcin 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> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| mm/zswap.c | 30 +++++++++++++++++++++++++++++- |
| 1 file changed, 29 insertions(+), 1 deletion(-) |
| |
| --- a/mm/zswap.c |
| +++ b/mm/zswap.c |
| @@ -78,7 +78,13 @@ static u64 zswap_duplicate_entry; |
| |
| /* Enable/disable zswap (disabled by default) */ |
| 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 */ |
| #define ZSWAP_COMPRESSOR_DEFAULT "lzo" |
| @@ -176,6 +182,9 @@ static atomic_t zswap_pools_count = ATOM |
| /* used by param callback function */ |
| static bool zswap_init_started; |
| |
| +/* fatal error during init */ |
| +static bool zswap_init_failed; |
| + |
| /********************************* |
| * helpers and fwd declarations |
| **********************************/ |
| @@ -706,6 +715,11 @@ static int __zswap_param_set(const char |
| char *s = strstrip((char *)val); |
| int ret; |
| |
| + if (zswap_init_failed) { |
| + pr_err("can't set param, initialization failed\n"); |
| + return -ENODEV; |
| + } |
| + |
| /* no change required */ |
| if (!strcmp(s, *(char **)kp->arg)) |
| return 0; |
| @@ -785,6 +799,17 @@ static int zswap_zpool_param_set(const c |
| 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 |
| **********************************/ |
| @@ -1271,6 +1296,9 @@ pool_fail: |
| dstmem_fail: |
| zswap_entry_cache_destroy(); |
| cache_fail: |
| + /* if built-in, we aren't unloaded on failure; don't allow use */ |
| + zswap_init_failed = true; |
| + zswap_enabled = false; |
| return -ENOMEM; |
| } |
| /* must be late so crypto has time to come up */ |