Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-03 Thread Tejun Heo
Hello, Glauber. Sorry about late replies. I'be been traveling for the Korean thanksgiving holidays. On Mon, Oct 01, 2012 at 12:28:28PM +0400, Glauber Costa wrote: > > That synchronous ref draining is going away. Maybe we can do that > > before kmemcg? Michal, do you have some timeframe on

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-03 Thread Tejun Heo
Hello, Glauber. Sorry about late replies. I'be been traveling for the Korean thanksgiving holidays. On Mon, Oct 01, 2012 at 12:28:28PM +0400, Glauber Costa wrote: That synchronous ref draining is going away. Maybe we can do that before kmemcg? Michal, do you have some timeframe on mind?

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 03:58 PM, Michal Hocko wrote: > On Mon 01-10-12 15:51:20, Glauber Costa wrote: >> On 10/01/2012 03:51 PM, Michal Hocko wrote: >>> On Mon 01-10-12 14:09:09, Glauber Costa wrote: On 10/01/2012 01:48 PM, Michal Hocko wrote: > On Fri 28-09-12 15:34:19, Glauber Costa wrote:

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Mon 01-10-12 15:51:20, Glauber Costa wrote: > On 10/01/2012 03:51 PM, Michal Hocko wrote: > > On Mon 01-10-12 14:09:09, Glauber Costa wrote: > >> On 10/01/2012 01:48 PM, Michal Hocko wrote: > >>> On Fri 28-09-12 15:34:19, Glauber Costa wrote: > On 09/27/2012 05:44 PM, Michal Hocko wrote: >

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 03:51 PM, Michal Hocko wrote: > On Mon 01-10-12 14:09:09, Glauber Costa wrote: >> On 10/01/2012 01:48 PM, Michal Hocko wrote: >>> On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: >>> the reference count aquired by mem_cgroup_get will

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Mon 01-10-12 14:09:09, Glauber Costa wrote: > On 10/01/2012 01:48 PM, Michal Hocko wrote: > > On Fri 28-09-12 15:34:19, Glauber Costa wrote: > >> On 09/27/2012 05:44 PM, Michal Hocko wrote: > > the reference count aquired by mem_cgroup_get will still prevent the > > memcg from going

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 01:48 PM, Michal Hocko wrote: > On Fri 28-09-12 15:34:19, Glauber Costa wrote: >> On 09/27/2012 05:44 PM, Michal Hocko wrote: > the reference count aquired by mem_cgroup_get will still prevent the > memcg from going away, no? >>> Yes but you are outside of the rcu now and we

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Fri 28-09-12 15:34:19, Glauber Costa wrote: > On 09/27/2012 05:44 PM, Michal Hocko wrote: > >> > the reference count aquired by mem_cgroup_get will still prevent the > >> > memcg from going away, no? > > Yes but you are outside of the rcu now and we usually do css_get before > > we rcu_unlock.

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Sun 30-09-12 17:25:42, Tejun Heo wrote: > On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: > > On 09/27/2012 05:44 PM, Michal Hocko wrote: > > > Anyway, I have just noticed that __mem_cgroup_try_charge does > > > VM_BUG_ON(css_is_removed(>css)) on a given memcg so you should > > >

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 09/30/2012 12:25 PM, Tejun Heo wrote: > On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: >> On 09/27/2012 05:44 PM, Michal Hocko wrote: >>> Anyway, I have just noticed that __mem_cgroup_try_charge does >>> VM_BUG_ON(css_is_removed(>css)) on a given memcg so you should >>> keep css

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 09/30/2012 12:25 PM, Tejun Heo wrote: On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: Anyway, I have just noticed that __mem_cgroup_try_charge does VM_BUG_ON(css_is_removed(memcg-css)) on a given memcg so you should keep css ref

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Sun 30-09-12 17:25:42, Tejun Heo wrote: On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: Anyway, I have just noticed that __mem_cgroup_try_charge does VM_BUG_ON(css_is_removed(memcg-css)) on a given memcg so you should keep

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference count aquired by mem_cgroup_get will still prevent the memcg from going away, no? Yes but you are outside of the rcu now and we usually do css_get before we rcu_unlock.

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 01:48 PM, Michal Hocko wrote: On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference count aquired by mem_cgroup_get will still prevent the memcg from going away, no? Yes but you are outside of the rcu now and we usually do

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Mon 01-10-12 14:09:09, Glauber Costa wrote: On 10/01/2012 01:48 PM, Michal Hocko wrote: On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference count aquired by mem_cgroup_get will still prevent the memcg from going away, no? Yes but

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 03:51 PM, Michal Hocko wrote: On Mon 01-10-12 14:09:09, Glauber Costa wrote: On 10/01/2012 01:48 PM, Michal Hocko wrote: On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference count aquired by mem_cgroup_get will still prevent

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Michal Hocko
On Mon 01-10-12 15:51:20, Glauber Costa wrote: On 10/01/2012 03:51 PM, Michal Hocko wrote: On Mon 01-10-12 14:09:09, Glauber Costa wrote: On 10/01/2012 01:48 PM, Michal Hocko wrote: On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-10-01 Thread Glauber Costa
On 10/01/2012 03:58 PM, Michal Hocko wrote: On Mon 01-10-12 15:51:20, Glauber Costa wrote: On 10/01/2012 03:51 PM, Michal Hocko wrote: On Mon 01-10-12 14:09:09, Glauber Costa wrote: On 10/01/2012 01:48 PM, Michal Hocko wrote: On Fri 28-09-12 15:34:19, Glauber Costa wrote: On 09/27/2012 05:44

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-30 Thread Tejun Heo
On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: > On 09/27/2012 05:44 PM, Michal Hocko wrote: > > Anyway, I have just noticed that __mem_cgroup_try_charge does > > VM_BUG_ON(css_is_removed(>css)) on a given memcg so you should > > keep css ref count up as well. > > IIRC, css_get

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-30 Thread Tejun Heo
On Fri, Sep 28, 2012 at 03:34:19PM +0400, Glauber Costa wrote: On 09/27/2012 05:44 PM, Michal Hocko wrote: Anyway, I have just noticed that __mem_cgroup_try_charge does VM_BUG_ON(css_is_removed(memcg-css)) on a given memcg so you should keep css ref count up as well. IIRC, css_get will

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-28 Thread Glauber Costa
On 09/27/2012 05:44 PM, Michal Hocko wrote: >> > the reference count aquired by mem_cgroup_get will still prevent the >> > memcg from going away, no? > Yes but you are outside of the rcu now and we usually do css_get before > we rcu_unlock. mem_cgroup_get just makes sure the group doesn't get >

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-28 Thread Glauber Costa
On 09/27/2012 05:44 PM, Michal Hocko wrote: the reference count aquired by mem_cgroup_get will still prevent the memcg from going away, no? Yes but you are outside of the rcu now and we usually do css_get before we rcu_unlock. mem_cgroup_get just makes sure the group doesn't get deallocated

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-27 Thread Michal Hocko
On Thu 27-09-12 15:31:57, Glauber Costa wrote: > On 09/26/2012 07:51 PM, Michal Hocko wrote: > > On Tue 18-09-12 18:04:03, Glauber Costa wrote: [...] > >> + *_memcg = NULL; > >> + rcu_read_lock(); > >> + p = rcu_dereference(current->mm->owner); > >> + memcg = mem_cgroup_from_task(p); > > > >

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-27 Thread Glauber Costa
On 09/26/2012 07:51 PM, Michal Hocko wrote: > On Tue 18-09-12 18:04:03, Glauber Costa wrote: >> This patch introduces infrastructure for tracking kernel memory pages to >> a given memcg. This will happen whenever the caller includes the flag >> __GFP_KMEMCG flag, and the task belong to a memcg

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-27 Thread Glauber Costa
On 09/26/2012 07:51 PM, Michal Hocko wrote: On Tue 18-09-12 18:04:03, Glauber Costa wrote: This patch introduces infrastructure for tracking kernel memory pages to a given memcg. This will happen whenever the caller includes the flag __GFP_KMEMCG flag, and the task belong to a memcg other than

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-27 Thread Michal Hocko
On Thu 27-09-12 15:31:57, Glauber Costa wrote: On 09/26/2012 07:51 PM, Michal Hocko wrote: On Tue 18-09-12 18:04:03, Glauber Costa wrote: [...] + *_memcg = NULL; + rcu_read_lock(); + p = rcu_dereference(current-mm-owner); + memcg = mem_cgroup_from_task(p); mem_cgroup_from_task

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-26 Thread Michal Hocko
On Tue 18-09-12 18:04:03, Glauber Costa wrote: > This patch introduces infrastructure for tracking kernel memory pages to > a given memcg. This will happen whenever the caller includes the flag > __GFP_KMEMCG flag, and the task belong to a memcg other than the root. > > In memcontrol.h those

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-26 Thread Michal Hocko
On Tue 18-09-12 18:04:03, Glauber Costa wrote: This patch introduces infrastructure for tracking kernel memory pages to a given memcg. This will happen whenever the caller includes the flag __GFP_KMEMCG flag, and the task belong to a memcg other than the root. In memcontrol.h those functions

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-21 Thread JoonSoo Kim
>> "*_memcg = memcg" should be executed when "memcg_charge_kmem" is success. >> "memcg_charge_kmem" return 0 if success in charging. >> Therefore, I think this code is wrong. >> If I am right, it is a serious bug that affect behavior of all the patchset. > > Which is precisely what it does. ret is

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-21 Thread Glauber Costa
On 09/20/2012 08:05 PM, JoonSoo Kim wrote: > Hi, Glauber. > > 2012/9/18 Glauber Costa : >> +/* >> + * We need to verify if the allocation against current->mm->owner's memcg is >> + * possible for the given order. But the page is not allocated yet, so we'll >> + * need a further commit step to do

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-21 Thread Glauber Costa
On 09/20/2012 08:05 PM, JoonSoo Kim wrote: Hi, Glauber. 2012/9/18 Glauber Costa glom...@parallels.com: +/* + * We need to verify if the allocation against current-mm-owner's memcg is + * possible for the given order. But the page is not allocated yet, so we'll + * need a further commit

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-21 Thread JoonSoo Kim
*_memcg = memcg should be executed when memcg_charge_kmem is success. memcg_charge_kmem return 0 if success in charging. Therefore, I think this code is wrong. If I am right, it is a serious bug that affect behavior of all the patchset. Which is precisely what it does. ret is a boolean, that

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-20 Thread JoonSoo Kim
Hi, Glauber. 2012/9/18 Glauber Costa : > +/* > + * We need to verify if the allocation against current->mm->owner's memcg is > + * possible for the given order. But the page is not allocated yet, so we'll > + * need a further commit step to do the final arrangements. > + * > + * It is possible

Re: [PATCH v3 06/13] memcg: kmem controller infrastructure

2012-09-20 Thread JoonSoo Kim
Hi, Glauber. 2012/9/18 Glauber Costa glom...@parallels.com: +/* + * We need to verify if the allocation against current-mm-owner's memcg is + * possible for the given order. But the page is not allocated yet, so we'll + * need a further commit step to do the final arrangements. + * + * It