Re: [RFC][PATCH 2/5] memcg: uncharge in batched manner

KAMEZAWA Hiroyuki
2009-08-31T11:59:23Z

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