headers
Peter Zijlstra | 1 Dec 2011 11:56
Picon

Re: [PATCH v2 0/3] cpuacct cgroup refactoring

On Mon, 2011-11-28 at 14:45 -0200, Glauber Costa wrote:
> Hi,
> 
> This is the same series I've already sent before. Just sending again
> after the conflict with the code that was in tip.git

Utter lack of complaints here.. lets just merge it :-)

Should show up in:

git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git

soonish.
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
KAMEZAWA Hiroyuki | 2 Dec 2011 11:06
Favicon

[RFC][PATCH] memcg: remove PCG_ACCT_LRU.

I'm now testing this patch, removing PCG_ACCT_LRU, onto mmotm.
How do you think ?

Here is a performance score at running page fault test.
==
[Before]
    11.20%   malloc  [kernel.kallsyms]  [k] clear_page_c
    ....
     1.80%   malloc  [kernel.kallsyms]  [k] __mem_cgroup_commit_charge
     0.94%   malloc  [kernel.kallsyms]  [k] mem_cgroup_lru_add_list
     0.87%   malloc  [kernel.kallsyms]  [k] mem_cgroup_lru_del_list

[After]
    11.66%   malloc  [kernel.kallsyms]  [k] clear_page_c
    2.17%   malloc  [kernel.kallsyms]  [k] __mem_cgroup_commit_charge
    0.56%   malloc  [kernel.kallsyms]  [k] mem_cgroup_lru_add_list
    0.25%   malloc  [kernel.kallsyms]  [k] mem_cgroup_lru_del_list

==

seems attractive to me.

==
From 882fefc1681591af5a1a3ac697012cef7952a462 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Fri, 2 Dec 2011 19:11:33 +0900
Subject: [PATCH] memcg: remove PCG_LRU_Acct.

Now. memcg uses PCG_LRU_ACCT bit for checking the page is on LRU or not.
Now, page->lru is used for lru and it seems not to be necessary
(Continue reading)

Permalink | Reply | 9 comments |
headers
Johannes Weiner | 2 Dec 2011 13:08

Re: [RFC][PATCH] memcg: remove PCG_ACCT_LRU.

On Fri, Dec 02, 2011 at 07:06:22PM +0900, KAMEZAWA Hiroyuki wrote:
> I'm now testing this patch, removing PCG_ACCT_LRU, onto mmotm.
> How do you think ?

>  <at>  <at>  -1024,18 +1026,8  <at>  <at>  void mem_cgroup_lru_del_list(struct page *page, enum lru_list lru)
>  		return;
>  
>  	pc = lookup_page_cgroup(page);
> -	/*
> -	 * root_mem_cgroup babysits uncharged LRU pages, but
> -	 * PageCgroupUsed is cleared when the page is about to get
> -	 * freed.  PageCgroupAcctLRU remembers whether the
> -	 * LRU-accounting happened against pc->mem_cgroup or
> -	 * root_mem_cgroup.
> -	 */
> -	if (TestClearPageCgroupAcctLRU(pc)) {
> -		VM_BUG_ON(!pc->mem_cgroup);
> -		memcg = pc->mem_cgroup;
> -	} else
> -		memcg = root_mem_cgroup;
> +	memcg = pc->mem_cgroup ? pc->mem_cgroup : root_mem_cgroup;
> +	VM_BUG_ON(memcg != pc->mem_cgroup_lru);
>  	mz = page_cgroup_zoneinfo(memcg, page);
>  	/* huge page split is done under lru_lock. so, we have no races. */
>  	MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);

Nobody clears pc->mem_cgroup upon uncharge, so this may end up
mistakenly lru-unaccount a page that was never charged against the
stale pc->mem_cgroup (e.g. a swap readahead page that has not been
charged yet gets isolated by reclaim).
(Continue reading)

Permalink | Reply | 8 comments |
headers
Glauber Costa | 2 Dec 2011 18:57
Favicon

Re: [PATCH v7 04/10] tcp memory pressure controls

