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

[KAMEZAWA Hiroyuki]

Balbir Singh wrote:
> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> [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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/