From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw0-f199.google.com (mail-yw0-f199.google.com [209.85.161.199]) by kanga.kvack.org (Postfix) with ESMTP id 2943C6B0253 for ; Thu, 4 Aug 2016 14:15:44 -0400 (EDT) Received: by mail-yw0-f199.google.com with SMTP id j12so389923462ywb.3 for ; Thu, 04 Aug 2016 11:15:44 -0700 (PDT) Received: from mail-ua0-x244.google.com (mail-ua0-x244.google.com. [2607:f8b0:400c:c08::244]) by mx.google.com with ESMTPS id d64si3662530vkb.125.2016.08.04.11.15.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Aug 2016 11:15:43 -0700 (PDT) Received: by mail-ua0-x244.google.com with SMTP id s7so4943147uas.1 for ; Thu, 04 Aug 2016 11:15:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20160804115809.GA447@swordfish> References: <2f8a65db-e5a8-75f0-8c08-daa41e1cd3ba@mejor.pl> <20160804115809.GA447@swordfish> From: Dan Streetman Date: Thu, 4 Aug 2016 14:15:02 -0400 Message-ID: Subject: Re: Choosing z3fold allocator in zswap gives WARNING: CPU: 0 PID: 5140 at mm/zswap.c:503 __zswap_pool_current+0x56/0x60 Content-Type: multipart/alternative; boundary=001a113ec7841e9769053942f0b8 Sender: owner-linux-mm@kvack.org List-ID: To: Sergey Senozhatsky Cc: Seth Jennings , Linux-MM , Vitaly Wool , =?UTF-8?Q?Marcin_Miros=C5=82aw?= , Andrew Morton --001a113ec7841e9769053942f0b8 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Thu, Aug 4, 2016 at 7:58 AM, Sergey Senozhatsky < sergey.senozhatsky.work@gmail.com> wrote: > Hello, > > Cc Seth, Dan > > On (08/01/16 11:03), Marcin Miros=C5=82aw 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] [] dump_stack+0x63/0x90 > > [ 429.740048] [] __warn+0xc7/0xf0 > > [ 429.740048] [] warn_slowpath_null+0x18/0x20 > > [ 429.740048] [] __zswap_pool_current+0x56/0x60 > > [ 429.740048] [] zswap_pool_current+0x13/0x20 > > [ 429.740048] [] __zswap_param_set+0x1db/0x2f0 > > [ 429.740048] [] zswap_zpool_param_set+0x12/0x20 > > [ 429.740048] [] param_attr_store+0x5f/0xc0 > > [ 429.740048] [] module_attr_store+0x19/0x30 > > [ 429.740048] [] sysfs_kf_write+0x32/0x40 > > [ 429.740048] [] kernfs_fop_write+0x113/0x190 > > [ 429.740048] [] __vfs_write+0x32/0x150 > > [ 429.740048] [] ? __fd_install+0x2e/0xe0 > > [ 429.740048] [] ? __alloc_fd+0x41/0x180 > > [ 429.740048] [] ? percpu_down_read+0xd/0x50 > > [ 429.740048] [] vfs_write+0xb3/0x1a0 > > [ 429.740048] [] ? filp_close+0x51/0x70 > > [ 429.740048] [] SyS_write+0x50/0xc0 > > [ 429.740048] [] entry_SYSCALL_64_fastpath+0x1e/0xa= 8 > > [ 429.764069] ---[ end trace ff7835fbf4d983b9 ]--- > > I think it's something like this. > > suppose there are no pools available - the list is empty (see later). > __zswap_param_set(): > > pool =3D zswap_pool_find_get(type, compressor); > > gives NULL. so it creates a new one > > pool =3D zswap_pool_create(type, compressor); > > then it does > > ret =3D param_set_charp(s, kp); > > which gives 0 -- all ok. so it goes to > > if (!ret) { > put_pool =3D 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 creati= on > 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 > --001a113ec7841e9769053942f0b8 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Thu, Aug 4, 2016 at 7:58 AM, Sergey Senozhatsky &l= t;se= rgey.senozhatsky.work@gmail.com> wrote:
Hello,

Cc Seth, Dan

On (08/01/16 11:03), Marcin Miros=C5=82aw wrote:
> [=C2=A0 429.722411] ------------[ cut here ]------------
> [=C2=A0 429.723476] WARNING: CPU: 0 PID: 5140 at mm/zswap.c:503 __zswa= p_pool_current+0x56/0x60
> [=C2=A0 429.740048] Call Trace:
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad255d43>] dump_stack+0x63/0= x90
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad04c997>] __warn+0xc7/0xf0<= br> > [=C2=A0 429.740048]=C2=A0 [<ffffffffad04cac8>] warn_slowpath_nul= l+0x18/0x20
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad1250c6>] __zswap_pool_curr= ent+0x56/0x60
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad1250e3>] zswap_pool_curren= t+0x13/0x20
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad125efb>] __zswap_param_set= +0x1db/0x2f0
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad126042>] zswap_zpool_param= _set+0x12/0x20
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad06645f>] param_attr_store+= 0x5f/0xc0
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad065b69>] module_attr_store= +0x19/0x30
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad1b0b02>] sysfs_kf_write+0x= 32/0x40
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad1b0663>] kernfs_fop_write+= 0x113/0x190
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad13fc52>] __vfs_write+0x32/= 0x150
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad15f0ae>] ? __fd_install+0x= 2e/0xe0
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad15ef11>] ? __alloc_fd+0x41= /0x180
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad0838dd>] ? percpu_down_rea= d+0xd/0x50
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad140d33>] vfs_write+0xb3/0x= 1a0
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad13db81>] ? filp_close+0x51= /0x70
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad142140>] SyS_write+0x50/0x= c0
> [=C2=A0 429.740048]=C2=A0 [<ffffffffad413836>] entry_SYSCALL_64_= fastpath+0x1e/0xa8
> [=C2=A0 429.764069] ---[ end trace ff7835fbf4d983b9 ]---

I think it's something like this.

suppose there are no pools available - the list is empty (see later).
__zswap_param_set():

=C2=A0 =C2=A0 =C2=A0 =C2=A0 pool =3D zswap_pool_find_get(type, compressor);=

gives NULL. so it creates a new one

=C2=A0 =C2=A0 =C2=A0 =C2=A0 pool =3D zswap_pool_create(type, compressor);
then it does

=C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D param_set_charp(s, kp);

which gives 0 -- all ok. so it goes to

=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!ret) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_pool =3D zswap_= pool_current();
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

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 exac= tly right.=C2=A0 I reproduced it with zbud compiled out.

=C2=A0

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 the= se, unless you're already working on it?


<= /div>
=C2=A0

=C2=A0 =C2=A0 =C2=A0 =C2=A0 -ss

--001a113ec7841e9769053942f0b8-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org