On 11/29/2011 11:49 PM, KAMEZAWA Hiroyuki wrote:
>
>> -static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>> +struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>   {
>>   	return container_of(cgroup_subsys_state(cont,
>>   				mem_cgroup_subsys_id), struct mem_cgroup,
>>  <at>  <at>  -4717,14 +4732,27  <at>  <at>  static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
>>
>>   	ret = cgroup_add_files(cont, ss, kmem_cgroup_files,
>>   			       ARRAY_SIZE(kmem_cgroup_files));
>> +
>> +	if (!ret)
>> +		ret = mem_cgroup_sockets_init(cont, ss);
>>   	return ret;
>>   };
>
> You does initizalication here. The reason what I think is
> 1. 'proto_list' is not available at createion of root cgroup and
>      you need to delay set up until mounting.
>
> If so, please add comment or find another way.
> This seems not very clean to me.

Yes, we do can run into some ordering issues. A part of the 
initialization can be done earlier. But I preferred to move it all later
instead of creating two functions for it. But I can change that if you 
want, no big deal.

>
(Continue reading)

Permalink | Reply | 1 comment |
headers
Glauber Costa | 2 Dec 2011 19:04
Favicon

Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure

On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
> On Tue, 29 Nov 2011 21:56:51 -0200
> Glauber Costa<glommer@...>  wrote:
>
>> Hi,
>>
>> This patchset implements per-cgroup tcp memory pressure controls. It did not change
>> significantly since last submission: rather, it just merges the comments Kame had.
>> Most of them are style-related and/or Documentation, but there are two real bugs he
>> managed to spot (thanks)
>>
>> Please let me know if there is anything else I should address.
>>
>
> After reading all codes again, I feel some strange. Could you clarify ?
>
> Here.
> ==
> +void sock_update_memcg(struct sock *sk)
> +{
> +	/* right now a socket spends its whole life in the same cgroup */
> +	if (sk->sk_cgrp) {
> +		WARN_ON(1);
> +		return;
> +	}
> +	if (static_branch(&memcg_socket_limit_enabled)) {
> +		struct mem_cgroup *memcg;
> +
> +		BUG_ON(!sk->sk_prot->proto_cgroup);
> +
(Continue reading)

Permalink | Reply | 4 comments |
headers
Glauber Costa | 2 Dec 2011 19:11
Favicon

Re: [PATCH v7 10/10] Disable task moving when using kernel memory accounting

On 11/30/2011 12:22 AM, KAMEZAWA Hiroyuki wrote:
> On Tue, 29 Nov 2011 21:57:01 -0200
> Glauber Costa<glommer@...>  wrote:
>
>> Since this code is still experimental, we are leaving the exact
>> details of how to move tasks between cgroups when kernel memory
>> accounting is used as future work.
>>
>> For now, we simply disallow movement if there are any pending
>> accounted memory.
>>
>> Signed-off-by: Glauber Costa<glommer@...>
>> CC: Hiroyouki Kamezawa<kamezawa.hiroyu@...>
>> ---
>>   mm/memcontrol.c |   23 ++++++++++++++++++++++-
>>   1 files changed, 22 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index a31a278..dd9a6d9 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>>  <at>  <at>  -5453,10 +5453,19  <at>  <at>  static int mem_cgroup_can_attach(struct cgroup_subsys *ss,
>>   {
>>   	int ret = 0;
>>   	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
>> +	struct mem_cgroup *from = mem_cgroup_from_task(p);
>> +
>> +#if defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM)&&  defined(CONFIG_INET)
>> +	if (from != memcg&&  !mem_cgroup_is_root(from)&&
>> +	    res_counter_read_u64(&from->tcp_mem.tcp_memory_allocated, RES_USAGE)) {
(Continue reading)

Permalink | Reply | 3 comments |
headers
KAMEZAWA Hiroyuki | 5 Dec 2011 01:50
Favicon

Re: [RFC][PATCH] memcg: remove PCG_ACCT_LRU.

On Fri, 2 Dec 2011 13:08:49 +0100
Johannes Weiner <hannes@...> wrote:

> On Fri, Dec 02, 2011 at 07:06:22PM +0900, KAMEZAWA Hiroyuki wrote:
> > I'm now testing this patch, removing PCG_ACCT_LRU, onto mmotm.
> > How do you think ?
> 
> >  <at>  <at>  -1024,18 +1026,8  <at>  <at>  void mem_cgroup_lru_del_list(struct page *page, enum lru_list lru)
> >  		return;
> >  
> >  	pc = lookup_page_cgroup(page);
> > -	/*
> > -	 * root_mem_cgroup babysits uncharged LRU pages, but
> > -	 * PageCgroupUsed is cleared when the page is about to get
> > -	 * freed.  PageCgroupAcctLRU remembers whether the
> > -	 * LRU-accounting happened against pc->mem_cgroup or
> > -	 * root_mem_cgroup.
> > -	 */
> > -	if (TestClearPageCgroupAcctLRU(pc)) {
> > -		VM_BUG_ON(!pc->mem_cgroup);
> > -		memcg = pc->mem_cgroup;
> > -	} else
> > -		memcg = root_mem_cgroup;
> > +	memcg = pc->mem_cgroup ? pc->mem_cgroup : root_mem_cgroup;
> > +	VM_BUG_ON(memcg != pc->mem_cgroup_lru);
> >  	mz = page_cgroup_zoneinfo(memcg, page);
> >  	/* huge page split is done under lru_lock. so, we have no races. */
> >  	MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);
> 
> Nobody clears pc->mem_cgroup upon uncharge, so this may end up
(Continue reading)

Permalink | Reply | 7 comments |
headers
KAMEZAWA Hiroyuki | 5 Dec 2011 03:01
Favicon

Re: [PATCH v7 04/10] tcp memory pressure controls

On Fri, 2 Dec 2011 15:57:28 -0200
Glauber Costa <glommer@...> wrote:

> On 11/29/2011 11:49 PM, KAMEZAWA Hiroyuki wrote:
> >
> >> -static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >> +struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >>   {
> >>   	return container_of(cgroup_subsys_state(cont,
> >>   				mem_cgroup_subsys_id), struct mem_cgroup,
> >>  <at>  <at>  -4717,14 +4732,27  <at>  <at>  static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
> >>
> >>   	ret = cgroup_add_files(cont, ss, kmem_cgroup_files,
> >>   			       ARRAY_SIZE(kmem_cgroup_files));
> >> +
> >> +	if (!ret)
> >> +		ret = mem_cgroup_sockets_init(cont, ss);
> >>   	return ret;
> >>   };
> >
> > You does initizalication here. The reason what I think is
> > 1. 'proto_list' is not available at createion of root cgroup and
> >      you need to delay set up until mounting.
> >
> > If so, please add comment or find another way.
> > This seems not very clean to me.
> 
> Yes, we do can run into some ordering issues. A part of the 
> initialization can be done earlier. But I preferred to move it all later
> instead of creating two functions for it. But I can change that if you
(Continue reading)

Permalink | Reply | 0 comments |
headers
KAMEZAWA Hiroyuki | 5 Dec 2011 03:06
Favicon

Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure

On Fri, 2 Dec 2011 16:04:08 -0200
Glauber Costa <glommer@...> wrote:

> On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
> > On Tue, 29 Nov 2011 21:56:51 -0200
> > Glauber Costa<glommer@...>  wrote:
> >
> >> Hi,
> >>
> >> This patchset implements per-cgroup tcp memory pressure controls. It did not change
> >> significantly since last submission: rather, it just merges the comments Kame had.
> >> Most of them are style-related and/or Documentation, but there are two real bugs he
> >> managed to spot (thanks)
> >>
> >> Please let me know if there is anything else I should address.
> >>
> >
> > After reading all codes again, I feel some strange. Could you clarify ?
> >
> > Here.
> > ==
> > +void sock_update_memcg(struct sock *sk)
> > +{
> > +	/* right now a socket spends its whole life in the same cgroup */
> > +	if (sk->sk_cgrp) {
> > +		WARN_ON(1);
> > +		return;
> > +	}
> > +	if (static_branch(&memcg_socket_limit_enabled)) {
> > +		struct mem_cgroup *memcg;
(Continue reading)

Permalink | Reply | 3 comments |
headers
KAMEZAWA Hiroyuki | 5 Dec 2011 03:18
Favicon

Re: [PATCH v7 10/10] Disable task moving when using kernel memory accounting

On Fri, 2 Dec 2011 16:11:56 -0200
Glauber Costa <glommer@...> wrote:

> On 11/30/2011 12:22 AM, KAMEZAWA Hiroyuki wrote:
> > On Tue, 29 Nov 2011 21:57:01 -0200
> > Glauber Costa<glommer@...>  wrote:
> >
> >> Since this code is still experimental, we are leaving the exact
> >> details of how to move tasks between cgroups when kernel memory
> >> accounting is used as future work.
> >>
> >> For now, we simply disallow movement if there are any pending
> >> accounted memory.
> >>
> >> Signed-off-by: Glauber Costa<glommer@...>
> >> CC: Hiroyouki Kamezawa<kamezawa.hiroyu@...>
> >> ---
> >>   mm/memcontrol.c |   23 ++++++++++++++++++++++-
> >>   1 files changed, 22 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index a31a278..dd9a6d9 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >>  <at>  <at>  -5453,10 +5453,19  <at>  <at>  static int mem_cgroup_can_attach(struct cgroup_subsys *ss,
> >>   {
> >>   	int ret = 0;
> >>   	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
> >> +	struct mem_cgroup *from = mem_cgroup_from_task(p);
> >> +
(Continue reading)

Permalink | Reply | 2 comments |

Gmane