Messages in this thread | | | Subject | Re: Re: [PATCH v3 2/2] zram: try vmalloc() after kmalloc() | From | "kyeongdon.kim" <> | Date | Tue, 1 Dec 2015 11:04:09 +0900 |
| |
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
| |