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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <119e8331d1210b1f56d0f6416863bfbc.squirrel@webmail-b.css.fujitsu.com>
Date:	Mon, 31 Aug 2009 20:59:18 +0900 (JST)
From:	"KAMEZAWA Hiroyuki" <kamezawa.hiroyu@...fujitsu.com>
To:	balbir@...ux.vnet.ibm.com
Cc:	"KAMEZAWA Hiroyuki" <kamezawa.hiroyu@...fujitsu.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"nishimura@....nes.nec.co.jp" <nishimura@....nes.nec.co.jp>
Subject: Re: [RFC][PATCH 2/5] memcg: uncharge in batched manner

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

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