Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758107AbbKSJok (ORCPT ); Thu, 19 Nov 2015 04:44:40 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:33939 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756140AbbKSJoh (ORCPT ); Thu, 19 Nov 2015 04:44:37 -0500 Date: Thu, 19 Nov 2015 18:45:35 +0900 From: Sergey Senozhatsky To: "kyeongdon.kim" Cc: minchan@kernel.org, Andrew Morton , linux-kernel@vger.kernel.org, sergey.senozhatsky@gmail.com Subject: Re: [PATCH] zram: Prevent page allocation failure during zcomp_strm_alloc Message-ID: <20151119094535.GA592@swordfish> References: <1447916068-28385-1-git-send-email-kyeongdon.kim@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1447916068-28385-1-git-send-email-kyeongdon.kim@lge.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5705 Lines: 162 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: > [] dump_backtrace+0x0/0x270 > [] show_stack+0x10/0x1c > [] dump_stack+0x1c/0x28 > [] warn_alloc_failed+0xfc/0x11c > [] __alloc_pages_nodemask+0x724/0x7f0 > [] __get_free_pages+0x14/0x5c > [] kmalloc_order_trace+0x38/0xd8 > [] zcomp_lz4_create+0x2c/0x38 > [] zcomp_strm_alloc+0x34/0x78 > [] zcomp_strm_multi_find+0x124/0x1ec > [] zcomp_strm_find+0xc/0x18 > [] zram_bvec_rw+0x2fc/0x780 > [] zram_make_request+0x25c/0x2d4 > [] generic_make_request+0x80/0xbc > [] submit_bio+0xa4/0x15c > [] __swap_writepage+0x218/0x230 > [] swap_writepage+0x3c/0x4c > [] shrink_page_list+0x51c/0x8d0 > [] shrink_inactive_list+0x3f8/0x60c > [] shrink_lruvec+0x33c/0x4cc > [] shrink_zone+0x3c/0x100 > [] try_to_free_pages+0x2b8/0x54c > [] __alloc_pages_nodemask+0x514/0x7f0 > [] __get_free_pages+0x14/0x5c > [] proc_info_read+0x50/0xe4 > [] vfs_read+0xa0/0x12c > [] 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 > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/