| 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
| ||
|
Message-ID: <20151119094535.GA592@swordfish>
Date: Thu, 19 Nov 2015 18:45:35 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: "kyeongdon.kim" <kyeongdon.kim@....com>
Cc: minchan@...nel.org, Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org, sergey.senozhatsky@...il.com
Subject: Re: [PATCH] zram: Prevent page allocation failure during
zcomp_strm_alloc
Hello,
On (11/19/15 15:54), kyeongdon.kim wrote:
> When we use lzo/lz4 multi compression streams for zram,
> we might face page allocation failure. In order to make parallel
> compression private data, we should call kzalloc() with order 2/3
> in runtime. But if there is no order 2/3 size memory to allocate
> in that time, page allocation fails.
>
> This patch makes new zstrm stream list in zcomp_create() or
> max_comp_streams set, which means calling (kzalloc()) in runtime
> is not needed. This prevents page alloc failure.
>
The decision here is to not create unneded streams (streams are not
free) -- we create one we need it. And if allocation fails we just
wait for already existing stream to become available (and we are
guaranteed to have at least one stream)
zcomp_strm_multi_find
{
...
while (1) {
spin_lock(&zs->strm_lock);
if (!list_empty(&zs->idle_strm)) {
zstrm = list_entry(zs->idle_strm.next,
struct zcomp_strm, list);
list_del(&zstrm->list);
spin_unlock(&zs->strm_lock);
return zstrm;
}
...
/* allocate new zstrm stream */
zs->avail_strm++;
spin_unlock(&zs->strm_lock);
zstrm = zcomp_strm_alloc(comp);
if (!zstrm) {
spin_lock(&zs->strm_lock);
zs->avail_strm--;
spin_unlock(&zs->strm_lock);
wait_event(zs->strm_wait, !list_empty(&zs->idle_strm));
continue;
}
break;
}
return zstrm;
}
So, are you trying to fix the warning from zcomp_strm_multi_find()? What
prevents this warning from happening in zcomp_strm_multi_set_max_streams()
or zcomp_strm_multi_create()? e.g. I requested 64 streams and ended up not
having enought memory? The warning will still be there and the streams will
not be allocated. What's the gain?
(besides, your implementation is racy. ->list and ->avail_strm should
not be modified outside of ->strm_lock).
-ss
> For reference a call trace :
>
> Binder_1: page allocation failure: order:3, mode:0x10c0d0
> CPU: 0 PID: 424 Comm: Binder_1 Tainted: GW 3.10.49-perf-g991d02b-dirty #20
> Call trace:
> [<ffffffc0002069c8>] dump_backtrace+0x0/0x270
> [<ffffffc000206c48>] show_stack+0x10/0x1c
> [<ffffffc000cb51c8>] dump_stack+0x1c/0x28
> [<ffffffc0002bbfc8>] warn_alloc_failed+0xfc/0x11c
> [<ffffffc0002bf518>] __alloc_pages_nodemask+0x724/0x7f0
> [<ffffffc0002bf5f8>] __get_free_pages+0x14/0x5c
> [<ffffffc0002ed6a4>] kmalloc_order_trace+0x38/0xd8
> [<ffffffc0005d9738>] zcomp_lz4_create+0x2c/0x38
> [<ffffffc0005d78f0>] zcomp_strm_alloc+0x34/0x78
> [<ffffffc0005d7a58>] zcomp_strm_multi_find+0x124/0x1ec
> [<ffffffc0005d7c14>] zcomp_strm_find+0xc/0x18
> [<ffffffc0005d8fa0>] zram_bvec_rw+0x2fc/0x780
> [<ffffffc0005d9680>] zram_make_request+0x25c/0x2d4
> [<ffffffc00040f8ac>] generic_make_request+0x80/0xbc
> [<ffffffc00040f98c>] submit_bio+0xa4/0x15c
> [<ffffffc0002e8bb0>] __swap_writepage+0x218/0x230
> [<ffffffc0002e8c04>] swap_writepage+0x3c/0x4c
> [<ffffffc0002c7384>] shrink_page_list+0x51c/0x8d0
> [<ffffffc0002c7e88>] shrink_inactive_list+0x3f8/0x60c
> [<ffffffc0002c86c8>] shrink_lruvec+0x33c/0x4cc
> [<ffffffc0002c8894>] shrink_zone+0x3c/0x100
> [<ffffffc0002c8c10>] try_to_free_pages+0x2b8/0x54c
> [<ffffffc0002bf308>] __alloc_pages_nodemask+0x514/0x7f0
> [<ffffffc0002bf5f8>] __get_free_pages+0x14/0x5c
> [<ffffffc0003446cc>] proc_info_read+0x50/0xe4
> [<ffffffc0002f5204>] vfs_read+0xa0/0x12c
> [<ffffffc0002f59c8>] SyS_read+0x44/0x74
> DMA: 3397*4kB (MC) 26*8kB (RC) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB
> 0*512kB 0*1024kB 0*2048kB 0*4096kB = 13796kB
>
> Signed-off-by: kyeongdon.kim <kyeongdon.kim@....com>
> ---
> drivers/block/zram/zcomp.c | 33 ++++++++++++++++++++++++++++-----
> 1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index f1ff39a..45f39b9 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -158,6 +158,21 @@ static bool zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
> struct zcomp_strm_multi *zs = comp->stream;
> struct zcomp_strm *zstrm;
>
> + if (zs->avail_strm < num_strm) {
> + while (1) {
> + /* allocate new zstrm stream */
> + zs->avail_strm++;
> + zstrm = zcomp_strm_alloc(comp);
> + if (!zstrm) {
> + zs->avail_strm--;
> + kfree(zs);
> + return false;
> + }
> + list_add(&zstrm->list, &zs->idle_strm);
> + if (zs->avail_strm == num_strm)
> + break;
> + }
> + }
> spin_lock(&zs->strm_lock);
> zs->max_strm = num_strm;
> /*
> @@ -209,12 +224,20 @@ static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
> zs->max_strm = max_strm;
> zs->avail_strm = 1;
>
> - zstrm = zcomp_strm_alloc(comp);
> - if (!zstrm) {
> - kfree(zs);
> - return -ENOMEM;
> + while (1) {
> + /* allocate new zstrm stream */
> + zs->avail_strm++;
> + zstrm = zcomp_strm_alloc(comp);
> + if (!zstrm) {
> + zs->avail_strm--;
> + kfree(zs);
> + return -ENOMEM;
> + }
> + list_add(&zstrm->list, &zs->idle_strm);
> + if (zs->avail_strm == max_strm)
> + break;
> }
> - list_add(&zstrm->list, &zs->idle_strm);
> +
> return 0;
> }
>
> --
> 1.8.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists