Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755487AbbLACET (ORCPT <rfc822;w@1wt.eu>);
	Mon, 30 Nov 2015 21:04:19 -0500
Received: from LGEAMRELO13.lge.com ([156.147.23.53]:46708 "EHLO
	lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752698AbbLACES (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 30 Nov 2015 21:04:18 -0500
X-Original-SENDERIP: 156.147.1.121
X-Original-MAILFROM: kyeongdon.kim@lge.com
X-Original-SENDERIP: 10.168.83.150
X-Original-MAILFROM: kyeongdon.kim@lge.com
Subject: Re: Re: [PATCH v3 2/2] zram: try vmalloc() after kmalloc()
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
References: <1448597449-17579-1-git-send-email-sergey.senozhatsky@gmail.com>
 <20151130111424.GB1483@swordfish>
Cc: Minchan Kim <minchan@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
From: "kyeongdon.kim" <kyeongdon.kim@lge.com>
Message-ID: <565D0019.5010502@lge.com>
Date: Tue, 1 Dec 2015 11:04:09 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <20151130111424.GB1483@swordfish>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3093
Lines: 96

On 2015-11-30 오후 8:14, Sergey Senozhatsky wrote:
> On (11/30/15 19:42), kyeongdon.kim wrote:
> [..]
>> Sorry to have kept you waiting,
>> Obviously, I couldn't see allocation fail message with this patch.
>> But, there is something to make some delay(not sure yet this is normal).
> 
> what delay? how significant it is? do you see it in practice or it's just
> a guess?
> 
Now, I just checked with printk() log in zcomp_lz4_create()/lzo_create()
to see address 'ret'. and these 'null' values are called several times
from kzalloc. also __vmalloc. - these are making the delay.
So, not significant status I guess. but if this allocation try is many.
I doubt is is fine.

>> static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp)
>> {
>> <snip>
>>
>> zstrm->private = comp->backend->create();
>> ^ // sometimes, return 'null' continually(2-5times)
>>
>> As you know, if there is 'null' return, this function is called again to
>> get a memory in while() loop. I just checked this one with printk().
> 
> well, not always.
> 
> a) current wait_event() for available stream to become idle.
> b) once current awaken it attempts to get an idle stream
> c) if zstrm then return
> d) if there is no idle stream then goto a)
> e) else try to allocate stream again, if !zstrm goto a), else return
> 
> 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;
> }
> /* zstrm streams limit reached, wait for idle stream */
> if (zs->avail_strm >= zs->max_strm) {
> spin_unlock(&zs->strm_lock);
> wait_event(zs->strm_wait, !list_empty(&zs->idle_strm));
> continue;
> }
> /* 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;
> }
> 
> so it's possible for current to zcomp_strm_alloc() several times...
> 
> do you see the same process doing N zcomp_strm_alloc() calls, or it's N
> processes
> doing one zcomp_strm_alloc()? I think the latter one is more likely;
> once we failed
> to zcomp_strm_alloc() quite possible that N concurrent or succeeding IOs
> will do
> the same. That's why I proposed to decrease ->max_strm; but we basically
> don't know
> when we shall rollback it to the original value; I'm not sure I want to
> do something
> like: every 42nd IO try to increment ->max_strm by one, until it's less
> than the
> original value.
> 
> so I'd probably prefer to keep it the way it is; but let's see the
> numbers from
> you first.
> 
> -ss

I didn't check detailed yet.I'll explain after checking this.

Thanks,
Kyeongdon Kim

--
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/