Messages in this thread | | | Date | Mon, 31 Aug 2009 20:59:18 +0900 (JST) | Subject | Re: [RFC][PATCH 2/5] memcg: uncharge in batched manner | From | "KAMEZAWA Hiroyuki" <> |
| |
Balbir Singh wrote: > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-08-28 > 13:24:38]:
>> + } >> + if (!batch || batch->memcg != mem) { >> + res_counter_uncharge(&mem->res, PAGE_SIZE); >> + if (uncharge_memsw) >> + res_counter_uncharge(&mem->memsw, PAGE_SIZE); > > Could you please add a comment stating that if memcg is different that > we do a direct uncharge else we batch. > really necessary ?. ok. I'll do.
>> + } else { >> + batch->pages += PAGE_SIZE; >> + if (uncharge_memsw) >> + batch->memsw += PAGE_SIZE; >> + } >> + return soft_limit_excess; >> +} >> /* >> * uncharge if !page_mapped(page) >> */ >> @@ -1886,12 +1914,8 @@ __mem_cgroup_uncharge_common(struct page >> break; >> } >> >> - if (!mem_cgroup_is_root(mem)) { >> - res_counter_uncharge(&mem->res, PAGE_SIZE); >> - if (do_swap_account && >> - (ctype != MEM_CGROUP_CHARGE_TYPE_SWAPOUT)) >> - res_counter_uncharge(&mem->memsw, PAGE_SIZE); >> - } >> + if (!mem_cgroup_is_root(mem)) >> + __do_batch_uncharge(mem, ctype); > > Now I am beginning to think we need a cond_mem_cgroup_is_not_root() > function. > I can't catch waht cond_mem_cgroup_is_not_root() means.
>> if (ctype == MEM_CGROUP_CHARGE_TYPE_SWAPOUT) >> mem_cgroup_swap_statistics(mem, true); >> mem_cgroup_charge_statistics(mem, pc, false); >> @@ -1938,6 +1962,40 @@ void mem_cgroup_uncharge_cache_page(stru >> __mem_cgroup_uncharge_common(page, MEM_CGROUP_CHARGE_TYPE_CACHE); >> } >> >> +void mem_cgroup_uncharge_batch_start(void) >> +{ >> + VM_BUG_ON(current->memcg_batch.do_batch); >> + /* avoid batch if killed by OOM */ >> + if (test_thread_flag(TIF_MEMDIE)) >> + return; >> + current->memcg_batch.do_batch = 1; >> + current->memcg_batch.memcg = NULL; >> + current->memcg_batch.pages = 0; >> + current->memcg_batch.memsw = 0; >> +} >> + >> +void mem_cgroup_uncharge_batch_end(void) >> +{ >> + struct mem_cgroup *mem; >> + >> + if (!current->memcg_batch.do_batch) >> + return; >> + >> + current->memcg_batch.do_batch = 0; >> + >> + mem = current->memcg_batch.memcg; >> + if (!mem) >> + return; >> + if (current->memcg_batch.pages) >> + res_counter_uncharge(&mem->res, >> + current->memcg_batch.pages, NULL); >> + if (current->memcg_batch.memsw) >> + res_counter_uncharge(&mem->memsw, >> + current->memcg_batch.memsw, NULL); >> + /* we got css's refcnt */ >> + cgroup_release_and_wakeup_rmdir(&mem->css); > > > Does this effect deleting of a group and delay it by a large amount? > plz see what cgroup_release_and_xxxx fixed. This is not for delay but for race-condition, which makes rmdir sleep permanently.
Thanks, -Kame
| |