On Thu, Aug 4, 2016 at 7:58 AM, Sergey Senozhatsky <sergey.senozhatsky.work@xxxxxxxxx> wrote:
Hello,
Cc Seth, Dan
On (08/01/16 11:03), Marcin Mirosław wrote:
> [ 429.722411] ------------[ cut here ]------------
> [ 429.723476] WARNING: CPU: 0 PID: 5140 at mm/zswap.c:503 __zswap_pool_current+0x56/0x60
> [ 429.740048] Call Trace:
> [ 429.740048] [<ffffffffad255d43>] dump_stack+0x63/0x90
> [ 429.740048] [<ffffffffad04c997>] __warn+0xc7/0xf0
> [ 429.740048] [<ffffffffad04cac8>] warn_slowpath_null+0x18/0x20
> [ 429.740048] [<ffffffffad1250c6>] __zswap_pool_current+0x56/0x60
> [ 429.740048] [<ffffffffad1250e3>] zswap_pool_current+0x13/0x20
> [ 429.740048] [<ffffffffad125efb>] __zswap_param_set+0x1db/0x2f0
> [ 429.740048] [<ffffffffad126042>] zswap_zpool_param_set+0x12/0x20 I think it's something like this.
> [ 429.740048] [<ffffffffad06645f>] param_attr_store+0x5f/0xc0
> [ 429.740048] [<ffffffffad065b69>] module_attr_store+0x19/0x30
> [ 429.740048] [<ffffffffad1b0b02>] sysfs_kf_write+0x32/0x40
> [ 429.740048] [<ffffffffad1b0663>] kernfs_fop_write+0x113/0x190
> [ 429.740048] [<ffffffffad13fc52>] __vfs_write+0x32/0x150
> [ 429.740048] [<ffffffffad15f0ae>] ? __fd_install+0x2e/0xe0
> [ 429.740048] [<ffffffffad15ef11>] ? __alloc_fd+0x41/0x180
> [ 429.740048] [<ffffffffad0838dd>] ? percpu_down_read+0xd/0x50
> [ 429.740048] [<ffffffffad140d33>] vfs_write+0xb3/0x1a0
> [ 429.740048] [<ffffffffad13db81>] ? filp_close+0x51/0x70
> [ 429.740048] [<ffffffffad142140>] SyS_write+0x50/0xc0
> [ 429.740048] [<ffffffffad413836>] entry_SYSCALL_64_fastpath+0x1e/0xa8
> [ 429.764069] ---[ end trace ff7835fbf4d983b9 ]---
suppose there are no pools available - the list is empty (see later).
__zswap_param_set():
pool = zswap_pool_find_get(type, compressor);
gives NULL. so it creates a new one
pool = zswap_pool_create(type, compressor);
then it does
ret = param_set_charp(s, kp);
which gives 0 -- all ok. so it goes to
if (!ret) {
put_pool = zswap_pool_current();
}
which gives WARN_ON(), as the list is still empty.
now, how is this possible. for example, we init a zswap with the default
configuration; but zbud is not available (can it be?). so the pool creation
fails, but init_zswap() does not set zswap_init_started back to false. it
either must clear it at the error path, or set it to true right before
'return 0'.
yep that's exactly right. I reproduced it with zbud compiled out.
one more problem here is that param_set_charp() does GFP_KERNEL
under zswap_pools_lock.
yep that's true as well.
i can get patches going for both these, unless you're already working on it?
-ss