Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Johannes Weiner
On Wed, Jul 29, 2015 at 05:05:49PM +0200, Michal Hocko wrote: > On Wed 29-07-15 09:14:54, Johannes Weiner wrote: > > On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: > [...] > > > 3) fail mem_cgroup_can_attach if we are trying to migrate a task sharing > > > mm_struct with a process

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Michal Hocko
On Wed 29-07-15 09:14:54, Johannes Weiner wrote: > On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: [...] > > 3) fail mem_cgroup_can_attach if we are trying to migrate a task sharing > > mm_struct with a process outside of the tset. If I understand the > > tset properly this would

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Johannes Weiner
On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: > On Fri 10-07-15 16:05:33, Michal Hocko wrote: > > JFYI: I've found some more issues while hamerring this more. > > OK so the main issue is quite simple but I have completely missed it when > thinking about the patch before.

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Michal Hocko
On Tue 14-07-15 17:18:23, Michal Hocko wrote: > On Fri 10-07-15 16:05:33, Michal Hocko wrote: > > JFYI: I've found some more issues while hamerring this more. > > OK so the main issue is quite simple but I have completely missed it when > thinking about the patch before. clone(CLONE_VM) without

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Johannes Weiner
On Wed, Jul 29, 2015 at 05:05:49PM +0200, Michal Hocko wrote: On Wed 29-07-15 09:14:54, Johannes Weiner wrote: On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: [...] 3) fail mem_cgroup_can_attach if we are trying to migrate a task sharing mm_struct with a process outside of

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Michal Hocko
On Tue 14-07-15 17:18:23, Michal Hocko wrote: On Fri 10-07-15 16:05:33, Michal Hocko wrote: JFYI: I've found some more issues while hamerring this more. OK so the main issue is quite simple but I have completely missed it when thinking about the patch before. clone(CLONE_VM) without

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Johannes Weiner
On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: On Fri 10-07-15 16:05:33, Michal Hocko wrote: JFYI: I've found some more issues while hamerring this more. OK so the main issue is quite simple but I have completely missed it when thinking about the patch before. clone(CLONE_VM)

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-29 Thread Michal Hocko
On Wed 29-07-15 09:14:54, Johannes Weiner wrote: On Tue, Jul 14, 2015 at 05:18:23PM +0200, Michal Hocko wrote: [...] 3) fail mem_cgroup_can_attach if we are trying to migrate a task sharing mm_struct with a process outside of the tset. If I understand the tset properly this would require

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-14 Thread Michal Hocko
On Sat 11-07-15 10:09:06, Vladimir Davydov wrote: [...] > Why can't we make root_mem_cgroup statically allocated? AFAICS it's a > common practice - e.g. see blkcg_root, root_task_group. It's not that easy. mem_cgroup doesn't have a fixed size. It depends on the number of online nodes. I haven't

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-14 Thread Michal Hocko
On Fri 10-07-15 16:05:33, Michal Hocko wrote: > JFYI: I've found some more issues while hamerring this more. OK so the main issue is quite simple but I have completely missed it when thinking about the patch before. clone(CLONE_VM) without CLONE_THREAD is really nasty and it will easily lockup

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-14 Thread Michal Hocko
On Fri 10-07-15 16:05:33, Michal Hocko wrote: JFYI: I've found some more issues while hamerring this more. OK so the main issue is quite simple but I have completely missed it when thinking about the patch before. clone(CLONE_VM) without CLONE_THREAD is really nasty and it will easily lockup the

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-14 Thread Michal Hocko
On Sat 11-07-15 10:09:06, Vladimir Davydov wrote: [...] Why can't we make root_mem_cgroup statically allocated? AFAICS it's a common practice - e.g. see blkcg_root, root_task_group. It's not that easy. mem_cgroup doesn't have a fixed size. It depends on the number of online nodes. I haven't

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-11 Thread Vladimir Davydov
On Fri, Jul 10, 2015 at 02:45:20PM +0200, Michal Hocko wrote: > On Fri 10-07-15 10:54:00, Vladimir Davydov wrote: > > On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: > > > On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: > > > > On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-11 Thread Vladimir Davydov
On Fri, Jul 10, 2015 at 02:45:20PM +0200, Michal Hocko wrote: On Fri 10-07-15 10:54:00, Vladimir Davydov wrote: On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote:

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Michal Hocko
JFYI: I've found some more issues while hamerring this more. Please ignore this and the follow up patch for now. If others are OK with the cleanups preceding this patch I will repost with the changes based on the feedback so far and let them merge into mm tree before I settle with this much more

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Michal Hocko
On Fri 10-07-15 10:54:00, Vladimir Davydov wrote: > On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: > > On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: > > > On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: > [...] > > > > @@ -474,7 +519,7 @@ static inline void

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Vladimir Davydov
On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: > On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: > > On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] > > > @@ -474,7 +519,7 @@ static inline void mem_cgroup_count_vm_event(struct > > > mm_struct *mm, > > >

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Vladimir Davydov
On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] @@ -474,7 +519,7 @@ static inline void mem_cgroup_count_vm_event(struct mm_struct *mm, return;

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Michal Hocko
On Fri 10-07-15 10:54:00, Vladimir Davydov wrote: On Thu, Jul 09, 2015 at 04:09:41PM +0200, Michal Hocko wrote: On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] @@ -474,7 +519,7 @@ static inline void

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-10 Thread Michal Hocko
JFYI: I've found some more issues while hamerring this more. Please ignore this and the follow up patch for now. If others are OK with the cleanups preceding this patch I will repost with the changes based on the feedback so far and let them merge into mm tree before I settle with this much more

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-09 Thread Michal Hocko
On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: > I like the gist of this patch. A few comments below. > > On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] > > +/** > > + * mm_inherit_memcg - Initialize mm_struct::memcg from an existing > > mm_struct > > + * @newmm: new mm

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-09 Thread Michal Hocko
On Wed 08-07-15 20:32:51, Vladimir Davydov wrote: I like the gist of this patch. A few comments below. On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] +/** + * mm_inherit_memcg - Initialize mm_struct::memcg from an existing mm_struct + * @newmm: new mm struct + *

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-08 Thread Vladimir Davydov
I like the gist of this patch. A few comments below. On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] > diff --git a/fs/exec.c b/fs/exec.c > index 1977c2a553ac..3ed9c0abc9f5 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -870,7 +870,7 @@ static int exec_mmap(struct mm_struct

[PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-08 Thread Michal Hocko
From: Michal Hocko mm_struct::owner keeps track of the task which is in charge for the specific mm. This is usually the thread group leader of the process but there are exotic cases where this doesn't hold. The most prominent one is when separate tasks (not in the same thread group) share the

[PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-08 Thread Michal Hocko
From: Michal Hocko mho...@suse.cz mm_struct::owner keeps track of the task which is in charge for the specific mm. This is usually the thread group leader of the process but there are exotic cases where this doesn't hold. The most prominent one is when separate tasks (not in the same thread

Re: [PATCH 7/8] memcg: get rid of mm_struct::owner

2015-07-08 Thread Vladimir Davydov
I like the gist of this patch. A few comments below. On Wed, Jul 08, 2015 at 02:27:51PM +0200, Michal Hocko wrote: [...] diff --git a/fs/exec.c b/fs/exec.c index 1977c2a553ac..3ed9c0abc9f5 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -870,7 +870,7 @@ static int exec_mmap(struct mm_struct *mm)